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/CONTRIBUTING.md b/CONTRIBUTING.md index dfd39385e..02600590a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,7 +1,7 @@ # Contributing to Boulder > **Note:** We are currently in a *General Availability* only merge window, meaning -> we will only be reviewing & merging patches which close a issue tagged with the *General +> we will only be reviewing & merging patches which close an issue tagged with the *General > Availability* milestone. Thanks for helping us build Boulder, if you haven't already had a chance to look diff --git a/DESIGN.md b/DESIGN.md index c2b46fa0b..7ed53c1dd 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -214,7 +214,7 @@ Notes: * 7-8: WFE does the following: * Create a URL from the certificate's serial number - * Return the certificate with it's URL + * Return the certificate with its URL ## Revoke Certificate @@ -244,4 +244,4 @@ Notes: * Log the success or failure of the revocation * 5-6: WFE does the following: - * Return an indication of the sucess or failure of the revocation + * Return an indication of the success or failure of the revocation 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/admin-revoker/main.go b/cmd/admin-revoker/main.go index 56cf8596e..20f3b6889 100644 --- a/cmd/admin-revoker/main.go +++ b/cmd/admin-revoker/main.go @@ -163,7 +163,7 @@ func main() { // 1: serial, 2: reasonCode (3: deny flag) serial := c.Args().First() reasonCode, err := strconv.Atoi(c.Args().Get(1)) - cmd.FailOnError(err, "Reason code argument must be a integer") + cmd.FailOnError(err, "Reason code argument must be an integer") deny := c.GlobalBool("deny") cac, auditlogger, dbMap, _ := setupContext(c) @@ -190,9 +190,9 @@ func main() { Action: func(c *cli.Context) { // 1: registration ID, 2: reasonCode (3: deny flag) regID, err := strconv.ParseInt(c.Args().First(), 10, 64) - cmd.FailOnError(err, "Registration ID argument must be a integer") + cmd.FailOnError(err, "Registration ID argument must be an integer") reasonCode, err := strconv.Atoi(c.Args().Get(1)) - cmd.FailOnError(err, "Reason code argument must be a integer") + cmd.FailOnError(err, "Reason code argument must be an integer") deny := c.GlobalBool("deny") cac, auditlogger, dbMap, sac := setupContext(c) 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..0e251fc62 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 { @@ -131,7 +141,7 @@ type Config struct { Path string ListenAddress string - // MaxAge is the max-age to set in the Cache-Controler response + // MaxAge is the max-age to set in the Cache-Control response // header. It is a time.Duration formatted string. MaxAge ConfigDuration diff --git a/cmd/expiration-mailer/main.go b/cmd/expiration-mailer/main.go index bf56aea75..e84532f5b 100644 --- a/cmd/expiration-mailer/main.go +++ b/cmd/expiration-mailer/main.go @@ -110,7 +110,7 @@ func (m *mailer) updateCertStatus(serial string) error { err = tx.Commit() if err != nil { - m.log.Err(fmt.Sprintf("Error commiting transaction for certificate %s: %s", serial, err)) + m.log.Err(fmt.Sprintf("Error committing transaction for certificate %s: %s", serial, err)) tx.Rollback() return err } diff --git a/cmd/shell.go b/cmd/shell.go index 466af0232..cc6a7178f 100644 --- a/cmd/shell.go +++ b/cmd/shell.go @@ -157,7 +157,7 @@ func (as *AppShell) Run() { FailOnError(err, "Failed to run application") } -// StatsAndLogging constructs a Statter and and AuditLogger based on its config +// StatsAndLogging constructs a Statter and an AuditLogger based on its config // parameters, and return them both. Crashes if any setup fails. // Also sets the constructed AuditLogger as the default logger. func StatsAndLogging(statConf StatsdConfig, logConf SyslogConfig) (statsd.Statter, *blog.AuditLogger) { diff --git a/core/objects.go b/core/objects.go index b8379a89b..2314d84e1 100644 --- a/core/objects.go +++ b/core/objects.go @@ -294,7 +294,7 @@ type Challenge struct { // The status of this challenge Status AcmeStatus `json:"status,omitempty"` - // Contains the error that occured during challenge validation, if any + // Contains the error that occurred during challenge validation, if any Error *probs.ProblemDetails `json:"error,omitempty"` // If successful, the time at which this challenge @@ -487,7 +487,7 @@ type Certificate struct { } // IdentifierData holds information about what certificates are known for a -// given identifier. This is used to present Proof of Posession challenges in +// given identifier. This is used to present Proof of Possession challenges in // the case where a certificate already exists. The DB table holding // IdentifierData rows contains information about certs issued by Boulder and // also information about certs observed from third parties. diff --git a/core/util.go b/core/util.go index 7de56a624..4ea7e8026 100644 --- a/core/util.go +++ b/core/util.go @@ -460,7 +460,7 @@ func LoadCertBundle(filename string) ([]*x509.Certificate, error) { return bundle, nil } -// LoadCert loads a PEM certificate specified by filename or returns a error +// LoadCert loads a PEM certificate specified by filename or returns an error func LoadCert(filename string) (cert *x509.Certificate, err error) { certPEM, err := ioutil.ReadFile(filename) if err != nil { diff --git a/log/audit-logger.go b/log/audit-logger.go index 1f6741697..d13f669b8 100644 --- a/log/audit-logger.go +++ b/log/audit-logger.go @@ -119,7 +119,7 @@ func SetAuditLogger(logger *AuditLogger) (err error) { // GetAuditLogger obtains the singleton audit logger. If SetAuditLogger // has not been called first, this method initializes with basic defaults. -// The basic defaults cannot error, and subequent access to an already-set +// The basic defaults cannot error, and subsequent access to an already-set // AuditLogger also cannot error, so this method is error-safe. func GetAuditLogger() *AuditLogger { _Singleton.once.Do(func() { @@ -271,7 +271,7 @@ func (log *AuditLogger) AuditObject(msg string, obj interface{}) (err error) { return log.auditAtLevel(syslog.LOG_NOTICE, formattedEvent) } -// InfoObject sends a INFO-severity JSON-serialized object message. +// InfoObject sends an INFO-severity JSON-serialized object message. func (log *AuditLogger) InfoObject(msg string, obj interface{}) (err error) { formattedEvent, logErr := log.formatObjectMessage(msg, obj) if logErr != nil { diff --git a/mocks/mocks.go b/mocks/mocks.go index 79446f94a..1d3954b77 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") } @@ -46,7 +47,7 @@ func (mock *DNSResolver) LookupTXT(hostname string) ([]string, error) { return []string{"hostname"}, nil } -// TimeoutError returns a a net.OpError for which Timeout() returns true. +// TimeoutError returns a net.OpError for which Timeout() returns true. func TimeoutError() *net.OpError { return &net.OpError{ Err: os.NewSyscallError("ugh timeout", timeoutError{}), @@ -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/publisher/publisher.go b/publisher/publisher.go index a464d3c34..1d4ce8dc4 100644 --- a/publisher/publisher.go +++ b/publisher/publisher.go @@ -25,7 +25,7 @@ type Log struct { verifier *ct.SignatureVerifier } -// NewLog returns a initialized Log struct +// NewLog returns an initialized Log struct func NewLog(uri, b64PK string) (*Log, error) { if strings.HasSuffix(uri, "/") { uri = uri[0 : len(uri)-2] diff --git a/ra/registration-authority.go b/ra/registration-authority.go index 455c0c5ac..8a4ffded6 100644 --- a/ra/registration-authority.go +++ b/ra/registration-authority.go @@ -20,6 +20,8 @@ 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/metrics" "github.com/letsencrypt/boulder/probs" "github.com/letsencrypt/boulder/bdns" @@ -61,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, @@ -75,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 } @@ -84,7 +98,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 +110,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{ @@ -185,8 +199,11 @@ func (ra *RegistrationAuthorityImpl) checkRegistrationLimit(ip net.IP) error { return err } 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) } return nil } @@ -209,7 +226,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 +244,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 +260,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) @@ -258,9 +276,10 @@ func (ra *RegistrationAuthorityImpl) validateContacts(contacts []*core.AcmeURL) 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 } @@ -268,13 +287,16 @@ 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 } -// NewAuthorization constuct a new Authz from a request. Values (domains) in +// NewAuthorization constructs a new Authz from a request. Values (domains) in // request.Identifier will be lowercased before storage. func (ra *RegistrationAuthorityImpl) NewAuthorization(request core.Authorization, regID int64) (authz core.Authorization, err error) { reg, err := ra.SA.GetRegistration(regID) @@ -291,8 +313,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 } @@ -607,10 +628,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 } @@ -622,8 +648,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) @@ -638,7 +668,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/rpc/amqp-rpc.go b/rpc/amqp-rpc.go index 9c4a7f397..624bd15e6 100644 --- a/rpc/amqp-rpc.go +++ b/rpc/amqp-rpc.go @@ -192,7 +192,7 @@ type rpcError struct { HTTPStatus int `json:"status,omitempty"` } -// Wraps a error in a rpcError so it can be marshalled to +// Wraps an error in a rpcError so it can be marshalled to // JSON. func wrapError(err error) *rpcError { if err != nil { @@ -298,7 +298,7 @@ func (r rpcResponse) debugString() string { return fmt.Sprintf("%s, RPCERR: %v", ret, r.Error) } -// makeAmqpChannel sets a AMQP connection up using SSL if configuration is provided +// makeAmqpChannel sets an AMQP connection up using SSL if configuration is provided func makeAmqpChannel(conf *cmd.AMQPConfig) (*amqp.Channel, error) { var conn *amqp.Connection var err error diff --git a/sa/storage-authority.go b/sa/storage-authority.go index 1d8937f0d..f70cb2f41 100644 --- a/sa/storage-authority.go +++ b/sa/storage-authority.go @@ -52,7 +52,7 @@ type authzModel struct { } // NewSQLStorageAuthority provides persistence using a SQL backend for -// Boulder. It will modify the given gorp.DbMap by adding relevent tables. +// Boulder. It will modify the given gorp.DbMap by adding relevant tables. func NewSQLStorageAuthority(dbMap *gorp.DbMap, clk clock.Clock) (*SQLStorageAuthority, error) { logger := blog.GetAuditLogger() @@ -318,7 +318,7 @@ func (t TooManyCertificatesError) Error() string { // subdomains. It returns a map from domains to counts, which is guaranteed to // contain an entry for each input domain, so long as err is nil. // The highest count this function can return is 10,000. If there are more -// certificates than that matching one ofthe provided domain names, it will return +// certificates than that matching one of the provided domain names, it will return // TooManyCertificatesError. func (ssa *SQLStorageAuthority) CountCertificatesByNames(domains []string, earliest, latest time.Time) (map[string]int, error) { ret := make(map[string]int, len(domains)) @@ -336,7 +336,7 @@ func (ssa *SQLStorageAuthority) CountCertificatesByNames(domains []string, earli // certificates issued in the given time range for that domain and its // subdomains. // The highest count this function can return is 10,000. If there are more -// certificates than that matching one ofthe provided domain names, it will return +// certificates than that matching one of the provided domain names, it will return // TooManyCertificatesError. func (ssa *SQLStorageAuthority) countCertificatesByName(domain string, earliest, latest time.Time) (int, error) { var count int64 @@ -633,7 +633,7 @@ func (ssa *SQLStorageAuthority) FinalizeAuthorization(authz core.Authorization) // Check that a pending authz exists if !existingPending(tx, authz.ID) { - err = errors.New("Cannot finalize a authorization that is not pending") + err = errors.New("Cannot finalize an authorization that is not pending") tx.Rollback() return } @@ -790,7 +790,7 @@ func (ssa *SQLStorageAuthority) CountPendingAuthorizations(regID int64) (count i return } -// ErrNoReceipt is a error type for non-existent SCT receipt +// ErrNoReceipt is an error type for non-existent SCT receipt type ErrNoReceipt string func (e ErrNoReceipt) Error() string { @@ -817,7 +817,7 @@ func (ssa *SQLStorageAuthority) GetSCTReceipt(serial string, logID string) (rece return } -// ErrDuplicateReceipt is a error type for duplicate SCT receipts +// ErrDuplicateReceipt is an error type for duplicate SCT receipts type ErrDuplicateReceipt string func (e ErrDuplicateReceipt) Error() string { 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/test/create_db.sh b/test/create_db.sh index 3c1600f52..783934a8c 100755 --- a/test/create_db.sh +++ b/test/create_db.sh @@ -3,7 +3,7 @@ set -o errexit cd $(dirname $0)/.. source test/db-common.sh -# set db connection for if running in a seperate container or not +# set db connection for if running in a separate container or not dbconn="-u root" if [[ ! -z "$MYSQL_CONTAINER" ]]; then dbconn="-u root -h 127.0.0.1 --port 3306" 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 + $@ 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', diff --git a/va/validation-authority.go b/va/validation-authority.go index 2007fcade..e5b16fd18 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" @@ -85,12 +86,12 @@ type verificationRequestEvent struct { } // getAddr will query for all A records associated with hostname and return the -// prefered address, the first net.IP in the addrs slice, and all addresses resolved. +// preferred address, the first net.IP in the addrs slice, and all addresses resolved. // 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) @@ -118,9 +119,9 @@ func (d *dialer) Dial(_, _ string) (net.Conn, error) { return realDialer.Dial("tcp", net.JoinHostPort(d.record.AddressUsed.String(), d.record.Port)) } -// resolveAndConstructDialer gets the prefered address using va.getAddr and returns +// resolveAndConstructDialer gets the preferred 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..beab8ce28 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" @@ -122,7 +123,7 @@ func httpSrv(t *testing.T, token string) *httptest.Server { test.AssertNotError(t, err, "failed to get server test port") http.Redirect(w, r, fmt.Sprintf("http://other.valid:%d/path", port), 302) } else if strings.HasSuffix(r.URL.Path, pathReLookupInvalid) { - t.Logf("HTTPSRV: Got a redirect req to a invalid hostname\n") + t.Logf("HTTPSRV: Got a redirect req to an invalid hostname\n") http.Redirect(w, r, "http://invalid.invalid/path", 302) } else if strings.HasSuffix(r.URL.Path, pathLooper) { t.Logf("HTTPSRV: Got a loop req\n") @@ -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,14 +862,14 @@ 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.") } // TestDNSValidationLive is an integration test, depending on -// the existance of some Internet resources. Because of that, +// the existence of some Internet resources. Because of that, // it asserts nothing; it is intended for coverage. func TestDNSValidationLive(t *testing.T) { stats, _ := statsd.NewNoopClient() @@ -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) } diff --git a/wfe/web-front-end.go b/wfe/web-front-end.go index 4a2566bab..469244cdb 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 @@ -296,7 +285,7 @@ const ( // the key in the JWS headers, and return the key plus a dummy registration if // successful. If a caller passes regCheck = false, it should plan on validating // the key itself. verifyPOST also appends its errors to requestEvent.Errors so -// code calling it does not need to if they imediately return a response to the +// code calling it does not need to if they immediately return a response to the // user. func (wfe *WebFrontEndImpl) verifyPOST(logEvent *requestEvent, request *http.Request, regCheck bool, resource core.AcmeResource) ([]byte, *jose.JsonWebKey, core.Registration, *probs.ProblemDetails) { // TODO: We should return a pointer to a registration, which can be nil, @@ -608,7 +597,7 @@ func (wfe *WebFrontEndImpl) NewAuthorization(logEvent *requestEvent, response ht // RevokeCertificate is used by clients to request the revocation of a cert. func (wfe *WebFrontEndImpl) RevokeCertificate(logEvent *requestEvent, response http.ResponseWriter, request *http.Request) { - // We don't ask verifyPOST to verify there is a correponding registration, + // We don't ask verifyPOST to verify there is a corresponding registration, // because anyone with the right private key can revoke a certificate. body, requestKey, registration, prob := wfe.verifyPOST(logEvent, request, false, core.ResourceRevokeCert) if prob != nil { diff --git a/wfe/web-front-end_test.go b/wfe/web-front-end_test.go index 4fa20474d..88f22712b 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" @@ -605,7 +606,7 @@ func TestIssueCertificate(t *testing.T) { responseWriter.Body.String(), `{"type":"urn:acme:error:malformed","detail":"Error unmarshaling certificate request","status":400}`) - // Valid, signed JWS body, payload has a invalid signature on CSR and no authorizations: + // Valid, signed JWS body, payload has an invalid signature on CSR and no authorizations: // alias b64url="base64 -w0 | sed -e 's,+,-,g' -e 's,/,_,g'" // openssl req -outform der -new -nodes -key wfe/test/178.key -subj /CN=foo.com | \ // sed 's/foo.com/fob.com/' | b64url @@ -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{})} }