Merge branch 'master' into delete_old_challenges

Godeps/Godeps.json

@@ -114,7 +114,7 @@
 		},
 		{
 			"ImportPath": "github.com/letsencrypt/net/publicsuffix",
-			"Rev": "adbe5512bf2d8766546da71cf90b681c519cb39f"
+			"Rev": "f564ebf2f38110ad192d071f20536b605349e2f1"
 		},
 		{
 			"ImportPath": "github.com/miekg/dns",

mocks/mocks.go

@@ -62,8 +62,13 @@ func (t timeoutError) Timeout() bool {
 }
 
 // LookupHost is a mock
+//
+// Note: see comments on LookupMX regarding email.only
+//
 func (mock *DNSResolver) LookupHost(hostname string) ([]net.IP, time.Duration, error) {
-	if hostname == "always.invalid" || hostname == "invalid.invalid" {
+	if hostname == "always.invalid" ||
+		hostname == "invalid.invalid" ||
+		hostname == "email.only" {
 		return []net.IP{}, 0, nil
 	}
 	if hostname == "always.timeout" {
@@ -109,10 +114,18 @@ func (mock *DNSResolver) LookupCAA(domain string) ([]*dns.CAA, time.Duration, er
 }
 
 // LookupMX is a mock
+//
+// Note: the email.only domain must have an MX but no A or AAAA
+// records. The mock LookupHost returns an address of 127.0.0.1 for
+// 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, time.Duration, error) {
 	switch strings.TrimRight(domain, ".") {
 	case "letsencrypt.org":
 		fallthrough
+	case "email.only":
+		fallthrough
 	case "email.com":
 		return []string{"mail.email.com"}, 0, nil
 	}

ra/registration-authority.go

@@ -81,21 +81,30 @@ func NewRegistrationAuthorityImpl(clk clock.Clock, logger *blog.AuditLogger, sta
 var errUnparseableEmail = errors.New("not a valid e-mail address")
 var errEmptyDNSResponse = errors.New("empty DNS response")
 
-func validateEmail(address string, resolver core.DNSResolver) (rtt time.Duration, err error) {
+func validateEmail(address string, resolver core.DNSResolver) (rtt time.Duration, count int64, err error) {
 	_, err = mail.ParseAddress(address)
 	if err != nil {
-		return time.Duration(0), errUnparseableEmail
+		return time.Duration(0), 0, errUnparseableEmail
 	}
 	splitEmail := strings.SplitN(address, "@", -1)
 	domain := strings.ToLower(splitEmail[len(splitEmail)-1])
-	result, rtt, err := resolver.LookupHost(domain)
+	var rtt1, rtt2 time.Duration
-	if err == nil && len(result) == 0 {
+	var resultMX []string
-		err = errEmptyDNSResponse
+	var resultA []net.IP
+	resultMX, rtt1, err = resolver.LookupMX(domain)
+	count++
+	if err == nil && len(resultMX) == 0 {
+		resultA, rtt2, err = resolver.LookupHost(domain)
+		count++
+		if err == nil && len(resultA) == 0 {
+			err = errEmptyDNSResponse
+		}
 	}
 	if err != nil {
 		problem := dns.ProblemDetailsFromDNSError(err)
 		err = core.MalformedRequestError(problem.Detail)
 	}
+	rtt = rtt1 + rtt2
 
 	return
 }
@@ -223,9 +232,14 @@ func (ra *RegistrationAuthorityImpl) validateContacts(contacts []*core.AcmeURL)
 		case "tel":
 			continue
 		case "mailto":
-			rtt, err := validateEmail(contact.Opaque, ra.DNSResolver)
+			// Note: the stats handling here is a bit of a lie,
-			ra.stats.TimingDuration("RA.DNS.RTT.A", rtt, 1.0)
+			// since validateEmail() mainly does MX lookups and
-			ra.stats.Inc("RA.DNS.Rate", 1, 1.0)
+			// only does A lookups when the MX is missing.
+			rtt, count, err := validateEmail(contact.Opaque, ra.DNSResolver)
+			if count > 0 {
+				ra.stats.TimingDuration("RA.DNS.RTT.A", time.Duration(int64(rtt)/count), 1.0)
+				ra.stats.Inc("RA.DNS.Rate", count, 1.0)
+			}
 			if err != nil {
 				return core.MalformedRequestError(fmt.Sprintf(
 					"Validation of contact %s failed: %s", contact, err))

ra/registration-authority_test.go

@@ -310,7 +310,7 @@ func TestValidateContacts(t *testing.T) {
 }
 
 func TestValidateEmail(t *testing.T) {
-	testCases := []struct {
+	testFailures := []struct {
 		input    string
 		expected string
 	}{
@@ -319,15 +319,22 @@ func TestValidateEmail(t *testing.T) {
 		{"a@always.timeout", "DNS query timed out"},
 		{"a@always.error", "DNS networking error"},
 	}
-	for _, tc := range testCases {
+	testSuccesses := []string{
-		_, err := validateEmail(tc.input, &mocks.DNSResolver{})
+		"a@email.com",
+		"b@email.only",
+	}
+	for _, tc := range testFailures {
+		_, _, err := validateEmail(tc.input, &mocks.DNSResolver{})
 		if err.Error() != tc.expected {
 			t.Errorf("validateEmail(%q): got %#v, expected %#v",
 				tc.input, err, tc.expected)
 		}
 	}
-	if _, err := validateEmail("a@email.com", &mocks.DNSResolver{}); err != nil {
+	for _, addr := range testSuccesses {
-		t.Errorf("Expected a@email.com to validate, but it failed: %s", err)
+		if _, _, err := validateEmail(addr, &mocks.DNSResolver{}); err != nil {
+			t.Errorf("validateEmail(%q): expected success, but it failed: %s",
+				addr, err)
+		}
 	}
 }