From 116ce96326152b27e91bb9c3170b0a802b85ce07 Mon Sep 17 00:00:00 2001 From: Jeff Hodges Date: Mon, 14 Dec 2015 21:49:28 -0800 Subject: [PATCH 1/7] add retries and context deadlines to DNSResolver This provides a means to add retries to DNS look ups, and, with some future work, end retries early if our request deadline is blown. That future work is tagged with #1292. Updates #1258 --- Godeps/Godeps.json | 5 +- .../src/golang.org/x/net/context/context.go | 447 ++++++++++++++++++ .../x/net/context/ctxhttp/cancelreq.go | 18 + .../x/net/context/ctxhttp/cancelreq_go14.go | 23 + .../x/net/context/ctxhttp/ctxhttp.go | 79 ++++ bdns/dns.go | 137 ++++-- bdns/dns_test.go | 231 +++++++-- cmd/boulder-ra/main.go | 8 +- cmd/boulder-va/main.go | 11 +- cmd/config.go | 10 + mocks/mocks.go | 9 +- ra/registration-authority.go | 17 +- ra/registration-authority_test.go | 20 +- test/boulder-config.json | 2 + va/validation-authority.go | 68 +-- va/validation-authority_test.go | 75 +-- 16 files changed, 998 insertions(+), 162 deletions(-) create mode 100644 Godeps/_workspace/src/golang.org/x/net/context/context.go create mode 100644 Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/cancelreq.go create mode 100644 Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/cancelreq_go14.go create mode 100644 Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/ctxhttp.go diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 23bcfed6c..5f476b3e0 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -1,6 +1,6 @@ { "ImportPath": "github.com/letsencrypt/boulder", - "GoVersion": "go1.5.1", + "GoVersion": "go1.5.2", "Packages": [ "./..." ], @@ -141,6 +141,9 @@ "Rev": "beef0f4390813b96e8e68fd78570396d0f4751fc" }, { + "ImportPath": "golang.org/x/net/context", + "Rev": "ce84af2e5bf21582345e478b116afc7d4efaba3d" + }, "ImportPath": "gopkg.in/gorp.v1", "Comment": "v1.7.1", "Rev": "c87af80f3cc5036b55b83d77171e156791085e2e" diff --git a/Godeps/_workspace/src/golang.org/x/net/context/context.go b/Godeps/_workspace/src/golang.org/x/net/context/context.go new file mode 100644 index 000000000..ef2f3e86f --- /dev/null +++ b/Godeps/_workspace/src/golang.org/x/net/context/context.go @@ -0,0 +1,447 @@ +// Copyright 2014 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package context defines the Context type, which carries deadlines, +// cancelation signals, and other request-scoped values across API boundaries +// and between processes. +// +// Incoming requests to a server should create a Context, and outgoing calls to +// servers should accept a Context. The chain of function calls between must +// propagate the Context, optionally replacing it with a modified copy created +// using WithDeadline, WithTimeout, WithCancel, or WithValue. +// +// Programs that use Contexts should follow these rules to keep interfaces +// consistent across packages and enable static analysis tools to check context +// propagation: +// +// Do not store Contexts inside a struct type; instead, pass a Context +// explicitly to each function that needs it. The Context should be the first +// parameter, typically named ctx: +// +// func DoSomething(ctx context.Context, arg Arg) error { +// // ... use ctx ... +// } +// +// Do not pass a nil Context, even if a function permits it. Pass context.TODO +// if you are unsure about which Context to use. +// +// Use context Values only for request-scoped data that transits processes and +// APIs, not for passing optional parameters to functions. +// +// The same Context may be passed to functions running in different goroutines; +// Contexts are safe for simultaneous use by multiple goroutines. +// +// See http://blog.golang.org/context for example code for a server that uses +// Contexts. +package context + +import ( + "errors" + "fmt" + "sync" + "time" +) + +// A Context carries a deadline, a cancelation signal, and other values across +// API boundaries. +// +// Context's methods may be called by multiple goroutines simultaneously. +type Context interface { + // Deadline returns the time when work done on behalf of this context + // should be canceled. Deadline returns ok==false when no deadline is + // set. Successive calls to Deadline return the same results. + Deadline() (deadline time.Time, ok bool) + + // Done returns a channel that's closed when work done on behalf of this + // context should be canceled. Done may return nil if this context can + // never be canceled. Successive calls to Done return the same value. + // + // WithCancel arranges for Done to be closed when cancel is called; + // WithDeadline arranges for Done to be closed when the deadline + // expires; WithTimeout arranges for Done to be closed when the timeout + // elapses. + // + // Done is provided for use in select statements: + // + // // Stream generates values with DoSomething and sends them to out + // // until DoSomething returns an error or ctx.Done is closed. + // func Stream(ctx context.Context, out <-chan Value) error { + // for { + // v, err := DoSomething(ctx) + // if err != nil { + // return err + // } + // select { + // case <-ctx.Done(): + // return ctx.Err() + // case out <- v: + // } + // } + // } + // + // See http://blog.golang.org/pipelines for more examples of how to use + // a Done channel for cancelation. + Done() <-chan struct{} + + // Err returns a non-nil error value after Done is closed. Err returns + // Canceled if the context was canceled or DeadlineExceeded if the + // context's deadline passed. No other values for Err are defined. + // After Done is closed, successive calls to Err return the same value. + Err() error + + // Value returns the value associated with this context for key, or nil + // if no value is associated with key. Successive calls to Value with + // the same key returns the same result. + // + // Use context values only for request-scoped data that transits + // processes and API boundaries, not for passing optional parameters to + // functions. + // + // A key identifies a specific value in a Context. Functions that wish + // to store values in Context typically allocate a key in a global + // variable then use that key as the argument to context.WithValue and + // Context.Value. A key can be any type that supports equality; + // packages should define keys as an unexported type to avoid + // collisions. + // + // Packages that define a Context key should provide type-safe accessors + // for the values stores using that key: + // + // // Package user defines a User type that's stored in Contexts. + // package user + // + // import "golang.org/x/net/context" + // + // // User is the type of value stored in the Contexts. + // type User struct {...} + // + // // key is an unexported type for keys defined in this package. + // // This prevents collisions with keys defined in other packages. + // type key int + // + // // userKey is the key for user.User values in Contexts. It is + // // unexported; clients use user.NewContext and user.FromContext + // // instead of using this key directly. + // var userKey key = 0 + // + // // NewContext returns a new Context that carries value u. + // func NewContext(ctx context.Context, u *User) context.Context { + // return context.WithValue(ctx, userKey, u) + // } + // + // // FromContext returns the User value stored in ctx, if any. + // func FromContext(ctx context.Context) (*User, bool) { + // u, ok := ctx.Value(userKey).(*User) + // return u, ok + // } + Value(key interface{}) interface{} +} + +// Canceled is the error returned by Context.Err when the context is canceled. +var Canceled = errors.New("context canceled") + +// DeadlineExceeded is the error returned by Context.Err when the context's +// deadline passes. +var DeadlineExceeded = errors.New("context deadline exceeded") + +// An emptyCtx is never canceled, has no values, and has no deadline. It is not +// struct{}, since vars of this type must have distinct addresses. +type emptyCtx int + +func (*emptyCtx) Deadline() (deadline time.Time, ok bool) { + return +} + +func (*emptyCtx) Done() <-chan struct{} { + return nil +} + +func (*emptyCtx) Err() error { + return nil +} + +func (*emptyCtx) Value(key interface{}) interface{} { + return nil +} + +func (e *emptyCtx) String() string { + switch e { + case background: + return "context.Background" + case todo: + return "context.TODO" + } + return "unknown empty Context" +} + +var ( + background = new(emptyCtx) + todo = new(emptyCtx) +) + +// Background returns a non-nil, empty Context. It is never canceled, has no +// values, and has no deadline. It is typically used by the main function, +// initialization, and tests, and as the top-level Context for incoming +// requests. +func Background() Context { + return background +} + +// TODO returns a non-nil, empty Context. Code should use context.TODO when +// it's unclear which Context to use or it's is not yet available (because the +// surrounding function has not yet been extended to accept a Context +// parameter). TODO is recognized by static analysis tools that determine +// whether Contexts are propagated correctly in a program. +func TODO() Context { + return todo +} + +// A CancelFunc tells an operation to abandon its work. +// A CancelFunc does not wait for the work to stop. +// After the first call, subsequent calls to a CancelFunc do nothing. +type CancelFunc func() + +// WithCancel returns a copy of parent with a new Done channel. The returned +// context's Done channel is closed when the returned cancel function is called +// or when the parent context's Done channel is closed, whichever happens first. +// +// Canceling this context releases resources associated with it, so code should +// call cancel as soon as the operations running in this Context complete. +func WithCancel(parent Context) (ctx Context, cancel CancelFunc) { + c := newCancelCtx(parent) + propagateCancel(parent, &c) + return &c, func() { c.cancel(true, Canceled) } +} + +// newCancelCtx returns an initialized cancelCtx. +func newCancelCtx(parent Context) cancelCtx { + return cancelCtx{ + Context: parent, + done: make(chan struct{}), + } +} + +// propagateCancel arranges for child to be canceled when parent is. +func propagateCancel(parent Context, child canceler) { + if parent.Done() == nil { + return // parent is never canceled + } + if p, ok := parentCancelCtx(parent); ok { + p.mu.Lock() + if p.err != nil { + // parent has already been canceled + child.cancel(false, p.err) + } else { + if p.children == nil { + p.children = make(map[canceler]bool) + } + p.children[child] = true + } + p.mu.Unlock() + } else { + go func() { + select { + case <-parent.Done(): + child.cancel(false, parent.Err()) + case <-child.Done(): + } + }() + } +} + +// parentCancelCtx follows a chain of parent references until it finds a +// *cancelCtx. This function understands how each of the concrete types in this +// package represents its parent. +func parentCancelCtx(parent Context) (*cancelCtx, bool) { + for { + switch c := parent.(type) { + case *cancelCtx: + return c, true + case *timerCtx: + return &c.cancelCtx, true + case *valueCtx: + parent = c.Context + default: + return nil, false + } + } +} + +// removeChild removes a context from its parent. +func removeChild(parent Context, child canceler) { + p, ok := parentCancelCtx(parent) + if !ok { + return + } + p.mu.Lock() + if p.children != nil { + delete(p.children, child) + } + p.mu.Unlock() +} + +// A canceler is a context type that can be canceled directly. The +// implementations are *cancelCtx and *timerCtx. +type canceler interface { + cancel(removeFromParent bool, err error) + Done() <-chan struct{} +} + +// A cancelCtx can be canceled. When canceled, it also cancels any children +// that implement canceler. +type cancelCtx struct { + Context + + done chan struct{} // closed by the first cancel call. + + mu sync.Mutex + children map[canceler]bool // set to nil by the first cancel call + err error // set to non-nil by the first cancel call +} + +func (c *cancelCtx) Done() <-chan struct{} { + return c.done +} + +func (c *cancelCtx) Err() error { + c.mu.Lock() + defer c.mu.Unlock() + return c.err +} + +func (c *cancelCtx) String() string { + return fmt.Sprintf("%v.WithCancel", c.Context) +} + +// cancel closes c.done, cancels each of c's children, and, if +// removeFromParent is true, removes c from its parent's children. +func (c *cancelCtx) cancel(removeFromParent bool, err error) { + if err == nil { + panic("context: internal error: missing cancel error") + } + c.mu.Lock() + if c.err != nil { + c.mu.Unlock() + return // already canceled + } + c.err = err + close(c.done) + for child := range c.children { + // NOTE: acquiring the child's lock while holding parent's lock. + child.cancel(false, err) + } + c.children = nil + c.mu.Unlock() + + if removeFromParent { + removeChild(c.Context, c) + } +} + +// WithDeadline returns a copy of the parent context with the deadline adjusted +// to be no later than d. If the parent's deadline is already earlier than d, +// WithDeadline(parent, d) is semantically equivalent to parent. The returned +// context's Done channel is closed when the deadline expires, when the returned +// cancel function is called, or when the parent context's Done channel is +// closed, whichever happens first. +// +// Canceling this context releases resources associated with it, so code should +// call cancel as soon as the operations running in this Context complete. +func WithDeadline(parent Context, deadline time.Time) (Context, CancelFunc) { + if cur, ok := parent.Deadline(); ok && cur.Before(deadline) { + // The current deadline is already sooner than the new one. + return WithCancel(parent) + } + c := &timerCtx{ + cancelCtx: newCancelCtx(parent), + deadline: deadline, + } + propagateCancel(parent, c) + d := deadline.Sub(time.Now()) + if d <= 0 { + c.cancel(true, DeadlineExceeded) // deadline has already passed + return c, func() { c.cancel(true, Canceled) } + } + c.mu.Lock() + defer c.mu.Unlock() + if c.err == nil { + c.timer = time.AfterFunc(d, func() { + c.cancel(true, DeadlineExceeded) + }) + } + return c, func() { c.cancel(true, Canceled) } +} + +// A timerCtx carries a timer and a deadline. It embeds a cancelCtx to +// implement Done and Err. It implements cancel by stopping its timer then +// delegating to cancelCtx.cancel. +type timerCtx struct { + cancelCtx + timer *time.Timer // Under cancelCtx.mu. + + deadline time.Time +} + +func (c *timerCtx) Deadline() (deadline time.Time, ok bool) { + return c.deadline, true +} + +func (c *timerCtx) String() string { + return fmt.Sprintf("%v.WithDeadline(%s [%s])", c.cancelCtx.Context, c.deadline, c.deadline.Sub(time.Now())) +} + +func (c *timerCtx) cancel(removeFromParent bool, err error) { + c.cancelCtx.cancel(false, err) + if removeFromParent { + // Remove this timerCtx from its parent cancelCtx's children. + removeChild(c.cancelCtx.Context, c) + } + c.mu.Lock() + if c.timer != nil { + c.timer.Stop() + c.timer = nil + } + c.mu.Unlock() +} + +// WithTimeout returns WithDeadline(parent, time.Now().Add(timeout)). +// +// Canceling this context releases resources associated with it, so code should +// call cancel as soon as the operations running in this Context complete: +// +// func slowOperationWithTimeout(ctx context.Context) (Result, error) { +// ctx, cancel := context.WithTimeout(ctx, 100*time.Millisecond) +// defer cancel() // releases resources if slowOperation completes before timeout elapses +// return slowOperation(ctx) +// } +func WithTimeout(parent Context, timeout time.Duration) (Context, CancelFunc) { + return WithDeadline(parent, time.Now().Add(timeout)) +} + +// WithValue returns a copy of parent in which the value associated with key is +// val. +// +// Use context Values only for request-scoped data that transits processes and +// APIs, not for passing optional parameters to functions. +func WithValue(parent Context, key interface{}, val interface{}) Context { + return &valueCtx{parent, key, val} +} + +// A valueCtx carries a key-value pair. It implements Value for that key and +// delegates all other calls to the embedded Context. +type valueCtx struct { + Context + key, val interface{} +} + +func (c *valueCtx) String() string { + return fmt.Sprintf("%v.WithValue(%#v, %#v)", c.Context, c.key, c.val) +} + +func (c *valueCtx) Value(key interface{}) interface{} { + if c.key == key { + return c.val + } + return c.Context.Value(key) +} diff --git a/Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/cancelreq.go b/Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/cancelreq.go new file mode 100644 index 000000000..48610e362 --- /dev/null +++ b/Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/cancelreq.go @@ -0,0 +1,18 @@ +// Copyright 2015 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// +build go1.5 + +package ctxhttp + +import "net/http" + +func canceler(client *http.Client, req *http.Request) func() { + ch := make(chan struct{}) + req.Cancel = ch + + return func() { + close(ch) + } +} diff --git a/Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/cancelreq_go14.go b/Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/cancelreq_go14.go new file mode 100644 index 000000000..56bcbadb8 --- /dev/null +++ b/Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/cancelreq_go14.go @@ -0,0 +1,23 @@ +// Copyright 2015 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// +build !go1.5 + +package ctxhttp + +import "net/http" + +type requestCanceler interface { + CancelRequest(*http.Request) +} + +func canceler(client *http.Client, req *http.Request) func() { + rc, ok := client.Transport.(requestCanceler) + if !ok { + return func() {} + } + return func() { + rc.CancelRequest(req) + } +} diff --git a/Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/ctxhttp.go b/Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/ctxhttp.go new file mode 100644 index 000000000..fcbd73b8a --- /dev/null +++ b/Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/ctxhttp.go @@ -0,0 +1,79 @@ +// Copyright 2015 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package ctxhttp provides helper functions for performing context-aware HTTP requests. +package ctxhttp + +import ( + "io" + "net/http" + "net/url" + "strings" + + "github.com/letsencrypt/boulder/Godeps/_workspace/src/golang.org/x/net/context" +) + +// Do sends an HTTP request with the provided http.Client and returns an HTTP response. +// If the client is nil, http.DefaultClient is used. +// If the context is canceled or times out, ctx.Err() will be returned. +func Do(ctx context.Context, client *http.Client, req *http.Request) (*http.Response, error) { + if client == nil { + client = http.DefaultClient + } + + // Request cancelation changed in Go 1.5, see cancelreq.go and cancelreq_go14.go. + cancel := canceler(client, req) + + type responseAndError struct { + resp *http.Response + err error + } + result := make(chan responseAndError, 1) + + go func() { + resp, err := client.Do(req) + result <- responseAndError{resp, err} + }() + + select { + case <-ctx.Done(): + cancel() + return nil, ctx.Err() + case r := <-result: + return r.resp, r.err + } +} + +// Get issues a GET request via the Do function. +func Get(ctx context.Context, client *http.Client, url string) (*http.Response, error) { + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return nil, err + } + return Do(ctx, client, req) +} + +// Head issues a HEAD request via the Do function. +func Head(ctx context.Context, client *http.Client, url string) (*http.Response, error) { + req, err := http.NewRequest("HEAD", url, nil) + if err != nil { + return nil, err + } + return Do(ctx, client, req) +} + +// Post issues a POST request via the Do function. +func Post(ctx context.Context, client *http.Client, url string, bodyType string, body io.Reader) (*http.Response, error) { + req, err := http.NewRequest("POST", url, body) + if err != nil { + return nil, err + } + req.Header.Set("Content-Type", bodyType) + return Do(ctx, client, req) +} + +// PostForm issues a POST request via the Do function. +func PostForm(ctx context.Context, client *http.Client, url string, data url.Values) (*http.Response, error) { + return Post(ctx, client, url, "application/x-www-form-urlencoded", strings.NewReader(data.Encode())) +} diff --git a/bdns/dns.go b/bdns/dns.go index ba7e82be2..d86497c72 100644 --- a/bdns/dns.go +++ b/bdns/dns.go @@ -12,7 +12,9 @@ import ( "strings" "time" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/miekg/dns" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/golang.org/x/net/context" "github.com/letsencrypt/boulder/metrics" ) @@ -114,19 +116,21 @@ var ( } ) -// DNSResolver defines methods used for DNS resolution +// DNSResolver queries for DNS records type DNSResolver interface { - LookupTXT(string) ([]string, error) - LookupHost(string) ([]net.IP, error) - LookupCAA(string) ([]*dns.CAA, error) - LookupMX(string) ([]string, error) + LookupTXT(context.Context, string) ([]string, error) + LookupHost(context.Context, string) ([]net.IP, error) + LookupCAA(context.Context, string) ([]*dns.CAA, error) + LookupMX(context.Context, string) ([]string, error) } // DNSResolverImpl represents a client that talks to an external resolver type DNSResolverImpl struct { - DNSClient *dns.Client + DNSClient exchanger Servers []string allowRestrictedAddresses bool + maxTries int + clk clock.Clock stats metrics.Scope txtStats metrics.Scope aStats metrics.Scope @@ -136,9 +140,14 @@ type DNSResolverImpl struct { var _ DNSResolver = &DNSResolverImpl{} +type exchanger interface { + Exchange(m *dns.Msg, a string) (*dns.Msg, time.Duration, error) +} + // NewDNSResolverImpl constructs a new DNS resolver object that utilizes the // provided list of DNS servers for resolution. -func NewDNSResolverImpl(readTimeout time.Duration, servers []string, stats metrics.Scope) *DNSResolverImpl { +func NewDNSResolverImpl(readTimeout time.Duration, servers []string, stats metrics.Scope, clk clock.Clock, maxTries int) *DNSResolverImpl { + // TODO(jmhodges): make constructor use an Option func pattern dnsClient := new(dns.Client) // Set timeout for underlying net.Conn @@ -149,19 +158,21 @@ func NewDNSResolverImpl(readTimeout time.Duration, servers []string, stats metri DNSClient: dnsClient, Servers: servers, allowRestrictedAddresses: false, - stats: stats, - txtStats: stats.NewScope("TXT"), - aStats: stats.NewScope("A"), - caaStats: stats.NewScope("CAA"), - mxStats: stats.NewScope("MX"), + maxTries: maxTries, + clk: clk, + stats: stats, + txtStats: stats.NewScope("TXT"), + aStats: stats.NewScope("A"), + caaStats: stats.NewScope("CAA"), + mxStats: stats.NewScope("MX"), } } // NewTestDNSResolverImpl constructs a new DNS resolver object that utilizes the // provided list of DNS servers for resolution and will allow loopback addresses. // This constructor should *only* be called from tests (unit or integration). -func NewTestDNSResolverImpl(readTimeout time.Duration, servers []string, stats metrics.Scope) *DNSResolverImpl { - resolver := NewDNSResolverImpl(readTimeout, servers, stats) +func NewTestDNSResolverImpl(readTimeout time.Duration, servers []string, stats metrics.Scope, clk clock.Clock, maxTries int) *DNSResolverImpl { + resolver := NewDNSResolverImpl(readTimeout, servers, stats, clk, maxTries) resolver.allowRestrictedAddresses = true return resolver } @@ -170,7 +181,7 @@ func NewTestDNSResolverImpl(readTimeout time.Duration, servers []string, stats m // out of the server list, returning the response, time, and error (if any). // This method sets the DNSSEC OK bit on the message to true before sending // it to the resolver in case validation isn't the resolvers default behaviour. -func (dnsResolver *DNSResolverImpl) exchangeOne(hostname string, qtype uint16, msgStats metrics.Scope) (rsp *dns.Msg, err error) { +func (dnsResolver *DNSResolverImpl) exchangeOne(ctx context.Context, hostname string, qtype uint16, msgStats metrics.Scope) (*dns.Msg, error) { m := new(dns.Msg) // Set question type m.SetQuestion(dns.Fqdn(hostname), qtype) @@ -178,8 +189,7 @@ func (dnsResolver *DNSResolverImpl) exchangeOne(hostname string, qtype uint16, m m.SetEdns0(4096, true) if len(dnsResolver.Servers) < 1 { - err = fmt.Errorf("Not configured with at least one DNS Server") - return + return nil, fmt.Errorf("Not configured with at least one DNS Server") } dnsResolver.stats.Inc("Rate", 1) @@ -187,21 +197,58 @@ func (dnsResolver *DNSResolverImpl) exchangeOne(hostname string, qtype uint16, m // Randomly pick a server chosenServer := dnsResolver.Servers[rand.Intn(len(dnsResolver.Servers))] - msg, rtt, err := dnsResolver.DNSClient.Exchange(m, chosenServer) - msgStats.TimingDuration("RTT", rtt) - if err == nil { - msgStats.Inc("Successes", 1) - } else { - msgStats.Inc("Errors", 1) + client := dnsResolver.DNSClient + + tries := 1 + start := dnsResolver.clk.Now() + msgStats.Inc("Calls", 1) + defer msgStats.TimingDuration("Latency", dnsResolver.clk.Now().Sub(start)) + for { + msgStats.Inc("Tries", 1) + ch := make(chan dnsResp, 1) + + go func() { + rsp, rtt, err := client.Exchange(m, chosenServer) + msgStats.TimingDuration("SingleTryLatency", rtt) + ch <- dnsResp{m: rsp, err: err} + }() + select { + case <-ctx.Done(): + msgStats.Inc("Cancels", 1) + msgStats.Inc("Errors", 1) + return nil, ctx.Err() + case r := <-ch: + if r.err != nil { + msgStats.Inc("Errors", 1) + operr, ok := r.err.(*net.OpError) + isRetryable := ok && operr.Temporary() + hasRetriesLeft := tries < dnsResolver.maxTries + if isRetryable && hasRetriesLeft { + tries++ + continue + } else if isRetryable && !hasRetriesLeft { + msgStats.Inc("RanOutOfTries", 1) + } + } else { + msgStats.Inc("Successes", 1) + } + return r.m, r.err + } } - return msg, err } -// LookupTXT sends a DNS query to find all TXT records associated with -// the provided hostname. -func (dnsResolver *DNSResolverImpl) LookupTXT(hostname string) ([]string, error) { +type dnsResp struct { + m *dns.Msg + err error +} + +// LookupTXT sends a DNS query to find all TXT records associated with the +// provided hostname. It will retry requests in the case of temporary network +// errors. It can return net package, context.Canceled, and +// context.DeadlineExceeded errors. +func (dnsResolver *DNSResolverImpl) LookupTXT(ctx context.Context, hostname string) ([]string, error) { var txt []string - r, err := dnsResolver.exchangeOne(hostname, dns.TypeTXT, dnsResolver.txtStats) + r, err := dnsResolver.exchangeOne(ctx, hostname, dns.TypeTXT, dnsResolver.txtStats) if err != nil { return nil, err } @@ -230,13 +277,15 @@ func isPrivateV4(ip net.IP) bool { return false } -// LookupHost sends a DNS query to find all A records associated with the provided -// hostname. This method assumes that the external resolver will chase CNAME/DNAME -// aliases and return relevant A records. -func (dnsResolver *DNSResolverImpl) LookupHost(hostname string) ([]net.IP, error) { +// LookupHost sends a DNS query to find all A records associated with the +// provided hostname. This method assumes that the external resolver will chase +// CNAME/DNAME aliases and return relevant A records. It will retry requests in +// the case of temporary network errors. It can return net package, +// context.Canceled, and context.DeadlineExceeded errors. +func (dnsResolver *DNSResolverImpl) LookupHost(ctx context.Context, hostname string) ([]net.IP, error) { var addrs []net.IP - r, err := dnsResolver.exchangeOne(hostname, dns.TypeA, dnsResolver.aStats) + r, err := dnsResolver.exchangeOne(ctx, hostname, dns.TypeA, dnsResolver.aStats) if err != nil { return addrs, err } @@ -256,11 +305,13 @@ func (dnsResolver *DNSResolverImpl) LookupHost(hostname string) ([]net.IP, error return addrs, nil } -// LookupCAA sends a DNS query to find all CAA records associated with -// the provided hostname. If the response code from the resolver is -// SERVFAIL an empty slice of CAA records is returned. -func (dnsResolver *DNSResolverImpl) LookupCAA(hostname string) ([]*dns.CAA, error) { - r, err := dnsResolver.exchangeOne(hostname, dns.TypeCAA, dnsResolver.caaStats) +// LookupCAA sends a DNS query to find all CAA records associated with the +// provided hostname. If the response code from the resolver is SERVFAIL an +// empty slice of CAA records is returned. It will retry requests in the case +// of temporary network errors. It can return net package, context.Canceled, and +// context.DeadlineExceeded errors. +func (dnsResolver *DNSResolverImpl) LookupCAA(ctx context.Context, hostname string) ([]*dns.CAA, error) { + r, err := dnsResolver.exchangeOne(ctx, hostname, dns.TypeCAA, dnsResolver.caaStats) if err != nil { return nil, err } @@ -282,10 +333,12 @@ func (dnsResolver *DNSResolverImpl) LookupCAA(hostname string) ([]*dns.CAA, erro return CAAs, nil } -// LookupMX sends a DNS query to find a MX record associated hostname and returns the -// record target. -func (dnsResolver *DNSResolverImpl) LookupMX(hostname string) ([]string, error) { - r, err := dnsResolver.exchangeOne(hostname, dns.TypeMX, dnsResolver.mxStats) +// LookupMX sends a DNS query to find a MX record associated hostname and +// returns the record target. It will retry requests in the case of temporary +// network errors. It can return net package, context.Canceled, and +// context.DeadlineExceeded errors. +func (dnsResolver *DNSResolverImpl) LookupMX(ctx context.Context, hostname string) ([]string, error) { + r, err := dnsResolver.exchangeOne(ctx, hostname, dns.TypeMX, dnsResolver.mxStats) if err != nil { return nil, err } diff --git a/bdns/dns_test.go b/bdns/dns_test.go index d26ea00cf..b0fa1d0ae 100644 --- a/bdns/dns_test.go +++ b/bdns/dns_test.go @@ -6,14 +6,19 @@ package bdns import ( + "errors" "fmt" "net" "os" "strings" + "sync" "testing" "time" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/golang.org/x/net/context" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/cactus/go-statsd-client/statsd" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/miekg/dns" "github.com/letsencrypt/boulder/metrics" "github.com/letsencrypt/boulder/test" @@ -151,67 +156,67 @@ func newTestStats() metrics.Scope { var testStats = newTestStats() func TestDNSNoServers(t *testing.T) { - obj := NewTestDNSResolverImpl(time.Hour, []string{}, testStats) + obj := NewTestDNSResolverImpl(time.Hour, []string{}, testStats, clock.NewFake(), 1) - _, err := obj.LookupHost("letsencrypt.org") + _, err := obj.LookupHost(context.Background(), "letsencrypt.org") test.AssertError(t, err, "No servers") } func TestDNSOneServer(t *testing.T) { - obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats) + obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats, clock.NewFake(), 1) - _, err := obj.LookupHost("letsencrypt.org") + _, err := obj.LookupHost(context.Background(), "letsencrypt.org") test.AssertNotError(t, err, "No message") } func TestDNSDuplicateServers(t *testing.T) { - obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr, dnsLoopbackAddr}, testStats) + obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr, dnsLoopbackAddr}, testStats, clock.NewFake(), 1) - _, err := obj.LookupHost("letsencrypt.org") + _, err := obj.LookupHost(context.Background(), "letsencrypt.org") test.AssertNotError(t, err, "No message") } func TestDNSLookupsNoServer(t *testing.T) { - obj := NewTestDNSResolverImpl(time.Second*10, []string{}, testStats) + obj := NewTestDNSResolverImpl(time.Second*10, []string{}, testStats, clock.NewFake(), 1) - _, err := obj.LookupTXT("letsencrypt.org") + _, err := obj.LookupTXT(context.Background(), "letsencrypt.org") test.AssertError(t, err, "No servers") - _, err = obj.LookupHost("letsencrypt.org") + _, err = obj.LookupHost(context.Background(), "letsencrypt.org") test.AssertError(t, err, "No servers") - _, err = obj.LookupCAA("letsencrypt.org") + _, err = obj.LookupCAA(context.Background(), "letsencrypt.org") test.AssertError(t, err, "No servers") } func TestDNSServFail(t *testing.T) { - obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats) + obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats, clock.NewFake(), 1) bad := "servfail.com" - _, err := obj.LookupTXT(bad) + _, err := obj.LookupTXT(context.Background(), bad) test.AssertError(t, err, "LookupTXT didn't return an error") - _, err = obj.LookupHost(bad) + _, err = obj.LookupHost(context.Background(), bad) test.AssertError(t, err, "LookupHost didn't return an error") // CAA lookup ignores validation failures from the resolver for now // and returns an empty list of CAA records. - emptyCaa, err := obj.LookupCAA(bad) + emptyCaa, err := obj.LookupCAA(context.Background(), bad) test.Assert(t, len(emptyCaa) == 0, "Query returned non-empty list of CAA records") test.AssertNotError(t, err, "LookupCAA returned an error") } func TestDNSLookupTXT(t *testing.T) { - obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats) + obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats, clock.NewFake(), 1) - a, err := obj.LookupTXT("letsencrypt.org") + a, err := obj.LookupTXT(context.Background(), "letsencrypt.org") t.Logf("A: %v", a) test.AssertNotError(t, err, "No message") - a, err = obj.LookupTXT("split-txt.letsencrypt.org") + a, err = obj.LookupTXT(context.Background(), "split-txt.letsencrypt.org") t.Logf("A: %v ", a) test.AssertNotError(t, err, "No message") test.AssertEquals(t, len(a), 1) @@ -219,47 +224,219 @@ func TestDNSLookupTXT(t *testing.T) { } func TestDNSLookupHost(t *testing.T) { - obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats) + obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats, clock.NewFake(), 1) - ip, err := obj.LookupHost("servfail.com") + ip, err := obj.LookupHost(context.Background(), "servfail.com") t.Logf("servfail.com - IP: %s, Err: %s", ip, err) test.AssertError(t, err, "Server failure") test.Assert(t, len(ip) == 0, "Should not have IPs") - ip, err = obj.LookupHost("nonexistent.letsencrypt.org") + ip, err = obj.LookupHost(context.Background(), "nonexistent.letsencrypt.org") t.Logf("nonexistent.letsencrypt.org - IP: %s, Err: %s", ip, err) test.AssertNotError(t, err, "Not an error to not exist") test.Assert(t, len(ip) == 0, "Should not have IPs") // Single IPv4 address - ip, err = obj.LookupHost("cps.letsencrypt.org") + ip, err = obj.LookupHost(context.Background(), "cps.letsencrypt.org") t.Logf("cps.letsencrypt.org - IP: %s, Err: %s", ip, err) test.AssertNotError(t, err, "Not an error to exist") test.Assert(t, len(ip) == 1, "Should have IP") - ip, err = obj.LookupHost("cps.letsencrypt.org") + ip, err = obj.LookupHost(context.Background(), "cps.letsencrypt.org") t.Logf("cps.letsencrypt.org - IP: %s, Err: %s", ip, err) test.AssertNotError(t, err, "Not an error to exist") test.Assert(t, len(ip) == 1, "Should have IP") // No IPv6 - ip, err = obj.LookupHost("v6.letsencrypt.org") + ip, err = obj.LookupHost(context.Background(), "v6.letsencrypt.org") t.Logf("v6.letsencrypt.org - IP: %s, Err: %s", ip, err) test.AssertNotError(t, err, "Not an error to exist") test.Assert(t, len(ip) == 0, "Should not have IPs") } func TestDNSLookupCAA(t *testing.T) { - obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats) + obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats, clock.NewFake(), 1) - caas, err := obj.LookupCAA("bracewel.net") + caas, err := obj.LookupCAA(context.Background(), "bracewel.net") test.AssertNotError(t, err, "CAA lookup failed") test.Assert(t, len(caas) > 0, "Should have CAA records") - caas, err = obj.LookupCAA("nonexistent.letsencrypt.org") + caas, err = obj.LookupCAA(context.Background(), "nonexistent.letsencrypt.org") test.AssertNotError(t, err, "CAA lookup failed") test.Assert(t, len(caas) == 0, "Shouldn't have CAA records") - caas, err = obj.LookupCAA("cname.example.com") + caas, err = obj.LookupCAA(context.Background(), "cname.example.com") test.AssertNotError(t, err, "CAA lookup failed") test.Assert(t, len(caas) > 0, "Should follow CNAME to find CAA") } + +type testExchanger struct { + sync.Mutex + count int + errs []error +} + +var errTooManyRequests = errors.New("too many requests") + +func (te *testExchanger) Exchange(m *dns.Msg, a string) (*dns.Msg, time.Duration, error) { + te.Lock() + defer te.Unlock() + msg := &dns.Msg{ + MsgHdr: dns.MsgHdr{Rcode: dns.RcodeSuccess}, + } + if len(te.errs) <= te.count { + return nil, 0, errTooManyRequests + } + err := te.errs[te.count] + te.count++ + + return msg, 2 * time.Millisecond, err +} + +func TestRetry(t *testing.T) { + isTempErr := &net.OpError{Op: "read", Err: tempError(true)} + nonTempErr := &net.OpError{Op: "read", Err: tempError(false)} + type testCase struct { + maxTries int + expected int + te *testExchanger + } + tests := []*testCase{ + // The success on first try case + { + maxTries: 3, + expected: 1, + te: &testExchanger{ + errs: []error{nil}, + }, + }, + // Immediate non-OpError, error returns immediately + { + maxTries: 3, + expected: 1, + te: &testExchanger{ + errs: []error{errors.New("nope")}, + }, + }, + // Temporary err, then non-OpError stops at two tries + { + maxTries: 3, + expected: 2, + te: &testExchanger{ + errs: []error{isTempErr, errors.New("nope")}, + }, + }, + // Temporary error given always + { + maxTries: 3, + expected: 3, + te: &testExchanger{ + errs: []error{ + isTempErr, + isTempErr, + isTempErr, + }, + }, + }, + // Even with maxTries at 0, we should still let a single request go + // through + { + maxTries: 0, + expected: 1, + te: &testExchanger{ + errs: []error{nil}, + }, + }, + // Temporary error given just once causes two tries + { + maxTries: 3, + expected: 2, + te: &testExchanger{ + errs: []error{ + isTempErr, + nil, + }, + }, + }, + // Temporary error given twice causes three tries + { + maxTries: 3, + expected: 3, + te: &testExchanger{ + errs: []error{ + isTempErr, + isTempErr, + nil, + }, + }, + }, + // Temporary error given thrice causes three tries and fails + { + maxTries: 3, + expected: 3, + te: &testExchanger{ + errs: []error{ + isTempErr, + isTempErr, + isTempErr, + }, + }, + }, + // temporary then non-Temporary error causes two retries + { + maxTries: 3, + expected: 2, + te: &testExchanger{ + errs: []error{ + isTempErr, + nonTempErr, + }, + }, + }, + } + + for i, tc := range tests { + dr := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats, clock.NewFake(), tc.maxTries) + + dr.DNSClient = tc.te + _, err := dr.LookupTXT(context.Background(), "example.com") + if err == errTooManyRequests { + t.Errorf("#%d, sent more requests than the test case handles", i) + } + expectedErr := tc.te.errs[tc.expected-1] + if err != expectedErr { + t.Errorf("#%d, error, expected %v, got %v", i, expectedErr, err) + } + if tc.expected != tc.te.count { + t.Errorf("#%d, count, expected %d, got %d", i, tc.expected, tc.te.count) + } + } + + dr := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats, clock.NewFake(), 3) + dr.DNSClient = &testExchanger{errs: []error{isTempErr, isTempErr, nil}} + ctx, cancel := context.WithCancel(context.Background()) + cancel() + _, err := dr.LookupTXT(ctx, "example.com") + if err != context.Canceled { + t.Errorf("expected %s, got %s", context.Canceled, err) + } + + dr.DNSClient = &testExchanger{errs: []error{isTempErr, isTempErr, nil}} + ctx, _ = context.WithTimeout(context.Background(), -10*time.Hour) + _, err = dr.LookupTXT(ctx, "example.com") + if err != context.DeadlineExceeded { + t.Errorf("expected %s, got %s", context.DeadlineExceeded, err) + } + + dr.DNSClient = &testExchanger{errs: []error{isTempErr, isTempErr, nil}} + ctx, deadlineCancel := context.WithTimeout(context.Background(), -10*time.Hour) + deadlineCancel() + _, err = dr.LookupTXT(ctx, "example.com") + if err != context.DeadlineExceeded { + t.Errorf("expected %s, got %s", context.DeadlineExceeded, err) + } +} + +type tempError bool + +func (t tempError) Temporary() bool { return bool(t) } +func (t tempError) Error() string { return fmt.Sprintf("Temporary: %t", t) } diff --git a/cmd/boulder-ra/main.go b/cmd/boulder-ra/main.go index a5f51e201..f64940157 100644 --- a/cmd/boulder-ra/main.go +++ b/cmd/boulder-ra/main.go @@ -64,10 +64,14 @@ func main() { raDNSTimeout, err := time.ParseDuration(c.Common.DNSTimeout) cmd.FailOnError(err, "Couldn't parse RA DNS timeout") scoped := metrics.NewStatsdScope(stats, "RA", "DNS") + dnsTries := c.RA.DNSTries + if dnsTries < 1 { + dnsTries = 1 + } if !c.Common.DNSAllowLoopbackAddresses { - rai.DNSResolver = bdns.NewDNSResolverImpl(raDNSTimeout, []string{c.Common.DNSResolver}, scoped) + rai.DNSResolver = bdns.NewDNSResolverImpl(raDNSTimeout, []string{c.Common.DNSResolver}, scoped, clock.Default(), dnsTries) } else { - rai.DNSResolver = bdns.NewTestDNSResolverImpl(raDNSTimeout, []string{c.Common.DNSResolver}, scoped) + rai.DNSResolver = bdns.NewTestDNSResolverImpl(raDNSTimeout, []string{c.Common.DNSResolver}, scoped, clock.Default(), dnsTries) } rai.VA = vac diff --git a/cmd/boulder-va/main.go b/cmd/boulder-va/main.go index 4d1f33203..a50354ba0 100644 --- a/cmd/boulder-va/main.go +++ b/cmd/boulder-va/main.go @@ -42,15 +42,20 @@ func main() { if c.VA.PortConfig.TLSPort != 0 { pc.TLSPort = c.VA.PortConfig.TLSPort } + clk := clock.Default() sbc := newGoogleSafeBrowsing(c.VA.GoogleSafeBrowsing) - vai := va.NewValidationAuthorityImpl(pc, sbc, stats, clock.Default()) + vai := va.NewValidationAuthorityImpl(pc, sbc, stats, clk) dnsTimeout, err := time.ParseDuration(c.Common.DNSTimeout) cmd.FailOnError(err, "Couldn't parse DNS timeout") scoped := metrics.NewStatsdScope(stats, "VA", "DNS") + dnsTries := c.VA.DNSTries + if dnsTries < 1 { + dnsTries = 1 + } if !c.Common.DNSAllowLoopbackAddresses { - vai.DNSResolver = bdns.NewDNSResolverImpl(dnsTimeout, []string{c.Common.DNSResolver}, scoped) + vai.DNSResolver = bdns.NewDNSResolverImpl(dnsTimeout, []string{c.Common.DNSResolver}, scoped, clk, dnsTries) } else { - vai.DNSResolver = bdns.NewTestDNSResolverImpl(dnsTimeout, []string{c.Common.DNSResolver}, scoped) + vai.DNSResolver = bdns.NewTestDNSResolverImpl(dnsTimeout, []string{c.Common.DNSResolver}, scoped, clk, dnsTries) } vai.UserAgent = c.VA.UserAgent vai.IssuerDomain = c.VA.IssuerDomain diff --git a/cmd/config.go b/cmd/config.go index 2d29a6b55..fc3fc9d88 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -62,6 +62,11 @@ type Config struct { // UseIsSafeDomain determines whether to call VA.IsSafeDomain UseIsSafeDomain bool // TODO(jmhodges): remove after va IsSafeDomain deploy + + // The number of times to try a DNS query (that has a temporary error) + // before giving up. May be short-circuited by deadlines. A zero value + // will be turned into 1. + DNSTries int } SA struct { @@ -83,6 +88,11 @@ type Config struct { MaxConcurrentRPCServerRequests int64 GoogleSafeBrowsing *GoogleSafeBrowsingConfig + + // The number of times to try a DNS query (that has a temporary error) + // before giving up. May be short-circuited by deadlines. A zero value + // will be turned into 1. + DNSTries int } SQL struct { diff --git a/mocks/mocks.go b/mocks/mocks.go index 860453a23..160785f5b 100644 --- a/mocks/mocks.go +++ b/mocks/mocks.go @@ -24,6 +24,7 @@ import ( "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/letsencrypt/go-jose" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/miekg/dns" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/golang.org/x/net/context" "github.com/letsencrypt/boulder/core" ) @@ -33,7 +34,7 @@ type DNSResolver struct { } // LookupTXT is a mock -func (mock *DNSResolver) LookupTXT(hostname string) ([]string, error) { +func (mock *DNSResolver) LookupTXT(ctx context.Context, hostname string) ([]string, error) { if hostname == "_acme-challenge.servfail.com" { return nil, fmt.Errorf("SERVFAIL") } @@ -66,7 +67,7 @@ func (t timeoutError) Timeout() bool { // // Note: see comments on LookupMX regarding email.only // -func (mock *DNSResolver) LookupHost(hostname string) ([]net.IP, error) { +func (mock *DNSResolver) LookupHost(ctx context.Context, hostname string) ([]net.IP, error) { if hostname == "always.invalid" || hostname == "invalid.invalid" || hostname == "email.only" { @@ -85,7 +86,7 @@ func (mock *DNSResolver) LookupHost(hostname string) ([]net.IP, error) { } // LookupCAA is a mock -func (mock *DNSResolver) LookupCAA(domain string) ([]*dns.CAA, error) { +func (mock *DNSResolver) LookupCAA(ctx context.Context, domain string) ([]*dns.CAA, error) { var results []*dns.CAA var record dns.CAA switch strings.TrimRight(domain, ".") { @@ -121,7 +122,7 @@ func (mock *DNSResolver) LookupCAA(domain string) ([]*dns.CAA, error) { // all domains except for special cases, so MX-only domains must be // handled in both LookupHost and LookupMX. // -func (mock *DNSResolver) LookupMX(domain string) ([]string, error) { +func (mock *DNSResolver) LookupMX(ctx context.Context, domain string) ([]string, error) { switch strings.TrimRight(domain, ".") { case "letsencrypt.org": fallthrough diff --git a/ra/registration-authority.go b/ra/registration-authority.go index 455c0c5ac..6654580b9 100644 --- a/ra/registration-authority.go +++ b/ra/registration-authority.go @@ -20,6 +20,7 @@ import ( "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/cactus/go-statsd-client/statsd" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/letsencrypt/net/publicsuffix" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/golang.org/x/net/context" "github.com/letsencrypt/boulder/probs" "github.com/letsencrypt/boulder/bdns" @@ -84,7 +85,7 @@ const ( emptyDNSResponseDetail = "empty DNS response" ) -func validateEmail(address string, resolver bdns.DNSResolver) (prob *probs.ProblemDetails) { +func validateEmail(ctx context.Context, address string, resolver bdns.DNSResolver) (prob *probs.ProblemDetails) { _, err := mail.ParseAddress(address) if err != nil { return &probs.ProblemDetails{ @@ -96,10 +97,10 @@ func validateEmail(address string, resolver bdns.DNSResolver) (prob *probs.Probl domain := strings.ToLower(splitEmail[len(splitEmail)-1]) var resultMX []string var resultA []net.IP - resultMX, err = resolver.LookupMX(domain) + resultMX, err = resolver.LookupMX(ctx, domain) recQ := "MX" if err == nil && len(resultMX) == 0 { - resultA, err = resolver.LookupHost(domain) + resultA, err = resolver.LookupHost(ctx, domain) recQ = "A" if err == nil && len(resultA) == 0 { return &probs.ProblemDetails{ @@ -209,7 +210,8 @@ func (ra *RegistrationAuthorityImpl) NewRegistration(init core.Registration) (re // MergeUpdate. But we need to fill it in for new registrations. reg.InitialIP = init.InitialIP - err = ra.validateContacts(reg.Contact) + // TODO(#1292): add a proper deadline here + err = ra.validateContacts(context.TODO(), reg.Contact) if err != nil { return } @@ -226,7 +228,7 @@ func (ra *RegistrationAuthorityImpl) NewRegistration(init core.Registration) (re return } -func (ra *RegistrationAuthorityImpl) validateContacts(contacts []*core.AcmeURL) (err error) { +func (ra *RegistrationAuthorityImpl) validateContacts(ctx context.Context, contacts []*core.AcmeURL) (err error) { if ra.maxContactsPerReg > 0 && len(contacts) > ra.maxContactsPerReg { return core.MalformedRequestError(fmt.Sprintf("Too many contacts provided: %d > %d", len(contacts), ra.maxContactsPerReg)) @@ -242,7 +244,7 @@ func (ra *RegistrationAuthorityImpl) validateContacts(contacts []*core.AcmeURL) case "mailto": start := ra.clk.Now() ra.stats.Inc("RA.ValidateEmail.Calls", 1, 1.0) - problem := validateEmail(contact.Opaque, ra.DNSResolver) + problem := validateEmail(ctx, contact.Opaque, ra.DNSResolver) ra.stats.TimingDuration("RA.ValidateEmail.Latency", ra.clk.Now().Sub(start), 1.0) if problem != nil { ra.stats.Inc("RA.ValidateEmail.Errors", 1, 1.0) @@ -638,7 +640,8 @@ func (ra *RegistrationAuthorityImpl) checkLimits(names []string, regID int64) er func (ra *RegistrationAuthorityImpl) UpdateRegistration(base core.Registration, update core.Registration) (reg core.Registration, err error) { base.MergeUpdate(update) - err = ra.validateContacts(base.Contact) + // TODO(#1292): add a proper deadline here + err = ra.validateContacts(context.TODO(), base.Contact) if err != nil { return } diff --git a/ra/registration-authority_test.go b/ra/registration-authority_test.go index e81c7d90e..2678d6117 100644 --- a/ra/registration-authority_test.go +++ b/ra/registration-authority_test.go @@ -22,6 +22,7 @@ import ( cfsslConfig "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/cloudflare/cfssl/config" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" jose "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/letsencrypt/go-jose" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/golang.org/x/net/context" "github.com/letsencrypt/boulder/ca" "github.com/letsencrypt/boulder/cmd" "github.com/letsencrypt/boulder/core" @@ -288,25 +289,25 @@ func TestValidateContacts(t *testing.T) { validEmail, _ := core.ParseAcmeURL("mailto:admin@email.com") malformedEmail, _ := core.ParseAcmeURL("mailto:admin.com") - err := ra.validateContacts([]*core.AcmeURL{}) + err := ra.validateContacts(context.Background(), []*core.AcmeURL{}) test.AssertNotError(t, err, "No Contacts") - err = ra.validateContacts([]*core.AcmeURL{tel, validEmail}) + err = ra.validateContacts(context.Background(), []*core.AcmeURL{tel, validEmail}) test.AssertError(t, err, "Too Many Contacts") - err = ra.validateContacts([]*core.AcmeURL{tel}) + err = ra.validateContacts(context.Background(), []*core.AcmeURL{tel}) test.AssertNotError(t, err, "Simple Telephone") - err = ra.validateContacts([]*core.AcmeURL{validEmail}) + err = ra.validateContacts(context.Background(), []*core.AcmeURL{validEmail}) test.AssertNotError(t, err, "Valid Email") - err = ra.validateContacts([]*core.AcmeURL{malformedEmail}) + err = ra.validateContacts(context.Background(), []*core.AcmeURL{malformedEmail}) test.AssertError(t, err, "Malformed Email") - err = ra.validateContacts([]*core.AcmeURL{ansible}) + err = ra.validateContacts(context.Background(), []*core.AcmeURL{ansible}) test.AssertError(t, err, "Unknown scheme") - err = ra.validateContacts([]*core.AcmeURL{nil}) + err = ra.validateContacts(context.Background(), []*core.AcmeURL{nil}) test.AssertError(t, err, "Nil AcmeURL") } @@ -324,8 +325,9 @@ func TestValidateEmail(t *testing.T) { "a@email.com", "b@email.only", } + for _, tc := range testFailures { - problem := validateEmail(tc.input, &mocks.DNSResolver{}) + problem := validateEmail(context.Background(), tc.input, &mocks.DNSResolver{}) if problem.Type != probs.InvalidEmailProblem { t.Errorf("validateEmail(%q): got problem type %#v, expected %#v", tc.input, problem.Type, probs.InvalidEmailProblem) } @@ -336,7 +338,7 @@ func TestValidateEmail(t *testing.T) { } for _, addr := range testSuccesses { - if prob := validateEmail(addr, &mocks.DNSResolver{}); prob != nil { + if prob := validateEmail(context.Background(), addr, &mocks.DNSResolver{}); prob != nil { t.Errorf("validateEmail(%q): expected success, but it failed: %s", addr, prob) } diff --git a/test/boulder-config.json b/test/boulder-config.json index ebc1bf68a..72d802d89 100644 --- a/test/boulder-config.json +++ b/test/boulder-config.json @@ -123,6 +123,7 @@ "rateLimitPoliciesFilename": "test/rate-limit-policies.yml", "maxConcurrentRPCServerRequests": 16, "maxContactsPerRegistration": 100, + "dnsTries": 3, "debugAddr": "localhost:8002", "amqp": { "serverURLFile": "test/secrets/amqp_url", @@ -165,6 +166,7 @@ "tlsPort": 5001 }, "maxConcurrentRPCServerRequests": 16, + "dnsTries": 3, "amqp": { "serverURLFile": "test/secrets/amqp_url", "insecure": true, diff --git a/va/validation-authority.go b/va/validation-authority.go index 2007fcade..9bc4f0177 100644 --- a/va/validation-authority.go +++ b/va/validation-authority.go @@ -24,6 +24,7 @@ import ( "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/letsencrypt/net/publicsuffix" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/miekg/dns" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/golang.org/x/net/context" "github.com/letsencrypt/boulder/probs" "github.com/letsencrypt/boulder/bdns" @@ -89,8 +90,8 @@ type verificationRequestEvent struct { // This is the same choice made by the Go internal resolution library used by // net/http, except we only send A queries and accept IPv4 addresses. // TODO(#593): Add IPv6 support -func (va ValidationAuthorityImpl) getAddr(hostname string) (net.IP, []net.IP, *probs.ProblemDetails) { - addrs, err := va.DNSResolver.LookupHost(hostname) +func (va ValidationAuthorityImpl) getAddr(ctx context.Context, hostname string) (net.IP, []net.IP, *probs.ProblemDetails) { + addrs, err := va.DNSResolver.LookupHost(ctx, hostname) if err != nil { va.log.Debug(fmt.Sprintf("%s DNS failure: %s", hostname, err)) problem := bdns.ProblemDetailsFromDNSError("A", hostname, err) @@ -120,7 +121,7 @@ func (d *dialer) Dial(_, _ string) (net.Conn, error) { // resolveAndConstructDialer gets the prefered address using va.getAddr and returns // the chosen address and dialer for that address and correct port. -func (va *ValidationAuthorityImpl) resolveAndConstructDialer(name string, port int) (dialer, *probs.ProblemDetails) { +func (va *ValidationAuthorityImpl) resolveAndConstructDialer(ctx context.Context, name string, port int) (dialer, *probs.ProblemDetails) { d := dialer{ record: core.ValidationRecord{ Hostname: name, @@ -128,7 +129,7 @@ func (va *ValidationAuthorityImpl) resolveAndConstructDialer(name string, port i }, } - addr, allAddrs, err := va.getAddr(name) + addr, allAddrs, err := va.getAddr(ctx, name) if err != nil { return d, err } @@ -139,7 +140,7 @@ func (va *ValidationAuthorityImpl) resolveAndConstructDialer(name string, port i // Validation methods -func (va *ValidationAuthorityImpl) fetchHTTP(identifier core.AcmeIdentifier, path string, useTLS bool, input core.Challenge) ([]byte, []core.ValidationRecord, *probs.ProblemDetails) { +func (va *ValidationAuthorityImpl) fetchHTTP(ctx context.Context, identifier core.AcmeIdentifier, path string, useTLS bool, input core.Challenge) ([]byte, []core.ValidationRecord, *probs.ProblemDetails) { challenge := input host := identifier.Value @@ -177,7 +178,7 @@ func (va *ValidationAuthorityImpl) fetchHTTP(identifier core.AcmeIdentifier, pat httpRequest.Header["User-Agent"] = []string{va.UserAgent} } - dialer, prob := va.resolveAndConstructDialer(host, port) + dialer, prob := va.resolveAndConstructDialer(ctx, host, port) dialer.record.URL = url.String() validationRecords := []core.ValidationRecord{dialer.record} if prob != nil { @@ -236,7 +237,7 @@ func (va *ValidationAuthorityImpl) fetchHTTP(identifier core.AcmeIdentifier, pat reqPort = 80 } - dialer, err := va.resolveAndConstructDialer(reqHost, reqPort) + dialer, err := va.resolveAndConstructDialer(ctx, reqHost, reqPort) dialer.record.URL = req.URL.String() validationRecords = append(validationRecords, dialer.record) if err != nil { @@ -279,8 +280,8 @@ func (va *ValidationAuthorityImpl) fetchHTTP(identifier core.AcmeIdentifier, pat return body, validationRecords, nil } -func (va *ValidationAuthorityImpl) validateTLSWithZName(identifier core.AcmeIdentifier, challenge core.Challenge, zName string) ([]core.ValidationRecord, *probs.ProblemDetails) { - addr, allAddrs, problem := va.getAddr(identifier.Value) +func (va *ValidationAuthorityImpl) validateTLSWithZName(ctx context.Context, identifier core.AcmeIdentifier, challenge core.Challenge, zName string) ([]core.ValidationRecord, *probs.ProblemDetails) { + addr, allAddrs, problem := va.getAddr(ctx, identifier.Value) validationRecords := []core.ValidationRecord{ core.ValidationRecord{ Hostname: identifier.Value, @@ -332,7 +333,7 @@ func (va *ValidationAuthorityImpl) validateTLSWithZName(identifier core.AcmeIden } } -func (va *ValidationAuthorityImpl) validateHTTP01(identifier core.AcmeIdentifier, challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) { +func (va *ValidationAuthorityImpl) validateHTTP01(ctx context.Context, identifier core.AcmeIdentifier, challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) { if identifier.Type != core.IdentifierDNS { va.log.Debug(fmt.Sprintf("%s [%s] Identifier failure", challenge.Type, identifier)) return nil, &probs.ProblemDetails{ @@ -343,7 +344,7 @@ func (va *ValidationAuthorityImpl) validateHTTP01(identifier core.AcmeIdentifier // Perform the fetch path := fmt.Sprintf(".well-known/acme-challenge/%s", challenge.Token) - body, validationRecords, err := va.fetchHTTP(identifier, path, false, challenge) + body, validationRecords, err := va.fetchHTTP(ctx, identifier, path, false, challenge) if err != nil { return validationRecords, err } @@ -374,7 +375,7 @@ func (va *ValidationAuthorityImpl) validateHTTP01(identifier core.AcmeIdentifier return validationRecords, nil } -func (va *ValidationAuthorityImpl) validateTLSSNI01(identifier core.AcmeIdentifier, challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) { +func (va *ValidationAuthorityImpl) validateTLSSNI01(ctx context.Context, identifier core.AcmeIdentifier, challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) { if identifier.Type != "dns" { va.log.Debug(fmt.Sprintf("TLS-SNI [%s] Identifier failure", identifier)) return nil, &probs.ProblemDetails{ @@ -389,7 +390,7 @@ func (va *ValidationAuthorityImpl) validateTLSSNI01(identifier core.AcmeIdentifi Z := hex.EncodeToString(h.Sum(nil)) ZName := fmt.Sprintf("%s.%s.%s", Z[:32], Z[32:], core.TLSSNISuffix) - return va.validateTLSWithZName(identifier, challenge, ZName) + return va.validateTLSWithZName(ctx, identifier, challenge, ZName) } // parseHTTPConnError returns the ACME ProblemType corresponding to an error @@ -414,7 +415,7 @@ func parseHTTPConnError(err error) probs.ProblemType { return probs.ConnectionProblem } -func (va *ValidationAuthorityImpl) validateDNS01(identifier core.AcmeIdentifier, challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) { +func (va *ValidationAuthorityImpl) validateDNS01(ctx context.Context, identifier core.AcmeIdentifier, challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) { if identifier.Type != core.IdentifierDNS { va.log.Debug(fmt.Sprintf("DNS [%s] Identifier failure", identifier)) return nil, &probs.ProblemDetails{ @@ -430,7 +431,7 @@ func (va *ValidationAuthorityImpl) validateDNS01(identifier core.AcmeIdentifier, // Look for the required record in the DNS challengeSubdomain := fmt.Sprintf("%s.%s", core.DNSPrefix, identifier.Value) - txts, err := va.DNSResolver.LookupTXT(challengeSubdomain) + txts, err := va.DNSResolver.LookupTXT(ctx, challengeSubdomain) if err != nil { va.log.Debug(fmt.Sprintf("%s [%s] DNS failure: %s", challenge.Type, identifier, err)) @@ -451,9 +452,9 @@ func (va *ValidationAuthorityImpl) validateDNS01(identifier core.AcmeIdentifier, } } -func (va *ValidationAuthorityImpl) checkCAA(identifier core.AcmeIdentifier, regID int64) *probs.ProblemDetails { +func (va *ValidationAuthorityImpl) checkCAA(ctx context.Context, identifier core.AcmeIdentifier, regID int64) *probs.ProblemDetails { // Check CAA records for the requested identifier - present, valid, err := va.CheckCAARecords(identifier) + present, valid, err := va.checkCAARecords(ctx, identifier) if err != nil { va.log.Warning(fmt.Sprintf("Problem checking CAA: %s", err)) return bdns.ProblemDetailsFromDNSError("CAA", identifier.Value, err) @@ -471,7 +472,7 @@ func (va *ValidationAuthorityImpl) checkCAA(identifier core.AcmeIdentifier, regI // Overall validation process -func (va *ValidationAuthorityImpl) validate(authz core.Authorization, challengeIndex int) { +func (va *ValidationAuthorityImpl) validate(ctx context.Context, authz core.Authorization, challengeIndex int) { logEvent := verificationRequestEvent{ ID: authz.ID, Requester: authz.RegistrationID, @@ -479,7 +480,7 @@ func (va *ValidationAuthorityImpl) validate(authz core.Authorization, challengeI } challenge := &authz.Challenges[challengeIndex] vStart := va.clk.Now() - validationRecords, prob := va.validateChallengeAndCAA(authz.Identifier, *challenge, authz.RegistrationID) + validationRecords, prob := va.validateChallengeAndCAA(ctx, authz.Identifier, *challenge, authz.RegistrationID) va.stats.TimingDuration(fmt.Sprintf("VA.Validations.%s.%s", challenge.Type, challenge.Status), time.Since(vStart), 1.0) challenge.ValidationRecord = validationRecords @@ -505,13 +506,14 @@ func (va *ValidationAuthorityImpl) validate(authz core.Authorization, challengeI va.RA.OnValidationUpdate(authz) } -func (va *ValidationAuthorityImpl) validateChallengeAndCAA(identifier core.AcmeIdentifier, challenge core.Challenge, regID int64) ([]core.ValidationRecord, *probs.ProblemDetails) { +func (va *ValidationAuthorityImpl) validateChallengeAndCAA(ctx context.Context, identifier core.AcmeIdentifier, challenge core.Challenge, regID int64) ([]core.ValidationRecord, *probs.ProblemDetails) { ch := make(chan *probs.ProblemDetails, 1) go func() { - ch <- va.checkCAA(identifier, regID) + ch <- va.checkCAA(ctx, identifier, regID) }() - validationRecords, err := va.validateChallenge(identifier, challenge) + // TODO(#1292): send into another goroutine + validationRecords, err := va.validateChallenge(ctx, identifier, challenge) if err != nil { return validationRecords, err } @@ -523,7 +525,7 @@ func (va *ValidationAuthorityImpl) validateChallengeAndCAA(identifier core.AcmeI return validationRecords, nil } -func (va *ValidationAuthorityImpl) validateChallenge(identifier core.AcmeIdentifier, challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) { +func (va *ValidationAuthorityImpl) validateChallenge(ctx context.Context, identifier core.AcmeIdentifier, challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) { if !challenge.IsSane(true) { return nil, &probs.ProblemDetails{ Type: probs.MalformedProblem, @@ -532,11 +534,11 @@ func (va *ValidationAuthorityImpl) validateChallenge(identifier core.AcmeIdentif } switch challenge.Type { case core.ChallengeTypeHTTP01: - return va.validateHTTP01(identifier, challenge) + return va.validateHTTP01(ctx, identifier, challenge) case core.ChallengeTypeTLSSNI01: - return va.validateTLSSNI01(identifier, challenge) + return va.validateTLSSNI01(ctx, identifier, challenge) case core.ChallengeTypeDNS01: - return va.validateDNS01(identifier, challenge) + return va.validateDNS01(ctx, identifier, challenge) } return nil, &probs.ProblemDetails{ Type: probs.MalformedProblem, @@ -546,7 +548,8 @@ func (va *ValidationAuthorityImpl) validateChallenge(identifier core.AcmeIdentif // UpdateValidations runs the validate() method asynchronously using goroutines. func (va *ValidationAuthorityImpl) UpdateValidations(authz core.Authorization, challengeIndex int) error { - go va.validate(authz, challengeIndex) + // TODO(#1292): add a proper deadline here + go va.validate(context.TODO(), authz, challengeIndex) return nil } @@ -593,7 +596,7 @@ func newCAASet(CAAs []*dns.CAA) *CAASet { return &filtered } -func (va *ValidationAuthorityImpl) getCAASet(hostname string) (*CAASet, error) { +func (va *ValidationAuthorityImpl) getCAASet(ctx context.Context, hostname string) (*CAASet, error) { hostname = strings.TrimRight(hostname, ".") labels := strings.Split(hostname, ".") // See RFC 6844 "Certification Authority Processing" for pseudocode. @@ -606,7 +609,7 @@ func (va *ValidationAuthorityImpl) getCAASet(hostname string) (*CAASet, error) { if tld, err := publicsuffix.ICANNTLD(name); err != nil || tld == name { break } - CAAs, err := va.DNSResolver.LookupCAA(name) + CAAs, err := va.DNSResolver.LookupCAA(ctx, name) if err != nil { return nil, err } @@ -621,8 +624,13 @@ func (va *ValidationAuthorityImpl) getCAASet(hostname string) (*CAASet, error) { // CheckCAARecords verifies that, if the indicated subscriber domain has any CAA // records, they authorize the configured CA domain to issue a certificate func (va *ValidationAuthorityImpl) CheckCAARecords(identifier core.AcmeIdentifier) (present, valid bool, err error) { + // TODO(#1292): add a proper deadline here + return va.checkCAARecords(context.TODO(), identifier) +} + +func (va *ValidationAuthorityImpl) checkCAARecords(ctx context.Context, identifier core.AcmeIdentifier) (present, valid bool, err error) { hostname := strings.ToLower(identifier.Value) - caaSet, err := va.getCAASet(hostname) + caaSet, err := va.getCAASet(ctx, hostname) if err != nil { return } diff --git a/va/validation-authority_test.go b/va/validation-authority_test.go index 6b94a74d2..7ae3aff6c 100644 --- a/va/validation-authority_test.go +++ b/va/validation-authority_test.go @@ -28,6 +28,7 @@ import ( "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/cactus/go-statsd-client/statsd" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/letsencrypt/go-jose" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/golang.org/x/net/context" "github.com/letsencrypt/boulder/bdns" "github.com/letsencrypt/boulder/metrics" "github.com/letsencrypt/boulder/probs" @@ -223,7 +224,7 @@ func TestHTTP(t *testing.T) { va := NewValidationAuthorityImpl(&PortConfig{HTTPPort: badPort}, nil, stats, clock.Default()) va.DNSResolver = &mocks.DNSResolver{} - _, prob := va.validateHTTP01(ident, chall) + _, prob := va.validateHTTP01(context.Background(), ident, chall) if prob == nil { t.Fatalf("Server's down; expected refusal. Where did we connect?") } @@ -234,7 +235,7 @@ func TestHTTP(t *testing.T) { log.Clear() t.Logf("Trying to validate: %+v\n", chall) - _, prob = va.validateHTTP01(ident, chall) + _, prob = va.validateHTTP01(context.Background(), ident, chall) if prob != nil { t.Errorf("Unexpected failure in HTTP validation: %s", prob) } @@ -242,7 +243,7 @@ func TestHTTP(t *testing.T) { log.Clear() setChallengeToken(&chall, path404) - _, prob = va.validateHTTP01(ident, chall) + _, prob = va.validateHTTP01(context.Background(), ident, chall) if prob == nil { t.Fatalf("Should have found a 404 for the challenge.") } @@ -253,7 +254,7 @@ func TestHTTP(t *testing.T) { setChallengeToken(&chall, pathWrongToken) // The "wrong token" will actually be the expectedToken. It's wrong // because it doesn't match pathWrongToken. - _, prob = va.validateHTTP01(ident, chall) + _, prob = va.validateHTTP01(context.Background(), ident, chall) if prob == nil { t.Fatalf("Should have found the wrong token value.") } @@ -262,7 +263,7 @@ func TestHTTP(t *testing.T) { log.Clear() setChallengeToken(&chall, pathMoved) - _, prob = va.validateHTTP01(ident, chall) + _, prob = va.validateHTTP01(context.Background(), ident, chall) if prob != nil { t.Fatalf("Failed to follow 301 redirect") } @@ -270,7 +271,7 @@ func TestHTTP(t *testing.T) { log.Clear() setChallengeToken(&chall, pathFound) - _, prob = va.validateHTTP01(ident, chall) + _, prob = va.validateHTTP01(context.Background(), ident, chall) if prob != nil { t.Fatalf("Failed to follow 302 redirect") } @@ -278,13 +279,13 @@ func TestHTTP(t *testing.T) { test.AssertEquals(t, len(log.GetAllMatching(`redirect from ".*/`+pathMoved+`" to ".*/`+pathValid+`"`)), 1) ipIdentifier := core.AcmeIdentifier{Type: core.IdentifierType("ip"), Value: "127.0.0.1"} - _, prob = va.validateHTTP01(ipIdentifier, chall) + _, prob = va.validateHTTP01(context.Background(), ipIdentifier, chall) if prob == nil { t.Fatalf("IdentifierType IP shouldn't have worked.") } test.AssertEquals(t, prob.Type, probs.MalformedProblem) - _, prob = va.validateHTTP01(core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "always.invalid"}, chall) + _, prob = va.validateHTTP01(context.Background(), core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "always.invalid"}, chall) if prob == nil { t.Fatalf("Domain name is invalid.") } @@ -292,7 +293,7 @@ func TestHTTP(t *testing.T) { setChallengeToken(&chall, pathWaitLong) started := time.Now() - _, prob = va.validateHTTP01(ident, chall) + _, prob = va.validateHTTP01(context.Background(), ident, chall) took := time.Since(started) // Check that the HTTP connection times out after 5 seconds and doesn't block for 10 seconds test.Assert(t, (took > (time.Second * 5)), "HTTP timed out before 5 seconds") @@ -318,7 +319,7 @@ func TestHTTPRedirectLookup(t *testing.T) { log.Clear() setChallengeToken(&chall, pathMoved) - _, prob := va.validateHTTP01(ident, chall) + _, prob := va.validateHTTP01(context.Background(), ident, chall) if prob != nil { t.Fatalf("Unexpected failure in redirect (%s): %s", pathMoved, prob) } @@ -327,7 +328,7 @@ func TestHTTPRedirectLookup(t *testing.T) { log.Clear() setChallengeToken(&chall, pathFound) - _, prob = va.validateHTTP01(ident, chall) + _, prob = va.validateHTTP01(context.Background(), ident, chall) if prob != nil { t.Fatalf("Unexpected failure in redirect (%s): %s", pathFound, prob) } @@ -337,14 +338,14 @@ func TestHTTPRedirectLookup(t *testing.T) { log.Clear() setChallengeToken(&chall, pathReLookupInvalid) - _, err = va.validateHTTP01(ident, chall) + _, err = va.validateHTTP01(context.Background(), ident, chall) test.AssertError(t, err, chall.Token) test.AssertEquals(t, len(log.GetAllMatching(`Resolved addresses for localhost \[using 127.0.0.1\]: \[127.0.0.1\]`)), 1) test.AssertEquals(t, len(log.GetAllMatching(`No IPv4 addresses found for invalid.invalid`)), 1) log.Clear() setChallengeToken(&chall, pathReLookup) - _, prob = va.validateHTTP01(ident, chall) + _, prob = va.validateHTTP01(context.Background(), ident, chall) if prob != nil { t.Fatalf("Unexpected error in redirect (%s): %s", pathReLookup, prob) } @@ -354,7 +355,7 @@ func TestHTTPRedirectLookup(t *testing.T) { log.Clear() setChallengeToken(&chall, pathRedirectPort) - _, err = va.validateHTTP01(ident, chall) + _, err = va.validateHTTP01(context.Background(), ident, chall) test.AssertError(t, err, chall.Token) test.AssertEquals(t, len(log.GetAllMatching(`redirect from ".*/port-redirect" to ".*other.valid:8080/path"`)), 1) test.AssertEquals(t, len(log.GetAllMatching(`Resolved addresses for localhost \[using 127.0.0.1\]: \[127.0.0.1\]`)), 1) @@ -375,7 +376,7 @@ func TestHTTPRedirectLoop(t *testing.T) { va.DNSResolver = &mocks.DNSResolver{} log.Clear() - _, prob := va.validateHTTP01(ident, chall) + _, prob := va.validateHTTP01(context.Background(), ident, chall) if prob == nil { t.Fatalf("Challenge should have failed for %s", chall.Token) } @@ -396,13 +397,13 @@ func TestHTTPRedirectUserAgent(t *testing.T) { va.UserAgent = rejectUserAgent setChallengeToken(&chall, pathMoved) - _, prob := va.validateHTTP01(ident, chall) + _, prob := va.validateHTTP01(context.Background(), ident, chall) if prob == nil { t.Fatalf("Challenge with rejectUserAgent should have failed (%s).", pathMoved) } setChallengeToken(&chall, pathFound) - _, prob = va.validateHTTP01(ident, chall) + _, prob = va.validateHTTP01(context.Background(), ident, chall) if prob == nil { t.Fatalf("Challenge with rejectUserAgent should have failed (%s).", pathFound) } @@ -437,14 +438,14 @@ func TestTLSSNI(t *testing.T) { va.DNSResolver = &mocks.DNSResolver{} log.Clear() - _, prob := va.validateTLSSNI01(ident, chall) + _, prob := va.validateTLSSNI01(context.Background(), ident, chall) if prob != nil { t.Fatalf("Unexpected failre in validateTLSSNI01: %s", prob) } test.AssertEquals(t, len(log.GetAllMatching(`Resolved addresses for localhost \[using 127.0.0.1\]: \[127.0.0.1\]`)), 1) log.Clear() - _, prob = va.validateTLSSNI01(core.AcmeIdentifier{ + _, prob = va.validateTLSSNI01(context.Background(), core.AcmeIdentifier{ Type: core.IdentifierType("ip"), Value: net.JoinHostPort("127.0.0.1", fmt.Sprintf("%d", port)), }, chall) @@ -454,7 +455,7 @@ func TestTLSSNI(t *testing.T) { test.AssertEquals(t, prob.Type, probs.MalformedProblem) log.Clear() - _, prob = va.validateTLSSNI01(core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "always.invalid"}, chall) + _, prob = va.validateTLSSNI01(context.Background(), core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "always.invalid"}, chall) if prob == nil { t.Fatalf("Domain name was supposed to be invalid.") } @@ -467,7 +468,7 @@ func TestTLSSNI(t *testing.T) { log.Clear() started := time.Now() - _, prob = va.validateTLSSNI01(ident, chall) + _, prob = va.validateTLSSNI01(context.Background(), ident, chall) took := time.Since(started) // Check that the HTTP connection times out after 5 seconds and doesn't block for 10 seconds test.Assert(t, (took > (time.Second * 5)), "HTTP timed out before 5 seconds") @@ -480,7 +481,7 @@ func TestTLSSNI(t *testing.T) { // Take down validation server and check that validation fails. hs.Close() - _, err = va.validateTLSSNI01(ident, chall) + _, err = va.validateTLSSNI01(context.Background(), ident, chall) if err == nil { t.Fatalf("Server's down; expected refusal. Where did we connect?") } @@ -508,7 +509,7 @@ func TestTLSError(t *testing.T) { va := NewValidationAuthorityImpl(&PortConfig{TLSPort: port}, nil, stats, clock.Default()) va.DNSResolver = &mocks.DNSResolver{} - _, prob := va.validateTLSSNI01(ident, chall) + _, prob := va.validateTLSSNI01(context.Background(), ident, chall) if prob == nil { t.Fatalf("TLS validation should have failed: What cert was used?") } @@ -537,7 +538,7 @@ func TestValidateHTTP(t *testing.T) { Identifier: ident, Challenges: []core.Challenge{chall}, } - va.validate(authz, 0) + va.validate(context.Background(), authz, 0) test.AssertEquals(t, core.StatusValid, mockRA.lastAuthz.Challenges[0].Status) } @@ -592,7 +593,7 @@ func TestValidateTLSSNI01(t *testing.T) { Identifier: ident, Challenges: []core.Challenge{chall}, } - va.validate(authz, 0) + va.validate(context.Background(), authz, 0) test.AssertEquals(t, core.StatusValid, mockRA.lastAuthz.Challenges[0].Status) } @@ -614,7 +615,7 @@ func TestValidateTLSSNINotSane(t *testing.T) { Identifier: ident, Challenges: []core.Challenge{chall}, } - va.validate(authz, 0) + va.validate(context.Background(), authz, 0) test.AssertEquals(t, core.StatusInvalid, mockRA.lastAuthz.Challenges[0].Status) } @@ -651,7 +652,7 @@ func TestCAATimeout(t *testing.T) { va := NewValidationAuthorityImpl(&PortConfig{}, nil, stats, clock.Default()) va.DNSResolver = &mocks.DNSResolver{} va.IssuerDomain = "letsencrypt.org" - err := va.checkCAA(core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "caa-timeout.com"}, 101) + err := va.checkCAA(context.Background(), core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "caa-timeout.com"}, 101) if err.Type != probs.ConnectionProblem { t.Errorf("Expected timeout error type %s, got %s", probs.ConnectionProblem, err.Type) } @@ -724,7 +725,7 @@ func TestDNSValidationFailure(t *testing.T) { Identifier: ident, Challenges: []core.Challenge{chalDNS}, } - va.validate(authz, 0) + va.validate(context.Background(), authz, 0) t.Logf("Resulting Authz: %+v", authz) test.AssertNotNil(t, mockRA.lastAuthz, "Should have gotten an authorization") @@ -753,7 +754,7 @@ func TestDNSValidationInvalid(t *testing.T) { mockRA := &MockRegistrationAuthority{} va.RA = mockRA - va.validate(authz, 0) + va.validate(context.Background(), authz, 0) test.AssertNotNil(t, mockRA.lastAuthz, "Should have gotten an authorization") test.Assert(t, authz.Challenges[0].Status == core.StatusInvalid, "Should be invalid.") @@ -781,7 +782,7 @@ func TestDNSValidationNotSane(t *testing.T) { } for i := 0; i < len(authz.Challenges); i++ { - va.validate(authz, i) + va.validate(context.Background(), authz, i) test.AssertEquals(t, authz.Challenges[i].Status, core.StatusInvalid) test.AssertEquals(t, authz.Challenges[i].Error.Type, probs.MalformedProblem) } @@ -806,7 +807,7 @@ func TestDNSValidationServFail(t *testing.T) { Identifier: badIdent, Challenges: []core.Challenge{chalDNS}, } - va.validate(authz, 0) + va.validate(context.Background(), authz, 0) test.AssertNotNil(t, mockRA.lastAuthz, "Should have gotten an authorization") test.Assert(t, authz.Challenges[0].Status == core.StatusInvalid, "Should be invalid.") @@ -817,7 +818,7 @@ func TestDNSValidationNoServer(t *testing.T) { c, _ := statsd.NewNoopClient() stats := metrics.NewNoopScope() va := NewValidationAuthorityImpl(&PortConfig{}, nil, c, clock.Default()) - va.DNSResolver = bdns.NewTestDNSResolverImpl(time.Second*5, []string{}, stats) + va.DNSResolver = bdns.NewTestDNSResolverImpl(time.Second*5, []string{}, stats, clock.Default(), 1) mockRA := &MockRegistrationAuthority{} va.RA = mockRA @@ -829,7 +830,7 @@ func TestDNSValidationNoServer(t *testing.T) { Identifier: ident, Challenges: []core.Challenge{chalDNS}, } - va.validate(authz, 0) + va.validate(context.Background(), authz, 0) test.AssertNotNil(t, mockRA.lastAuthz, "Should have gotten an authorization") test.Assert(t, authz.Challenges[0].Status == core.StatusInvalid, "Should be invalid.") @@ -861,7 +862,7 @@ func TestDNSValidationOK(t *testing.T) { Identifier: goodIdent, Challenges: []core.Challenge{chalDNS}, } - va.validate(authz, 0) + va.validate(context.Background(), authz, 0) test.AssertNotNil(t, mockRA.lastAuthz, "Should have gotten an authorization") test.Assert(t, authz.Challenges[0].Status == core.StatusValid, "Should be valid.") @@ -899,7 +900,7 @@ func TestDNSValidationLive(t *testing.T) { Challenges: []core.Challenge{goodChalDNS}, } - va.validate(authzGood, 0) + va.validate(context.Background(), authzGood, 0) if authzGood.Challenges[0].Status != core.StatusValid { t.Logf("TestDNSValidationLive on Good did not succeed.") @@ -916,7 +917,7 @@ func TestDNSValidationLive(t *testing.T) { Challenges: []core.Challenge{badChalDNS}, } - va.validate(authzBad, 0) + va.validate(context.Background(), authzBad, 0) if authzBad.Challenges[0].Status != core.StatusInvalid { t.Logf("TestDNSValidationLive on Bad did succeed inappropriately.") } @@ -943,7 +944,7 @@ func TestCAAFailure(t *testing.T) { Identifier: ident, Challenges: []core.Challenge{chall}, } - va.validate(authz, 0) + va.validate(context.Background(), authz, 0) test.AssertEquals(t, core.StatusInvalid, mockRA.lastAuthz.Challenges[0].Status) } From 426ec155aad0759def8782bf95e0405c3d7f9a20 Mon Sep 17 00:00:00 2001 From: Jeff Hodges Date: Wed, 30 Dec 2015 18:25:51 -0800 Subject: [PATCH 2/7] correct Content-Length/Transfer-Encoding on HEAD Fixes #1320 --- wfe/web-front-end.go | 17 +++-------------- wfe/web-front-end_test.go | 31 ++++++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/wfe/web-front-end.go b/wfe/web-front-end.go index 4a2566bab..bcc29f04d 100644 --- a/wfe/web-front-end.go +++ b/wfe/web-front-end.go @@ -107,16 +107,6 @@ func NewWebFrontEndImpl(stats statsd.Statter, clk clock.Clock) (WebFrontEndImpl, }, nil } -// BodylessResponseWriter wraps http.ResponseWriter, discarding -// anything written to the body. -type BodylessResponseWriter struct { - http.ResponseWriter -} - -func (mrw BodylessResponseWriter) Write(buf []byte) (int, error) { - return len(buf), nil -} - // HandleFunc registers a handler at the given path. It's // http.HandleFunc(), but with a wrapper around the handler that // provides some generic per-request functionality: @@ -157,10 +147,9 @@ func (wfe *WebFrontEndImpl) HandleFunc(mux *http.ServeMux, pattern string, h wfe switch request.Method { case "HEAD": - // Whether or not we're sending a 405 error, - // we should comply with HTTP spec by not - // sending a body. - response = BodylessResponseWriter{response} + // Go's net/http (and httptest) servers will strip our the body + // of responses for us. This keeps the Content-Length for HEAD + // requests as the same as GET requests per the spec. case "OPTIONS": wfe.Options(response, request, methodsStr, methodsMap) return diff --git a/wfe/web-front-end_test.go b/wfe/web-front-end_test.go index 4fa20474d..a15aec3f1 100644 --- a/wfe/web-front-end_test.go +++ b/wfe/web-front-end_test.go @@ -19,6 +19,7 @@ import ( "net/http/httptest" "net/url" "sort" + "strconv" "strings" "testing" "time" @@ -327,7 +328,7 @@ func TestHandleFunc(t *testing.T) { test.AssertEquals(t, rw.Code, http.StatusMethodNotAllowed) test.AssertEquals(t, rw.Header().Get("Content-Type"), "application/problem+json") test.AssertEquals(t, rw.Header().Get("Allow"), "POST") - test.AssertEquals(t, rw.Body.String(), "") + test.AssertEquals(t, rw.Body.String(), `{"type":"urn:acme:error:malformed","detail":"Method not allowed","status":405}`) wfe.AllowOrigins = []string{"*"} testOrigin := "https://example.com" @@ -1400,6 +1401,34 @@ func TestBadKeyCSR(t *testing.T) { `{"type":"urn:acme:error:malformed","detail":"Invalid key in certificate request :: Key too small: 512","status":400}`) } +// This uses httptest.NewServer because ServeMux.ServeHTTP won't prevent the +// body from being sent like the net/http Server's actually do. +func TestGetCertificateHEADHasCorrectBodyLength(t *testing.T) { + wfe, _ := setupWFE(t) + + certPemBytes, _ := ioutil.ReadFile("test/178.crt") + certBlock, _ := pem.Decode(certPemBytes) + + mockLog := wfe.log.SyslogWriter.(*mocks.SyslogWriter) + mockLog.Clear() + + mux, _ := wfe.Handler() + s := httptest.NewServer(mux) + req, _ := http.NewRequest("HEAD", s.URL+"/acme/cert/0000000000000000000000000000000000b2", nil) + resp, err := http.DefaultClient.Do(req) + if err != nil { + test.AssertNotError(t, err, "do error") + } + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + test.AssertNotEquals(t, err, "readall error") + } + defer resp.Body.Close() + test.AssertEquals(t, resp.StatusCode, 200) + test.AssertEquals(t, strconv.Itoa(len(certBlock.Bytes)), resp.Header.Get("Content-Length")) + test.AssertEquals(t, 0, len(body)) +} + func newRequestEvent() *requestEvent { return &requestEvent{Extra: make(map[string]interface{})} } From 673f927d851498c90935c611cbaa106827e43748 Mon Sep 17 00:00:00 2001 From: Reinaldo de Souza Jr Date: Tue, 22 Dec 2015 21:44:28 -0500 Subject: [PATCH 3/7] Initialize rabbitmq in Docker entrypoint --- test/entrypoint.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/entrypoint.sh b/test/entrypoint.sh index 5a3977688..4ad357a9f 100755 --- a/test/entrypoint.sh +++ b/test/entrypoint.sh @@ -12,10 +12,19 @@ while ! exec 6<>/dev/tcp/0.0.0.0/3306; do sleep 1 done +# make sure we can reach the rabbitmq +while ! exec 6<>/dev/tcp/0.0.0.0/5672; do + echo "$(date) - still trying to connect to rabbitmq at 0.0.0.0:5672" + sleep 1 +done + exec 6>&- exec 6<&- # create the database source $DIR/create_db.sh +# Set up rabbitmq exchange and activity monitor queue +go run cmd/rabbitmq-setup/main.go -server amqp://localhost + $@ From a76cb3c4140097eab70d8c5f79d76212678dc846 Mon Sep 17 00:00:00 2001 From: Jeff Hodges Date: Mon, 21 Dec 2015 21:18:34 -0800 Subject: [PATCH 4/7] stat and log rate limit data where available It doesn't log the IP information on its rate limit. IP is considered personally identifiable information and against our policies to log. Fixes #1120 --- ra/registration-authority.go | 39 ++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/ra/registration-authority.go b/ra/registration-authority.go index 003557434..1a1dc3501 100644 --- a/ra/registration-authority.go +++ b/ra/registration-authority.go @@ -21,6 +21,7 @@ import ( "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/letsencrypt/net/publicsuffix" "github.com/letsencrypt/boulder/Godeps/_workspace/src/golang.org/x/net/context" + "github.com/letsencrypt/boulder/metrics" "github.com/letsencrypt/boulder/probs" "github.com/letsencrypt/boulder/bdns" @@ -62,10 +63,17 @@ type RegistrationAuthorityImpl struct { totalIssuedCache int lastIssuedCount *time.Time maxContactsPerReg int + + regByIPStats metrics.Scope + pendAuthByRegIDStats metrics.Scope + certsForDomainStats metrics.Scope + totalCertsStats metrics.Scope } // NewRegistrationAuthorityImpl constructs a new RA object. func NewRegistrationAuthorityImpl(clk clock.Clock, logger *blog.AuditLogger, stats statsd.Statter, dc *DomainCheck, policies cmd.RateLimitConfig, maxContactsPerReg int) *RegistrationAuthorityImpl { + // TODO(jmhodges): making RA take a "RA" stats.Scope, not Statter + scope := metrics.NewStatsdScope(stats, "RA") ra := &RegistrationAuthorityImpl{ stats: stats, clk: clk, @@ -76,6 +84,11 @@ func NewRegistrationAuthorityImpl(clk clock.Clock, logger *blog.AuditLogger, sta rlPolicies: policies, tiMu: new(sync.RWMutex), maxContactsPerReg: maxContactsPerReg, + + regByIPStats: scope.NewScope("RA", "RateLimit", "RegistrationsByIP"), + pendAuthByRegIDStats: scope.NewScope("RA", "RateLimit", "PendingAuthorizationsByRegID"), + certsForDomainStats: scope.NewScope("RA", "RateLimit", "CertificatesForDomain"), + totalCertsStats: scope.NewScope("RA", "RateLimit", "TotalCertificates"), } return ra } @@ -186,8 +199,10 @@ func (ra *RegistrationAuthorityImpl) checkRegistrationLimit(ip net.IP) error { return err } if count >= limit.GetThreshold(ip.String(), noRegistrationID) { + ra.regByIPStats.Inc("Exceeded", 1) return core.RateLimitedError("Too many registrations from this IP") } + ra.regByIPStats.Inc("Pass", 1) } return nil } @@ -260,9 +275,10 @@ func (ra *RegistrationAuthorityImpl) validateContacts(ctx context.Context, conta return } -func checkPendingAuthorizationLimit(sa core.StorageGetter, limit *cmd.RateLimitPolicy, regID int64) error { +func (ra *RegistrationAuthorityImpl) checkPendingAuthorizationLimit(regID int64) error { + limit := ra.rlPolicies.PendingAuthorizationsPerAccount if limit.Enabled() { - count, err := sa.CountPendingAuthorizations(regID) + count, err := ra.SA.CountPendingAuthorizations(regID) if err != nil { return err } @@ -270,8 +286,11 @@ func checkPendingAuthorizationLimit(sa core.StorageGetter, limit *cmd.RateLimitP // here. noKey := "" if count >= limit.GetThreshold(noKey, regID) { + ra.pendAuthByRegIDStats.Inc("Exceeded", 1) + ra.log.Info(fmt.Sprintf("Rate limit exceeded, PendingAuthorizationsByRegID, regID: %d", regID)) return core.RateLimitedError("Too many currently pending authorizations.") } + ra.pendAuthByRegIDStats.Inc("Pass", 1) } return nil } @@ -293,8 +312,7 @@ func (ra *RegistrationAuthorityImpl) NewAuthorization(request core.Authorization return authz, err } - limit := &ra.rlPolicies.PendingAuthorizationsPerAccount - if err = checkPendingAuthorizationLimit(ra.SA, limit, regID); err != nil { + if err = ra.checkPendingAuthorizationLimit(regID); err != nil { return authz, err } @@ -609,10 +627,15 @@ func (ra *RegistrationAuthorityImpl) checkCertificatesPerNameLimit(names []strin } } if len(badNames) > 0 { + domains := strings.Join(badNames, ", ") + ra.certsForDomainStats.Inc("Exceeded", 1) + ra.log.Info(fmt.Sprintf("Rate limit exceeded, CertificatesForDomain, regID: %d, domains: %s", regID, domains)) return core.RateLimitedError(fmt.Sprintf( - "Too many certificates already issued for: %s", - strings.Join(badNames, ", "))) + "Too many certificates already issued for: %s", domains)) + } + ra.certsForDomainStats.Inc("Pass", 1) + return nil } @@ -624,8 +647,12 @@ func (ra *RegistrationAuthorityImpl) checkLimits(names []string, regID int64) er return err } if totalIssued >= ra.rlPolicies.TotalCertificates.Threshold { + domains := strings.Join(names, ",") + ra.totalCertsStats.Inc("Exceeded", 1) + ra.log.Info(fmt.Sprintf("Rate limit exceeded, TotalCertificates, regID: %d, domains: %s, totalIssued: %d", regID, domains, totalIssued)) return core.RateLimitedError("Certificate issuance limit reached") } + ra.totalCertsStats.Inc("Pass", 1) } if limits.CertificatesPerName.Enabled() { err := ra.checkCertificatesPerNameLimit(names, limits.CertificatesPerName, regID) From 4fe05d08d195c3b343fdc1cd7ab825caff379af9 Mon Sep 17 00:00:00 2001 From: Jeff Hodges Date: Tue, 5 Jan 2016 11:33:15 -0800 Subject: [PATCH 5/7] add IP to rate limit logging I was wrong about it not being okay in #1300 --- ra/registration-authority.go | 1 + 1 file changed, 1 insertion(+) diff --git a/ra/registration-authority.go b/ra/registration-authority.go index 1a1dc3501..8a4ffded6 100644 --- a/ra/registration-authority.go +++ b/ra/registration-authority.go @@ -200,6 +200,7 @@ func (ra *RegistrationAuthorityImpl) checkRegistrationLimit(ip net.IP) error { } if count >= limit.GetThreshold(ip.String(), noRegistrationID) { ra.regByIPStats.Inc("Exceeded", 1) + ra.log.Info(fmt.Sprintf("Rate limit exceeded, RegistrationsByIP, IP: %s", ip)) return core.RateLimitedError("Too many registrations from this IP") } ra.regByIPStats.Inc("Pass", 1) From f67648d22f1394fde6e529bb7fdd837fc05712e2 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Tue, 5 Jan 2016 14:50:25 -0800 Subject: [PATCH 6/7] Disable activity-monitor. We no longer run this in prod, so we shouldn't run it in test / dev. --- .gitignore | 3 --- test/integration-test.py | 8 -------- test/startservers.py | 1 - 3 files changed, 12 deletions(-) diff --git a/.gitignore b/.gitignore index 75bee6fea..a61289c17 100644 --- a/.gitignore +++ b/.gitignore @@ -34,6 +34,3 @@ _testmain.go *.test *.prof *.coverprofile - -boulder-start/boulder-start -activity-monitor/activity-monitor diff --git a/test/integration-test.py b/test/integration-test.py index 01f4c0ad9..7b657aa97 100644 --- a/test/integration-test.py +++ b/test/integration-test.py @@ -201,12 +201,6 @@ def run_client_tests(): if subprocess.Popen(cmd, shell=True, cwd=root, executable='/bin/bash').wait() != 0: die(ExitStatus.PythonFailure) -def check_activity_monitor(): - """Ensure that the activity monitor is running and received some messages.""" - resp = urllib2.urlopen("http://localhost:8007/debug/vars") - debug_vars = json.loads(resp.read()) - assert debug_vars['messages'] > 0, "Activity Monitor received zero messages." - @atexit.register def cleanup(): import shutil @@ -271,8 +265,6 @@ def main(): if args.run_all or args.run_letsencrypt: run_client_tests() - check_activity_monitor() - if not startservers.check(): die(ExitStatus.Error) exit_status = ExitStatus.OK diff --git a/test/startservers.py b/test/startservers.py index 73a7471d5..8ae3cf76e 100644 --- a/test/startservers.py +++ b/test/startservers.py @@ -63,7 +63,6 @@ def start(race_detection): t.daemon = True t.start() progs = [ - 'activity-monitor', 'boulder-wfe', 'boulder-ra', 'boulder-sa', From 2ea173d3ce4cf8d252815357df66bc30d23b4907 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Tue, 5 Jan 2016 16:35:34 -0800 Subject: [PATCH 7/7] Restore signing to ct-test-srv. This restores an earlier version of https://github.com/letsencrypt/boulder/pull/1282. During code review on that change, I requested that we replace the realtime signing with a static signature. However, that is now generating failures. I think the signature validation is now validating more of the content. --- test/ct-test-srv/main.go | 84 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 76 insertions(+), 8 deletions(-) diff --git a/test/ct-test-srv/main.go b/test/ct-test-srv/main.go index 6f74b2965..34065178d 100644 --- a/test/ct-test-srv/main.go +++ b/test/ct-test-srv/main.go @@ -9,20 +9,77 @@ package main import ( + "crypto/ecdsa" + "crypto/rand" + "crypto/sha256" + "crypto/x509" + "encoding/asn1" + "encoding/base64" "encoding/json" "fmt" "io/ioutil" "log" + "math/big" "net/http" + "os" "sync/atomic" + + ct "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/google/certificate-transparency/go" ) +func createSignedSCT(leaf []byte, k *ecdsa.PrivateKey) []byte { + rawKey, _ := x509.MarshalPKIXPublicKey(&k.PublicKey) + pkHash := sha256.Sum256(rawKey) + sct := ct.SignedCertificateTimestamp{ + SCTVersion: ct.V1, + LogID: pkHash, + Timestamp: 1337, + } + serialized, _ := ct.SerializeSCTSignatureInput(sct, ct.LogEntry{ + Leaf: ct.MerkleTreeLeaf{ + LeafType: ct.TimestampedEntryLeafType, + TimestampedEntry: ct.TimestampedEntry{ + X509Entry: ct.ASN1Cert(leaf), + EntryType: ct.X509LogEntryType, + }, + }, + }) + hashed := sha256.Sum256(serialized) + var ecdsaSig struct { + R, S *big.Int + } + ecdsaSig.R, ecdsaSig.S, _ = ecdsa.Sign(rand.Reader, k, hashed[:]) + sig, _ := asn1.Marshal(ecdsaSig) + + ds := ct.DigitallySigned{ + HashAlgorithm: ct.SHA256, + SignatureAlgorithm: ct.ECDSA, + Signature: sig, + } + + var jsonSCTObj struct { + SCTVersion ct.Version `json:"sct_version"` + ID string `json:"id"` + Timestamp uint64 `json:"timestamp"` + Extensions string `json:"extensions"` + Signature string `json:"signature"` + } + jsonSCTObj.SCTVersion = ct.V1 + jsonSCTObj.ID = base64.StdEncoding.EncodeToString(pkHash[:]) + jsonSCTObj.Timestamp = 1337 + jsonSCTObj.Signature, _ = ds.Base64String() + + jsonSCT, _ := json.Marshal(jsonSCTObj) + return jsonSCT +} + type ctSubmissionRequest struct { Chain []string `json:"chain"` } type integrationSrv struct { submissions int64 + key *ecdsa.PrivateKey } func (is *integrationSrv) handler(w http.ResponseWriter, r *http.Request) { @@ -47,14 +104,16 @@ func (is *integrationSrv) handler(w http.ResponseWriter, r *http.Request) { return } + leaf, err := base64.StdEncoding.DecodeString(addChainReq.Chain[0]) + if err != nil { + w.WriteHeader(400) + return + } + w.WriteHeader(http.StatusOK) - w.Write([]byte(`{ - "sct_version":0, - "id":"KHYaGJAn++880NYaAY12sFBXKcenQRvMvfYE9F1CYVM=", - "timestamp":1337, - "extensions":"", - "signature":"BAMARjBEAiAka/W0eYq23Iaih2wB2CGrAqlo92KyQuuY6WWumi1eNwIgBirYV/wsJvmZfGP5NrNYoWGIx1VV6NaNBIaSXh9hiYA=" - }`)) + // id is a sha256 of a random EC key. Generate your own with: + // openssl ecparam -name prime256v1 -genkey -outform der | openssl sha256 -binary | base64 + w.Write(createSignedSCT(leaf, is.key)) atomic.AddInt64(&is.submissions, 1) case "/submissions": if r.Method != "GET" { @@ -72,7 +131,16 @@ func (is *integrationSrv) handler(w http.ResponseWriter, r *http.Request) { } func main() { - is := integrationSrv{} + signingKey := "MHcCAQEEIOCtGlGt/WT7471dOHdfBg43uJWJoZDkZAQjWfTitcVNoAoGCCqGSM49AwEHoUQDQgAEYggOxPnPkzKBIhTacSYoIfnSL2jPugcbUKx83vFMvk5gKAz/AGe87w20riuPwEGn229hKVbEKHFB61NIqNHC3Q==" + decodedKey, _ := base64.StdEncoding.DecodeString(signingKey) + + key, err := x509.ParseECPrivateKey(decodedKey) + if err != nil { + fmt.Fprintf(os.Stderr, "failed to parse signing key: %s\n", err) + return + } + + is := integrationSrv{key: key} s := &http.Server{ Addr: "localhost:4500", Handler: http.HandlerFunc(is.handler),