From eb52f02d0663aacf5bb2589214f6a7f7919898fd Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Tue, 29 Dec 2015 11:48:02 +0000 Subject: [PATCH 01/26] Make expiration mailer RFC 822 compliant (and satisfy SpamAssassin) --- cmd/expiration-mailer/main.go | 3 ++- cmd/expiration-mailer/main_test.go | 2 +- mail/mailer.go | 29 +++++++++++++++++++++++++---- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/cmd/expiration-mailer/main.go b/cmd/expiration-mailer/main.go index bf56aea75..e9b976729 100644 --- a/cmd/expiration-mailer/main.go +++ b/cmd/expiration-mailer/main.go @@ -19,6 +19,7 @@ import ( "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/codegangsta/cli" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" "github.com/letsencrypt/boulder/Godeps/_workspace/src/gopkg.in/gorp.v1" + "github.com/letsencrypt/boulder/cmd" "github.com/letsencrypt/boulder/core" blog "github.com/letsencrypt/boulder/log" @@ -72,7 +73,7 @@ func (m *mailer) sendNags(parsedCert *x509.Certificate, contacts []*core.AcmeURL return err } startSending := m.clk.Now() - err = m.mailer.SendMail(emails, msgBuf.String()) + err = m.mailer.SendMail(emails, "Certificate expiration notice", msgBuf.String()) if err != nil { m.stats.Inc("Mailer.Expiration.Errors.SendingNag.SendFailure", 1, 1.0) return err diff --git a/cmd/expiration-mailer/main_test.go b/cmd/expiration-mailer/main_test.go index 26ac81c17..d0a83576c 100644 --- a/cmd/expiration-mailer/main_test.go +++ b/cmd/expiration-mailer/main_test.go @@ -50,7 +50,7 @@ func (m *mockMail) Clear() { m.Messages = []string{} } -func (m *mockMail) SendMail(to []string, msg string) (err error) { +func (m *mockMail) SendMail(to []string, subject, msg string) (err error) { for range to { m.Messages = append(m.Messages, msg) } diff --git a/mail/mailer.go b/mail/mailer.go index dc3a6b366..094583c41 100644 --- a/mail/mailer.go +++ b/mail/mailer.go @@ -6,13 +6,17 @@ package mail import ( + "fmt" + "math/rand" "net" "net/smtp" + "strings" + "time" ) // Mailer provides the interface for a mailer type Mailer interface { - SendMail([]string, string) error + SendMail([]string, string, string) error } // MailerImpl defines a mail transfer agent to use for sending mail @@ -35,9 +39,26 @@ func New(server, port, username, password string) MailerImpl { } } +func (m *MailerImpl) generateMessage(to []string, subject, body string) []byte { + now := time.Now().UTC() + headers := []string{ + fmt.Sprintf("To: %s", strings.Join(to, ", ")), + fmt.Sprintf("From: %s", m.From), + fmt.Sprintf("Subject: %s", subject), + fmt.Sprintf("Date: %s", now.Format("Mon Jan 2 2006 15:04:05 -0700")), + fmt.Sprintf("Message-Id: <%s.%d.%s>", now.Format("20060102T150405"), rand.Int63(), m.From), + } + return []byte(fmt.Sprintf("%s\r\n\r\n%s\r\n", strings.Join(headers, "\r\n"), body)) +} + // SendMail sends an email to the provided list of recipients. The email body // is simple text. -func (m *MailerImpl) SendMail(to []string, msg string) (err error) { - err = smtp.SendMail(net.JoinHostPort(m.Server, m.Port), m.Auth, m.From, to, []byte(msg)) - return +func (m *MailerImpl) SendMail(to []string, subject, msg string) error { + return smtp.SendMail( + net.JoinHostPort(m.Server, m.Port), + m.Auth, + m.From, + to, + m.generateMessage(to, subject, msg), + ) } From aacafb7ff58c66ef69ff04cb6545a89eae15e286 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Tue, 29 Dec 2015 19:48:33 +0000 Subject: [PATCH 02/26] Add unit test --- cmd/expiration-mailer/main_test.go | 22 ++++------------------ mail/mailer.go | 8 ++++++-- mail/mailer_test.go | 25 +++++++++++++++++++++++++ mocks/mocks.go | 18 ++++++++++++++++++ 4 files changed, 53 insertions(+), 20 deletions(-) diff --git a/cmd/expiration-mailer/main_test.go b/cmd/expiration-mailer/main_test.go index d0a83576c..ed7d06fb7 100644 --- a/cmd/expiration-mailer/main_test.go +++ b/cmd/expiration-mailer/main_test.go @@ -23,6 +23,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/gopkg.in/gorp.v1" + "github.com/letsencrypt/boulder/core" blog "github.com/letsencrypt/boulder/log" "github.com/letsencrypt/boulder/mocks" @@ -42,21 +43,6 @@ func intFromB64(b64 string) int { return int(bigIntFromB64(b64).Int64()) } -type mockMail struct { - Messages []string -} - -func (m *mockMail) Clear() { - m.Messages = []string{} -} - -func (m *mockMail) SendMail(to []string, subject, msg string) (err error) { - for range to { - m.Messages = append(m.Messages, msg) - } - return -} - type fakeRegStore struct { RegByID map[int64]core.Registration } @@ -104,7 +90,7 @@ var ( func TestSendNags(t *testing.T) { stats, _ := statsd.NewNoopClient(nil) - mc := mockMail{} + mc := mocks.Mailer{} rs := newFakeRegStore() fc := newFakeClock(t) @@ -461,7 +447,7 @@ func TestDontFindRevokedCert(t *testing.T) { type testCtx struct { dbMap *gorp.DbMap ssa core.StorageAdder - mc *mockMail + mc *mocks.Mailer fc clock.FakeClock m *mailer cleanUp func() @@ -482,7 +468,7 @@ func setup(t *testing.T, nagTimes []time.Duration) *testCtx { cleanUp := test.ResetSATestDatabase(t) stats, _ := statsd.NewNoopClient(nil) - mc := &mockMail{} + mc := &mocks.Mailer{} offsetNags := make([]time.Duration, len(nagTimes)) for i, t := range nagTimes { diff --git a/mail/mailer.go b/mail/mailer.go index 094583c41..239257a50 100644 --- a/mail/mailer.go +++ b/mail/mailer.go @@ -11,7 +11,8 @@ import ( "net" "net/smtp" "strings" - "time" + + "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" ) // Mailer provides the interface for a mailer @@ -25,6 +26,7 @@ type MailerImpl struct { Port string Auth smtp.Auth From string + clk clock.Clock } // New constructs a Mailer to represent an account on a particular mail @@ -36,11 +38,13 @@ func New(server, port, username, password string) MailerImpl { Port: port, Auth: auth, From: username, + clk: clock.Default(), } } func (m *MailerImpl) generateMessage(to []string, subject, body string) []byte { - now := time.Now().UTC() + now := m.clk.Now().UTC() + rand.Seed(int64(now.Nanosecond())) headers := []string{ fmt.Sprintf("To: %s", strings.Join(to, ", ")), fmt.Sprintf("From: %s", m.From), diff --git a/mail/mailer_test.go b/mail/mailer_test.go index 41b645f9f..e9cb49b92 100644 --- a/mail/mailer_test.go +++ b/mail/mailer_test.go @@ -4,3 +4,28 @@ // file, You can obtain one at http://mozilla.org/MPL/2.0/. package mail + +import ( + "strings" + "testing" + + "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" + + "github.com/letsencrypt/boulder/test" +) + +func TestGenerateMessage(t *testing.T) { + fc := clock.NewFake() + m := MailerImpl{From: "send@email.com", clk: fc} + message := string(m.generateMessage([]string{"recv@email.com"}, "test subject", "this is the body")) + fields := strings.Split(message, "\r\n") + test.AssertEquals(t, len(fields), 8) + test.AssertEquals(t, fields[0], "To: recv@email.com") + test.AssertEquals(t, fields[1], "From: send@email.com") + test.AssertEquals(t, fields[2], "Subject: test subject") + test.AssertEquals(t, fields[3], "Date: Thu Jan 1 1970 00:00:00 +0000") + test.AssertEquals(t, fields[4], "Message-Id: <19700101T000000.8717895732742165505.send@email.com>") + test.AssertEquals(t, fields[5], "") + test.AssertEquals(t, fields[6], "this is the body") + test.AssertEquals(t, fields[7], "") +} diff --git a/mocks/mocks.go b/mocks/mocks.go index 28dfdfd7b..2070054d4 100644 --- a/mocks/mocks.go +++ b/mocks/mocks.go @@ -422,3 +422,21 @@ func (s *Statter) Inc(metric string, value int64, rate float32) error { func NewStatter() Statter { return Statter{statsd.NoopClient{}, map[string]int64{}} } + +// Mailer is a mock +type Mailer struct { + Messages []string +} + +// Clear removes any previously recorded messages +func (m *Mailer) Clear() { + m.Messages = []string{} +} + +// SendMail is a mock +func (m *Mailer) SendMail(to []string, subject, msg string) (err error) { + for range to { + m.Messages = append(m.Messages, msg) + } + return +} From cb18c22c0d62c9f19493a83a73e24a12e0962e57 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Thu, 31 Dec 2015 23:59:35 +0000 Subject: [PATCH 03/26] Use crypto/rand for Message-Id --- mail/mailer.go | 22 ++++++++++++++++------ mail/mailer_test.go | 4 +++- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/mail/mailer.go b/mail/mailer.go index 239257a50..a381ae837 100644 --- a/mail/mailer.go +++ b/mail/mailer.go @@ -6,8 +6,9 @@ package mail import ( + "crypto/rand" "fmt" - "math/rand" + "math/big" "net" "net/smtp" "strings" @@ -42,27 +43,36 @@ func New(server, port, username, password string) MailerImpl { } } -func (m *MailerImpl) generateMessage(to []string, subject, body string) []byte { +var maxBigInt = big.NewInt(1000000000) // idk + +func (m *MailerImpl) generateMessage(to []string, subject, body string) ([]byte, error) { + mid, err := rand.Int(rand.Reader, maxBigInt) + if err != nil { + return nil, err + } now := m.clk.Now().UTC() - rand.Seed(int64(now.Nanosecond())) headers := []string{ fmt.Sprintf("To: %s", strings.Join(to, ", ")), fmt.Sprintf("From: %s", m.From), fmt.Sprintf("Subject: %s", subject), fmt.Sprintf("Date: %s", now.Format("Mon Jan 2 2006 15:04:05 -0700")), - fmt.Sprintf("Message-Id: <%s.%d.%s>", now.Format("20060102T150405"), rand.Int63(), m.From), + fmt.Sprintf("Message-Id: <%s.%s.%s>", now.Format("20060102T150405"), mid.String(), m.From), } - return []byte(fmt.Sprintf("%s\r\n\r\n%s\r\n", strings.Join(headers, "\r\n"), body)) + return []byte(fmt.Sprintf("%s\r\n\r\n%s\r\n", strings.Join(headers, "\r\n"), body)), nil } // SendMail sends an email to the provided list of recipients. The email body // is simple text. func (m *MailerImpl) SendMail(to []string, subject, msg string) error { + body, err := m.generateMessage(to, subject, msg) + if err != nil { + return err + } return smtp.SendMail( net.JoinHostPort(m.Server, m.Port), m.Auth, m.From, to, - m.generateMessage(to, subject, msg), + body, ) } diff --git a/mail/mailer_test.go b/mail/mailer_test.go index e9cb49b92..2bb63bc9d 100644 --- a/mail/mailer_test.go +++ b/mail/mailer_test.go @@ -17,7 +17,9 @@ import ( func TestGenerateMessage(t *testing.T) { fc := clock.NewFake() m := MailerImpl{From: "send@email.com", clk: fc} - message := string(m.generateMessage([]string{"recv@email.com"}, "test subject", "this is the body")) + messageBytes, err := m.generateMessage([]string{"recv@email.com"}, "test subject", "this is the body") + test.AssertNotError(t, err, "Failed to generate email body") + message := string(messageBytes) fields := strings.Split(message, "\r\n") test.AssertEquals(t, len(fields), 8) test.AssertEquals(t, fields[0], "To: recv@email.com") From 043bfd8041598ded4c468ee1e86973a49ac913b2 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Mon, 4 Jan 2016 10:39:33 -0800 Subject: [PATCH 04/26] Add header/body escaping --- mail/mailer.go | 56 +++++++++++++++++++++++++++++++++------------ mail/mailer_test.go | 18 +++++++++++---- 2 files changed, 55 insertions(+), 19 deletions(-) diff --git a/mail/mailer.go b/mail/mailer.go index a381ae837..7b4e798bc 100644 --- a/mail/mailer.go +++ b/mail/mailer.go @@ -8,14 +8,27 @@ package mail import ( "crypto/rand" "fmt" + "io" + "math" "math/big" "net" "net/smtp" + "strconv" "strings" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" ) +type csprg interface { + Int(io.Reader, *big.Int) (*big.Int, error) +} + +type realSource struct{} + +func (s realSource) Int(reader io.Reader, max *big.Int) (*big.Int, error) { + return rand.Int(reader, max) +} + // Mailer provides the interface for a mailer type Mailer interface { SendMail([]string, string, string) error @@ -23,11 +36,12 @@ type Mailer interface { // MailerImpl defines a mail transfer agent to use for sending mail type MailerImpl struct { - Server string - Port string - Auth smtp.Auth - From string - clk clock.Clock + Server string + Port string + Auth smtp.Auth + From string + clk clock.Clock + csprgSource csprg } // New constructs a Mailer to represent an account on a particular mail @@ -35,30 +49,44 @@ type MailerImpl struct { func New(server, port, username, password string) MailerImpl { auth := smtp.PlainAuth("", username, password, server) return MailerImpl{ - Server: server, - Port: port, - Auth: auth, - From: username, - clk: clock.Default(), + Server: server, + Port: port, + Auth: auth, + From: username, + clk: clock.Default(), + csprgSource: realSource{}, } } -var maxBigInt = big.NewInt(1000000000) // idk +var maxBigInt = big.NewInt(math.MaxInt64) // ??? func (m *MailerImpl) generateMessage(to []string, subject, body string) ([]byte, error) { - mid, err := rand.Int(rand.Reader, maxBigInt) + mid, err := m.csprgSource.Int(rand.Reader, maxBigInt) if err != nil { return nil, err } now := m.clk.Now().UTC() + addrs := []string{} + for _, a := range to { + addrs = append(addrs, strconv.QuoteToASCII(a)) + } headers := []string{ - fmt.Sprintf("To: %s", strings.Join(to, ", ")), + fmt.Sprintf("To: %s", strings.Join(addrs, ", ")), fmt.Sprintf("From: %s", m.From), fmt.Sprintf("Subject: %s", subject), fmt.Sprintf("Date: %s", now.Format("Mon Jan 2 2006 15:04:05 -0700")), fmt.Sprintf("Message-Id: <%s.%s.%s>", now.Format("20060102T150405"), mid.String(), m.From), } - return []byte(fmt.Sprintf("%s\r\n\r\n%s\r\n", strings.Join(headers, "\r\n"), body)), nil + for _, h := range headers[1:] { + quoted := strconv.QuoteToASCII(h) + h = quoted[1 : len(quoted)-1] // remove forced "" quotes + } + quotedBody := strconv.QuoteToASCII(body) + return []byte(fmt.Sprintf( + "%s\r\n\r\n%s\r\n", + strings.Join(headers, "\r\n"), + quotedBody[1:len(quotedBody)-1], + )), nil } // SendMail sends an email to the provided list of recipients. The email body diff --git a/mail/mailer_test.go b/mail/mailer_test.go index 2bb63bc9d..1a52b889c 100644 --- a/mail/mailer_test.go +++ b/mail/mailer_test.go @@ -6,6 +6,8 @@ package mail import ( + "io" + "math/big" "strings" "testing" @@ -14,20 +16,26 @@ import ( "github.com/letsencrypt/boulder/test" ) +type fakeSource struct{} + +func (f fakeSource) Int(reader io.Reader, max *big.Int) (*big.Int, error) { + return big.NewInt(1991), nil +} + func TestGenerateMessage(t *testing.T) { fc := clock.NewFake() - m := MailerImpl{From: "send@email.com", clk: fc} - messageBytes, err := m.generateMessage([]string{"recv@email.com"}, "test subject", "this is the body") + m := MailerImpl{From: "send@email.com", clk: fc, csprgSource: fakeSource{}} + messageBytes, err := m.generateMessage([]string{"recv@email.com"}, "test subject", "this is the body\n") test.AssertNotError(t, err, "Failed to generate email body") message := string(messageBytes) fields := strings.Split(message, "\r\n") test.AssertEquals(t, len(fields), 8) - test.AssertEquals(t, fields[0], "To: recv@email.com") + test.AssertEquals(t, fields[0], "To: \"recv@email.com\"") test.AssertEquals(t, fields[1], "From: send@email.com") test.AssertEquals(t, fields[2], "Subject: test subject") test.AssertEquals(t, fields[3], "Date: Thu Jan 1 1970 00:00:00 +0000") - test.AssertEquals(t, fields[4], "Message-Id: <19700101T000000.8717895732742165505.send@email.com>") + test.AssertEquals(t, fields[4], "Message-Id: <19700101T000000.1991.send@email.com>") test.AssertEquals(t, fields[5], "") - test.AssertEquals(t, fields[6], "this is the body") + test.AssertEquals(t, fields[6], "this is the body\\n") test.AssertEquals(t, fields[7], "") } From d18b8a536d4e07386f29b04683ae502b0c725194 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Mon, 4 Jan 2016 12:20:45 -0800 Subject: [PATCH 05/26] Add DNS ValidationRecord metadata --- bdns/dns.go | 15 ++++++++++----- bdns/dns_test.go | 28 ++++++++++++++++++++++++---- core/objects.go | 11 ++++++++++- mocks/mocks.go | 6 +++--- test/dns-test-srv/main.go | 11 +++++++++++ va/validation-authority.go | 8 +++++--- 6 files changed, 63 insertions(+), 16 deletions(-) diff --git a/bdns/dns.go b/bdns/dns.go index ba7e82be2..91ca23cba 100644 --- a/bdns/dns.go +++ b/bdns/dns.go @@ -116,7 +116,7 @@ var ( // DNSResolver defines methods used for DNS resolution type DNSResolver interface { - LookupTXT(string) ([]string, error) + LookupTXT(string) ([]string, []string, error) LookupHost(string) ([]net.IP, error) LookupCAA(string) ([]*dns.CAA, error) LookupMX(string) ([]string, error) @@ -199,15 +199,15 @@ func (dnsResolver *DNSResolverImpl) exchangeOne(hostname string, qtype uint16, m // LookupTXT sends a DNS query to find all TXT records associated with // the provided hostname. -func (dnsResolver *DNSResolverImpl) LookupTXT(hostname string) ([]string, error) { +func (dnsResolver *DNSResolverImpl) LookupTXT(hostname string) ([]string, []string, error) { var txt []string r, err := dnsResolver.exchangeOne(hostname, dns.TypeTXT, dnsResolver.txtStats) if err != nil { - return nil, err + return nil, nil, err } if r.Rcode != dns.RcodeSuccess { err = fmt.Errorf("DNS failure: %d-%s for TXT query", r.Rcode, dns.RcodeToString[r.Rcode]) - return nil, err + return nil, nil, err } for _, answer := range r.Answer { @@ -218,7 +218,12 @@ func (dnsResolver *DNSResolverImpl) LookupTXT(hostname string) ([]string, error) } } - return txt, err + authorities := []string{} + for _, a := range r.Ns { + authorities = append(authorities, a.String()) + } + + return txt, authorities, err } func isPrivateV4(ip net.IP) bool { diff --git a/bdns/dns_test.go b/bdns/dns_test.go index d26ea00cf..abe4950c4 100644 --- a/bdns/dns_test.go +++ b/bdns/dns_test.go @@ -105,6 +105,17 @@ func mockDNSQuery(w dns.ResponseWriter, r *dns.Msg) { record.Txt = []string{"a", "b", "c"} appendAnswer(record) } + + auth := new(dns.SOA) + auth.Hdr = dns.RR_Header{Name: "letsencrypt.org.", Rrtype: dns.TypeSOA, Class: dns.ClassINET, Ttl: 0} + auth.Ns = "ns.letsencrypt.org." + auth.Mbox = "master.letsencrypt.org." + auth.Serial = 1 + auth.Refresh = 1 + auth.Retry = 1 + auth.Expire = 1 + auth.Minttl = 1 + m.Ns = append(m.Ns, auth) } } @@ -177,7 +188,7 @@ func TestDNSDuplicateServers(t *testing.T) { func TestDNSLookupsNoServer(t *testing.T) { obj := NewTestDNSResolverImpl(time.Second*10, []string{}, testStats) - _, err := obj.LookupTXT("letsencrypt.org") + _, _, err := obj.LookupTXT("letsencrypt.org") test.AssertError(t, err, "No servers") _, err = obj.LookupHost("letsencrypt.org") @@ -191,7 +202,7 @@ func TestDNSServFail(t *testing.T) { obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats) bad := "servfail.com" - _, err := obj.LookupTXT(bad) + _, _, err := obj.LookupTXT(bad) test.AssertError(t, err, "LookupTXT didn't return an error") _, err = obj.LookupHost(bad) @@ -207,11 +218,11 @@ func TestDNSServFail(t *testing.T) { func TestDNSLookupTXT(t *testing.T) { obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats) - a, err := obj.LookupTXT("letsencrypt.org") + a, _, err := obj.LookupTXT("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("split-txt.letsencrypt.org") t.Logf("A: %v ", a) test.AssertNotError(t, err, "No message") test.AssertEquals(t, len(a), 1) @@ -263,3 +274,12 @@ func TestDNSLookupCAA(t *testing.T) { test.AssertNotError(t, err, "CAA lookup failed") test.Assert(t, len(caas) > 0, "Should follow CNAME to find CAA") } + +func TestDNSTXTAuthorities(t *testing.T) { + obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats) + + _, auths, err := obj.LookupTXT("letsencrypt.org") + test.AssertNotError(t, err, "TXT lookup failed") + test.AssertEquals(t, len(auths), 1) + test.AssertEquals(t, auths[0], "letsencrypt.org. 0 IN SOA ns.letsencrypt.org. master.letsencrypt.org. 1 1 1 1 1") +} diff --git a/core/objects.go b/core/objects.go index b8379a89b..18f7e583f 100644 --- a/core/objects.go +++ b/core/objects.go @@ -178,6 +178,9 @@ func (r *Registration) MergeUpdate(input Registration) { // ValidationRecord represents a validation attempt against a specific URL/hostname // and the IP addresses that were resolved and used type ValidationRecord struct { + // DNS only + Authorities []string + // SimpleHTTP only URL string `json:"url,omitempty"` @@ -328,7 +331,7 @@ type Challenge struct { // RecordsSane checks the sanity of a ValidationRecord object before sending it // back to the RA to be stored. func (ch Challenge) RecordsSane() bool { - if ch.Type != ChallengeTypeDNS01 && (ch.ValidationRecord == nil || len(ch.ValidationRecord) == 0) { + if ch.ValidationRecord == nil || len(ch.ValidationRecord) == 0 { return false } @@ -352,6 +355,12 @@ func (ch Challenge) RecordsSane() bool { return false } case ChallengeTypeDNS01: + if len(ch.ValidationRecord) > 1 { + return false + } + if len(ch.ValidationRecord[0].Authorities) == 0 || ch.ValidationRecord[0].Hostname == "" { + return false + } return true default: // Unsupported challenge type return false diff --git a/mocks/mocks.go b/mocks/mocks.go index 28dfdfd7b..892767942 100644 --- a/mocks/mocks.go +++ b/mocks/mocks.go @@ -33,11 +33,11 @@ type DNSResolver struct { } // LookupTXT is a mock -func (mock *DNSResolver) LookupTXT(hostname string) ([]string, error) { +func (mock *DNSResolver) LookupTXT(hostname string) ([]string, []string, error) { if hostname == "_acme-challenge.servfail.com" { - return nil, fmt.Errorf("SERVFAIL") + return nil, nil, fmt.Errorf("SERVFAIL") } - return []string{"hostname"}, nil + return []string{"hostname"}, []string{"respect my authority!"}, nil } // TimeoutError returns a a net.OpError for which Timeout() returns true. diff --git a/test/dns-test-srv/main.go b/test/dns-test-srv/main.go index bbbb0e0ad..fa1842242 100644 --- a/test/dns-test-srv/main.go +++ b/test/dns-test-srv/main.go @@ -134,6 +134,17 @@ func (ts *testSrv) dnsHandler(w dns.ResponseWriter, r *dns.Msg) { } } + auth := new(dns.SOA) + auth.Hdr = dns.RR_Header{Name: "boulder.invalid.", Rrtype: dns.TypeSOA, Class: dns.ClassINET, Ttl: 0} + auth.Ns = "ns.boulder.invalid." + auth.Mbox = "master.boulder.invalid." + auth.Serial = 1 + auth.Refresh = 1 + auth.Retry = 1 + auth.Expire = 1 + auth.Minttl = 1 + m.Ns = append(m.Ns, auth) + w.WriteMsg(m) return } diff --git a/va/validation-authority.go b/va/validation-authority.go index 241c2224f..3b39770a1 100644 --- a/va/validation-authority.go +++ b/va/validation-authority.go @@ -429,8 +429,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, authorities, err := va.DNSResolver.LookupTXT(challengeSubdomain) if err != nil { va.log.Debug(fmt.Sprintf("%s [%s] DNS failure: %s", challenge.Type, identifier, err)) return nil, bdns.ProblemDetailsFromDNSError(err) @@ -439,7 +438,10 @@ func (va *ValidationAuthorityImpl) validateDNS01(identifier core.AcmeIdentifier, for _, element := range txts { if subtle.ConstantTimeCompare([]byte(element), []byte(authorizedKeysDigest)) == 1 { // Successful challenge validation - return nil, nil + return []core.ValidationRecord{{ + Authorities: authorities, + Hostname: identifier.Value, + }}, nil } } From 8022d3a8fe028fd46779954e421c53c9bb60bbda Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Mon, 4 Jan 2016 13:39:26 -0800 Subject: [PATCH 06/26] Update comment --- bdns/dns.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bdns/dns.go b/bdns/dns.go index 91ca23cba..b06378bab 100644 --- a/bdns/dns.go +++ b/bdns/dns.go @@ -198,7 +198,8 @@ func (dnsResolver *DNSResolverImpl) exchangeOne(hostname string, qtype uint16, m } // LookupTXT sends a DNS query to find all TXT records associated with -// the provided hostname. +// the provided hostname which it returns along with the returned +// DNS authority section. func (dnsResolver *DNSResolverImpl) LookupTXT(hostname string) ([]string, []string, error) { var txt []string r, err := dnsResolver.exchangeOne(hostname, dns.TypeTXT, dnsResolver.txtStats) From bee1f46f28febccae9af723bb36d9ef2945b5a42 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Mon, 4 Jan 2016 14:19:54 -0800 Subject: [PATCH 07/26] Add return value names --- bdns/dns.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bdns/dns.go b/bdns/dns.go index b06378bab..04bce2ddb 100644 --- a/bdns/dns.go +++ b/bdns/dns.go @@ -116,7 +116,7 @@ var ( // DNSResolver defines methods used for DNS resolution type DNSResolver interface { - LookupTXT(string) ([]string, []string, error) + LookupTXT(string) (txts []string, authorities []string, err error) LookupHost(string) ([]net.IP, error) LookupCAA(string) ([]*dns.CAA, error) LookupMX(string) ([]string, error) From 116ce96326152b27e91bb9c3170b0a802b85ce07 Mon Sep 17 00:00:00 2001 From: Jeff Hodges Date: Mon, 14 Dec 2015 21:49:28 -0800 Subject: [PATCH 08/26] add retries and context deadlines to DNSResolver This provides a means to add retries to DNS look ups, and, with some future work, end retries early if our request deadline is blown. That future work is tagged with #1292. Updates #1258 --- Godeps/Godeps.json | 5 +- .../src/golang.org/x/net/context/context.go | 447 ++++++++++++++++++ .../x/net/context/ctxhttp/cancelreq.go | 18 + .../x/net/context/ctxhttp/cancelreq_go14.go | 23 + .../x/net/context/ctxhttp/ctxhttp.go | 79 ++++ bdns/dns.go | 137 ++++-- bdns/dns_test.go | 231 +++++++-- cmd/boulder-ra/main.go | 8 +- cmd/boulder-va/main.go | 11 +- cmd/config.go | 10 + mocks/mocks.go | 9 +- ra/registration-authority.go | 17 +- ra/registration-authority_test.go | 20 +- test/boulder-config.json | 2 + va/validation-authority.go | 68 +-- va/validation-authority_test.go | 75 +-- 16 files changed, 998 insertions(+), 162 deletions(-) create mode 100644 Godeps/_workspace/src/golang.org/x/net/context/context.go create mode 100644 Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/cancelreq.go create mode 100644 Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/cancelreq_go14.go create mode 100644 Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/ctxhttp.go diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 23bcfed6c..5f476b3e0 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -1,6 +1,6 @@ { "ImportPath": "github.com/letsencrypt/boulder", - "GoVersion": "go1.5.1", + "GoVersion": "go1.5.2", "Packages": [ "./..." ], @@ -141,6 +141,9 @@ "Rev": "beef0f4390813b96e8e68fd78570396d0f4751fc" }, { + "ImportPath": "golang.org/x/net/context", + "Rev": "ce84af2e5bf21582345e478b116afc7d4efaba3d" + }, "ImportPath": "gopkg.in/gorp.v1", "Comment": "v1.7.1", "Rev": "c87af80f3cc5036b55b83d77171e156791085e2e" diff --git a/Godeps/_workspace/src/golang.org/x/net/context/context.go b/Godeps/_workspace/src/golang.org/x/net/context/context.go new file mode 100644 index 000000000..ef2f3e86f --- /dev/null +++ b/Godeps/_workspace/src/golang.org/x/net/context/context.go @@ -0,0 +1,447 @@ +// Copyright 2014 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package context defines the Context type, which carries deadlines, +// cancelation signals, and other request-scoped values across API boundaries +// and between processes. +// +// Incoming requests to a server should create a Context, and outgoing calls to +// servers should accept a Context. The chain of function calls between must +// propagate the Context, optionally replacing it with a modified copy created +// using WithDeadline, WithTimeout, WithCancel, or WithValue. +// +// Programs that use Contexts should follow these rules to keep interfaces +// consistent across packages and enable static analysis tools to check context +// propagation: +// +// Do not store Contexts inside a struct type; instead, pass a Context +// explicitly to each function that needs it. The Context should be the first +// parameter, typically named ctx: +// +// func DoSomething(ctx context.Context, arg Arg) error { +// // ... use ctx ... +// } +// +// Do not pass a nil Context, even if a function permits it. Pass context.TODO +// if you are unsure about which Context to use. +// +// Use context Values only for request-scoped data that transits processes and +// APIs, not for passing optional parameters to functions. +// +// The same Context may be passed to functions running in different goroutines; +// Contexts are safe for simultaneous use by multiple goroutines. +// +// See http://blog.golang.org/context for example code for a server that uses +// Contexts. +package context + +import ( + "errors" + "fmt" + "sync" + "time" +) + +// A Context carries a deadline, a cancelation signal, and other values across +// API boundaries. +// +// Context's methods may be called by multiple goroutines simultaneously. +type Context interface { + // Deadline returns the time when work done on behalf of this context + // should be canceled. Deadline returns ok==false when no deadline is + // set. Successive calls to Deadline return the same results. + Deadline() (deadline time.Time, ok bool) + + // Done returns a channel that's closed when work done on behalf of this + // context should be canceled. Done may return nil if this context can + // never be canceled. Successive calls to Done return the same value. + // + // WithCancel arranges for Done to be closed when cancel is called; + // WithDeadline arranges for Done to be closed when the deadline + // expires; WithTimeout arranges for Done to be closed when the timeout + // elapses. + // + // Done is provided for use in select statements: + // + // // Stream generates values with DoSomething and sends them to out + // // until DoSomething returns an error or ctx.Done is closed. + // func Stream(ctx context.Context, out <-chan Value) error { + // for { + // v, err := DoSomething(ctx) + // if err != nil { + // return err + // } + // select { + // case <-ctx.Done(): + // return ctx.Err() + // case out <- v: + // } + // } + // } + // + // See http://blog.golang.org/pipelines for more examples of how to use + // a Done channel for cancelation. + Done() <-chan struct{} + + // Err returns a non-nil error value after Done is closed. Err returns + // Canceled if the context was canceled or DeadlineExceeded if the + // context's deadline passed. No other values for Err are defined. + // After Done is closed, successive calls to Err return the same value. + Err() error + + // Value returns the value associated with this context for key, or nil + // if no value is associated with key. Successive calls to Value with + // the same key returns the same result. + // + // Use context values only for request-scoped data that transits + // processes and API boundaries, not for passing optional parameters to + // functions. + // + // A key identifies a specific value in a Context. Functions that wish + // to store values in Context typically allocate a key in a global + // variable then use that key as the argument to context.WithValue and + // Context.Value. A key can be any type that supports equality; + // packages should define keys as an unexported type to avoid + // collisions. + // + // Packages that define a Context key should provide type-safe accessors + // for the values stores using that key: + // + // // Package user defines a User type that's stored in Contexts. + // package user + // + // import "golang.org/x/net/context" + // + // // User is the type of value stored in the Contexts. + // type User struct {...} + // + // // key is an unexported type for keys defined in this package. + // // This prevents collisions with keys defined in other packages. + // type key int + // + // // userKey is the key for user.User values in Contexts. It is + // // unexported; clients use user.NewContext and user.FromContext + // // instead of using this key directly. + // var userKey key = 0 + // + // // NewContext returns a new Context that carries value u. + // func NewContext(ctx context.Context, u *User) context.Context { + // return context.WithValue(ctx, userKey, u) + // } + // + // // FromContext returns the User value stored in ctx, if any. + // func FromContext(ctx context.Context) (*User, bool) { + // u, ok := ctx.Value(userKey).(*User) + // return u, ok + // } + Value(key interface{}) interface{} +} + +// Canceled is the error returned by Context.Err when the context is canceled. +var Canceled = errors.New("context canceled") + +// DeadlineExceeded is the error returned by Context.Err when the context's +// deadline passes. +var DeadlineExceeded = errors.New("context deadline exceeded") + +// An emptyCtx is never canceled, has no values, and has no deadline. It is not +// struct{}, since vars of this type must have distinct addresses. +type emptyCtx int + +func (*emptyCtx) Deadline() (deadline time.Time, ok bool) { + return +} + +func (*emptyCtx) Done() <-chan struct{} { + return nil +} + +func (*emptyCtx) Err() error { + return nil +} + +func (*emptyCtx) Value(key interface{}) interface{} { + return nil +} + +func (e *emptyCtx) String() string { + switch e { + case background: + return "context.Background" + case todo: + return "context.TODO" + } + return "unknown empty Context" +} + +var ( + background = new(emptyCtx) + todo = new(emptyCtx) +) + +// Background returns a non-nil, empty Context. It is never canceled, has no +// values, and has no deadline. It is typically used by the main function, +// initialization, and tests, and as the top-level Context for incoming +// requests. +func Background() Context { + return background +} + +// TODO returns a non-nil, empty Context. Code should use context.TODO when +// it's unclear which Context to use or it's is not yet available (because the +// surrounding function has not yet been extended to accept a Context +// parameter). TODO is recognized by static analysis tools that determine +// whether Contexts are propagated correctly in a program. +func TODO() Context { + return todo +} + +// A CancelFunc tells an operation to abandon its work. +// A CancelFunc does not wait for the work to stop. +// After the first call, subsequent calls to a CancelFunc do nothing. +type CancelFunc func() + +// WithCancel returns a copy of parent with a new Done channel. The returned +// context's Done channel is closed when the returned cancel function is called +// or when the parent context's Done channel is closed, whichever happens first. +// +// Canceling this context releases resources associated with it, so code should +// call cancel as soon as the operations running in this Context complete. +func WithCancel(parent Context) (ctx Context, cancel CancelFunc) { + c := newCancelCtx(parent) + propagateCancel(parent, &c) + return &c, func() { c.cancel(true, Canceled) } +} + +// newCancelCtx returns an initialized cancelCtx. +func newCancelCtx(parent Context) cancelCtx { + return cancelCtx{ + Context: parent, + done: make(chan struct{}), + } +} + +// propagateCancel arranges for child to be canceled when parent is. +func propagateCancel(parent Context, child canceler) { + if parent.Done() == nil { + return // parent is never canceled + } + if p, ok := parentCancelCtx(parent); ok { + p.mu.Lock() + if p.err != nil { + // parent has already been canceled + child.cancel(false, p.err) + } else { + if p.children == nil { + p.children = make(map[canceler]bool) + } + p.children[child] = true + } + p.mu.Unlock() + } else { + go func() { + select { + case <-parent.Done(): + child.cancel(false, parent.Err()) + case <-child.Done(): + } + }() + } +} + +// parentCancelCtx follows a chain of parent references until it finds a +// *cancelCtx. This function understands how each of the concrete types in this +// package represents its parent. +func parentCancelCtx(parent Context) (*cancelCtx, bool) { + for { + switch c := parent.(type) { + case *cancelCtx: + return c, true + case *timerCtx: + return &c.cancelCtx, true + case *valueCtx: + parent = c.Context + default: + return nil, false + } + } +} + +// removeChild removes a context from its parent. +func removeChild(parent Context, child canceler) { + p, ok := parentCancelCtx(parent) + if !ok { + return + } + p.mu.Lock() + if p.children != nil { + delete(p.children, child) + } + p.mu.Unlock() +} + +// A canceler is a context type that can be canceled directly. The +// implementations are *cancelCtx and *timerCtx. +type canceler interface { + cancel(removeFromParent bool, err error) + Done() <-chan struct{} +} + +// A cancelCtx can be canceled. When canceled, it also cancels any children +// that implement canceler. +type cancelCtx struct { + Context + + done chan struct{} // closed by the first cancel call. + + mu sync.Mutex + children map[canceler]bool // set to nil by the first cancel call + err error // set to non-nil by the first cancel call +} + +func (c *cancelCtx) Done() <-chan struct{} { + return c.done +} + +func (c *cancelCtx) Err() error { + c.mu.Lock() + defer c.mu.Unlock() + return c.err +} + +func (c *cancelCtx) String() string { + return fmt.Sprintf("%v.WithCancel", c.Context) +} + +// cancel closes c.done, cancels each of c's children, and, if +// removeFromParent is true, removes c from its parent's children. +func (c *cancelCtx) cancel(removeFromParent bool, err error) { + if err == nil { + panic("context: internal error: missing cancel error") + } + c.mu.Lock() + if c.err != nil { + c.mu.Unlock() + return // already canceled + } + c.err = err + close(c.done) + for child := range c.children { + // NOTE: acquiring the child's lock while holding parent's lock. + child.cancel(false, err) + } + c.children = nil + c.mu.Unlock() + + if removeFromParent { + removeChild(c.Context, c) + } +} + +// WithDeadline returns a copy of the parent context with the deadline adjusted +// to be no later than d. If the parent's deadline is already earlier than d, +// WithDeadline(parent, d) is semantically equivalent to parent. The returned +// context's Done channel is closed when the deadline expires, when the returned +// cancel function is called, or when the parent context's Done channel is +// closed, whichever happens first. +// +// Canceling this context releases resources associated with it, so code should +// call cancel as soon as the operations running in this Context complete. +func WithDeadline(parent Context, deadline time.Time) (Context, CancelFunc) { + if cur, ok := parent.Deadline(); ok && cur.Before(deadline) { + // The current deadline is already sooner than the new one. + return WithCancel(parent) + } + c := &timerCtx{ + cancelCtx: newCancelCtx(parent), + deadline: deadline, + } + propagateCancel(parent, c) + d := deadline.Sub(time.Now()) + if d <= 0 { + c.cancel(true, DeadlineExceeded) // deadline has already passed + return c, func() { c.cancel(true, Canceled) } + } + c.mu.Lock() + defer c.mu.Unlock() + if c.err == nil { + c.timer = time.AfterFunc(d, func() { + c.cancel(true, DeadlineExceeded) + }) + } + return c, func() { c.cancel(true, Canceled) } +} + +// A timerCtx carries a timer and a deadline. It embeds a cancelCtx to +// implement Done and Err. It implements cancel by stopping its timer then +// delegating to cancelCtx.cancel. +type timerCtx struct { + cancelCtx + timer *time.Timer // Under cancelCtx.mu. + + deadline time.Time +} + +func (c *timerCtx) Deadline() (deadline time.Time, ok bool) { + return c.deadline, true +} + +func (c *timerCtx) String() string { + return fmt.Sprintf("%v.WithDeadline(%s [%s])", c.cancelCtx.Context, c.deadline, c.deadline.Sub(time.Now())) +} + +func (c *timerCtx) cancel(removeFromParent bool, err error) { + c.cancelCtx.cancel(false, err) + if removeFromParent { + // Remove this timerCtx from its parent cancelCtx's children. + removeChild(c.cancelCtx.Context, c) + } + c.mu.Lock() + if c.timer != nil { + c.timer.Stop() + c.timer = nil + } + c.mu.Unlock() +} + +// WithTimeout returns WithDeadline(parent, time.Now().Add(timeout)). +// +// Canceling this context releases resources associated with it, so code should +// call cancel as soon as the operations running in this Context complete: +// +// func slowOperationWithTimeout(ctx context.Context) (Result, error) { +// ctx, cancel := context.WithTimeout(ctx, 100*time.Millisecond) +// defer cancel() // releases resources if slowOperation completes before timeout elapses +// return slowOperation(ctx) +// } +func WithTimeout(parent Context, timeout time.Duration) (Context, CancelFunc) { + return WithDeadline(parent, time.Now().Add(timeout)) +} + +// WithValue returns a copy of parent in which the value associated with key is +// val. +// +// Use context Values only for request-scoped data that transits processes and +// APIs, not for passing optional parameters to functions. +func WithValue(parent Context, key interface{}, val interface{}) Context { + return &valueCtx{parent, key, val} +} + +// A valueCtx carries a key-value pair. It implements Value for that key and +// delegates all other calls to the embedded Context. +type valueCtx struct { + Context + key, val interface{} +} + +func (c *valueCtx) String() string { + return fmt.Sprintf("%v.WithValue(%#v, %#v)", c.Context, c.key, c.val) +} + +func (c *valueCtx) Value(key interface{}) interface{} { + if c.key == key { + return c.val + } + return c.Context.Value(key) +} diff --git a/Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/cancelreq.go b/Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/cancelreq.go new file mode 100644 index 000000000..48610e362 --- /dev/null +++ b/Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/cancelreq.go @@ -0,0 +1,18 @@ +// Copyright 2015 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// +build go1.5 + +package ctxhttp + +import "net/http" + +func canceler(client *http.Client, req *http.Request) func() { + ch := make(chan struct{}) + req.Cancel = ch + + return func() { + close(ch) + } +} diff --git a/Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/cancelreq_go14.go b/Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/cancelreq_go14.go new file mode 100644 index 000000000..56bcbadb8 --- /dev/null +++ b/Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/cancelreq_go14.go @@ -0,0 +1,23 @@ +// Copyright 2015 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// +build !go1.5 + +package ctxhttp + +import "net/http" + +type requestCanceler interface { + CancelRequest(*http.Request) +} + +func canceler(client *http.Client, req *http.Request) func() { + rc, ok := client.Transport.(requestCanceler) + if !ok { + return func() {} + } + return func() { + rc.CancelRequest(req) + } +} diff --git a/Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/ctxhttp.go b/Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/ctxhttp.go new file mode 100644 index 000000000..fcbd73b8a --- /dev/null +++ b/Godeps/_workspace/src/golang.org/x/net/context/ctxhttp/ctxhttp.go @@ -0,0 +1,79 @@ +// Copyright 2015 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package ctxhttp provides helper functions for performing context-aware HTTP requests. +package ctxhttp + +import ( + "io" + "net/http" + "net/url" + "strings" + + "github.com/letsencrypt/boulder/Godeps/_workspace/src/golang.org/x/net/context" +) + +// Do sends an HTTP request with the provided http.Client and returns an HTTP response. +// If the client is nil, http.DefaultClient is used. +// If the context is canceled or times out, ctx.Err() will be returned. +func Do(ctx context.Context, client *http.Client, req *http.Request) (*http.Response, error) { + if client == nil { + client = http.DefaultClient + } + + // Request cancelation changed in Go 1.5, see cancelreq.go and cancelreq_go14.go. + cancel := canceler(client, req) + + type responseAndError struct { + resp *http.Response + err error + } + result := make(chan responseAndError, 1) + + go func() { + resp, err := client.Do(req) + result <- responseAndError{resp, err} + }() + + select { + case <-ctx.Done(): + cancel() + return nil, ctx.Err() + case r := <-result: + return r.resp, r.err + } +} + +// Get issues a GET request via the Do function. +func Get(ctx context.Context, client *http.Client, url string) (*http.Response, error) { + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return nil, err + } + return Do(ctx, client, req) +} + +// Head issues a HEAD request via the Do function. +func Head(ctx context.Context, client *http.Client, url string) (*http.Response, error) { + req, err := http.NewRequest("HEAD", url, nil) + if err != nil { + return nil, err + } + return Do(ctx, client, req) +} + +// Post issues a POST request via the Do function. +func Post(ctx context.Context, client *http.Client, url string, bodyType string, body io.Reader) (*http.Response, error) { + req, err := http.NewRequest("POST", url, body) + if err != nil { + return nil, err + } + req.Header.Set("Content-Type", bodyType) + return Do(ctx, client, req) +} + +// PostForm issues a POST request via the Do function. +func PostForm(ctx context.Context, client *http.Client, url string, data url.Values) (*http.Response, error) { + return Post(ctx, client, url, "application/x-www-form-urlencoded", strings.NewReader(data.Encode())) +} diff --git a/bdns/dns.go b/bdns/dns.go index ba7e82be2..d86497c72 100644 --- a/bdns/dns.go +++ b/bdns/dns.go @@ -12,7 +12,9 @@ import ( "strings" "time" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/miekg/dns" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/golang.org/x/net/context" "github.com/letsencrypt/boulder/metrics" ) @@ -114,19 +116,21 @@ var ( } ) -// DNSResolver defines methods used for DNS resolution +// DNSResolver queries for DNS records type DNSResolver interface { - LookupTXT(string) ([]string, error) - LookupHost(string) ([]net.IP, error) - LookupCAA(string) ([]*dns.CAA, error) - LookupMX(string) ([]string, error) + LookupTXT(context.Context, string) ([]string, error) + LookupHost(context.Context, string) ([]net.IP, error) + LookupCAA(context.Context, string) ([]*dns.CAA, error) + LookupMX(context.Context, string) ([]string, error) } // DNSResolverImpl represents a client that talks to an external resolver type DNSResolverImpl struct { - DNSClient *dns.Client + DNSClient exchanger Servers []string allowRestrictedAddresses bool + maxTries int + clk clock.Clock stats metrics.Scope txtStats metrics.Scope aStats metrics.Scope @@ -136,9 +140,14 @@ type DNSResolverImpl struct { var _ DNSResolver = &DNSResolverImpl{} +type exchanger interface { + Exchange(m *dns.Msg, a string) (*dns.Msg, time.Duration, error) +} + // NewDNSResolverImpl constructs a new DNS resolver object that utilizes the // provided list of DNS servers for resolution. -func NewDNSResolverImpl(readTimeout time.Duration, servers []string, stats metrics.Scope) *DNSResolverImpl { +func NewDNSResolverImpl(readTimeout time.Duration, servers []string, stats metrics.Scope, clk clock.Clock, maxTries int) *DNSResolverImpl { + // TODO(jmhodges): make constructor use an Option func pattern dnsClient := new(dns.Client) // Set timeout for underlying net.Conn @@ -149,19 +158,21 @@ func NewDNSResolverImpl(readTimeout time.Duration, servers []string, stats metri DNSClient: dnsClient, Servers: servers, allowRestrictedAddresses: false, - stats: stats, - txtStats: stats.NewScope("TXT"), - aStats: stats.NewScope("A"), - caaStats: stats.NewScope("CAA"), - mxStats: stats.NewScope("MX"), + maxTries: maxTries, + clk: clk, + stats: stats, + txtStats: stats.NewScope("TXT"), + aStats: stats.NewScope("A"), + caaStats: stats.NewScope("CAA"), + mxStats: stats.NewScope("MX"), } } // NewTestDNSResolverImpl constructs a new DNS resolver object that utilizes the // provided list of DNS servers for resolution and will allow loopback addresses. // This constructor should *only* be called from tests (unit or integration). -func NewTestDNSResolverImpl(readTimeout time.Duration, servers []string, stats metrics.Scope) *DNSResolverImpl { - resolver := NewDNSResolverImpl(readTimeout, servers, stats) +func NewTestDNSResolverImpl(readTimeout time.Duration, servers []string, stats metrics.Scope, clk clock.Clock, maxTries int) *DNSResolverImpl { + resolver := NewDNSResolverImpl(readTimeout, servers, stats, clk, maxTries) resolver.allowRestrictedAddresses = true return resolver } @@ -170,7 +181,7 @@ func NewTestDNSResolverImpl(readTimeout time.Duration, servers []string, stats m // out of the server list, returning the response, time, and error (if any). // This method sets the DNSSEC OK bit on the message to true before sending // it to the resolver in case validation isn't the resolvers default behaviour. -func (dnsResolver *DNSResolverImpl) exchangeOne(hostname string, qtype uint16, msgStats metrics.Scope) (rsp *dns.Msg, err error) { +func (dnsResolver *DNSResolverImpl) exchangeOne(ctx context.Context, hostname string, qtype uint16, msgStats metrics.Scope) (*dns.Msg, error) { m := new(dns.Msg) // Set question type m.SetQuestion(dns.Fqdn(hostname), qtype) @@ -178,8 +189,7 @@ func (dnsResolver *DNSResolverImpl) exchangeOne(hostname string, qtype uint16, m m.SetEdns0(4096, true) if len(dnsResolver.Servers) < 1 { - err = fmt.Errorf("Not configured with at least one DNS Server") - return + return nil, fmt.Errorf("Not configured with at least one DNS Server") } dnsResolver.stats.Inc("Rate", 1) @@ -187,21 +197,58 @@ func (dnsResolver *DNSResolverImpl) exchangeOne(hostname string, qtype uint16, m // Randomly pick a server chosenServer := dnsResolver.Servers[rand.Intn(len(dnsResolver.Servers))] - msg, rtt, err := dnsResolver.DNSClient.Exchange(m, chosenServer) - msgStats.TimingDuration("RTT", rtt) - if err == nil { - msgStats.Inc("Successes", 1) - } else { - msgStats.Inc("Errors", 1) + client := dnsResolver.DNSClient + + tries := 1 + start := dnsResolver.clk.Now() + msgStats.Inc("Calls", 1) + defer msgStats.TimingDuration("Latency", dnsResolver.clk.Now().Sub(start)) + for { + msgStats.Inc("Tries", 1) + ch := make(chan dnsResp, 1) + + go func() { + rsp, rtt, err := client.Exchange(m, chosenServer) + msgStats.TimingDuration("SingleTryLatency", rtt) + ch <- dnsResp{m: rsp, err: err} + }() + select { + case <-ctx.Done(): + msgStats.Inc("Cancels", 1) + msgStats.Inc("Errors", 1) + return nil, ctx.Err() + case r := <-ch: + if r.err != nil { + msgStats.Inc("Errors", 1) + operr, ok := r.err.(*net.OpError) + isRetryable := ok && operr.Temporary() + hasRetriesLeft := tries < dnsResolver.maxTries + if isRetryable && hasRetriesLeft { + tries++ + continue + } else if isRetryable && !hasRetriesLeft { + msgStats.Inc("RanOutOfTries", 1) + } + } else { + msgStats.Inc("Successes", 1) + } + return r.m, r.err + } } - return msg, err } -// LookupTXT sends a DNS query to find all TXT records associated with -// the provided hostname. -func (dnsResolver *DNSResolverImpl) LookupTXT(hostname string) ([]string, error) { +type dnsResp struct { + m *dns.Msg + err error +} + +// LookupTXT sends a DNS query to find all TXT records associated with the +// provided hostname. It will retry requests in the case of temporary network +// errors. It can return net package, context.Canceled, and +// context.DeadlineExceeded errors. +func (dnsResolver *DNSResolverImpl) LookupTXT(ctx context.Context, hostname string) ([]string, error) { var txt []string - r, err := dnsResolver.exchangeOne(hostname, dns.TypeTXT, dnsResolver.txtStats) + r, err := dnsResolver.exchangeOne(ctx, hostname, dns.TypeTXT, dnsResolver.txtStats) if err != nil { return nil, err } @@ -230,13 +277,15 @@ func isPrivateV4(ip net.IP) bool { return false } -// LookupHost sends a DNS query to find all A records associated with the provided -// hostname. This method assumes that the external resolver will chase CNAME/DNAME -// aliases and return relevant A records. -func (dnsResolver *DNSResolverImpl) LookupHost(hostname string) ([]net.IP, error) { +// LookupHost sends a DNS query to find all A records associated with the +// provided hostname. This method assumes that the external resolver will chase +// CNAME/DNAME aliases and return relevant A records. It will retry requests in +// the case of temporary network errors. It can return net package, +// context.Canceled, and context.DeadlineExceeded errors. +func (dnsResolver *DNSResolverImpl) LookupHost(ctx context.Context, hostname string) ([]net.IP, error) { var addrs []net.IP - r, err := dnsResolver.exchangeOne(hostname, dns.TypeA, dnsResolver.aStats) + r, err := dnsResolver.exchangeOne(ctx, hostname, dns.TypeA, dnsResolver.aStats) if err != nil { return addrs, err } @@ -256,11 +305,13 @@ func (dnsResolver *DNSResolverImpl) LookupHost(hostname string) ([]net.IP, error return addrs, nil } -// LookupCAA sends a DNS query to find all CAA records associated with -// the provided hostname. If the response code from the resolver is -// SERVFAIL an empty slice of CAA records is returned. -func (dnsResolver *DNSResolverImpl) LookupCAA(hostname string) ([]*dns.CAA, error) { - r, err := dnsResolver.exchangeOne(hostname, dns.TypeCAA, dnsResolver.caaStats) +// LookupCAA sends a DNS query to find all CAA records associated with the +// provided hostname. If the response code from the resolver is SERVFAIL an +// empty slice of CAA records is returned. It will retry requests in the case +// of temporary network errors. It can return net package, context.Canceled, and +// context.DeadlineExceeded errors. +func (dnsResolver *DNSResolverImpl) LookupCAA(ctx context.Context, hostname string) ([]*dns.CAA, error) { + r, err := dnsResolver.exchangeOne(ctx, hostname, dns.TypeCAA, dnsResolver.caaStats) if err != nil { return nil, err } @@ -282,10 +333,12 @@ func (dnsResolver *DNSResolverImpl) LookupCAA(hostname string) ([]*dns.CAA, erro return CAAs, nil } -// LookupMX sends a DNS query to find a MX record associated hostname and returns the -// record target. -func (dnsResolver *DNSResolverImpl) LookupMX(hostname string) ([]string, error) { - r, err := dnsResolver.exchangeOne(hostname, dns.TypeMX, dnsResolver.mxStats) +// LookupMX sends a DNS query to find a MX record associated hostname and +// returns the record target. It will retry requests in the case of temporary +// network errors. It can return net package, context.Canceled, and +// context.DeadlineExceeded errors. +func (dnsResolver *DNSResolverImpl) LookupMX(ctx context.Context, hostname string) ([]string, error) { + r, err := dnsResolver.exchangeOne(ctx, hostname, dns.TypeMX, dnsResolver.mxStats) if err != nil { return nil, err } diff --git a/bdns/dns_test.go b/bdns/dns_test.go index d26ea00cf..b0fa1d0ae 100644 --- a/bdns/dns_test.go +++ b/bdns/dns_test.go @@ -6,14 +6,19 @@ package bdns import ( + "errors" "fmt" "net" "os" "strings" + "sync" "testing" "time" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/golang.org/x/net/context" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/cactus/go-statsd-client/statsd" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/miekg/dns" "github.com/letsencrypt/boulder/metrics" "github.com/letsencrypt/boulder/test" @@ -151,67 +156,67 @@ func newTestStats() metrics.Scope { var testStats = newTestStats() func TestDNSNoServers(t *testing.T) { - obj := NewTestDNSResolverImpl(time.Hour, []string{}, testStats) + obj := NewTestDNSResolverImpl(time.Hour, []string{}, testStats, clock.NewFake(), 1) - _, err := obj.LookupHost("letsencrypt.org") + _, err := obj.LookupHost(context.Background(), "letsencrypt.org") test.AssertError(t, err, "No servers") } func TestDNSOneServer(t *testing.T) { - obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats) + obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats, clock.NewFake(), 1) - _, err := obj.LookupHost("letsencrypt.org") + _, err := obj.LookupHost(context.Background(), "letsencrypt.org") test.AssertNotError(t, err, "No message") } func TestDNSDuplicateServers(t *testing.T) { - obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr, dnsLoopbackAddr}, testStats) + obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr, dnsLoopbackAddr}, testStats, clock.NewFake(), 1) - _, err := obj.LookupHost("letsencrypt.org") + _, err := obj.LookupHost(context.Background(), "letsencrypt.org") test.AssertNotError(t, err, "No message") } func TestDNSLookupsNoServer(t *testing.T) { - obj := NewTestDNSResolverImpl(time.Second*10, []string{}, testStats) + obj := NewTestDNSResolverImpl(time.Second*10, []string{}, testStats, clock.NewFake(), 1) - _, err := obj.LookupTXT("letsencrypt.org") + _, err := obj.LookupTXT(context.Background(), "letsencrypt.org") test.AssertError(t, err, "No servers") - _, err = obj.LookupHost("letsencrypt.org") + _, err = obj.LookupHost(context.Background(), "letsencrypt.org") test.AssertError(t, err, "No servers") - _, err = obj.LookupCAA("letsencrypt.org") + _, err = obj.LookupCAA(context.Background(), "letsencrypt.org") test.AssertError(t, err, "No servers") } func TestDNSServFail(t *testing.T) { - obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats) + obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats, clock.NewFake(), 1) bad := "servfail.com" - _, err := obj.LookupTXT(bad) + _, err := obj.LookupTXT(context.Background(), bad) test.AssertError(t, err, "LookupTXT didn't return an error") - _, err = obj.LookupHost(bad) + _, err = obj.LookupHost(context.Background(), bad) test.AssertError(t, err, "LookupHost didn't return an error") // CAA lookup ignores validation failures from the resolver for now // and returns an empty list of CAA records. - emptyCaa, err := obj.LookupCAA(bad) + emptyCaa, err := obj.LookupCAA(context.Background(), bad) test.Assert(t, len(emptyCaa) == 0, "Query returned non-empty list of CAA records") test.AssertNotError(t, err, "LookupCAA returned an error") } func TestDNSLookupTXT(t *testing.T) { - obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats) + obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats, clock.NewFake(), 1) - a, err := obj.LookupTXT("letsencrypt.org") + a, err := obj.LookupTXT(context.Background(), "letsencrypt.org") t.Logf("A: %v", a) test.AssertNotError(t, err, "No message") - a, err = obj.LookupTXT("split-txt.letsencrypt.org") + a, err = obj.LookupTXT(context.Background(), "split-txt.letsencrypt.org") t.Logf("A: %v ", a) test.AssertNotError(t, err, "No message") test.AssertEquals(t, len(a), 1) @@ -219,47 +224,219 @@ func TestDNSLookupTXT(t *testing.T) { } func TestDNSLookupHost(t *testing.T) { - obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats) + obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats, clock.NewFake(), 1) - ip, err := obj.LookupHost("servfail.com") + ip, err := obj.LookupHost(context.Background(), "servfail.com") t.Logf("servfail.com - IP: %s, Err: %s", ip, err) test.AssertError(t, err, "Server failure") test.Assert(t, len(ip) == 0, "Should not have IPs") - ip, err = obj.LookupHost("nonexistent.letsencrypt.org") + ip, err = obj.LookupHost(context.Background(), "nonexistent.letsencrypt.org") t.Logf("nonexistent.letsencrypt.org - IP: %s, Err: %s", ip, err) test.AssertNotError(t, err, "Not an error to not exist") test.Assert(t, len(ip) == 0, "Should not have IPs") // Single IPv4 address - ip, err = obj.LookupHost("cps.letsencrypt.org") + ip, err = obj.LookupHost(context.Background(), "cps.letsencrypt.org") t.Logf("cps.letsencrypt.org - IP: %s, Err: %s", ip, err) test.AssertNotError(t, err, "Not an error to exist") test.Assert(t, len(ip) == 1, "Should have IP") - ip, err = obj.LookupHost("cps.letsencrypt.org") + ip, err = obj.LookupHost(context.Background(), "cps.letsencrypt.org") t.Logf("cps.letsencrypt.org - IP: %s, Err: %s", ip, err) test.AssertNotError(t, err, "Not an error to exist") test.Assert(t, len(ip) == 1, "Should have IP") // No IPv6 - ip, err = obj.LookupHost("v6.letsencrypt.org") + ip, err = obj.LookupHost(context.Background(), "v6.letsencrypt.org") t.Logf("v6.letsencrypt.org - IP: %s, Err: %s", ip, err) test.AssertNotError(t, err, "Not an error to exist") test.Assert(t, len(ip) == 0, "Should not have IPs") } func TestDNSLookupCAA(t *testing.T) { - obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats) + obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats, clock.NewFake(), 1) - caas, err := obj.LookupCAA("bracewel.net") + caas, err := obj.LookupCAA(context.Background(), "bracewel.net") test.AssertNotError(t, err, "CAA lookup failed") test.Assert(t, len(caas) > 0, "Should have CAA records") - caas, err = obj.LookupCAA("nonexistent.letsencrypt.org") + caas, err = obj.LookupCAA(context.Background(), "nonexistent.letsencrypt.org") test.AssertNotError(t, err, "CAA lookup failed") test.Assert(t, len(caas) == 0, "Shouldn't have CAA records") - caas, err = obj.LookupCAA("cname.example.com") + caas, err = obj.LookupCAA(context.Background(), "cname.example.com") test.AssertNotError(t, err, "CAA lookup failed") test.Assert(t, len(caas) > 0, "Should follow CNAME to find CAA") } + +type testExchanger struct { + sync.Mutex + count int + errs []error +} + +var errTooManyRequests = errors.New("too many requests") + +func (te *testExchanger) Exchange(m *dns.Msg, a string) (*dns.Msg, time.Duration, error) { + te.Lock() + defer te.Unlock() + msg := &dns.Msg{ + MsgHdr: dns.MsgHdr{Rcode: dns.RcodeSuccess}, + } + if len(te.errs) <= te.count { + return nil, 0, errTooManyRequests + } + err := te.errs[te.count] + te.count++ + + return msg, 2 * time.Millisecond, err +} + +func TestRetry(t *testing.T) { + isTempErr := &net.OpError{Op: "read", Err: tempError(true)} + nonTempErr := &net.OpError{Op: "read", Err: tempError(false)} + type testCase struct { + maxTries int + expected int + te *testExchanger + } + tests := []*testCase{ + // The success on first try case + { + maxTries: 3, + expected: 1, + te: &testExchanger{ + errs: []error{nil}, + }, + }, + // Immediate non-OpError, error returns immediately + { + maxTries: 3, + expected: 1, + te: &testExchanger{ + errs: []error{errors.New("nope")}, + }, + }, + // Temporary err, then non-OpError stops at two tries + { + maxTries: 3, + expected: 2, + te: &testExchanger{ + errs: []error{isTempErr, errors.New("nope")}, + }, + }, + // Temporary error given always + { + maxTries: 3, + expected: 3, + te: &testExchanger{ + errs: []error{ + isTempErr, + isTempErr, + isTempErr, + }, + }, + }, + // Even with maxTries at 0, we should still let a single request go + // through + { + maxTries: 0, + expected: 1, + te: &testExchanger{ + errs: []error{nil}, + }, + }, + // Temporary error given just once causes two tries + { + maxTries: 3, + expected: 2, + te: &testExchanger{ + errs: []error{ + isTempErr, + nil, + }, + }, + }, + // Temporary error given twice causes three tries + { + maxTries: 3, + expected: 3, + te: &testExchanger{ + errs: []error{ + isTempErr, + isTempErr, + nil, + }, + }, + }, + // Temporary error given thrice causes three tries and fails + { + maxTries: 3, + expected: 3, + te: &testExchanger{ + errs: []error{ + isTempErr, + isTempErr, + isTempErr, + }, + }, + }, + // temporary then non-Temporary error causes two retries + { + maxTries: 3, + expected: 2, + te: &testExchanger{ + errs: []error{ + isTempErr, + nonTempErr, + }, + }, + }, + } + + for i, tc := range tests { + dr := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats, clock.NewFake(), tc.maxTries) + + dr.DNSClient = tc.te + _, err := dr.LookupTXT(context.Background(), "example.com") + if err == errTooManyRequests { + t.Errorf("#%d, sent more requests than the test case handles", i) + } + expectedErr := tc.te.errs[tc.expected-1] + if err != expectedErr { + t.Errorf("#%d, error, expected %v, got %v", i, expectedErr, err) + } + if tc.expected != tc.te.count { + t.Errorf("#%d, count, expected %d, got %d", i, tc.expected, tc.te.count) + } + } + + dr := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats, clock.NewFake(), 3) + dr.DNSClient = &testExchanger{errs: []error{isTempErr, isTempErr, nil}} + ctx, cancel := context.WithCancel(context.Background()) + cancel() + _, err := dr.LookupTXT(ctx, "example.com") + if err != context.Canceled { + t.Errorf("expected %s, got %s", context.Canceled, err) + } + + dr.DNSClient = &testExchanger{errs: []error{isTempErr, isTempErr, nil}} + ctx, _ = context.WithTimeout(context.Background(), -10*time.Hour) + _, err = dr.LookupTXT(ctx, "example.com") + if err != context.DeadlineExceeded { + t.Errorf("expected %s, got %s", context.DeadlineExceeded, err) + } + + dr.DNSClient = &testExchanger{errs: []error{isTempErr, isTempErr, nil}} + ctx, deadlineCancel := context.WithTimeout(context.Background(), -10*time.Hour) + deadlineCancel() + _, err = dr.LookupTXT(ctx, "example.com") + if err != context.DeadlineExceeded { + t.Errorf("expected %s, got %s", context.DeadlineExceeded, err) + } +} + +type tempError bool + +func (t tempError) Temporary() bool { return bool(t) } +func (t tempError) Error() string { return fmt.Sprintf("Temporary: %t", t) } diff --git a/cmd/boulder-ra/main.go b/cmd/boulder-ra/main.go index a5f51e201..f64940157 100644 --- a/cmd/boulder-ra/main.go +++ b/cmd/boulder-ra/main.go @@ -64,10 +64,14 @@ func main() { raDNSTimeout, err := time.ParseDuration(c.Common.DNSTimeout) cmd.FailOnError(err, "Couldn't parse RA DNS timeout") scoped := metrics.NewStatsdScope(stats, "RA", "DNS") + dnsTries := c.RA.DNSTries + if dnsTries < 1 { + dnsTries = 1 + } if !c.Common.DNSAllowLoopbackAddresses { - rai.DNSResolver = bdns.NewDNSResolverImpl(raDNSTimeout, []string{c.Common.DNSResolver}, scoped) + rai.DNSResolver = bdns.NewDNSResolverImpl(raDNSTimeout, []string{c.Common.DNSResolver}, scoped, clock.Default(), dnsTries) } else { - rai.DNSResolver = bdns.NewTestDNSResolverImpl(raDNSTimeout, []string{c.Common.DNSResolver}, scoped) + rai.DNSResolver = bdns.NewTestDNSResolverImpl(raDNSTimeout, []string{c.Common.DNSResolver}, scoped, clock.Default(), dnsTries) } rai.VA = vac diff --git a/cmd/boulder-va/main.go b/cmd/boulder-va/main.go index 4d1f33203..a50354ba0 100644 --- a/cmd/boulder-va/main.go +++ b/cmd/boulder-va/main.go @@ -42,15 +42,20 @@ func main() { if c.VA.PortConfig.TLSPort != 0 { pc.TLSPort = c.VA.PortConfig.TLSPort } + clk := clock.Default() sbc := newGoogleSafeBrowsing(c.VA.GoogleSafeBrowsing) - vai := va.NewValidationAuthorityImpl(pc, sbc, stats, clock.Default()) + vai := va.NewValidationAuthorityImpl(pc, sbc, stats, clk) dnsTimeout, err := time.ParseDuration(c.Common.DNSTimeout) cmd.FailOnError(err, "Couldn't parse DNS timeout") scoped := metrics.NewStatsdScope(stats, "VA", "DNS") + dnsTries := c.VA.DNSTries + if dnsTries < 1 { + dnsTries = 1 + } if !c.Common.DNSAllowLoopbackAddresses { - vai.DNSResolver = bdns.NewDNSResolverImpl(dnsTimeout, []string{c.Common.DNSResolver}, scoped) + vai.DNSResolver = bdns.NewDNSResolverImpl(dnsTimeout, []string{c.Common.DNSResolver}, scoped, clk, dnsTries) } else { - vai.DNSResolver = bdns.NewTestDNSResolverImpl(dnsTimeout, []string{c.Common.DNSResolver}, scoped) + vai.DNSResolver = bdns.NewTestDNSResolverImpl(dnsTimeout, []string{c.Common.DNSResolver}, scoped, clk, dnsTries) } vai.UserAgent = c.VA.UserAgent vai.IssuerDomain = c.VA.IssuerDomain diff --git a/cmd/config.go b/cmd/config.go index 2d29a6b55..fc3fc9d88 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -62,6 +62,11 @@ type Config struct { // UseIsSafeDomain determines whether to call VA.IsSafeDomain UseIsSafeDomain bool // TODO(jmhodges): remove after va IsSafeDomain deploy + + // The number of times to try a DNS query (that has a temporary error) + // before giving up. May be short-circuited by deadlines. A zero value + // will be turned into 1. + DNSTries int } SA struct { @@ -83,6 +88,11 @@ type Config struct { MaxConcurrentRPCServerRequests int64 GoogleSafeBrowsing *GoogleSafeBrowsingConfig + + // The number of times to try a DNS query (that has a temporary error) + // before giving up. May be short-circuited by deadlines. A zero value + // will be turned into 1. + DNSTries int } SQL struct { diff --git a/mocks/mocks.go b/mocks/mocks.go index 860453a23..160785f5b 100644 --- a/mocks/mocks.go +++ b/mocks/mocks.go @@ -24,6 +24,7 @@ import ( "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/letsencrypt/go-jose" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/miekg/dns" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/golang.org/x/net/context" "github.com/letsencrypt/boulder/core" ) @@ -33,7 +34,7 @@ type DNSResolver struct { } // LookupTXT is a mock -func (mock *DNSResolver) LookupTXT(hostname string) ([]string, error) { +func (mock *DNSResolver) LookupTXT(ctx context.Context, hostname string) ([]string, error) { if hostname == "_acme-challenge.servfail.com" { return nil, fmt.Errorf("SERVFAIL") } @@ -66,7 +67,7 @@ func (t timeoutError) Timeout() bool { // // Note: see comments on LookupMX regarding email.only // -func (mock *DNSResolver) LookupHost(hostname string) ([]net.IP, error) { +func (mock *DNSResolver) LookupHost(ctx context.Context, hostname string) ([]net.IP, error) { if hostname == "always.invalid" || hostname == "invalid.invalid" || hostname == "email.only" { @@ -85,7 +86,7 @@ func (mock *DNSResolver) LookupHost(hostname string) ([]net.IP, error) { } // LookupCAA is a mock -func (mock *DNSResolver) LookupCAA(domain string) ([]*dns.CAA, error) { +func (mock *DNSResolver) LookupCAA(ctx context.Context, domain string) ([]*dns.CAA, error) { var results []*dns.CAA var record dns.CAA switch strings.TrimRight(domain, ".") { @@ -121,7 +122,7 @@ func (mock *DNSResolver) LookupCAA(domain string) ([]*dns.CAA, error) { // all domains except for special cases, so MX-only domains must be // handled in both LookupHost and LookupMX. // -func (mock *DNSResolver) LookupMX(domain string) ([]string, error) { +func (mock *DNSResolver) LookupMX(ctx context.Context, domain string) ([]string, error) { switch strings.TrimRight(domain, ".") { case "letsencrypt.org": fallthrough diff --git a/ra/registration-authority.go b/ra/registration-authority.go index 455c0c5ac..6654580b9 100644 --- a/ra/registration-authority.go +++ b/ra/registration-authority.go @@ -20,6 +20,7 @@ import ( "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/cactus/go-statsd-client/statsd" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/letsencrypt/net/publicsuffix" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/golang.org/x/net/context" "github.com/letsencrypt/boulder/probs" "github.com/letsencrypt/boulder/bdns" @@ -84,7 +85,7 @@ const ( emptyDNSResponseDetail = "empty DNS response" ) -func validateEmail(address string, resolver bdns.DNSResolver) (prob *probs.ProblemDetails) { +func validateEmail(ctx context.Context, address string, resolver bdns.DNSResolver) (prob *probs.ProblemDetails) { _, err := mail.ParseAddress(address) if err != nil { return &probs.ProblemDetails{ @@ -96,10 +97,10 @@ func validateEmail(address string, resolver bdns.DNSResolver) (prob *probs.Probl domain := strings.ToLower(splitEmail[len(splitEmail)-1]) var resultMX []string var resultA []net.IP - resultMX, err = resolver.LookupMX(domain) + resultMX, err = resolver.LookupMX(ctx, domain) recQ := "MX" if err == nil && len(resultMX) == 0 { - resultA, err = resolver.LookupHost(domain) + resultA, err = resolver.LookupHost(ctx, domain) recQ = "A" if err == nil && len(resultA) == 0 { return &probs.ProblemDetails{ @@ -209,7 +210,8 @@ func (ra *RegistrationAuthorityImpl) NewRegistration(init core.Registration) (re // MergeUpdate. But we need to fill it in for new registrations. reg.InitialIP = init.InitialIP - err = ra.validateContacts(reg.Contact) + // TODO(#1292): add a proper deadline here + err = ra.validateContacts(context.TODO(), reg.Contact) if err != nil { return } @@ -226,7 +228,7 @@ func (ra *RegistrationAuthorityImpl) NewRegistration(init core.Registration) (re return } -func (ra *RegistrationAuthorityImpl) validateContacts(contacts []*core.AcmeURL) (err error) { +func (ra *RegistrationAuthorityImpl) validateContacts(ctx context.Context, contacts []*core.AcmeURL) (err error) { if ra.maxContactsPerReg > 0 && len(contacts) > ra.maxContactsPerReg { return core.MalformedRequestError(fmt.Sprintf("Too many contacts provided: %d > %d", len(contacts), ra.maxContactsPerReg)) @@ -242,7 +244,7 @@ func (ra *RegistrationAuthorityImpl) validateContacts(contacts []*core.AcmeURL) case "mailto": start := ra.clk.Now() ra.stats.Inc("RA.ValidateEmail.Calls", 1, 1.0) - problem := validateEmail(contact.Opaque, ra.DNSResolver) + problem := validateEmail(ctx, contact.Opaque, ra.DNSResolver) ra.stats.TimingDuration("RA.ValidateEmail.Latency", ra.clk.Now().Sub(start), 1.0) if problem != nil { ra.stats.Inc("RA.ValidateEmail.Errors", 1, 1.0) @@ -638,7 +640,8 @@ func (ra *RegistrationAuthorityImpl) checkLimits(names []string, regID int64) er func (ra *RegistrationAuthorityImpl) UpdateRegistration(base core.Registration, update core.Registration) (reg core.Registration, err error) { base.MergeUpdate(update) - err = ra.validateContacts(base.Contact) + // TODO(#1292): add a proper deadline here + err = ra.validateContacts(context.TODO(), base.Contact) if err != nil { return } diff --git a/ra/registration-authority_test.go b/ra/registration-authority_test.go index e81c7d90e..2678d6117 100644 --- a/ra/registration-authority_test.go +++ b/ra/registration-authority_test.go @@ -22,6 +22,7 @@ import ( cfsslConfig "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/cloudflare/cfssl/config" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" jose "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/letsencrypt/go-jose" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/golang.org/x/net/context" "github.com/letsencrypt/boulder/ca" "github.com/letsencrypt/boulder/cmd" "github.com/letsencrypt/boulder/core" @@ -288,25 +289,25 @@ func TestValidateContacts(t *testing.T) { validEmail, _ := core.ParseAcmeURL("mailto:admin@email.com") malformedEmail, _ := core.ParseAcmeURL("mailto:admin.com") - err := ra.validateContacts([]*core.AcmeURL{}) + err := ra.validateContacts(context.Background(), []*core.AcmeURL{}) test.AssertNotError(t, err, "No Contacts") - err = ra.validateContacts([]*core.AcmeURL{tel, validEmail}) + err = ra.validateContacts(context.Background(), []*core.AcmeURL{tel, validEmail}) test.AssertError(t, err, "Too Many Contacts") - err = ra.validateContacts([]*core.AcmeURL{tel}) + err = ra.validateContacts(context.Background(), []*core.AcmeURL{tel}) test.AssertNotError(t, err, "Simple Telephone") - err = ra.validateContacts([]*core.AcmeURL{validEmail}) + err = ra.validateContacts(context.Background(), []*core.AcmeURL{validEmail}) test.AssertNotError(t, err, "Valid Email") - err = ra.validateContacts([]*core.AcmeURL{malformedEmail}) + err = ra.validateContacts(context.Background(), []*core.AcmeURL{malformedEmail}) test.AssertError(t, err, "Malformed Email") - err = ra.validateContacts([]*core.AcmeURL{ansible}) + err = ra.validateContacts(context.Background(), []*core.AcmeURL{ansible}) test.AssertError(t, err, "Unknown scheme") - err = ra.validateContacts([]*core.AcmeURL{nil}) + err = ra.validateContacts(context.Background(), []*core.AcmeURL{nil}) test.AssertError(t, err, "Nil AcmeURL") } @@ -324,8 +325,9 @@ func TestValidateEmail(t *testing.T) { "a@email.com", "b@email.only", } + for _, tc := range testFailures { - problem := validateEmail(tc.input, &mocks.DNSResolver{}) + problem := validateEmail(context.Background(), tc.input, &mocks.DNSResolver{}) if problem.Type != probs.InvalidEmailProblem { t.Errorf("validateEmail(%q): got problem type %#v, expected %#v", tc.input, problem.Type, probs.InvalidEmailProblem) } @@ -336,7 +338,7 @@ func TestValidateEmail(t *testing.T) { } for _, addr := range testSuccesses { - if prob := validateEmail(addr, &mocks.DNSResolver{}); prob != nil { + if prob := validateEmail(context.Background(), addr, &mocks.DNSResolver{}); prob != nil { t.Errorf("validateEmail(%q): expected success, but it failed: %s", addr, prob) } diff --git a/test/boulder-config.json b/test/boulder-config.json index ebc1bf68a..72d802d89 100644 --- a/test/boulder-config.json +++ b/test/boulder-config.json @@ -123,6 +123,7 @@ "rateLimitPoliciesFilename": "test/rate-limit-policies.yml", "maxConcurrentRPCServerRequests": 16, "maxContactsPerRegistration": 100, + "dnsTries": 3, "debugAddr": "localhost:8002", "amqp": { "serverURLFile": "test/secrets/amqp_url", @@ -165,6 +166,7 @@ "tlsPort": 5001 }, "maxConcurrentRPCServerRequests": 16, + "dnsTries": 3, "amqp": { "serverURLFile": "test/secrets/amqp_url", "insecure": true, diff --git a/va/validation-authority.go b/va/validation-authority.go index 2007fcade..9bc4f0177 100644 --- a/va/validation-authority.go +++ b/va/validation-authority.go @@ -24,6 +24,7 @@ import ( "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/letsencrypt/net/publicsuffix" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/miekg/dns" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/golang.org/x/net/context" "github.com/letsencrypt/boulder/probs" "github.com/letsencrypt/boulder/bdns" @@ -89,8 +90,8 @@ type verificationRequestEvent struct { // This is the same choice made by the Go internal resolution library used by // net/http, except we only send A queries and accept IPv4 addresses. // TODO(#593): Add IPv6 support -func (va ValidationAuthorityImpl) getAddr(hostname string) (net.IP, []net.IP, *probs.ProblemDetails) { - addrs, err := va.DNSResolver.LookupHost(hostname) +func (va ValidationAuthorityImpl) getAddr(ctx context.Context, hostname string) (net.IP, []net.IP, *probs.ProblemDetails) { + addrs, err := va.DNSResolver.LookupHost(ctx, hostname) if err != nil { va.log.Debug(fmt.Sprintf("%s DNS failure: %s", hostname, err)) problem := bdns.ProblemDetailsFromDNSError("A", hostname, err) @@ -120,7 +121,7 @@ func (d *dialer) Dial(_, _ string) (net.Conn, error) { // resolveAndConstructDialer gets the prefered address using va.getAddr and returns // the chosen address and dialer for that address and correct port. -func (va *ValidationAuthorityImpl) resolveAndConstructDialer(name string, port int) (dialer, *probs.ProblemDetails) { +func (va *ValidationAuthorityImpl) resolveAndConstructDialer(ctx context.Context, name string, port int) (dialer, *probs.ProblemDetails) { d := dialer{ record: core.ValidationRecord{ Hostname: name, @@ -128,7 +129,7 @@ func (va *ValidationAuthorityImpl) resolveAndConstructDialer(name string, port i }, } - addr, allAddrs, err := va.getAddr(name) + addr, allAddrs, err := va.getAddr(ctx, name) if err != nil { return d, err } @@ -139,7 +140,7 @@ func (va *ValidationAuthorityImpl) resolveAndConstructDialer(name string, port i // Validation methods -func (va *ValidationAuthorityImpl) fetchHTTP(identifier core.AcmeIdentifier, path string, useTLS bool, input core.Challenge) ([]byte, []core.ValidationRecord, *probs.ProblemDetails) { +func (va *ValidationAuthorityImpl) fetchHTTP(ctx context.Context, identifier core.AcmeIdentifier, path string, useTLS bool, input core.Challenge) ([]byte, []core.ValidationRecord, *probs.ProblemDetails) { challenge := input host := identifier.Value @@ -177,7 +178,7 @@ func (va *ValidationAuthorityImpl) fetchHTTP(identifier core.AcmeIdentifier, pat httpRequest.Header["User-Agent"] = []string{va.UserAgent} } - dialer, prob := va.resolveAndConstructDialer(host, port) + dialer, prob := va.resolveAndConstructDialer(ctx, host, port) dialer.record.URL = url.String() validationRecords := []core.ValidationRecord{dialer.record} if prob != nil { @@ -236,7 +237,7 @@ func (va *ValidationAuthorityImpl) fetchHTTP(identifier core.AcmeIdentifier, pat reqPort = 80 } - dialer, err := va.resolveAndConstructDialer(reqHost, reqPort) + dialer, err := va.resolveAndConstructDialer(ctx, reqHost, reqPort) dialer.record.URL = req.URL.String() validationRecords = append(validationRecords, dialer.record) if err != nil { @@ -279,8 +280,8 @@ func (va *ValidationAuthorityImpl) fetchHTTP(identifier core.AcmeIdentifier, pat return body, validationRecords, nil } -func (va *ValidationAuthorityImpl) validateTLSWithZName(identifier core.AcmeIdentifier, challenge core.Challenge, zName string) ([]core.ValidationRecord, *probs.ProblemDetails) { - addr, allAddrs, problem := va.getAddr(identifier.Value) +func (va *ValidationAuthorityImpl) validateTLSWithZName(ctx context.Context, identifier core.AcmeIdentifier, challenge core.Challenge, zName string) ([]core.ValidationRecord, *probs.ProblemDetails) { + addr, allAddrs, problem := va.getAddr(ctx, identifier.Value) validationRecords := []core.ValidationRecord{ core.ValidationRecord{ Hostname: identifier.Value, @@ -332,7 +333,7 @@ func (va *ValidationAuthorityImpl) validateTLSWithZName(identifier core.AcmeIden } } -func (va *ValidationAuthorityImpl) validateHTTP01(identifier core.AcmeIdentifier, challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) { +func (va *ValidationAuthorityImpl) validateHTTP01(ctx context.Context, identifier core.AcmeIdentifier, challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) { if identifier.Type != core.IdentifierDNS { va.log.Debug(fmt.Sprintf("%s [%s] Identifier failure", challenge.Type, identifier)) return nil, &probs.ProblemDetails{ @@ -343,7 +344,7 @@ func (va *ValidationAuthorityImpl) validateHTTP01(identifier core.AcmeIdentifier // Perform the fetch path := fmt.Sprintf(".well-known/acme-challenge/%s", challenge.Token) - body, validationRecords, err := va.fetchHTTP(identifier, path, false, challenge) + body, validationRecords, err := va.fetchHTTP(ctx, identifier, path, false, challenge) if err != nil { return validationRecords, err } @@ -374,7 +375,7 @@ func (va *ValidationAuthorityImpl) validateHTTP01(identifier core.AcmeIdentifier return validationRecords, nil } -func (va *ValidationAuthorityImpl) validateTLSSNI01(identifier core.AcmeIdentifier, challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) { +func (va *ValidationAuthorityImpl) validateTLSSNI01(ctx context.Context, identifier core.AcmeIdentifier, challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) { if identifier.Type != "dns" { va.log.Debug(fmt.Sprintf("TLS-SNI [%s] Identifier failure", identifier)) return nil, &probs.ProblemDetails{ @@ -389,7 +390,7 @@ func (va *ValidationAuthorityImpl) validateTLSSNI01(identifier core.AcmeIdentifi Z := hex.EncodeToString(h.Sum(nil)) ZName := fmt.Sprintf("%s.%s.%s", Z[:32], Z[32:], core.TLSSNISuffix) - return va.validateTLSWithZName(identifier, challenge, ZName) + return va.validateTLSWithZName(ctx, identifier, challenge, ZName) } // parseHTTPConnError returns the ACME ProblemType corresponding to an error @@ -414,7 +415,7 @@ func parseHTTPConnError(err error) probs.ProblemType { return probs.ConnectionProblem } -func (va *ValidationAuthorityImpl) validateDNS01(identifier core.AcmeIdentifier, challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) { +func (va *ValidationAuthorityImpl) validateDNS01(ctx context.Context, identifier core.AcmeIdentifier, challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) { if identifier.Type != core.IdentifierDNS { va.log.Debug(fmt.Sprintf("DNS [%s] Identifier failure", identifier)) return nil, &probs.ProblemDetails{ @@ -430,7 +431,7 @@ func (va *ValidationAuthorityImpl) validateDNS01(identifier core.AcmeIdentifier, // Look for the required record in the DNS challengeSubdomain := fmt.Sprintf("%s.%s", core.DNSPrefix, identifier.Value) - txts, err := va.DNSResolver.LookupTXT(challengeSubdomain) + txts, err := va.DNSResolver.LookupTXT(ctx, challengeSubdomain) if err != nil { va.log.Debug(fmt.Sprintf("%s [%s] DNS failure: %s", challenge.Type, identifier, err)) @@ -451,9 +452,9 @@ func (va *ValidationAuthorityImpl) validateDNS01(identifier core.AcmeIdentifier, } } -func (va *ValidationAuthorityImpl) checkCAA(identifier core.AcmeIdentifier, regID int64) *probs.ProblemDetails { +func (va *ValidationAuthorityImpl) checkCAA(ctx context.Context, identifier core.AcmeIdentifier, regID int64) *probs.ProblemDetails { // Check CAA records for the requested identifier - present, valid, err := va.CheckCAARecords(identifier) + present, valid, err := va.checkCAARecords(ctx, identifier) if err != nil { va.log.Warning(fmt.Sprintf("Problem checking CAA: %s", err)) return bdns.ProblemDetailsFromDNSError("CAA", identifier.Value, err) @@ -471,7 +472,7 @@ func (va *ValidationAuthorityImpl) checkCAA(identifier core.AcmeIdentifier, regI // Overall validation process -func (va *ValidationAuthorityImpl) validate(authz core.Authorization, challengeIndex int) { +func (va *ValidationAuthorityImpl) validate(ctx context.Context, authz core.Authorization, challengeIndex int) { logEvent := verificationRequestEvent{ ID: authz.ID, Requester: authz.RegistrationID, @@ -479,7 +480,7 @@ func (va *ValidationAuthorityImpl) validate(authz core.Authorization, challengeI } challenge := &authz.Challenges[challengeIndex] vStart := va.clk.Now() - validationRecords, prob := va.validateChallengeAndCAA(authz.Identifier, *challenge, authz.RegistrationID) + validationRecords, prob := va.validateChallengeAndCAA(ctx, authz.Identifier, *challenge, authz.RegistrationID) va.stats.TimingDuration(fmt.Sprintf("VA.Validations.%s.%s", challenge.Type, challenge.Status), time.Since(vStart), 1.0) challenge.ValidationRecord = validationRecords @@ -505,13 +506,14 @@ func (va *ValidationAuthorityImpl) validate(authz core.Authorization, challengeI va.RA.OnValidationUpdate(authz) } -func (va *ValidationAuthorityImpl) validateChallengeAndCAA(identifier core.AcmeIdentifier, challenge core.Challenge, regID int64) ([]core.ValidationRecord, *probs.ProblemDetails) { +func (va *ValidationAuthorityImpl) validateChallengeAndCAA(ctx context.Context, identifier core.AcmeIdentifier, challenge core.Challenge, regID int64) ([]core.ValidationRecord, *probs.ProblemDetails) { ch := make(chan *probs.ProblemDetails, 1) go func() { - ch <- va.checkCAA(identifier, regID) + ch <- va.checkCAA(ctx, identifier, regID) }() - validationRecords, err := va.validateChallenge(identifier, challenge) + // TODO(#1292): send into another goroutine + validationRecords, err := va.validateChallenge(ctx, identifier, challenge) if err != nil { return validationRecords, err } @@ -523,7 +525,7 @@ func (va *ValidationAuthorityImpl) validateChallengeAndCAA(identifier core.AcmeI return validationRecords, nil } -func (va *ValidationAuthorityImpl) validateChallenge(identifier core.AcmeIdentifier, challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) { +func (va *ValidationAuthorityImpl) validateChallenge(ctx context.Context, identifier core.AcmeIdentifier, challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) { if !challenge.IsSane(true) { return nil, &probs.ProblemDetails{ Type: probs.MalformedProblem, @@ -532,11 +534,11 @@ func (va *ValidationAuthorityImpl) validateChallenge(identifier core.AcmeIdentif } switch challenge.Type { case core.ChallengeTypeHTTP01: - return va.validateHTTP01(identifier, challenge) + return va.validateHTTP01(ctx, identifier, challenge) case core.ChallengeTypeTLSSNI01: - return va.validateTLSSNI01(identifier, challenge) + return va.validateTLSSNI01(ctx, identifier, challenge) case core.ChallengeTypeDNS01: - return va.validateDNS01(identifier, challenge) + return va.validateDNS01(ctx, identifier, challenge) } return nil, &probs.ProblemDetails{ Type: probs.MalformedProblem, @@ -546,7 +548,8 @@ func (va *ValidationAuthorityImpl) validateChallenge(identifier core.AcmeIdentif // UpdateValidations runs the validate() method asynchronously using goroutines. func (va *ValidationAuthorityImpl) UpdateValidations(authz core.Authorization, challengeIndex int) error { - go va.validate(authz, challengeIndex) + // TODO(#1292): add a proper deadline here + go va.validate(context.TODO(), authz, challengeIndex) return nil } @@ -593,7 +596,7 @@ func newCAASet(CAAs []*dns.CAA) *CAASet { return &filtered } -func (va *ValidationAuthorityImpl) getCAASet(hostname string) (*CAASet, error) { +func (va *ValidationAuthorityImpl) getCAASet(ctx context.Context, hostname string) (*CAASet, error) { hostname = strings.TrimRight(hostname, ".") labels := strings.Split(hostname, ".") // See RFC 6844 "Certification Authority Processing" for pseudocode. @@ -606,7 +609,7 @@ func (va *ValidationAuthorityImpl) getCAASet(hostname string) (*CAASet, error) { if tld, err := publicsuffix.ICANNTLD(name); err != nil || tld == name { break } - CAAs, err := va.DNSResolver.LookupCAA(name) + CAAs, err := va.DNSResolver.LookupCAA(ctx, name) if err != nil { return nil, err } @@ -621,8 +624,13 @@ func (va *ValidationAuthorityImpl) getCAASet(hostname string) (*CAASet, error) { // CheckCAARecords verifies that, if the indicated subscriber domain has any CAA // records, they authorize the configured CA domain to issue a certificate func (va *ValidationAuthorityImpl) CheckCAARecords(identifier core.AcmeIdentifier) (present, valid bool, err error) { + // TODO(#1292): add a proper deadline here + return va.checkCAARecords(context.TODO(), identifier) +} + +func (va *ValidationAuthorityImpl) checkCAARecords(ctx context.Context, identifier core.AcmeIdentifier) (present, valid bool, err error) { hostname := strings.ToLower(identifier.Value) - caaSet, err := va.getCAASet(hostname) + caaSet, err := va.getCAASet(ctx, hostname) if err != nil { return } diff --git a/va/validation-authority_test.go b/va/validation-authority_test.go index 6b94a74d2..7ae3aff6c 100644 --- a/va/validation-authority_test.go +++ b/va/validation-authority_test.go @@ -28,6 +28,7 @@ import ( "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/cactus/go-statsd-client/statsd" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/letsencrypt/go-jose" + "github.com/letsencrypt/boulder/Godeps/_workspace/src/golang.org/x/net/context" "github.com/letsencrypt/boulder/bdns" "github.com/letsencrypt/boulder/metrics" "github.com/letsencrypt/boulder/probs" @@ -223,7 +224,7 @@ func TestHTTP(t *testing.T) { va := NewValidationAuthorityImpl(&PortConfig{HTTPPort: badPort}, nil, stats, clock.Default()) va.DNSResolver = &mocks.DNSResolver{} - _, prob := va.validateHTTP01(ident, chall) + _, prob := va.validateHTTP01(context.Background(), ident, chall) if prob == nil { t.Fatalf("Server's down; expected refusal. Where did we connect?") } @@ -234,7 +235,7 @@ func TestHTTP(t *testing.T) { log.Clear() t.Logf("Trying to validate: %+v\n", chall) - _, prob = va.validateHTTP01(ident, chall) + _, prob = va.validateHTTP01(context.Background(), ident, chall) if prob != nil { t.Errorf("Unexpected failure in HTTP validation: %s", prob) } @@ -242,7 +243,7 @@ func TestHTTP(t *testing.T) { log.Clear() setChallengeToken(&chall, path404) - _, prob = va.validateHTTP01(ident, chall) + _, prob = va.validateHTTP01(context.Background(), ident, chall) if prob == nil { t.Fatalf("Should have found a 404 for the challenge.") } @@ -253,7 +254,7 @@ func TestHTTP(t *testing.T) { setChallengeToken(&chall, pathWrongToken) // The "wrong token" will actually be the expectedToken. It's wrong // because it doesn't match pathWrongToken. - _, prob = va.validateHTTP01(ident, chall) + _, prob = va.validateHTTP01(context.Background(), ident, chall) if prob == nil { t.Fatalf("Should have found the wrong token value.") } @@ -262,7 +263,7 @@ func TestHTTP(t *testing.T) { log.Clear() setChallengeToken(&chall, pathMoved) - _, prob = va.validateHTTP01(ident, chall) + _, prob = va.validateHTTP01(context.Background(), ident, chall) if prob != nil { t.Fatalf("Failed to follow 301 redirect") } @@ -270,7 +271,7 @@ func TestHTTP(t *testing.T) { log.Clear() setChallengeToken(&chall, pathFound) - _, prob = va.validateHTTP01(ident, chall) + _, prob = va.validateHTTP01(context.Background(), ident, chall) if prob != nil { t.Fatalf("Failed to follow 302 redirect") } @@ -278,13 +279,13 @@ func TestHTTP(t *testing.T) { test.AssertEquals(t, len(log.GetAllMatching(`redirect from ".*/`+pathMoved+`" to ".*/`+pathValid+`"`)), 1) ipIdentifier := core.AcmeIdentifier{Type: core.IdentifierType("ip"), Value: "127.0.0.1"} - _, prob = va.validateHTTP01(ipIdentifier, chall) + _, prob = va.validateHTTP01(context.Background(), ipIdentifier, chall) if prob == nil { t.Fatalf("IdentifierType IP shouldn't have worked.") } test.AssertEquals(t, prob.Type, probs.MalformedProblem) - _, prob = va.validateHTTP01(core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "always.invalid"}, chall) + _, prob = va.validateHTTP01(context.Background(), core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "always.invalid"}, chall) if prob == nil { t.Fatalf("Domain name is invalid.") } @@ -292,7 +293,7 @@ func TestHTTP(t *testing.T) { setChallengeToken(&chall, pathWaitLong) started := time.Now() - _, prob = va.validateHTTP01(ident, chall) + _, prob = va.validateHTTP01(context.Background(), ident, chall) took := time.Since(started) // Check that the HTTP connection times out after 5 seconds and doesn't block for 10 seconds test.Assert(t, (took > (time.Second * 5)), "HTTP timed out before 5 seconds") @@ -318,7 +319,7 @@ func TestHTTPRedirectLookup(t *testing.T) { log.Clear() setChallengeToken(&chall, pathMoved) - _, prob := va.validateHTTP01(ident, chall) + _, prob := va.validateHTTP01(context.Background(), ident, chall) if prob != nil { t.Fatalf("Unexpected failure in redirect (%s): %s", pathMoved, prob) } @@ -327,7 +328,7 @@ func TestHTTPRedirectLookup(t *testing.T) { log.Clear() setChallengeToken(&chall, pathFound) - _, prob = va.validateHTTP01(ident, chall) + _, prob = va.validateHTTP01(context.Background(), ident, chall) if prob != nil { t.Fatalf("Unexpected failure in redirect (%s): %s", pathFound, prob) } @@ -337,14 +338,14 @@ func TestHTTPRedirectLookup(t *testing.T) { log.Clear() setChallengeToken(&chall, pathReLookupInvalid) - _, err = va.validateHTTP01(ident, chall) + _, err = va.validateHTTP01(context.Background(), ident, chall) test.AssertError(t, err, chall.Token) test.AssertEquals(t, len(log.GetAllMatching(`Resolved addresses for localhost \[using 127.0.0.1\]: \[127.0.0.1\]`)), 1) test.AssertEquals(t, len(log.GetAllMatching(`No IPv4 addresses found for invalid.invalid`)), 1) log.Clear() setChallengeToken(&chall, pathReLookup) - _, prob = va.validateHTTP01(ident, chall) + _, prob = va.validateHTTP01(context.Background(), ident, chall) if prob != nil { t.Fatalf("Unexpected error in redirect (%s): %s", pathReLookup, prob) } @@ -354,7 +355,7 @@ func TestHTTPRedirectLookup(t *testing.T) { log.Clear() setChallengeToken(&chall, pathRedirectPort) - _, err = va.validateHTTP01(ident, chall) + _, err = va.validateHTTP01(context.Background(), ident, chall) test.AssertError(t, err, chall.Token) test.AssertEquals(t, len(log.GetAllMatching(`redirect from ".*/port-redirect" to ".*other.valid:8080/path"`)), 1) test.AssertEquals(t, len(log.GetAllMatching(`Resolved addresses for localhost \[using 127.0.0.1\]: \[127.0.0.1\]`)), 1) @@ -375,7 +376,7 @@ func TestHTTPRedirectLoop(t *testing.T) { va.DNSResolver = &mocks.DNSResolver{} log.Clear() - _, prob := va.validateHTTP01(ident, chall) + _, prob := va.validateHTTP01(context.Background(), ident, chall) if prob == nil { t.Fatalf("Challenge should have failed for %s", chall.Token) } @@ -396,13 +397,13 @@ func TestHTTPRedirectUserAgent(t *testing.T) { va.UserAgent = rejectUserAgent setChallengeToken(&chall, pathMoved) - _, prob := va.validateHTTP01(ident, chall) + _, prob := va.validateHTTP01(context.Background(), ident, chall) if prob == nil { t.Fatalf("Challenge with rejectUserAgent should have failed (%s).", pathMoved) } setChallengeToken(&chall, pathFound) - _, prob = va.validateHTTP01(ident, chall) + _, prob = va.validateHTTP01(context.Background(), ident, chall) if prob == nil { t.Fatalf("Challenge with rejectUserAgent should have failed (%s).", pathFound) } @@ -437,14 +438,14 @@ func TestTLSSNI(t *testing.T) { va.DNSResolver = &mocks.DNSResolver{} log.Clear() - _, prob := va.validateTLSSNI01(ident, chall) + _, prob := va.validateTLSSNI01(context.Background(), ident, chall) if prob != nil { t.Fatalf("Unexpected failre in validateTLSSNI01: %s", prob) } test.AssertEquals(t, len(log.GetAllMatching(`Resolved addresses for localhost \[using 127.0.0.1\]: \[127.0.0.1\]`)), 1) log.Clear() - _, prob = va.validateTLSSNI01(core.AcmeIdentifier{ + _, prob = va.validateTLSSNI01(context.Background(), core.AcmeIdentifier{ Type: core.IdentifierType("ip"), Value: net.JoinHostPort("127.0.0.1", fmt.Sprintf("%d", port)), }, chall) @@ -454,7 +455,7 @@ func TestTLSSNI(t *testing.T) { test.AssertEquals(t, prob.Type, probs.MalformedProblem) log.Clear() - _, prob = va.validateTLSSNI01(core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "always.invalid"}, chall) + _, prob = va.validateTLSSNI01(context.Background(), core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "always.invalid"}, chall) if prob == nil { t.Fatalf("Domain name was supposed to be invalid.") } @@ -467,7 +468,7 @@ func TestTLSSNI(t *testing.T) { log.Clear() started := time.Now() - _, prob = va.validateTLSSNI01(ident, chall) + _, prob = va.validateTLSSNI01(context.Background(), ident, chall) took := time.Since(started) // Check that the HTTP connection times out after 5 seconds and doesn't block for 10 seconds test.Assert(t, (took > (time.Second * 5)), "HTTP timed out before 5 seconds") @@ -480,7 +481,7 @@ func TestTLSSNI(t *testing.T) { // Take down validation server and check that validation fails. hs.Close() - _, err = va.validateTLSSNI01(ident, chall) + _, err = va.validateTLSSNI01(context.Background(), ident, chall) if err == nil { t.Fatalf("Server's down; expected refusal. Where did we connect?") } @@ -508,7 +509,7 @@ func TestTLSError(t *testing.T) { va := NewValidationAuthorityImpl(&PortConfig{TLSPort: port}, nil, stats, clock.Default()) va.DNSResolver = &mocks.DNSResolver{} - _, prob := va.validateTLSSNI01(ident, chall) + _, prob := va.validateTLSSNI01(context.Background(), ident, chall) if prob == nil { t.Fatalf("TLS validation should have failed: What cert was used?") } @@ -537,7 +538,7 @@ func TestValidateHTTP(t *testing.T) { Identifier: ident, Challenges: []core.Challenge{chall}, } - va.validate(authz, 0) + va.validate(context.Background(), authz, 0) test.AssertEquals(t, core.StatusValid, mockRA.lastAuthz.Challenges[0].Status) } @@ -592,7 +593,7 @@ func TestValidateTLSSNI01(t *testing.T) { Identifier: ident, Challenges: []core.Challenge{chall}, } - va.validate(authz, 0) + va.validate(context.Background(), authz, 0) test.AssertEquals(t, core.StatusValid, mockRA.lastAuthz.Challenges[0].Status) } @@ -614,7 +615,7 @@ func TestValidateTLSSNINotSane(t *testing.T) { Identifier: ident, Challenges: []core.Challenge{chall}, } - va.validate(authz, 0) + va.validate(context.Background(), authz, 0) test.AssertEquals(t, core.StatusInvalid, mockRA.lastAuthz.Challenges[0].Status) } @@ -651,7 +652,7 @@ func TestCAATimeout(t *testing.T) { va := NewValidationAuthorityImpl(&PortConfig{}, nil, stats, clock.Default()) va.DNSResolver = &mocks.DNSResolver{} va.IssuerDomain = "letsencrypt.org" - err := va.checkCAA(core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "caa-timeout.com"}, 101) + err := va.checkCAA(context.Background(), core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "caa-timeout.com"}, 101) if err.Type != probs.ConnectionProblem { t.Errorf("Expected timeout error type %s, got %s", probs.ConnectionProblem, err.Type) } @@ -724,7 +725,7 @@ func TestDNSValidationFailure(t *testing.T) { Identifier: ident, Challenges: []core.Challenge{chalDNS}, } - va.validate(authz, 0) + va.validate(context.Background(), authz, 0) t.Logf("Resulting Authz: %+v", authz) test.AssertNotNil(t, mockRA.lastAuthz, "Should have gotten an authorization") @@ -753,7 +754,7 @@ func TestDNSValidationInvalid(t *testing.T) { mockRA := &MockRegistrationAuthority{} va.RA = mockRA - va.validate(authz, 0) + va.validate(context.Background(), authz, 0) test.AssertNotNil(t, mockRA.lastAuthz, "Should have gotten an authorization") test.Assert(t, authz.Challenges[0].Status == core.StatusInvalid, "Should be invalid.") @@ -781,7 +782,7 @@ func TestDNSValidationNotSane(t *testing.T) { } for i := 0; i < len(authz.Challenges); i++ { - va.validate(authz, i) + va.validate(context.Background(), authz, i) test.AssertEquals(t, authz.Challenges[i].Status, core.StatusInvalid) test.AssertEquals(t, authz.Challenges[i].Error.Type, probs.MalformedProblem) } @@ -806,7 +807,7 @@ func TestDNSValidationServFail(t *testing.T) { Identifier: badIdent, Challenges: []core.Challenge{chalDNS}, } - va.validate(authz, 0) + va.validate(context.Background(), authz, 0) test.AssertNotNil(t, mockRA.lastAuthz, "Should have gotten an authorization") test.Assert(t, authz.Challenges[0].Status == core.StatusInvalid, "Should be invalid.") @@ -817,7 +818,7 @@ func TestDNSValidationNoServer(t *testing.T) { c, _ := statsd.NewNoopClient() stats := metrics.NewNoopScope() va := NewValidationAuthorityImpl(&PortConfig{}, nil, c, clock.Default()) - va.DNSResolver = bdns.NewTestDNSResolverImpl(time.Second*5, []string{}, stats) + va.DNSResolver = bdns.NewTestDNSResolverImpl(time.Second*5, []string{}, stats, clock.Default(), 1) mockRA := &MockRegistrationAuthority{} va.RA = mockRA @@ -829,7 +830,7 @@ func TestDNSValidationNoServer(t *testing.T) { Identifier: ident, Challenges: []core.Challenge{chalDNS}, } - va.validate(authz, 0) + va.validate(context.Background(), authz, 0) test.AssertNotNil(t, mockRA.lastAuthz, "Should have gotten an authorization") test.Assert(t, authz.Challenges[0].Status == core.StatusInvalid, "Should be invalid.") @@ -861,7 +862,7 @@ func TestDNSValidationOK(t *testing.T) { Identifier: goodIdent, Challenges: []core.Challenge{chalDNS}, } - va.validate(authz, 0) + va.validate(context.Background(), authz, 0) test.AssertNotNil(t, mockRA.lastAuthz, "Should have gotten an authorization") test.Assert(t, authz.Challenges[0].Status == core.StatusValid, "Should be valid.") @@ -899,7 +900,7 @@ func TestDNSValidationLive(t *testing.T) { Challenges: []core.Challenge{goodChalDNS}, } - va.validate(authzGood, 0) + va.validate(context.Background(), authzGood, 0) if authzGood.Challenges[0].Status != core.StatusValid { t.Logf("TestDNSValidationLive on Good did not succeed.") @@ -916,7 +917,7 @@ func TestDNSValidationLive(t *testing.T) { Challenges: []core.Challenge{badChalDNS}, } - va.validate(authzBad, 0) + va.validate(context.Background(), authzBad, 0) if authzBad.Challenges[0].Status != core.StatusInvalid { t.Logf("TestDNSValidationLive on Bad did succeed inappropriately.") } @@ -943,7 +944,7 @@ func TestCAAFailure(t *testing.T) { Identifier: ident, Challenges: []core.Challenge{chall}, } - va.validate(authz, 0) + va.validate(context.Background(), authz, 0) test.AssertEquals(t, core.StatusInvalid, mockRA.lastAuthz.Challenges[0].Status) } From 426ec155aad0759def8782bf95e0405c3d7f9a20 Mon Sep 17 00:00:00 2001 From: Jeff Hodges Date: Wed, 30 Dec 2015 18:25:51 -0800 Subject: [PATCH 09/26] correct Content-Length/Transfer-Encoding on HEAD Fixes #1320 --- wfe/web-front-end.go | 17 +++-------------- wfe/web-front-end_test.go | 31 ++++++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/wfe/web-front-end.go b/wfe/web-front-end.go index 4a2566bab..bcc29f04d 100644 --- a/wfe/web-front-end.go +++ b/wfe/web-front-end.go @@ -107,16 +107,6 @@ func NewWebFrontEndImpl(stats statsd.Statter, clk clock.Clock) (WebFrontEndImpl, }, nil } -// BodylessResponseWriter wraps http.ResponseWriter, discarding -// anything written to the body. -type BodylessResponseWriter struct { - http.ResponseWriter -} - -func (mrw BodylessResponseWriter) Write(buf []byte) (int, error) { - return len(buf), nil -} - // HandleFunc registers a handler at the given path. It's // http.HandleFunc(), but with a wrapper around the handler that // provides some generic per-request functionality: @@ -157,10 +147,9 @@ func (wfe *WebFrontEndImpl) HandleFunc(mux *http.ServeMux, pattern string, h wfe switch request.Method { case "HEAD": - // Whether or not we're sending a 405 error, - // we should comply with HTTP spec by not - // sending a body. - response = BodylessResponseWriter{response} + // Go's net/http (and httptest) servers will strip our the body + // of responses for us. This keeps the Content-Length for HEAD + // requests as the same as GET requests per the spec. case "OPTIONS": wfe.Options(response, request, methodsStr, methodsMap) return diff --git a/wfe/web-front-end_test.go b/wfe/web-front-end_test.go index 4fa20474d..a15aec3f1 100644 --- a/wfe/web-front-end_test.go +++ b/wfe/web-front-end_test.go @@ -19,6 +19,7 @@ import ( "net/http/httptest" "net/url" "sort" + "strconv" "strings" "testing" "time" @@ -327,7 +328,7 @@ func TestHandleFunc(t *testing.T) { test.AssertEquals(t, rw.Code, http.StatusMethodNotAllowed) test.AssertEquals(t, rw.Header().Get("Content-Type"), "application/problem+json") test.AssertEquals(t, rw.Header().Get("Allow"), "POST") - test.AssertEquals(t, rw.Body.String(), "") + test.AssertEquals(t, rw.Body.String(), `{"type":"urn:acme:error:malformed","detail":"Method not allowed","status":405}`) wfe.AllowOrigins = []string{"*"} testOrigin := "https://example.com" @@ -1400,6 +1401,34 @@ func TestBadKeyCSR(t *testing.T) { `{"type":"urn:acme:error:malformed","detail":"Invalid key in certificate request :: Key too small: 512","status":400}`) } +// This uses httptest.NewServer because ServeMux.ServeHTTP won't prevent the +// body from being sent like the net/http Server's actually do. +func TestGetCertificateHEADHasCorrectBodyLength(t *testing.T) { + wfe, _ := setupWFE(t) + + certPemBytes, _ := ioutil.ReadFile("test/178.crt") + certBlock, _ := pem.Decode(certPemBytes) + + mockLog := wfe.log.SyslogWriter.(*mocks.SyslogWriter) + mockLog.Clear() + + mux, _ := wfe.Handler() + s := httptest.NewServer(mux) + req, _ := http.NewRequest("HEAD", s.URL+"/acme/cert/0000000000000000000000000000000000b2", nil) + resp, err := http.DefaultClient.Do(req) + if err != nil { + test.AssertNotError(t, err, "do error") + } + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + test.AssertNotEquals(t, err, "readall error") + } + defer resp.Body.Close() + test.AssertEquals(t, resp.StatusCode, 200) + test.AssertEquals(t, strconv.Itoa(len(certBlock.Bytes)), resp.Header.Get("Content-Length")) + test.AssertEquals(t, 0, len(body)) +} + func newRequestEvent() *requestEvent { return &requestEvent{Extra: make(map[string]interface{})} } From cbeffe96a6d2184e784113c352f4bff9609634a7 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Mon, 4 Jan 2016 18:39:34 -0500 Subject: [PATCH 10/26] Fixed a bunch of typos --- CONTRIBUTING.md | 2 +- DESIGN.md | 4 ++-- cmd/admin-revoker/main.go | 6 +++--- cmd/config.go | 2 +- cmd/expiration-mailer/main.go | 2 +- cmd/shell.go | 2 +- core/objects.go | 4 ++-- core/util.go | 2 +- log/audit-logger.go | 4 ++-- mocks/mocks.go | 2 +- publisher/publisher.go | 2 +- ra/registration-authority.go | 2 +- rpc/amqp-rpc.go | 4 ++-- sa/storage-authority.go | 12 ++++++------ test/create_db.sh | 2 +- va/validation-authority.go | 4 ++-- va/validation-authority_test.go | 4 ++-- wfe/web-front-end.go | 4 ++-- wfe/web-front-end_test.go | 2 +- 19 files changed, 33 insertions(+), 33 deletions(-) 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/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/config.go b/cmd/config.go index 2d29a6b55..ee9cb44e4 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -131,7 +131,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..f15036915 100644 --- a/mocks/mocks.go +++ b/mocks/mocks.go @@ -46,7 +46,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{}), 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..cc1d76fdf 100644 --- a/ra/registration-authority.go +++ b/ra/registration-authority.go @@ -274,7 +274,7 @@ func checkPendingAuthorizationLimit(sa core.StorageGetter, limit *cmd.RateLimitP 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) 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/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/va/validation-authority.go b/va/validation-authority.go index 2007fcade..8aab57fb4 100644 --- a/va/validation-authority.go +++ b/va/validation-authority.go @@ -85,7 +85,7 @@ 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 @@ -118,7 +118,7 @@ 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) { d := dialer{ diff --git a/va/validation-authority_test.go b/va/validation-authority_test.go index 6b94a74d2..2c622a20e 100644 --- a/va/validation-authority_test.go +++ b/va/validation-authority_test.go @@ -122,7 +122,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") @@ -868,7 +868,7 @@ func TestDNSValidationOK(t *testing.T) { } // 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() diff --git a/wfe/web-front-end.go b/wfe/web-front-end.go index 4a2566bab..e083d9a9c 100644 --- a/wfe/web-front-end.go +++ b/wfe/web-front-end.go @@ -296,7 +296,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 +608,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..830cca842 100644 --- a/wfe/web-front-end_test.go +++ b/wfe/web-front-end_test.go @@ -605,7 +605,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 From de14c9274ce8ed3b15d978d23aa4f8ee6262a1e2 Mon Sep 17 00:00:00 2001 From: Jeff Hodges Date: Mon, 4 Jan 2016 17:04:25 -0800 Subject: [PATCH 11/26] s/our/out/ in wfe HEAD comment --- wfe/web-front-end.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wfe/web-front-end.go b/wfe/web-front-end.go index 469244cdb..e0dfb803d 100644 --- a/wfe/web-front-end.go +++ b/wfe/web-front-end.go @@ -147,7 +147,7 @@ func (wfe *WebFrontEndImpl) HandleFunc(mux *http.ServeMux, pattern string, h wfe switch request.Method { case "HEAD": - // Go's net/http (and httptest) servers will strip our the body + // Go's net/http (and httptest) servers will strip out 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": From 673f927d851498c90935c611cbaa106827e43748 Mon Sep 17 00:00:00 2001 From: Reinaldo de Souza Jr Date: Tue, 22 Dec 2015 21:44:28 -0500 Subject: [PATCH 12/26] Initialize rabbitmq in Docker entrypoint --- test/entrypoint.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/entrypoint.sh b/test/entrypoint.sh index 5a3977688..4ad357a9f 100755 --- a/test/entrypoint.sh +++ b/test/entrypoint.sh @@ -12,10 +12,19 @@ while ! exec 6<>/dev/tcp/0.0.0.0/3306; do sleep 1 done +# make sure we can reach the rabbitmq +while ! exec 6<>/dev/tcp/0.0.0.0/5672; do + echo "$(date) - still trying to connect to rabbitmq at 0.0.0.0:5672" + sleep 1 +done + exec 6>&- exec 6<&- # create the database source $DIR/create_db.sh +# Set up rabbitmq exchange and activity monitor queue +go run cmd/rabbitmq-setup/main.go -server amqp://localhost + $@ From a76cb3c4140097eab70d8c5f79d76212678dc846 Mon Sep 17 00:00:00 2001 From: Jeff Hodges Date: Mon, 21 Dec 2015 21:18:34 -0800 Subject: [PATCH 13/26] stat and log rate limit data where available It doesn't log the IP information on its rate limit. IP is considered personally identifiable information and against our policies to log. Fixes #1120 --- ra/registration-authority.go | 39 ++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/ra/registration-authority.go b/ra/registration-authority.go index 003557434..1a1dc3501 100644 --- a/ra/registration-authority.go +++ b/ra/registration-authority.go @@ -21,6 +21,7 @@ import ( "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/letsencrypt/net/publicsuffix" "github.com/letsencrypt/boulder/Godeps/_workspace/src/golang.org/x/net/context" + "github.com/letsencrypt/boulder/metrics" "github.com/letsencrypt/boulder/probs" "github.com/letsencrypt/boulder/bdns" @@ -62,10 +63,17 @@ type RegistrationAuthorityImpl struct { totalIssuedCache int lastIssuedCount *time.Time maxContactsPerReg int + + regByIPStats metrics.Scope + pendAuthByRegIDStats metrics.Scope + certsForDomainStats metrics.Scope + totalCertsStats metrics.Scope } // NewRegistrationAuthorityImpl constructs a new RA object. func NewRegistrationAuthorityImpl(clk clock.Clock, logger *blog.AuditLogger, stats statsd.Statter, dc *DomainCheck, policies cmd.RateLimitConfig, maxContactsPerReg int) *RegistrationAuthorityImpl { + // TODO(jmhodges): making RA take a "RA" stats.Scope, not Statter + scope := metrics.NewStatsdScope(stats, "RA") ra := &RegistrationAuthorityImpl{ stats: stats, clk: clk, @@ -76,6 +84,11 @@ func NewRegistrationAuthorityImpl(clk clock.Clock, logger *blog.AuditLogger, sta rlPolicies: policies, tiMu: new(sync.RWMutex), maxContactsPerReg: maxContactsPerReg, + + regByIPStats: scope.NewScope("RA", "RateLimit", "RegistrationsByIP"), + pendAuthByRegIDStats: scope.NewScope("RA", "RateLimit", "PendingAuthorizationsByRegID"), + certsForDomainStats: scope.NewScope("RA", "RateLimit", "CertificatesForDomain"), + totalCertsStats: scope.NewScope("RA", "RateLimit", "TotalCertificates"), } return ra } @@ -186,8 +199,10 @@ func (ra *RegistrationAuthorityImpl) checkRegistrationLimit(ip net.IP) error { return err } if count >= limit.GetThreshold(ip.String(), noRegistrationID) { + ra.regByIPStats.Inc("Exceeded", 1) return core.RateLimitedError("Too many registrations from this IP") } + ra.regByIPStats.Inc("Pass", 1) } return nil } @@ -260,9 +275,10 @@ func (ra *RegistrationAuthorityImpl) validateContacts(ctx context.Context, conta return } -func checkPendingAuthorizationLimit(sa core.StorageGetter, limit *cmd.RateLimitPolicy, regID int64) error { +func (ra *RegistrationAuthorityImpl) checkPendingAuthorizationLimit(regID int64) error { + limit := ra.rlPolicies.PendingAuthorizationsPerAccount if limit.Enabled() { - count, err := sa.CountPendingAuthorizations(regID) + count, err := ra.SA.CountPendingAuthorizations(regID) if err != nil { return err } @@ -270,8 +286,11 @@ func checkPendingAuthorizationLimit(sa core.StorageGetter, limit *cmd.RateLimitP // here. noKey := "" if count >= limit.GetThreshold(noKey, regID) { + ra.pendAuthByRegIDStats.Inc("Exceeded", 1) + ra.log.Info(fmt.Sprintf("Rate limit exceeded, PendingAuthorizationsByRegID, regID: %d", regID)) return core.RateLimitedError("Too many currently pending authorizations.") } + ra.pendAuthByRegIDStats.Inc("Pass", 1) } return nil } @@ -293,8 +312,7 @@ func (ra *RegistrationAuthorityImpl) NewAuthorization(request core.Authorization return authz, err } - limit := &ra.rlPolicies.PendingAuthorizationsPerAccount - if err = checkPendingAuthorizationLimit(ra.SA, limit, regID); err != nil { + if err = ra.checkPendingAuthorizationLimit(regID); err != nil { return authz, err } @@ -609,10 +627,15 @@ func (ra *RegistrationAuthorityImpl) checkCertificatesPerNameLimit(names []strin } } if len(badNames) > 0 { + domains := strings.Join(badNames, ", ") + ra.certsForDomainStats.Inc("Exceeded", 1) + ra.log.Info(fmt.Sprintf("Rate limit exceeded, CertificatesForDomain, regID: %d, domains: %s", regID, domains)) return core.RateLimitedError(fmt.Sprintf( - "Too many certificates already issued for: %s", - strings.Join(badNames, ", "))) + "Too many certificates already issued for: %s", domains)) + } + ra.certsForDomainStats.Inc("Pass", 1) + return nil } @@ -624,8 +647,12 @@ func (ra *RegistrationAuthorityImpl) checkLimits(names []string, regID int64) er return err } if totalIssued >= ra.rlPolicies.TotalCertificates.Threshold { + domains := strings.Join(names, ",") + ra.totalCertsStats.Inc("Exceeded", 1) + ra.log.Info(fmt.Sprintf("Rate limit exceeded, TotalCertificates, regID: %d, domains: %s, totalIssued: %d", regID, domains, totalIssued)) return core.RateLimitedError("Certificate issuance limit reached") } + ra.totalCertsStats.Inc("Pass", 1) } if limits.CertificatesPerName.Enabled() { err := ra.checkCertificatesPerNameLimit(names, limits.CertificatesPerName, regID) From f6473efcc2091767a99de390f17322d0904208bd Mon Sep 17 00:00:00 2001 From: Jeff Hodges Date: Mon, 4 Jan 2016 23:59:19 -0800 Subject: [PATCH 14/26] delete ca.RevokeCertificate Also, delete the unused core.CertificateAuthorityDatabase while we're here. Fixes #1319 --- ca/certificate-authority.go | 6 ------ cmd/ocsp-updater/main_test.go | 4 ---- core/interfaces.go | 8 -------- rpc/rpc-wrappers.go | 31 ------------------------------- 4 files changed, 49 deletions(-) diff --git a/ca/certificate-authority.go b/ca/certificate-authority.go index 82a863c62..96694aa62 100644 --- a/ca/certificate-authority.go +++ b/ca/certificate-authority.go @@ -218,12 +218,6 @@ func (ca *CertificateAuthorityImpl) GenerateOCSP(xferObj core.OCSPSigningRequest return ocspResponse, err } -// RevokeCertificate revokes the trust of the Cert referred to by the provided Serial. -func (ca *CertificateAuthorityImpl) RevokeCertificate(serial string, reasonCode core.RevocationCode) (err error) { - err = ca.SA.MarkCertificateRevoked(serial, reasonCode) - return err -} - // IssueCertificate attempts to convert a CSR into a signed Certificate, while // enforcing all policies. Names (domains) in the CertificateRequest will be // lowercased before storage. diff --git a/cmd/ocsp-updater/main_test.go b/cmd/ocsp-updater/main_test.go index a0273c82c..c4fac742b 100644 --- a/cmd/ocsp-updater/main_test.go +++ b/cmd/ocsp-updater/main_test.go @@ -29,10 +29,6 @@ func (ca *mockCA) GenerateOCSP(xferObj core.OCSPSigningRequest) (ocsp []byte, er return } -func (ca *mockCA) RevokeCertificate(serial string, reasonCode core.RevocationCode) (err error) { - return -} - type mockPub struct { sa core.StorageAuthority } diff --git a/core/interfaces.go b/core/interfaces.go index b46d21fb1..90b360664 100644 --- a/core/interfaces.go +++ b/core/interfaces.go @@ -12,7 +12,6 @@ import ( "time" jose "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/letsencrypt/go-jose" - gorp "github.com/letsencrypt/boulder/Godeps/_workspace/src/gopkg.in/gorp.v1" ) // A WebFrontEnd object supplies methods that can be hooked into @@ -83,7 +82,6 @@ type RegistrationAuthority interface { type CertificateAuthority interface { // [RegistrationAuthority] IssueCertificate(x509.CertificateRequest, int64) (Certificate, error) - RevokeCertificate(string, RevocationCode) error GenerateOCSP(OCSPSigningRequest) ([]byte, error) } @@ -133,12 +131,6 @@ type StorageAuthority interface { StorageAdder } -// CertificateAuthorityDatabase represents an atomic sequence source -type CertificateAuthorityDatabase interface { - IncrementAndGetSerial(*gorp.Transaction) (int64, error) - Begin() (*gorp.Transaction, error) -} - // Publisher defines the public interface for the Boulder Publisher type Publisher interface { SubmitToCT([]byte) error diff --git a/rpc/rpc-wrappers.go b/rpc/rpc-wrappers.go index 69134f013..8b1dbbb43 100644 --- a/rpc/rpc-wrappers.go +++ b/rpc/rpc-wrappers.go @@ -42,7 +42,6 @@ const ( MethodNewCertificate = "NewCertificate" // RA MethodUpdateRegistration = "UpdateRegistration" // RA, SA MethodUpdateAuthorization = "UpdateAuthorization" // RA - MethodRevokeCertificate = "RevokeCertificate" // CA MethodRevokeCertificateWithReg = "RevokeCertificateWithReg" // RA MethodAdministrativelyRevokeCertificate = "AdministrativelyRevokeCertificate" // RA MethodOnValidationUpdate = "OnValidationUpdate" // RA @@ -704,19 +703,6 @@ func NewCertificateAuthorityServer(rpc Server, impl core.CertificateAuthority) ( return }) - rpc.Handle(MethodRevokeCertificate, func(req []byte) (response []byte, err error) { - var revokeReq revokeCertificateRequest - err = json.Unmarshal(req, &revokeReq) - if err != nil { - // AUDIT[ Error Conditions ] 9cc4d537-8534-4970-8665-4b382abe82f3 - errorCondition(MethodRevokeCertificate, err, req) - return - } - - err = impl.RevokeCertificate(revokeReq.Serial, revokeReq.ReasonCode) - return - }) - rpc.Handle(MethodGenerateOCSP, func(req []byte) (response []byte, err error) { var xferObj core.OCSPSigningRequest err = json.Unmarshal(req, &xferObj) @@ -767,23 +753,6 @@ func (cac CertificateAuthorityClient) IssueCertificate(csr x509.CertificateReque return } -// RevokeCertificate sends a request to revoke a certificate -func (cac CertificateAuthorityClient) RevokeCertificate(serial string, reasonCode core.RevocationCode) (err error) { - var revokeReq revokeCertificateRequest - revokeReq.Serial = serial - revokeReq.ReasonCode = reasonCode - - data, err := json.Marshal(revokeReq) - if err != nil { - // AUDIT[ Error Conditions ] 9cc4d537-8534-4970-8665-4b382abe82f3 - errorCondition(MethodRevokeCertificate, err, revokeReq) - return - } - - _, err = cac.rpc.DispatchSync(MethodRevokeCertificate, data) - return -} - // GenerateOCSP sends a request to generate an OCSP response func (cac CertificateAuthorityClient) GenerateOCSP(signRequest core.OCSPSigningRequest) (resp []byte, err error) { data, err := json.Marshal(signRequest) From 4fe05d08d195c3b343fdc1cd7ab825caff379af9 Mon Sep 17 00:00:00 2001 From: Jeff Hodges Date: Tue, 5 Jan 2016 11:33:15 -0800 Subject: [PATCH 15/26] add IP to rate limit logging I was wrong about it not being okay in #1300 --- ra/registration-authority.go | 1 + 1 file changed, 1 insertion(+) diff --git a/ra/registration-authority.go b/ra/registration-authority.go index 1a1dc3501..8a4ffded6 100644 --- a/ra/registration-authority.go +++ b/ra/registration-authority.go @@ -200,6 +200,7 @@ func (ra *RegistrationAuthorityImpl) checkRegistrationLimit(ip net.IP) error { } if count >= limit.GetThreshold(ip.String(), noRegistrationID) { ra.regByIPStats.Inc("Exceeded", 1) + ra.log.Info(fmt.Sprintf("Rate limit exceeded, RegistrationsByIP, IP: %s", ip)) return core.RateLimitedError("Too many registrations from this IP") } ra.regByIPStats.Inc("Pass", 1) From f67648d22f1394fde6e529bb7fdd837fc05712e2 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Tue, 5 Jan 2016 14:50:25 -0800 Subject: [PATCH 16/26] Disable activity-monitor. We no longer run this in prod, so we shouldn't run it in test / dev. --- .gitignore | 3 --- test/integration-test.py | 8 -------- test/startservers.py | 1 - 3 files changed, 12 deletions(-) diff --git a/.gitignore b/.gitignore index 75bee6fea..a61289c17 100644 --- a/.gitignore +++ b/.gitignore @@ -34,6 +34,3 @@ _testmain.go *.test *.prof *.coverprofile - -boulder-start/boulder-start -activity-monitor/activity-monitor diff --git a/test/integration-test.py b/test/integration-test.py index 01f4c0ad9..7b657aa97 100644 --- a/test/integration-test.py +++ b/test/integration-test.py @@ -201,12 +201,6 @@ def run_client_tests(): if subprocess.Popen(cmd, shell=True, cwd=root, executable='/bin/bash').wait() != 0: die(ExitStatus.PythonFailure) -def check_activity_monitor(): - """Ensure that the activity monitor is running and received some messages.""" - resp = urllib2.urlopen("http://localhost:8007/debug/vars") - debug_vars = json.loads(resp.read()) - assert debug_vars['messages'] > 0, "Activity Monitor received zero messages." - @atexit.register def cleanup(): import shutil @@ -271,8 +265,6 @@ def main(): if args.run_all or args.run_letsencrypt: run_client_tests() - check_activity_monitor() - if not startservers.check(): die(ExitStatus.Error) exit_status = ExitStatus.OK diff --git a/test/startservers.py b/test/startservers.py index 73a7471d5..8ae3cf76e 100644 --- a/test/startservers.py +++ b/test/startservers.py @@ -63,7 +63,6 @@ def start(race_detection): t.daemon = True t.start() progs = [ - 'activity-monitor', 'boulder-wfe', 'boulder-ra', 'boulder-sa', From 2ea173d3ce4cf8d252815357df66bc30d23b4907 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Tue, 5 Jan 2016 16:35:34 -0800 Subject: [PATCH 17/26] Restore signing to ct-test-srv. This restores an earlier version of https://github.com/letsencrypt/boulder/pull/1282. During code review on that change, I requested that we replace the realtime signing with a static signature. However, that is now generating failures. I think the signature validation is now validating more of the content. --- test/ct-test-srv/main.go | 84 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 76 insertions(+), 8 deletions(-) diff --git a/test/ct-test-srv/main.go b/test/ct-test-srv/main.go index 6f74b2965..34065178d 100644 --- a/test/ct-test-srv/main.go +++ b/test/ct-test-srv/main.go @@ -9,20 +9,77 @@ package main import ( + "crypto/ecdsa" + "crypto/rand" + "crypto/sha256" + "crypto/x509" + "encoding/asn1" + "encoding/base64" "encoding/json" "fmt" "io/ioutil" "log" + "math/big" "net/http" + "os" "sync/atomic" + + ct "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/google/certificate-transparency/go" ) +func createSignedSCT(leaf []byte, k *ecdsa.PrivateKey) []byte { + rawKey, _ := x509.MarshalPKIXPublicKey(&k.PublicKey) + pkHash := sha256.Sum256(rawKey) + sct := ct.SignedCertificateTimestamp{ + SCTVersion: ct.V1, + LogID: pkHash, + Timestamp: 1337, + } + serialized, _ := ct.SerializeSCTSignatureInput(sct, ct.LogEntry{ + Leaf: ct.MerkleTreeLeaf{ + LeafType: ct.TimestampedEntryLeafType, + TimestampedEntry: ct.TimestampedEntry{ + X509Entry: ct.ASN1Cert(leaf), + EntryType: ct.X509LogEntryType, + }, + }, + }) + hashed := sha256.Sum256(serialized) + var ecdsaSig struct { + R, S *big.Int + } + ecdsaSig.R, ecdsaSig.S, _ = ecdsa.Sign(rand.Reader, k, hashed[:]) + sig, _ := asn1.Marshal(ecdsaSig) + + ds := ct.DigitallySigned{ + HashAlgorithm: ct.SHA256, + SignatureAlgorithm: ct.ECDSA, + Signature: sig, + } + + var jsonSCTObj struct { + SCTVersion ct.Version `json:"sct_version"` + ID string `json:"id"` + Timestamp uint64 `json:"timestamp"` + Extensions string `json:"extensions"` + Signature string `json:"signature"` + } + jsonSCTObj.SCTVersion = ct.V1 + jsonSCTObj.ID = base64.StdEncoding.EncodeToString(pkHash[:]) + jsonSCTObj.Timestamp = 1337 + jsonSCTObj.Signature, _ = ds.Base64String() + + jsonSCT, _ := json.Marshal(jsonSCTObj) + return jsonSCT +} + type ctSubmissionRequest struct { Chain []string `json:"chain"` } type integrationSrv struct { submissions int64 + key *ecdsa.PrivateKey } func (is *integrationSrv) handler(w http.ResponseWriter, r *http.Request) { @@ -47,14 +104,16 @@ func (is *integrationSrv) handler(w http.ResponseWriter, r *http.Request) { return } + leaf, err := base64.StdEncoding.DecodeString(addChainReq.Chain[0]) + if err != nil { + w.WriteHeader(400) + return + } + w.WriteHeader(http.StatusOK) - w.Write([]byte(`{ - "sct_version":0, - "id":"KHYaGJAn++880NYaAY12sFBXKcenQRvMvfYE9F1CYVM=", - "timestamp":1337, - "extensions":"", - "signature":"BAMARjBEAiAka/W0eYq23Iaih2wB2CGrAqlo92KyQuuY6WWumi1eNwIgBirYV/wsJvmZfGP5NrNYoWGIx1VV6NaNBIaSXh9hiYA=" - }`)) + // id is a sha256 of a random EC key. Generate your own with: + // openssl ecparam -name prime256v1 -genkey -outform der | openssl sha256 -binary | base64 + w.Write(createSignedSCT(leaf, is.key)) atomic.AddInt64(&is.submissions, 1) case "/submissions": if r.Method != "GET" { @@ -72,7 +131,16 @@ func (is *integrationSrv) handler(w http.ResponseWriter, r *http.Request) { } func main() { - is := integrationSrv{} + signingKey := "MHcCAQEEIOCtGlGt/WT7471dOHdfBg43uJWJoZDkZAQjWfTitcVNoAoGCCqGSM49AwEHoUQDQgAEYggOxPnPkzKBIhTacSYoIfnSL2jPugcbUKx83vFMvk5gKAz/AGe87w20riuPwEGn229hKVbEKHFB61NIqNHC3Q==" + decodedKey, _ := base64.StdEncoding.DecodeString(signingKey) + + key, err := x509.ParseECPrivateKey(decodedKey) + if err != nil { + fmt.Fprintf(os.Stderr, "failed to parse signing key: %s\n", err) + return + } + + is := integrationSrv{key: key} s := &http.Server{ Addr: "localhost:4500", Handler: http.HandlerFunc(is.handler), From a801766f05e22967e32db07fd1ba0eb536025a7e Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Wed, 6 Jan 2016 15:30:13 -0800 Subject: [PATCH 18/26] Fix silly merge error --- bdns/dns.go | 2 +- bdns/dns_test.go | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/bdns/dns.go b/bdns/dns.go index f5fd4de8d..1f0c07f9d 100644 --- a/bdns/dns.go +++ b/bdns/dns.go @@ -118,7 +118,7 @@ var ( // DNSResolver queries for DNS records type DNSResolver interface { - LookupTXT(context.Context, string) (txts []string, authorities []string, error) + LookupTXT(context.Context, string) (txts []string, authorities []string, err error) LookupHost(context.Context, string) ([]net.IP, error) LookupCAA(context.Context, string) ([]*dns.CAA, error) LookupMX(context.Context, string) ([]string, error) diff --git a/bdns/dns_test.go b/bdns/dns_test.go index bb395744d..cdbae9738 100644 --- a/bdns/dns_test.go +++ b/bdns/dns_test.go @@ -281,9 +281,9 @@ func TestDNSLookupCAA(t *testing.T) { } func TestDNSTXTAuthorities(t *testing.T) { - obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats) + obj := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats, clock.NewFake(), 1) - _, auths, err := obj.LookupTXT("letsencrypt.org") + _, auths, err := obj.LookupTXT(context.Background(), "letsencrypt.org") test.AssertNotError(t, err, "TXT lookup failed") test.AssertEquals(t, len(auths), 1) test.AssertEquals(t, auths[0], "letsencrypt.org. 0 IN SOA ns.letsencrypt.org. master.letsencrypt.org. 1 1 1 1 1") @@ -418,7 +418,7 @@ func TestRetry(t *testing.T) { dr := NewTestDNSResolverImpl(time.Second*10, []string{dnsLoopbackAddr}, testStats, clock.NewFake(), tc.maxTries) dr.DNSClient = tc.te - _, err := dr.LookupTXT(context.Background(), "example.com") + _, _, err := dr.LookupTXT(context.Background(), "example.com") if err == errTooManyRequests { t.Errorf("#%d, sent more requests than the test case handles", i) } @@ -435,14 +435,14 @@ func TestRetry(t *testing.T) { dr.DNSClient = &testExchanger{errs: []error{isTempErr, isTempErr, nil}} ctx, cancel := context.WithCancel(context.Background()) cancel() - _, err := dr.LookupTXT(ctx, "example.com") + _, _, 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") + _, _, err = dr.LookupTXT(ctx, "example.com") if err != context.DeadlineExceeded { t.Errorf("expected %s, got %s", context.DeadlineExceeded, err) } @@ -450,7 +450,7 @@ func TestRetry(t *testing.T) { dr.DNSClient = &testExchanger{errs: []error{isTempErr, isTempErr, nil}} ctx, deadlineCancel := context.WithTimeout(context.Background(), -10*time.Hour) deadlineCancel() - _, err = dr.LookupTXT(ctx, "example.com") + _, _, err = dr.LookupTXT(ctx, "example.com") if err != context.DeadlineExceeded { t.Errorf("expected %s, got %s", context.DeadlineExceeded, err) } From 95f5a5b0a0546661972c1c9e7deb764dfa478efa Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Wed, 6 Jan 2016 15:54:40 -0800 Subject: [PATCH 19/26] Strip LF from headers and encode body using MIME quoted-printable --- mail/mailer.go | 24 +++++++++++++++++++----- mail/mailer_test.go | 12 ++++++++---- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/mail/mailer.go b/mail/mailer.go index 7b4e798bc..bbb40e264 100644 --- a/mail/mailer.go +++ b/mail/mailer.go @@ -6,11 +6,13 @@ package mail import ( + "bytes" "crypto/rand" "fmt" "io" "math" "math/big" + "mime/quotedprintable" "net" "net/smtp" "strconv" @@ -76,16 +78,28 @@ func (m *MailerImpl) generateMessage(to []string, subject, body string) ([]byte, fmt.Sprintf("Subject: %s", subject), fmt.Sprintf("Date: %s", now.Format("Mon Jan 2 2006 15:04:05 -0700")), fmt.Sprintf("Message-Id: <%s.%s.%s>", now.Format("20060102T150405"), mid.String(), m.From), + "MIME-Version: 1.0", + "Content-Type: text/plain", + "Content-Transfer-Encoding: quoted-printable", } - for _, h := range headers[1:] { - quoted := strconv.QuoteToASCII(h) - h = quoted[1 : len(quoted)-1] // remove forced "" quotes + for i := range headers[1:] { + // strip LFs + headers[i] = strings.Replace(headers[i], "\n", "", -1) + } + bodyBuf := new(bytes.Buffer) + mimeWriter := quotedprintable.NewWriter(bodyBuf) + _, err = mimeWriter.Write([]byte(body)) + if err != nil { + return nil, err + } + err = mimeWriter.Close() + if err != nil { + return nil, err } - quotedBody := strconv.QuoteToASCII(body) return []byte(fmt.Sprintf( "%s\r\n\r\n%s\r\n", strings.Join(headers, "\r\n"), - quotedBody[1:len(quotedBody)-1], + bodyBuf.String(), )), nil } diff --git a/mail/mailer_test.go b/mail/mailer_test.go index 1a52b889c..275d70365 100644 --- a/mail/mailer_test.go +++ b/mail/mailer_test.go @@ -6,6 +6,7 @@ package mail import ( + "fmt" "io" "math/big" "strings" @@ -29,13 +30,16 @@ func TestGenerateMessage(t *testing.T) { test.AssertNotError(t, err, "Failed to generate email body") message := string(messageBytes) fields := strings.Split(message, "\r\n") - test.AssertEquals(t, len(fields), 8) + test.AssertEquals(t, len(fields), 12) + fmt.Println(message) test.AssertEquals(t, fields[0], "To: \"recv@email.com\"") test.AssertEquals(t, fields[1], "From: send@email.com") test.AssertEquals(t, fields[2], "Subject: test subject") test.AssertEquals(t, fields[3], "Date: Thu Jan 1 1970 00:00:00 +0000") test.AssertEquals(t, fields[4], "Message-Id: <19700101T000000.1991.send@email.com>") - test.AssertEquals(t, fields[5], "") - test.AssertEquals(t, fields[6], "this is the body\\n") - test.AssertEquals(t, fields[7], "") + test.AssertEquals(t, fields[5], "MIME-Version: 1.0") + test.AssertEquals(t, fields[6], "Content-Type: text/plain") + test.AssertEquals(t, fields[7], "Content-Transfer-Encoding: quoted-printable") + test.AssertEquals(t, fields[8], "") + test.AssertEquals(t, fields[9], "this is the body") } From aa94f070815493ba2ee67a34c174bb1744ca80f3 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Wed, 6 Jan 2016 17:13:04 -0800 Subject: [PATCH 20/26] Review fixes --- mail/mailer.go | 33 +++++++++++++++++---------------- mail/mailer_test.go | 9 ++++----- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/mail/mailer.go b/mail/mailer.go index bbb40e264..e34042878 100644 --- a/mail/mailer.go +++ b/mail/mailer.go @@ -9,7 +9,6 @@ import ( "bytes" "crypto/rand" "fmt" - "io" "math" "math/big" "mime/quotedprintable" @@ -17,18 +16,25 @@ import ( "net/smtp" "strconv" "strings" + "time" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" ) -type csprg interface { - Int(io.Reader, *big.Int) (*big.Int, error) +type idGenerator interface { + generate() *big.Int } +var maxBigInt = big.NewInt(math.MaxInt64) + type realSource struct{} -func (s realSource) Int(reader io.Reader, max *big.Int) (*big.Int, error) { - return rand.Int(reader, max) +func (s realSource) generate() *big.Int { + randInt, err := rand.Int(rand.Reader, maxBigInt) + if err != nil { + panic(err) + } + return randInt } // Mailer provides the interface for a mailer @@ -43,7 +49,7 @@ type MailerImpl struct { Auth smtp.Auth From string clk clock.Clock - csprgSource csprg + csprgSource idGenerator } // New constructs a Mailer to represent an account on a particular mail @@ -60,26 +66,21 @@ func New(server, port, username, password string) MailerImpl { } } -var maxBigInt = big.NewInt(math.MaxInt64) // ??? - func (m *MailerImpl) generateMessage(to []string, subject, body string) ([]byte, error) { - mid, err := m.csprgSource.Int(rand.Reader, maxBigInt) - if err != nil { - return nil, err - } + mid := m.csprgSource.generate() now := m.clk.Now().UTC() addrs := []string{} for _, a := range to { - addrs = append(addrs, strconv.QuoteToASCII(a)) + addrs = append(addrs, strconv.Quote(a)) } headers := []string{ fmt.Sprintf("To: %s", strings.Join(addrs, ", ")), fmt.Sprintf("From: %s", m.From), fmt.Sprintf("Subject: %s", subject), - fmt.Sprintf("Date: %s", now.Format("Mon Jan 2 2006 15:04:05 -0700")), + fmt.Sprintf("Date: %s", now.Format(time.RFC822)), fmt.Sprintf("Message-Id: <%s.%s.%s>", now.Format("20060102T150405"), mid.String(), m.From), "MIME-Version: 1.0", - "Content-Type: text/plain", + "Content-Type: text/plain; charset=UTF-8", "Content-Transfer-Encoding: quoted-printable", } for i := range headers[1:] { @@ -88,7 +89,7 @@ func (m *MailerImpl) generateMessage(to []string, subject, body string) ([]byte, } bodyBuf := new(bytes.Buffer) mimeWriter := quotedprintable.NewWriter(bodyBuf) - _, err = mimeWriter.Write([]byte(body)) + _, err := mimeWriter.Write([]byte(body)) if err != nil { return nil, err } diff --git a/mail/mailer_test.go b/mail/mailer_test.go index 275d70365..2b09d7c3e 100644 --- a/mail/mailer_test.go +++ b/mail/mailer_test.go @@ -7,7 +7,6 @@ package mail import ( "fmt" - "io" "math/big" "strings" "testing" @@ -19,8 +18,8 @@ import ( type fakeSource struct{} -func (f fakeSource) Int(reader io.Reader, max *big.Int) (*big.Int, error) { - return big.NewInt(1991), nil +func (f fakeSource) generate() *big.Int { + return big.NewInt(1991) } func TestGenerateMessage(t *testing.T) { @@ -35,10 +34,10 @@ func TestGenerateMessage(t *testing.T) { test.AssertEquals(t, fields[0], "To: \"recv@email.com\"") test.AssertEquals(t, fields[1], "From: send@email.com") test.AssertEquals(t, fields[2], "Subject: test subject") - test.AssertEquals(t, fields[3], "Date: Thu Jan 1 1970 00:00:00 +0000") + test.AssertEquals(t, fields[3], "Date: 01 Jan 70 00:00 UTC") test.AssertEquals(t, fields[4], "Message-Id: <19700101T000000.1991.send@email.com>") test.AssertEquals(t, fields[5], "MIME-Version: 1.0") - test.AssertEquals(t, fields[6], "Content-Type: text/plain") + test.AssertEquals(t, fields[6], "Content-Type: text/plain; charset=UTF-8") test.AssertEquals(t, fields[7], "Content-Transfer-Encoding: quoted-printable") test.AssertEquals(t, fields[8], "") test.AssertEquals(t, fields[9], "this is the body") From 4e569184d505a698ee0cbaf68fef13c27d0e3d1d Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Wed, 6 Jan 2016 17:26:59 -0800 Subject: [PATCH 21/26] Disallow non-ASCII addresses --- mail/mailer.go | 13 +++++++++++++ mail/mailer_test.go | 7 +++++++ 2 files changed, 20 insertions(+) diff --git a/mail/mailer.go b/mail/mailer.go index e34042878..43fafeb24 100644 --- a/mail/mailer.go +++ b/mail/mailer.go @@ -17,6 +17,7 @@ import ( "strconv" "strings" "time" + "unicode" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock" ) @@ -52,6 +53,15 @@ type MailerImpl struct { csprgSource idGenerator } +func isASCII(str string) bool { + for _, r := range str { + if r > unicode.MaxASCII { + return false + } + } + return true +} + // New constructs a Mailer to represent an account on a particular mail // transfer agent. func New(server, port, username, password string) MailerImpl { @@ -71,6 +81,9 @@ func (m *MailerImpl) generateMessage(to []string, subject, body string) ([]byte, now := m.clk.Now().UTC() addrs := []string{} for _, a := range to { + if !isASCII(a) { + return nil, fmt.Errorf("Non-ASCII email address") + } addrs = append(addrs, strconv.Quote(a)) } headers := []string{ diff --git a/mail/mailer_test.go b/mail/mailer_test.go index 2b09d7c3e..a0bc2226b 100644 --- a/mail/mailer_test.go +++ b/mail/mailer_test.go @@ -42,3 +42,10 @@ func TestGenerateMessage(t *testing.T) { test.AssertEquals(t, fields[8], "") test.AssertEquals(t, fields[9], "this is the body") } + +func TestFailNonASCIIAddress(t *testing.T) { + fc := clock.NewFake() + m := MailerImpl{From: "send@email.com", clk: fc, csprgSource: fakeSource{}} + _, err := m.generateMessage([]string{"遗憾@email.com"}, "test subject", "this is the body\n") + test.AssertError(t, err, "Allowed a non-ASCII to address incorrectly") +} From 6395064880c0d22daf472c0f4ddc3c0a4004ebc5 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Thu, 7 Jan 2016 10:56:56 -0800 Subject: [PATCH 22/26] Add CAA test domains to rate limit policies. This ensures that repeatedly running the integration test locally doesn't run afoul of rate limits. --- test/rate-limit-policies.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/rate-limit-policies.yml b/test/rate-limit-policies.yml index 05e76cf6f..c8b265c15 100644 --- a/test/rate-limit-policies.yml +++ b/test/rate-limit-policies.yml @@ -13,6 +13,8 @@ certificatesPerName: le2.wtf: 10000 le3.wtf: 10000 nginx.wtf: 10000 + good-caa-reserved.com: 10000 + bad-caa-reserved.com: 10000 registrationOverrides: 101: 1000 registrationsPerIP: From 2bb216441db92e7f0f47394ce4bedf8b216f9f4f Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Thu, 7 Jan 2016 11:06:34 -0800 Subject: [PATCH 23/26] Add mailer_dburl file. This is referenced in test/boulder-config.json but didn't exist. --- test/secrets/mailer_dburl | 1 + 1 file changed, 1 insertion(+) create mode 100644 test/secrets/mailer_dburl diff --git a/test/secrets/mailer_dburl b/test/secrets/mailer_dburl new file mode 100644 index 000000000..5b03a358b --- /dev/null +++ b/test/secrets/mailer_dburl @@ -0,0 +1 @@ +mysql+tcp://mailer@localhost:3306/boulder_sa_integration From b61c2a7e3a0c18395afd64e3d29b9ce04323bf20 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 6 Jan 2016 19:29:21 -0800 Subject: [PATCH 24/26] Add a From field to mailer config. Fixes #1351. --- cmd/config.go | 1 + cmd/expiration-mailer/main.go | 2 +- mail/mailer.go | 4 ++-- mail/mailer_test.go | 7 ++++--- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/cmd/config.go b/cmd/config.go index 0e251fc62..da069e187 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -118,6 +118,7 @@ type Config struct { Port string Username string Password string + From string CertLimit int NagTimes []string diff --git a/cmd/expiration-mailer/main.go b/cmd/expiration-mailer/main.go index 9f3304a23..1fffc7153 100644 --- a/cmd/expiration-mailer/main.go +++ b/cmd/expiration-mailer/main.go @@ -247,7 +247,7 @@ func main() { tmpl, err := template.New("expiry-email").Parse(string(emailTmpl)) cmd.FailOnError(err, "Could not parse email template") - mailClient := mail.New(c.Mailer.Server, c.Mailer.Port, c.Mailer.Username, c.Mailer.Password) + mailClient := mail.New(c.Mailer.Server, c.Mailer.Port, c.Mailer.Username, c.Mailer.Password, c.Mailer.From) nagCheckInterval := defaultNagCheckInterval if s := c.Mailer.NagCheckInterval; s != "" { diff --git a/mail/mailer.go b/mail/mailer.go index 43fafeb24..b85656491 100644 --- a/mail/mailer.go +++ b/mail/mailer.go @@ -64,13 +64,13 @@ func isASCII(str string) bool { // New constructs a Mailer to represent an account on a particular mail // transfer agent. -func New(server, port, username, password string) MailerImpl { +func New(server, port, username, password, from string) MailerImpl { auth := smtp.PlainAuth("", username, password, server) return MailerImpl{ Server: server, Port: port, Auth: auth, - From: username, + From: from, clk: clock.Default(), csprgSource: realSource{}, } diff --git a/mail/mailer_test.go b/mail/mailer_test.go index a0bc2226b..b40eafac4 100644 --- a/mail/mailer_test.go +++ b/mail/mailer_test.go @@ -24,7 +24,9 @@ func (f fakeSource) generate() *big.Int { func TestGenerateMessage(t *testing.T) { fc := clock.NewFake() - m := MailerImpl{From: "send@email.com", clk: fc, csprgSource: fakeSource{}} + m := New("", "", "", "", "send@email.com") + m.clk = fc + m.csprgSource = fakeSource{} messageBytes, err := m.generateMessage([]string{"recv@email.com"}, "test subject", "this is the body\n") test.AssertNotError(t, err, "Failed to generate email body") message := string(messageBytes) @@ -44,8 +46,7 @@ func TestGenerateMessage(t *testing.T) { } func TestFailNonASCIIAddress(t *testing.T) { - fc := clock.NewFake() - m := MailerImpl{From: "send@email.com", clk: fc, csprgSource: fakeSource{}} + m := New("", "", "", "", "send@email.com") _, err := m.generateMessage([]string{"遗憾@email.com"}, "test subject", "this is the body\n") test.AssertError(t, err, "Allowed a non-ASCII to address incorrectly") } From f218e314f8cb5914b78f516f6dcd39b7aca4f2a5 Mon Sep 17 00:00:00 2001 From: Hugo Landau Date: Fri, 18 Dec 2015 19:48:11 +0000 Subject: [PATCH 25/26] Add good key testing for ECDSA. --- ca/certificate-authority.go | 5 +- ca/certificate-authority_test.go | 42 ++++--- cmd/boulder-ca/main.go | 3 +- cmd/boulder-ra/main.go | 2 +- cmd/boulder-wfe/main.go | 4 +- cmd/config.go | 17 +++ core/good_key.go | 182 +++++++++++++++++++++++++----- core/good_key_test.go | 142 ++++++++++++++++++++--- ra/registration-authority.go | 6 +- ra/registration-authority_test.go | 11 +- test/boulder-config.json | 9 +- wfe/web-front-end.go | 10 +- wfe/web-front-end_test.go | 11 +- 13 files changed, 366 insertions(+), 78 deletions(-) diff --git a/ca/certificate-authority.go b/ca/certificate-authority.go index 96694aa62..900ad34d0 100644 --- a/ca/certificate-authority.go +++ b/ca/certificate-authority.go @@ -65,6 +65,7 @@ type CertificateAuthorityImpl struct { SA core.StorageAuthority PA core.PolicyAuthority Publisher core.Publisher + keyPolicy core.KeyPolicy clk clock.Clock // TODO(jmhodges): should be private, like log log *blog.AuditLogger stats statsd.Statter @@ -90,6 +91,7 @@ func NewCertificateAuthorityImpl( stats statsd.Statter, issuer *x509.Certificate, privateKey crypto.Signer, + keyPolicy core.KeyPolicy, ) (*CertificateAuthorityImpl, error) { var ca *CertificateAuthorityImpl var err error @@ -142,6 +144,7 @@ func NewCertificateAuthorityImpl( stats: stats, notAfter: issuer.NotAfter, hsmFaultTimeout: config.HSMFaultTimeout.Duration, + keyPolicy: keyPolicy, } if config.Expiry == "" { @@ -236,7 +239,7 @@ func (ca *CertificateAuthorityImpl) IssueCertificate(csr x509.CertificateRequest ca.log.AuditErr(err) return emptyCert, err } - if err = core.GoodKey(key); err != nil { + if err = ca.keyPolicy.GoodKey(key); err != nil { err = core.MalformedRequestError(fmt.Sprintf("Invalid public key in CSR: %s", err.Error())) // AUDIT[ Certificate Requests ] 11917fa4-10ef-4e0d-9105-bacbe7836a3c ca.log.AuditErr(err) diff --git a/ca/certificate-authority_test.go b/ca/certificate-authority_test.go index 081f73071..2341c605c 100644 --- a/ca/certificate-authority_test.go +++ b/ca/certificate-authority_test.go @@ -109,13 +109,14 @@ func mustRead(path string) []byte { } type testCtx struct { - sa core.StorageAuthority - caConfig cmd.CAConfig - reg core.Registration - pa core.PolicyAuthority - fc clock.FakeClock - stats *mocks.Statter - cleanUp func() + sa core.StorageAuthority + caConfig cmd.CAConfig + reg core.Registration + pa core.PolicyAuthority + keyPolicy core.KeyPolicy + fc clock.FakeClock + stats *mocks.Statter + cleanUp func() } var caKey crypto.Signer @@ -207,11 +208,18 @@ func setup(t *testing.T) *testCtx { stats := mocks.NewStatter() + keyPolicy := core.KeyPolicy{ + AllowRSA: true, + AllowECDSANISTP256: true, + AllowECDSANISTP384: true, + } + return &testCtx{ ssa, caConfig, reg, pa, + keyPolicy, fc, &stats, cleanUp, @@ -223,14 +231,14 @@ func TestFailNoSerial(t *testing.T) { defer ctx.cleanUp() ctx.caConfig.SerialPrefix = 0 - _, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCert, caKey) + _, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCert, caKey, ctx.keyPolicy) test.AssertError(t, err, "CA should have failed with no SerialPrefix") } func TestIssueCertificate(t *testing.T) { ctx := setup(t) defer ctx.cleanUp() - ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCert, caKey) + ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCert, caKey, ctx.keyPolicy) test.AssertNotError(t, err, "Failed to create CA") ca.Publisher = &mocks.Publisher{} ca.PA = ctx.pa @@ -307,7 +315,7 @@ func TestIssueCertificate(t *testing.T) { func TestRejectNoName(t *testing.T) { ctx := setup(t) defer ctx.cleanUp() - ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCert, caKey) + ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCert, caKey, ctx.keyPolicy) test.AssertNotError(t, err, "Failed to create CA") ca.Publisher = &mocks.Publisher{} ca.PA = ctx.pa @@ -324,7 +332,7 @@ func TestRejectNoName(t *testing.T) { func TestRejectTooManyNames(t *testing.T) { ctx := setup(t) defer ctx.cleanUp() - ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCert, caKey) + ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCert, caKey, ctx.keyPolicy) test.AssertNotError(t, err, "Failed to create CA") ca.Publisher = &mocks.Publisher{} ca.PA = ctx.pa @@ -341,7 +349,7 @@ func TestRejectTooManyNames(t *testing.T) { func TestDeduplication(t *testing.T) { ctx := setup(t) defer ctx.cleanUp() - ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCert, caKey) + ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCert, caKey, ctx.keyPolicy) test.AssertNotError(t, err, "Failed to create CA") ca.Publisher = &mocks.Publisher{} ca.PA = ctx.pa @@ -365,7 +373,7 @@ func TestDeduplication(t *testing.T) { func TestRejectValidityTooLong(t *testing.T) { ctx := setup(t) defer ctx.cleanUp() - ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCert, caKey) + ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCert, caKey, ctx.keyPolicy) test.AssertNotError(t, err, "Failed to create CA") ca.Publisher = &mocks.Publisher{} ca.PA = ctx.pa @@ -383,7 +391,7 @@ func TestRejectValidityTooLong(t *testing.T) { func TestShortKey(t *testing.T) { ctx := setup(t) defer ctx.cleanUp() - ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCert, caKey) + ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCert, caKey, ctx.keyPolicy) ca.Publisher = &mocks.Publisher{} ca.PA = ctx.pa ca.SA = ctx.sa @@ -399,7 +407,7 @@ func TestShortKey(t *testing.T) { func TestRejectBadAlgorithm(t *testing.T) { ctx := setup(t) defer ctx.cleanUp() - ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCert, caKey) + ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCert, caKey, ctx.keyPolicy) ca.Publisher = &mocks.Publisher{} ca.PA = ctx.pa ca.SA = ctx.sa @@ -416,7 +424,7 @@ func TestCapitalizedLetters(t *testing.T) { ctx := setup(t) defer ctx.cleanUp() ctx.caConfig.MaxNames = 3 - ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCert, caKey) + ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCert, caKey, ctx.keyPolicy) ca.Publisher = &mocks.Publisher{} ca.PA = ctx.pa ca.SA = ctx.sa @@ -437,7 +445,7 @@ func TestHSMFaultTimeout(t *testing.T) { ctx := setup(t) defer ctx.cleanUp() - ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCert, caKey) + ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, ctx.stats, caCert, caKey, ctx.keyPolicy) ca.Publisher = &mocks.Publisher{} ca.PA = ctx.pa ca.SA = ctx.sa diff --git a/cmd/boulder-ca/main.go b/cmd/boulder-ca/main.go index e04812998..a42d4666a 100644 --- a/cmd/boulder-ca/main.go +++ b/cmd/boulder-ca/main.go @@ -91,7 +91,8 @@ func main() { clock.Default(), stats, issuer, - priv) + priv, + c.KeyPolicy()) cmd.FailOnError(err, "Failed to create CA impl") cai.PA = pa diff --git a/cmd/boulder-ra/main.go b/cmd/boulder-ra/main.go index f64940157..4f44cfb5f 100644 --- a/cmd/boulder-ra/main.go +++ b/cmd/boulder-ra/main.go @@ -59,7 +59,7 @@ func main() { } rai := ra.NewRegistrationAuthorityImpl(clock.Default(), auditlogger, stats, - dc, rateLimitPolicies, c.RA.MaxContactsPerRegistration) + dc, rateLimitPolicies, c.RA.MaxContactsPerRegistration, c.KeyPolicy()) rai.PA = pa raDNSTimeout, err := time.ParseDuration(c.Common.DNSTimeout) cmd.FailOnError(err, "Couldn't parse RA DNS timeout") diff --git a/cmd/boulder-wfe/main.go b/cmd/boulder-wfe/main.go index 9b4c79c1e..d7124d1b9 100644 --- a/cmd/boulder-wfe/main.go +++ b/cmd/boulder-wfe/main.go @@ -53,7 +53,7 @@ func main() { app.Action = func(c cmd.Config, stats statsd.Statter, auditlogger *blog.AuditLogger) { go cmd.DebugServer(c.WFE.DebugAddr) - wfe, err := wfe.NewWebFrontEndImpl(stats, clock.Default()) + wfe, err := wfe.NewWebFrontEndImpl(stats, clock.Default(), c.KeyPolicy()) cmd.FailOnError(err, "Unable to create WFE") rac, sac := setupWFE(c, auditlogger, stats) wfe.RA = rac @@ -79,6 +79,8 @@ func main() { wfe.IssuerCert, err = cmd.LoadCert(c.Common.IssuerCert) cmd.FailOnError(err, fmt.Sprintf("Couldn't read issuer cert [%s]", c.Common.IssuerCert)) + auditlogger.Info(fmt.Sprintf("WFE using key policy: %#v", c.KeyPolicy())) + go cmd.ProfileCmd("WFE", stats) // Set up paths diff --git a/cmd/config.go b/cmd/config.go index 0e251fc62..e9a1ac15d 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -187,9 +187,26 @@ type Config struct { ReportDirectoryPath string } + AllowedSigningAlgos struct { + RSA bool + ECDSANISTP256 bool + ECDSANISTP384 bool + ECDSANISTP521 bool + } + SubscriberAgreementURL string } +// KeyPolicy returns a KeyPolicy reflecting the Boulder configuration. +func (config *Config) KeyPolicy() core.KeyPolicy { + return core.KeyPolicy{ + AllowRSA: config.AllowedSigningAlgos.RSA, + AllowECDSANISTP256: config.AllowedSigningAlgos.ECDSANISTP256, + AllowECDSANISTP384: config.AllowedSigningAlgos.ECDSANISTP384, + AllowECDSANISTP521: config.AllowedSigningAlgos.ECDSANISTP521, + } +} + // ServiceConfig contains config items that are common to all our services, to // be embedded in other config structs. type ServiceConfig struct { diff --git a/core/good_key.go b/core/good_key.go index 6d65ffe9f..d5b32ca55 100644 --- a/core/good_key.go +++ b/core/good_key.go @@ -8,9 +8,9 @@ package core import ( "crypto" "crypto/ecdsa" + "crypto/elliptic" "crypto/rsa" "fmt" - blog "github.com/letsencrypt/boulder/log" "math/big" "reflect" "sync" @@ -38,53 +38,163 @@ var ( smallPrimes []*big.Int ) +// KeyPolicy etermines which types of key may be used with various boulder +// operations. +type KeyPolicy struct { + AllowRSA bool // Whether RSA keys should be allowed. + AllowECDSANISTP256 bool // Whether ECDSA NISTP256 keys should be allowed. + AllowECDSANISTP384 bool // Whether ECDSA NISTP384 keys should be allowed. + AllowECDSANISTP521 bool // Whether ECDSA NISTP521 keys should be allowed. +} + // GoodKey returns true iff the key is acceptable for both TLS use and account // key use (our requirements are the same for either one), according to basic // strength and algorithm checking. // TODO: Support JsonWebKeys once go-jose migration is done. -func GoodKey(key crypto.PublicKey) error { - log := blog.GetAuditLogger() +func (policy *KeyPolicy) GoodKey(key crypto.PublicKey) error { switch t := key.(type) { case rsa.PublicKey: - return GoodKeyRSA(t) + return policy.goodKeyRSA(t) case *rsa.PublicKey: - return GoodKeyRSA(*t) + return policy.goodKeyRSA(*t) case ecdsa.PublicKey: - return GoodKeyECDSA(t) + return policy.goodKeyECDSA(t) case *ecdsa.PublicKey: - return GoodKeyECDSA(*t) + return policy.goodKeyECDSA(*t) default: - err := MalformedRequestError(fmt.Sprintf("Unknown key type %s", reflect.TypeOf(key))) - log.Debug(err.Error()) - return err + return MalformedRequestError(fmt.Sprintf("Unknown key type %s", reflect.TypeOf(key))) } } // GoodKeyECDSA determines if an ECDSA pubkey meets our requirements -func GoodKeyECDSA(key ecdsa.PublicKey) (err error) { - log := blog.GetAuditLogger() - err = NotSupportedError("ECDSA keys not yet supported") - log.Debug(err.Error()) - return +func (policy *KeyPolicy) goodKeyECDSA(key ecdsa.PublicKey) (err error) { + // Check the curve. + // + // The validity of the curve is an assumption for all following tests. + err = policy.goodCurve(key.Curve) + if err != nil { + return err + } + + // Key validation routine adapted from NIST SP800-56A § 5.6.2.3.2. + // + // + // Assuming a prime field since a) we are only allowing such curves and b) + // crypto/elliptic only supports prime curves. Where this assumption + // simplifies the code below, it is explicitly stated and explained. If ever + // adapting this code to support non-prime curves, refer to NIST SP800-56A § + // 5.6.2.3.2 and adapt this code appropriately. + params := key.Params() + + // SP800-56A § 5.6.2.3.2 Step 1. + // Partial check of the public key for an invalid range in the EC group: + // Verify that key is not the point at infinity O. + // This code assumes that the point at infinity is (0,0), which is the + // case for all supported curves. + if isPointAtInfinityNISTP(key.X, key.Y) { + return MalformedRequestError("Key x, y must not be the point at infinity") + } + + // SP800-56A § 5.6.2.3.2 Step 2. + // "Verify that x_Q and y_Q are integers in the interval [0,p-1] in the + // case that q is an odd prime p, or that x_Q and y_Q are bit strings + // of length m bits in the case that q = 2**m." + // + // Prove prime field: ASSUMED. + // Prove q != 2: ASSUMED. (Curve parameter. No supported curve has q == 2.) + // Prime field && q != 2 => q is an odd prime p + // Therefore "verify that x, y are in [0, p-1]" satisfies step 2. + // + // Therefore verify that both x and y of the public key point have the unique + // correct representation of an element in the underlying field by verifying + // that x and y are integers in [0, p-1]. + if key.X.Sign() < 0 || key.Y.Sign() < 0 { + return MalformedRequestError("Key x, y must not be negative") + } + + if key.X.Cmp(params.P) >= 0 || key.Y.Cmp(params.P) >= 0 { + return MalformedRequestError("Key x, y must not exceed P-1") + } + + // SP800-56A § 5.6.2.3.2 Step 3. + // "If q is an odd prime p, verify that (y_Q)**2 === (x_Q)***3 + a*x_Q + b (mod p). + // If q = 2**m, verify that (y_Q)**2 + (x_Q)*(y_Q) == (x_Q)**3 + a*(x_Q)*2 + b in + // the finite field of size 2**m. + // (Ensures that the public key is on the correct elliptic curve.)" + // + // q is an odd prime p: proven/assumed above. + // a = -3 for all supported curves. + // + // Therefore step 3 is satisfied simply by showing that + // y**2 === x**3 - 3*x + B (mod P). + // + // This proves that the public key is on the correct elliptic curve. + // But in practice, this test is provided by crypto/elliptic, so use that. + if !key.Curve.IsOnCurve(key.X, key.Y) { + return MalformedRequestError("Key point is not on the curve") + } + + // SP800-56A § 5.6.2.3.2 Step 4. + // "Verify that n*Q == O. + // (Ensures that the public key has the correct order. Along with check 1, + // ensures that the public key is in the correct range in the correct EC + // subgroup, that is, it is in the correct EC subgroup and is not the + // identity element.)" + // + // Ensure that public key has the correct order: + // verify that n*Q = O. + // + // n*Q = O iff n*Q is the point at infinity (see step 1). + ox, oy := key.Curve.ScalarMult(key.X, key.Y, params.N.Bytes()) + if !isPointAtInfinityNISTP(ox, oy) { + return MalformedRequestError("Public key does not have correct order") + } + + // End of SP800-56A § 5.6.2.3.2 Public Key Validation Routine. + // Key is valid. + return nil +} + +// Returns true iff the point (x,y) on NIST P-256, NIST P-384 or NIST P-521 is +// the point at infinity. These curves all have the same point at infinity +// (0,0). This function must ONLY be used on points on curves verified to have +// (0,0) as their point at infinity. +func isPointAtInfinityNISTP(x, y *big.Int) bool { + return x.Sign() == 0 && y.Sign() == 0 +} + +// GoodCurve determines if an elliptic curve meets our requirements. +func (policy *KeyPolicy) goodCurve(c elliptic.Curve) (err error) { + // Simply use a whitelist for now. + params := c.Params() + switch { + case policy.AllowECDSANISTP256 && params == elliptic.P256().Params(): + return nil + case policy.AllowECDSANISTP384 && params == elliptic.P384().Params(): + return nil + case policy.AllowECDSANISTP521 && params == elliptic.P521().Params(): + return nil + default: + return MalformedRequestError(fmt.Sprintf("ECDSA curve %v not allowed", params.Name)) + } } // GoodKeyRSA determines if a RSA pubkey meets our requirements -func GoodKeyRSA(key rsa.PublicKey) (err error) { - log := blog.GetAuditLogger() +func (policy *KeyPolicy) goodKeyRSA(key rsa.PublicKey) (err error) { + if !policy.AllowRSA { + return MalformedRequestError("RSA keys are not allowed") + } + // Baseline Requirements Appendix A // Modulus must be >= 2048 bits and <= 4096 bits modulus := key.N modulusBitLen := modulus.BitLen() const maxKeySize = 4096 if modulusBitLen < 2048 { - err = MalformedRequestError(fmt.Sprintf("Key too small: %d", modulusBitLen)) - log.Debug(err.Error()) - return err + return MalformedRequestError(fmt.Sprintf("Key too small: %d", modulusBitLen)) } if modulusBitLen > maxKeySize { - err = MalformedRequestError(fmt.Sprintf("Key too large: %d > %d", modulusBitLen, maxKeySize)) - log.Debug(err.Error()) - return err + return MalformedRequestError(fmt.Sprintf("Key too large: %d > %d", modulusBitLen, maxKeySize)) } // The CA SHALL confirm that the value of the public exponent is an // odd number equal to 3 or more. Additionally, the public exponent @@ -93,26 +203,36 @@ func GoodKeyRSA(key rsa.PublicKey) (err error) { // 2^32 - 1 or 2^64 - 1, because it stores E as an integer. So we // don't need to check the upper bound. if (key.E%2) == 0 || key.E < ((1<<16)+1) { - err = MalformedRequestError(fmt.Sprintf("Key exponent should be odd and >2^16: %d", key.E)) - log.Debug(err.Error()) - return err + return MalformedRequestError(fmt.Sprintf("Key exponent should be odd and >2^16: %d", key.E)) } // The modulus SHOULD also have the following characteristics: an odd // number, not the power of a prime, and have no factors smaller than 752. // TODO: We don't yet check for "power of a prime." + if checkSmallPrimes(modulus) { + return MalformedRequestError("Key divisible by small prime") + } + + return nil +} + +// Returns true iff integer i is divisible by any of the primes in smallPrimes. +// +// Short circuits; execution time is dependent on i. Do not use this on secret +// values. +func checkSmallPrimes(i *big.Int) bool { smallPrimesSingleton.Do(func() { for _, prime := range smallPrimeInts { smallPrimes = append(smallPrimes, big.NewInt(prime)) } }) + for _, prime := range smallPrimes { var result big.Int - result.Mod(modulus, prime) + result.Mod(i, prime) if result.Sign() == 0 { - err = MalformedRequestError(fmt.Sprintf("Key divisible by small prime: %d", prime)) - log.Debug(err.Error()) - return err + return true } } - return nil + + return false } diff --git a/core/good_key_test.go b/core/good_key_test.go index 33f6ded8a..6a0cfb4ac 100644 --- a/core/good_key_test.go +++ b/core/good_key_test.go @@ -7,6 +7,7 @@ package core import ( "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" "crypto/rsa" "math/big" @@ -15,29 +16,29 @@ import ( "github.com/letsencrypt/boulder/test" ) -func TestUnknownKeyType(t *testing.T) { - notAKey := struct{}{} - test.AssertError(t, GoodKey(notAKey), "Should have rejected a key of unknown type") +var testingPolicy = &KeyPolicy{ + AllowRSA: true, + AllowECDSANISTP256: true, + AllowECDSANISTP384: true, } -func TestWrongKeyType(t *testing.T) { - ecdsaKey := ecdsa.PublicKey{} - test.AssertError(t, GoodKey(&ecdsaKey), "Should have rejected ECDSA key.") - test.AssertError(t, GoodKey(ecdsaKey), "Should have rejected ECDSA key.") +func TestUnknownKeyType(t *testing.T) { + notAKey := struct{}{} + test.AssertError(t, testingPolicy.GoodKey(notAKey), "Should have rejected a key of unknown type") } func TestSmallModulus(t *testing.T) { private, err := rsa.GenerateKey(rand.Reader, 2040) test.AssertNotError(t, err, "Error generating key") - test.AssertError(t, GoodKey(&private.PublicKey), "Should have rejected too-short key.") - test.AssertError(t, GoodKey(private.PublicKey), "Should have rejected too-short key.") + test.AssertError(t, testingPolicy.GoodKey(&private.PublicKey), "Should have rejected too-short key.") + test.AssertError(t, testingPolicy.GoodKey(private.PublicKey), "Should have rejected too-short key.") } func TestLargeModulus(t *testing.T) { private, err := rsa.GenerateKey(rand.Reader, 4097) test.AssertNotError(t, err, "Error generating key") - test.AssertError(t, GoodKey(&private.PublicKey), "Should have rejected too-long key.") - test.AssertError(t, GoodKey(private.PublicKey), "Should have rejected too-long key.") + test.AssertError(t, testingPolicy.GoodKey(&private.PublicKey), "Should have rejected too-long key.") + test.AssertError(t, testingPolicy.GoodKey(private.PublicKey), "Should have rejected too-long key.") } func TestSmallExponent(t *testing.T) { @@ -46,7 +47,7 @@ func TestSmallExponent(t *testing.T) { N: bigOne.Lsh(bigOne, 2048), E: 5, } - test.AssertError(t, GoodKey(&key), "Should have rejected small exponent.") + test.AssertError(t, testingPolicy.GoodKey(&key), "Should have rejected small exponent.") } func TestEvenExponent(t *testing.T) { @@ -55,7 +56,7 @@ func TestEvenExponent(t *testing.T) { N: bigOne.Lsh(bigOne, 2048), E: 1 << 17, } - test.AssertError(t, GoodKey(&key), "Should have rejected even exponent.") + test.AssertError(t, testingPolicy.GoodKey(&key), "Should have rejected even exponent.") } func TestEvenModulus(t *testing.T) { @@ -64,7 +65,7 @@ func TestEvenModulus(t *testing.T) { N: bigOne.Lsh(bigOne, 2048), E: (1 << 17) + 1, } - test.AssertError(t, GoodKey(&key), "Should have rejected even modulus.") + test.AssertError(t, testingPolicy.GoodKey(&key), "Should have rejected even modulus.") } func TestModulusDivisibleBy752(t *testing.T) { @@ -76,11 +77,120 @@ func TestModulusDivisibleBy752(t *testing.T) { N: N, E: (1 << 17) + 1, } - test.AssertError(t, GoodKey(&key), "Should have rejected modulus divisible by 751.") + test.AssertError(t, testingPolicy.GoodKey(&key), "Should have rejected modulus divisible by 751.") } func TestGoodKey(t *testing.T) { private, err := rsa.GenerateKey(rand.Reader, 2048) test.AssertNotError(t, err, "Error generating key") - test.AssertNotError(t, GoodKey(&private.PublicKey), "Should have accepted good key.") + test.AssertNotError(t, testingPolicy.GoodKey(&private.PublicKey), "Should have accepted good key.") +} + +func TestECDSABadCurve(t *testing.T) { + for _, curve := range invalidCurves { + private, err := ecdsa.GenerateKey(curve, rand.Reader) + test.AssertNotError(t, err, "Error generating key") + test.AssertError(t, testingPolicy.GoodKey(&private.PublicKey), "Should have rejected key with unsupported curve.") + test.AssertError(t, testingPolicy.GoodKey(private.PublicKey), "Should have rejected key with unsupported curve.") + } +} + +var invalidCurves = []elliptic.Curve{ + elliptic.P224(), + elliptic.P521(), +} + +var validCurves = []elliptic.Curve{ + elliptic.P256(), + elliptic.P384(), +} + +func TestECDSAGoodKey(t *testing.T) { + for _, curve := range validCurves { + private, err := ecdsa.GenerateKey(curve, rand.Reader) + test.AssertNotError(t, err, "Error generating key") + test.AssertNotError(t, testingPolicy.GoodKey(&private.PublicKey), "Should have accepted good key.") + test.AssertNotError(t, testingPolicy.GoodKey(private.PublicKey), "Should have accepted good key.") + } +} + +func TestECDSANotOnCurveX(t *testing.T) { + for _, curve := range validCurves { + // Change a public key so that it is no longer on the curve. + private, err := ecdsa.GenerateKey(curve, rand.Reader) + test.AssertNotError(t, err, "Error generating key") + + private.X.Add(private.X, big.NewInt(1)) + test.AssertError(t, testingPolicy.GoodKey(&private.PublicKey), "Should not have accepted key not on the curve.") + test.AssertError(t, testingPolicy.GoodKey(private.PublicKey), "Should not have accepted key not on the curve.") + } +} + +func TestECDSANotOnCurveY(t *testing.T) { + for _, curve := range validCurves { + // Again with Y. + private, err := ecdsa.GenerateKey(curve, rand.Reader) + test.AssertNotError(t, err, "Error generating key") + + // Change the public key so that it is no longer on the curve. + private.Y.Add(private.Y, big.NewInt(1)) + test.AssertError(t, testingPolicy.GoodKey(&private.PublicKey), "Should not have accepted key not on the curve.") + test.AssertError(t, testingPolicy.GoodKey(private.PublicKey), "Should not have accepted key not on the curve.") + } +} + +func TestECDSANegative(t *testing.T) { + for _, curve := range validCurves { + // Check that negative X is not accepted. + private, err := ecdsa.GenerateKey(curve, rand.Reader) + test.AssertNotError(t, err, "Error generating key") + + private.X.Neg(private.X) + test.AssertError(t, testingPolicy.GoodKey(&private.PublicKey), "Should not have accepted key with negative X.") + test.AssertError(t, testingPolicy.GoodKey(private.PublicKey), "Should not have accepted key with negative X.") + + // Check that negative Y is not accepted. + private.X.Neg(private.X) + private.Y.Neg(private.Y) + test.AssertError(t, testingPolicy.GoodKey(&private.PublicKey), "Should not have accepted key with negative Y.") + test.AssertError(t, testingPolicy.GoodKey(private.PublicKey), "Should not have accepted key with negative Y.") + } +} + +func TestECDSANegativeUnmodulatedX(t *testing.T) { + for _, curve := range validCurves { + // Check that unmodulated X is not accepted. + private, err := ecdsa.GenerateKey(curve, rand.Reader) + test.AssertNotError(t, err, "Error generating key") + + private.X.Mul(private.X, private.Curve.Params().P) + test.AssertError(t, testingPolicy.GoodKey(&private.PublicKey), "Should not have accepted key with unmodulated X.") + test.AssertError(t, testingPolicy.GoodKey(private.PublicKey), "Should not have accepted key with unmodulated X.") + } +} + +func TestECDSANegativeUnmodulatedY(t *testing.T) { + for _, curve := range validCurves { + // Check that unmodulated Y is not accepted. + private, err := ecdsa.GenerateKey(curve, rand.Reader) + test.AssertNotError(t, err, "Error generating key") + + private.X.Mul(private.Y, private.Curve.Params().P) + test.AssertError(t, testingPolicy.GoodKey(&private.PublicKey), "Should not have accepted key with unmodulated Y.") + test.AssertError(t, testingPolicy.GoodKey(private.PublicKey), "Should not have accepted key with unmodulated Y.") + } +} + +func TestECDSAIdentity(t *testing.T) { + for _, curve := range validCurves { + // The point at infinity is 0,0, it should not be accepted. + public := ecdsa.PublicKey{ + Curve: curve, + X: big.NewInt(0), + Y: big.NewInt(0), + } + + test.AssertError(t, testingPolicy.GoodKey(&public), "Should not have accepted key with point at infinity.") + test.AssertError(t, testingPolicy.GoodKey(public), "Should not have accepted key with point at infinity.") + } } diff --git a/ra/registration-authority.go b/ra/registration-authority.go index 8a4ffded6..95202c6f4 100644 --- a/ra/registration-authority.go +++ b/ra/registration-authority.go @@ -55,6 +55,7 @@ type RegistrationAuthorityImpl struct { clk clock.Clock log *blog.AuditLogger dc *DomainCheck + keyPolicy core.KeyPolicy // How long before a newly created authorization expires. authorizationLifetime time.Duration pendingAuthorizationLifetime time.Duration @@ -71,7 +72,7 @@ type RegistrationAuthorityImpl struct { } // 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 { +func NewRegistrationAuthorityImpl(clk clock.Clock, logger *blog.AuditLogger, stats statsd.Statter, dc *DomainCheck, policies cmd.RateLimitConfig, maxContactsPerReg int, keyPolicy core.KeyPolicy) *RegistrationAuthorityImpl { // TODO(jmhodges): making RA take a "RA" stats.Scope, not Statter scope := metrics.NewStatsdScope(stats, "RA") ra := &RegistrationAuthorityImpl{ @@ -84,6 +85,7 @@ func NewRegistrationAuthorityImpl(clk clock.Clock, logger *blog.AuditLogger, sta rlPolicies: policies, tiMu: new(sync.RWMutex), maxContactsPerReg: maxContactsPerReg, + keyPolicy: keyPolicy, regByIPStats: scope.NewScope("RA", "RateLimit", "RegistrationsByIP"), pendAuthByRegIDStats: scope.NewScope("RA", "RateLimit", "PendingAuthorizationsByRegID"), @@ -210,7 +212,7 @@ func (ra *RegistrationAuthorityImpl) checkRegistrationLimit(ip net.IP) error { // NewRegistration constructs a new Registration from a request. func (ra *RegistrationAuthorityImpl) NewRegistration(init core.Registration) (reg core.Registration, err error) { - if err = core.GoodKey(init.Key.Key); err != nil { + if err = ra.keyPolicy.GoodKey(init.Key.Key); err != nil { return core.Registration{}, core.MalformedRequestError(fmt.Sprintf("Invalid public key: %s", err.Error())) } if err = ra.checkRegistrationLimit(init.InitialIP); err != nil { diff --git a/ra/registration-authority_test.go b/ra/registration-authority_test.go index 2678d6117..5e56289c9 100644 --- a/ra/registration-authority_test.go +++ b/ra/registration-authority_test.go @@ -146,6 +146,12 @@ func makeResponse(ch core.Challenge) (out core.Challenge, err error) { return } +var testKeyPolicy = core.KeyPolicy{ + AllowRSA: true, + AllowECDSANISTP256: true, + AllowECDSANISTP384: true, +} + func initAuthorities(t *testing.T) (*DummyValidationAuthority, *sa.SQLStorageAuthority, *RegistrationAuthorityImpl, clock.FakeClock, func()) { err := json.Unmarshal(AccountKeyJSONA, &AccountKeyA) test.AssertNotError(t, err, "Failed to unmarshal public JWK") @@ -215,7 +221,8 @@ func initAuthorities(t *testing.T) (*DummyValidationAuthority, *sa.SQLStorageAut fc, stats, caCert, - caKey) + caKey, + testKeyPolicy) test.AssertNotError(t, err, "Couldn't create CA") ca.SA = ssa ca.PA = pa @@ -242,7 +249,7 @@ func initAuthorities(t *testing.T) (*DummyValidationAuthority, *sa.SQLStorageAut Threshold: 100, Window: cmd.ConfigDuration{Duration: 24 * 90 * time.Hour}, }, - }, 1) + }, 1, testKeyPolicy) ra.SA = ssa ra.VA = va ra.CA = ca diff --git a/test/boulder-config.json b/test/boulder-config.json index 72d802d89..b2799fcc3 100644 --- a/test/boulder-config.json +++ b/test/boulder-config.json @@ -306,5 +306,12 @@ "dbConnectFile": "test/secrets/cert_checker_dburl" }, - "subscriberAgreementURL": "http://127.0.0.1:4001/terms/v1" + "subscriberAgreementURL": "http://127.0.0.1:4001/terms/v1", + + "allowedSigningAlgos": { + "rsa": true, + "ecdsanistp256": true, + "ecdsanistp384": true, + "ecdsanistp521": false + } } diff --git a/wfe/web-front-end.go b/wfe/web-front-end.go index e0dfb803d..04ab62b6f 100644 --- a/wfe/web-front-end.go +++ b/wfe/web-front-end.go @@ -75,6 +75,9 @@ type WebFrontEndImpl struct { // Register of anti-replay nonces nonceService *core.NonceService + // Key policy. + keyPolicy core.KeyPolicy + // Cache settings CertCacheDuration time.Duration CertNoCacheExpirationWindow time.Duration @@ -90,7 +93,7 @@ type WebFrontEndImpl struct { } // NewWebFrontEndImpl constructs a web service for Boulder -func NewWebFrontEndImpl(stats statsd.Statter, clk clock.Clock) (WebFrontEndImpl, error) { +func NewWebFrontEndImpl(stats statsd.Statter, clk clock.Clock, keyPolicy core.KeyPolicy) (WebFrontEndImpl, error) { logger := blog.GetAuditLogger() logger.Notice("Web Front End Starting") @@ -104,6 +107,7 @@ func NewWebFrontEndImpl(stats statsd.Statter, clk clock.Clock) (WebFrontEndImpl, clk: clk, nonceService: nonceService, stats: stats, + keyPolicy: keyPolicy, }, nil } @@ -355,7 +359,7 @@ func (wfe *WebFrontEndImpl) verifyPOST(logEvent *requestEvent, request *http.Req // When looking up keys from the registrations DB, we can be confident they // are "good". But when we are verifying against any submitted key, we want // to check its quality before doing the verify. - if err = core.GoodKey(submittedKey.Key); err != nil { + if err = wfe.keyPolicy.GoodKey(submittedKey.Key); err != nil { wfe.stats.Inc("WFE.Errors.JWKRejectedByGoodKey", 1, 1.0) logEvent.AddError("JWK in request was rejected by GoodKey: %s", err) return nil, nil, reg, probs.Malformed(err.Error()) @@ -719,7 +723,7 @@ func (wfe *WebFrontEndImpl) NewCertificate(logEvent *requestEvent, response http // bytes on the wire, and (b) the CA logs all rejections as audit events, but // a bad key from the client is just a malformed request and doesn't need to // be audited. - if err := core.GoodKey(certificateRequest.CSR.PublicKey); err != nil { + if err := wfe.keyPolicy.GoodKey(certificateRequest.CSR.PublicKey); err != nil { logEvent.AddError("CSR public key failed GoodKey: %s", err) wfe.sendError(response, logEvent, probs.Malformed("Invalid key in certificate request :: %s", err), err) return diff --git a/wfe/web-front-end_test.go b/wfe/web-front-end_test.go index 88f22712b..a2cffa5e1 100644 --- a/wfe/web-front-end_test.go +++ b/wfe/web-front-end_test.go @@ -197,10 +197,17 @@ func signRequest(t *testing.T, req string, nonceService *core.NonceService) stri return ret } +var testKeyPolicy = core.KeyPolicy{ + AllowRSA: true, + AllowECDSANISTP256: true, + AllowECDSANISTP384: true, +} + func setupWFE(t *testing.T) (WebFrontEndImpl, clock.FakeClock) { fc := clock.NewFake() stats, _ := statsd.NewNoopClient() - wfe, err := NewWebFrontEndImpl(stats, fc) + + wfe, err := NewWebFrontEndImpl(stats, fc, testKeyPolicy) test.AssertNotError(t, err, "Unable to create WFE") wfe.NewReg = wfe.BaseURL + NewRegPath @@ -546,7 +553,7 @@ func TestIssueCertificate(t *testing.T) { // TODO: Use a mock RA so we can test various conditions of authorized, not // authorized, etc. stats, _ := statsd.NewNoopClient(nil) - ra := ra.NewRegistrationAuthorityImpl(fc, wfe.log, stats, nil, cmd.RateLimitConfig{}, 0) + ra := ra.NewRegistrationAuthorityImpl(fc, wfe.log, stats, nil, cmd.RateLimitConfig{}, 0, testKeyPolicy) ra.SA = mocks.NewStorageAuthority(fc) ra.CA = &MockCA{} ra.PA = &MockPA{} From 945b727478a2b9f6d1bb32b0fd9a1d7016baddc8 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Thu, 7 Jan 2016 13:49:52 -0800 Subject: [PATCH 26/26] Parse from address to make sure it's valid. --- cmd/expiration-mailer/main.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmd/expiration-mailer/main.go b/cmd/expiration-mailer/main.go index 1fffc7153..eba4de705 100644 --- a/cmd/expiration-mailer/main.go +++ b/cmd/expiration-mailer/main.go @@ -10,6 +10,7 @@ import ( "crypto/x509" "fmt" "io/ioutil" + netmail "net/mail" "sort" "strings" "text/template" @@ -247,6 +248,9 @@ func main() { tmpl, err := template.New("expiry-email").Parse(string(emailTmpl)) cmd.FailOnError(err, "Could not parse email template") + _, err = netmail.ParseAddress(c.Mailer.From) + cmd.FailOnError(err, fmt.Sprintf("Could not parse from address: %s", c.Mailer.From)) + mailClient := mail.New(c.Mailer.Server, c.Mailer.Port, c.Mailer.Username, c.Mailer.Password, c.Mailer.From) nagCheckInterval := defaultNagCheckInterval