Merge branch 'master' into dns-meta

bdns/problem.go

@@ -6,6 +6,7 @@
 package bdns
 
 import (
+	"fmt"
 	"net"
 
 	"github.com/letsencrypt/boulder/probs"
@@ -15,20 +16,23 @@ const detailDNSTimeout = "DNS query timed out"
 const detailDNSNetFailure = "DNS networking error"
 const detailServerFailure = "Server failure at resolver"
 
-// ProblemDetailsFromDNSError checks the error returned from Lookup...
+// ProblemDetailsFromDNSError checks the error returned from Lookup...  methods
-// methods and tests if the error was an underlying net.OpError or an error
+// and tests if the error was an underlying net.OpError or an error caused by
-// caused by resolver returning SERVFAIL or other invalid Rcodes and returns
+// resolver returning SERVFAIL or other invalid Rcodes and returns the relevant
-// the relevant core.ProblemDetails.
+// core.ProblemDetails. The detail string will contain a mention of the DNS
-func ProblemDetailsFromDNSError(err error) *probs.ProblemDetails {
+// record type and domain given.
-	problem := &probs.ProblemDetails{Type: probs.ConnectionProblem}
+func ProblemDetailsFromDNSError(recordType, domain string, err error) *probs.ProblemDetails {
+	detail := detailServerFailure
 	if netErr, ok := err.(*net.OpError); ok {
 		if netErr.Timeout() {
-			problem.Detail = detailDNSTimeout
+			detail = detailDNSTimeout
 		} else {
-			problem.Detail = detailDNSNetFailure
+			detail = detailDNSNetFailure
 		}
-	} else {
-		problem.Detail = detailServerFailure
 	}
-	return problem
+	detail = fmt.Sprintf("%s during %s-record lookup of %s", detail, recordType, domain)
+	return &probs.ProblemDetails{
+		Type:   probs.ConnectionProblem,
+		Detail: detail,
+	}
 }

bdns/problem_test.go

@@ -31,11 +31,12 @@ func TestProblemDetailsFromDNSError(t *testing.T) {
 		},
 	}
 	for _, tc := range testCases {
-		err := ProblemDetailsFromDNSError(tc.err)
+		err := ProblemDetailsFromDNSError("TXT", "example.com", tc.err)
 		if err.Type != probs.ConnectionProblem {
 			t.Errorf("ProblemDetailsFromDNSError(%q).Type = %q, expected %q", tc.err, err.Type, probs.ConnectionProblem)
 		}
-		if err.Detail != tc.expected {
+		exp := tc.expected + " during TXT-record lookup of example.com"
+		if err.Detail != exp {
 			t.Errorf("ProblemDetailsFromDNSError(%q).Detail = %q, expected %q", tc.err, err.Detail, tc.expected)
 		}
 	}

ra/registration-authority.go

@@ -97,9 +97,10 @@ func validateEmail(address string, resolver bdns.DNSResolver) (prob *probs.Probl
 	var resultMX []string
 	var resultA []net.IP
 	resultMX, err = resolver.LookupMX(domain)
-
+	recQ := "MX"
 	if err == nil && len(resultMX) == 0 {
 		resultA, err = resolver.LookupHost(domain)
+		recQ = "A"
 		if err == nil && len(resultA) == 0 {
 			return &probs.ProblemDetails{
 				Type:   probs.InvalidEmailProblem,
@@ -108,11 +109,9 @@ func validateEmail(address string, resolver bdns.DNSResolver) (prob *probs.Probl
 		}
 	}
 	if err != nil {
-		dnsProblem := bdns.ProblemDetailsFromDNSError(err)
+		prob := bdns.ProblemDetailsFromDNSError(recQ, domain, err)
-		return &probs.ProblemDetails{
+		prob.Type = probs.InvalidEmailProblem
-			Type:   probs.InvalidEmailProblem,
+		return prob
-			Detail: dnsProblem.Detail,
-		}
 	}
 
 	return nil

ra/registration-authority_test.go

@@ -317,8 +317,8 @@ func TestValidateEmail(t *testing.T) {
 	}{
 		{"an email`", unparseableEmailDetail},
 		{"a@always.invalid", emptyDNSResponseDetail},
-		{"a@always.timeout", "DNS query timed out"},
+		{"a@always.timeout", "DNS query timed out during A-record lookup of always.timeout"},
-		{"a@always.error", "DNS networking error"},
+		{"a@always.error", "DNS networking error during A-record lookup of always.error"},
 	}
 	testSuccesses := []string{
 		"a@email.com",

va/validation-authority.go

@@ -88,24 +88,24 @@ 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) (addr net.IP, addrs []net.IP, problem *probs.ProblemDetails) {
+func (va ValidationAuthorityImpl) getAddr(hostname string) (net.IP, []net.IP, *probs.ProblemDetails) {
 	addrs, err := va.DNSResolver.LookupHost(hostname)
 	if err != nil {
-		problem = bdns.ProblemDetailsFromDNSError(err)
 		va.log.Debug(fmt.Sprintf("%s DNS failure: %s", hostname, err))
-		return
+		problem := bdns.ProblemDetailsFromDNSError("A", hostname, err)
+		return net.IP{}, nil, problem
 	}
 
 	if len(addrs) == 0 {
-		problem = &probs.ProblemDetails{
+		problem := &probs.ProblemDetails{
 			Type:   probs.UnknownHostProblem,
 			Detail: fmt.Sprintf("No IPv4 addresses found for %s", hostname),
 		}
-		return
+		return net.IP{}, nil, problem
 	}
-	addr = addrs[0]
+	addr := addrs[0]
 	va.log.Info(fmt.Sprintf("Resolved addresses for %s [using %s]: %s", hostname, addr, addrs))
-	return
+	return addr, addrs, nil
 }
 
 type dialer struct {
@@ -432,7 +432,8 @@ func (va *ValidationAuthorityImpl) validateDNS01(identifier core.AcmeIdentifier,
 	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)
+
+		return nil, bdns.ProblemDetailsFromDNSError("TXT", challengeSubdomain, err)
 	}
 
 	for _, element := range txts {
@@ -456,14 +457,14 @@ func (va *ValidationAuthorityImpl) checkCAA(identifier core.AcmeIdentifier, regI
 	present, valid, err := va.CheckCAARecords(identifier)
 	if err != nil {
 		va.log.Warning(fmt.Sprintf("Problem checking CAA: %s", err))
-		return bdns.ProblemDetailsFromDNSError(err)
+		return bdns.ProblemDetailsFromDNSError("CAA", identifier.Value, err)
 	}
 	// AUDIT[ Certificate Requests ] 11917fa4-10ef-4e0d-9105-bacbe7836a3c
 	va.log.Audit(fmt.Sprintf("Checked CAA records for %s, registration ID %d [Present: %t, Valid for issuance: %t]", identifier.Value, regID, present, valid))
 	if !valid {
 		return &probs.ProblemDetails{
 			Type:   probs.ConnectionProblem,
-			Detail: "CAA check for identifier failed",
+			Detail: fmt.Sprintf("CAA check for %s failed", identifier.Value),
 		}
 	}
 	return nil

va/validation-authority_test.go

@@ -655,9 +655,9 @@ func TestCAATimeout(t *testing.T) {
 	if err.Type != probs.ConnectionProblem {
 		t.Errorf("Expected timeout error type %s, got %s", probs.ConnectionProblem, err.Type)
 	}
-	expected := "DNS query timed out"
+	expected := "DNS query timed out during CAA-record lookup of caa-timeout.com"
 	if err.Detail != expected {
-		t.Errorf("checkCAA: got %s, expected %s", err.Detail, expected)
+		t.Errorf("checkCAA: got %#v, expected %#v", err.Detail, expected)
 	}
 }