diff --git a/va/validation-authority.go b/va/validation-authority.go index e828ae9b9..bcb2a2890 100644 --- a/va/validation-authority.go +++ b/va/validation-authority.go @@ -56,8 +56,8 @@ type verificationRequestEvent struct { // problemDetailsFromDNSError checks the error returned from Lookup... // methods and tests if the error was an underlying net.OpError or an error -// caused by resolver returning SERVFAIL or other invalid Rcodes returns the -// relevant core.ProblemDetails. +// caused by resolver returning SERVFAIL or other invalid Rcodes and returns +// the relevant core.ProblemDetails. func problemDetailsFromDNSError(err error) *core.ProblemDetails { problem := &core.ProblemDetails{Type: core.ConnectionProblem} if netErr, ok := err.(*net.OpError); ok { @@ -157,13 +157,15 @@ func (va ValidationAuthorityImpl) validateSimpleHTTP(identifier core.AcmeIdentif return defaultDial("tcp", net.JoinHostPort(addrs[0].String(), port)) } var redirects []string + addrUsed := addrs[0] logRedirect := func(req *http.Request, via []*http.Request) error { redirects = append(redirects, req.URL.String()) - va.log.Info(fmt.Sprintf("validateSimpleHTTP [%s] redirect from %q to %q", identifier, via[len(via)-1].URL.String(), req.URL.String())) addrs, problem := va.getAddrs(req.URL.Host) if problem != nil { return problem } + addrUsed = addrs[0] + va.log.Info(fmt.Sprintf("validateSimpleHTTP [%s] redirect from %q to %q [%s]", identifier, via[len(via)-1].URL.String(), req.URL.String(), addrs[0])) challenge.ResolvedAddrs = append(challenge.ResolvedAddrs, addrs...) redirectPort := "80" if va.TestMode { @@ -222,8 +224,8 @@ func (va ValidationAuthorityImpl) validateSimpleHTTP(identifier core.AcmeIdentif challenge.Status = core.StatusInvalid challenge.Error = &core.ProblemDetails{ Type: core.UnauthorizedProblem, - Detail: fmt.Sprintf("Invalid response from %s: %d", - url.String(), httpResponse.StatusCode), + Detail: fmt.Sprintf("Invalid response from %s [%s]: %d", + url.String(), addrUsed, httpResponse.StatusCode), } err = challenge.Error } diff --git a/va/validation-authority_test.go b/va/validation-authority_test.go index 451addad0..98c3ad67a 100644 --- a/va/validation-authority_test.go +++ b/va/validation-authority_test.go @@ -88,10 +88,10 @@ func simpleSrv(t *testing.T, token string, stopChan, waitChan chan bool, enableT t.Logf("SIMPLESRV: Got a wait-long req\n") time.Sleep(time.Second * 10) } else if strings.HasSuffix(r.URL.Path, "re-lookup") { - t.Logf("SIMPLESRV: Got a redirect to invalid lookup req\n") + t.Logf("SIMPLESRV: Got a redirect req to a valid hostname\n") http.Redirect(w, r, "http://localhost2/path", 302) } else if strings.HasSuffix(r.URL.Path, "re-lookup-invalid") { - t.Logf("SIMPLESRV: Got a redirect to invalid lookup req\n") + t.Logf("SIMPLESRV: Got a redirect req to a invalid hostname\n") http.Redirect(w, r, "http://invalid.super/path", 302) } else { t.Logf("SIMPLESRV: Got a valid req\n") @@ -265,41 +265,6 @@ func TestSimpleHttp(t *testing.T) { test.AssertNotError(t, err, chall.Path) test.AssertEquals(t, len(log.GetAllMatching(`^\[AUDIT\] `)), 1) - log.Clear() - chall.Path = pathMoved - finChall, err = va.validateSimpleHTTP(ident, chall) - test.AssertEquals(t, finChall.Status, core.StatusValid) - test.AssertNotError(t, err, chall.Path) - test.AssertEquals(t, len(log.GetAllMatching(`redirect from ".*/301" to ".*/valid"`)), 1) - test.AssertEquals(t, len(log.GetAllMatching(`Resolved addresses for localhost: \[127.0.0.1\]`)), 2) - - log.Clear() - chall.Path = pathFound - finChall, err = va.validateSimpleHTTP(ident, chall) - test.AssertEquals(t, finChall.Status, core.StatusValid) - test.AssertNotError(t, err, chall.Path) - test.AssertEquals(t, len(log.GetAllMatching(`redirect from ".*/302" to ".*/301"`)), 1) - test.AssertEquals(t, len(log.GetAllMatching(`redirect from ".*/301" to ".*/valid"`)), 1) - test.AssertEquals(t, len(log.GetAllMatching(`Resolved addresses for localhost: \[127.0.0.1\]`)), 3) - - log.Clear() - chall.Path = pathRedirectLookupInvalid - finChall, err = va.validateSimpleHTTP(ident, chall) - test.AssertEquals(t, finChall.Status, core.StatusInvalid) - test.AssertError(t, err, chall.Path) - test.AssertEquals(t, len(log.GetAllMatching(`redirect from ".*/re-lookup-invalid" to ".*invalid.super/path"`)), 1) - test.AssertEquals(t, len(log.GetAllMatching(`Resolved addresses for localhost: \[127.0.0.1\]`)), 1) - test.AssertEquals(t, len(log.GetAllMatching(`Could not resolve invalid.super`)), 1) - - log.Clear() - chall.Path = pathRedirectLookup - finChall, err = va.validateSimpleHTTP(ident, chall) - test.AssertEquals(t, finChall.Status, core.StatusValid) - test.AssertNotError(t, err, chall.Path) - test.AssertEquals(t, len(log.GetAllMatching(`redirect from ".*/re-lookup" to ".*localhost2/path"`)), 1) - test.AssertEquals(t, len(log.GetAllMatching(`Resolved addresses for localhost: \[127.0.0.1\]`)), 1) - test.AssertEquals(t, len(log.GetAllMatching(`Resolved addresses for localhost2: \[127.0.0.1\]`)), 1) - log.Clear() chall.Path = path404 invalidChall, err = va.validateSimpleHTTP(ident, chall) @@ -346,6 +311,54 @@ func TestSimpleHttp(t *testing.T) { test.AssertEquals(t, invalidChall.Error.Type, core.ConnectionProblem) } +func TestSimpleHttpRedirectLookup(t *testing.T) { + va := NewValidationAuthorityImpl(true) + va.DNSResolver = &mocks.MockDNS{} + + tls := false + chall := core.Challenge{Token: expectedToken, TLS: &tls} + + stopChan := make(chan bool, 1) + waitChan := make(chan bool, 1) + go simpleSrv(t, expectedToken, stopChan, waitChan, tls) + defer func() { stopChan <- true }() + <-waitChan + + log.Clear() + chall.Path = pathMoved + finChall, err := va.validateSimpleHTTP(ident, chall) + test.AssertEquals(t, finChall.Status, core.StatusValid) + test.AssertNotError(t, err, chall.Path) + test.AssertEquals(t, len(log.GetAllMatching(`redirect from ".*/301" to ".*/valid"`)), 1) + test.AssertEquals(t, len(log.GetAllMatching(`Resolved addresses for localhost: \[127.0.0.1\]`)), 2) + + log.Clear() + chall.Path = pathFound + finChall, err = va.validateSimpleHTTP(ident, chall) + test.AssertEquals(t, finChall.Status, core.StatusValid) + test.AssertNotError(t, err, chall.Path) + test.AssertEquals(t, len(log.GetAllMatching(`redirect from ".*/302" to ".*/301"`)), 1) + test.AssertEquals(t, len(log.GetAllMatching(`redirect from ".*/301" to ".*/valid"`)), 1) + test.AssertEquals(t, len(log.GetAllMatching(`Resolved addresses for localhost: \[127.0.0.1\]`)), 3) + + log.Clear() + chall.Path = pathRedirectLookupInvalid + finChall, err = va.validateSimpleHTTP(ident, chall) + test.AssertEquals(t, finChall.Status, core.StatusInvalid) + test.AssertError(t, err, chall.Path) + test.AssertEquals(t, len(log.GetAllMatching(`Resolved addresses for localhost: \[127.0.0.1\]`)), 1) + test.AssertEquals(t, len(log.GetAllMatching(`Could not resolve invalid.super`)), 1) + + log.Clear() + chall.Path = pathRedirectLookup + finChall, err = va.validateSimpleHTTP(ident, chall) + test.AssertEquals(t, finChall.Status, core.StatusValid) + test.AssertNotError(t, err, chall.Path) + test.AssertEquals(t, len(log.GetAllMatching(`redirect from ".*/re-lookup" to ".*localhost2/path"`)), 1) + test.AssertEquals(t, len(log.GetAllMatching(`Resolved addresses for localhost: \[127.0.0.1\]`)), 1) + test.AssertEquals(t, len(log.GetAllMatching(`Resolved addresses for localhost2: \[127.0.0.1\]`)), 1) +} + func TestDvsni(t *testing.T) { va := NewValidationAuthorityImpl(true) va.DNSResolver = &mocks.MockDNS{}