diff --git a/va/va.go b/va/va.go index 832aca8a2..675749f7b 100644 --- a/va/va.go +++ b/va/va.go @@ -338,8 +338,7 @@ func (va *ValidationAuthorityImpl) fetchHTTP(ctx context.Context, identifier cor validationRecords = append(validationRecords, dialer.record) if err != nil { va.log.Info(fmt.Sprintf("HTTP request to %s failed. err=[%#v] errStr=[%s]", url, err, err)) - return nil, validationRecords, - parseHTTPConnError(fmt.Sprintf("Could not connect to %s", urlHost), err) + return nil, validationRecords, detailedError(err) } body, err := ioutil.ReadAll(&io.LimitedReader{R: httpResponse.Body, N: maxResponseSize}) @@ -521,8 +520,7 @@ func (va *ValidationAuthorityImpl) getTLSSNICerts(hostPort string, identifier co if err != nil { va.log.Info(fmt.Sprintf("%s connection failure for %s. err=[%#v] errStr=[%s]", challenge.Type, identifier, err, err)) - return nil, - parseHTTPConnError(fmt.Sprintf("Failed to connect to %s for %s challenge", hostPort, challenge.Type), err) + return nil, detailedError(err) } // close errors are not important here defer func() { @@ -607,30 +605,34 @@ func (va *ValidationAuthorityImpl) validateTLSSNI02(ctx context.Context, identif // we try to talk TLS to a server that only talks HTTP var badTLSHeader = []byte{0x48, 0x54, 0x54, 0x50, 0x2f} -// parseHTTPConnError returns a ProblemDetails corresponding to an error -// that occurred during domain validation. -func parseHTTPConnError(detail string, err error) *probs.ProblemDetails { +// detailedError returns a ProblemDetails corresponding to an error +// that occurred during HTTP-01 or TLS-SNI domain validation. Specifically it +// tries to unwrap known Go error types and present something a little more +// meaningful. +func detailedError(err error) *probs.ProblemDetails { + // net/http wraps net.OpError in a url.Error. Unwrap them. if urlErr, ok := err.(*url.Error); ok { - err = urlErr.Err + prob := detailedError(urlErr.Err) + prob.Detail = fmt.Sprintf("Fetching %s: %s", urlErr.URL, prob.Detail) + return prob } if tlsErr, ok := err.(tls.RecordHeaderError); ok && bytes.Compare(tlsErr.RecordHeader[:], badTLSHeader) == 0 { - return probs.Malformed(fmt.Sprintf("%s: Server only speaks HTTP, not TLS", detail)) + return probs.Malformed(fmt.Sprintf("Server only speaks HTTP, not TLS")) } - // XXX: On all of the resolvers I tested that validate DNSSEC, there is - // no differentiation between a DNSSEC failure and an unknown host. If we - // do not verify DNSSEC ourselves, this function should be modified. if netErr, ok := err.(*net.OpError); ok { - dnsErr, ok := netErr.Err.(*net.DNSError) - if ok && !dnsErr.Timeout() && !dnsErr.Temporary() { - return probs.UnknownHost(detail) - } else if fmt.Sprintf("%T", netErr.Err) == "tls.alert" { - return probs.TLSError(detail) + if fmt.Sprintf("%T", netErr.Err) == "tls.alert" { + // As of Go 1.8, all the tls.alert error strings are reasonable to hand back to a + // user. + return probs.TLSError(netErr.Error()) } } + if err, ok := err.(net.Error); ok && err.Timeout() { + return probs.ConnectionFailure("Timeout") + } - return probs.ConnectionFailure(detail) + return probs.ConnectionFailure("Error getting validation data") } func (va *ValidationAuthorityImpl) validateDNS01(ctx context.Context, identifier core.AcmeIdentifier, challenge core.Challenge) ([]core.ValidationRecord, *probs.ProblemDetails) { diff --git a/va/va_test.go b/va/va_test.go index 94bf86c9c..3259bd873 100644 --- a/va/va_test.go +++ b/va/va_test.go @@ -17,6 +17,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "regexp" "strconv" "strings" "testing" @@ -316,10 +317,20 @@ func TestHTTP(t *testing.T) { t.Fatalf("Domain name is invalid.") } test.AssertEquals(t, prob.Type, probs.UnknownHostProblem) +} + +func TestHTTPTimeout(t *testing.T) { + chall := core.HTTPChallenge01() + setChallengeToken(&chall, expectedToken) + + hs := httpSrv(t, chall.Token) + // TODO(#1989): close hs + + va, _ := setup(hs) setChallengeToken(&chall, pathWaitLong) started := time.Now() - _, prob = va.validateHTTP01(ctx, ident, chall) + _, prob := va.validateHTTP01(ctx, 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") @@ -328,6 +339,12 @@ func TestHTTP(t *testing.T) { t.Fatalf("Connection should've timed out") } test.AssertEquals(t, prob.Type, probs.ConnectionProblem) + expectMatch := regexp.MustCompile( + "Fetching http://localhost:\\d+/.well-known/acme-challenge/wait-long: Timeout") + if !expectMatch.MatchString(prob.Detail) { + t.Errorf("Problem details incorrect. Got %q, expected to match %q", + prob.Detail, expectMatch) + } } func TestHTTPRedirectLookup(t *testing.T) { @@ -388,7 +405,7 @@ func TestHTTPRedirectLookup(t *testing.T) { setChallengeToken(&chall, pathRedirectToFailingURL) _, prob = va.validateHTTP01(ctx, ident, chall) test.AssertNotNil(t, prob, "Problem Details should not be nil") - test.AssertEquals(t, prob.Detail, "Could not connect to other.valid") + test.AssertEquals(t, prob.Detail, "Fetching http://other.valid/500: Error getting validation data") } func TestHTTPRedirectLoop(t *testing.T) { @@ -609,7 +626,10 @@ func TestTLSError(t *testing.T) { if prob == nil { t.Fatalf("TLS validation should have failed: What cert was used?") } - test.AssertEquals(t, prob.Type, probs.TLSProblem) + if prob.Type != probs.TLSProblem { + t.Errorf("Wrong problem type: got %s, expected type %s", + prob, probs.TLSProblem) + } } // misconfiguredTLSSrv is a TLS HTTP test server that returns a certificate