Review fixes

This commit is contained in:
Roland Shoemaker 2015-07-29 12:09:05 -07:00
parent 8a577df190
commit ec3aa67f4e
2 changed files with 57 additions and 42 deletions

View File

@ -56,8 +56,8 @@ type verificationRequestEvent struct {
// problemDetailsFromDNSError checks the error returned from Lookup... // problemDetailsFromDNSError checks the error returned from Lookup...
// methods and tests if the error was an underlying net.OpError or an error // 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 // caused by resolver returning SERVFAIL or other invalid Rcodes and returns
// relevant core.ProblemDetails. // the relevant core.ProblemDetails.
func problemDetailsFromDNSError(err error) *core.ProblemDetails { func problemDetailsFromDNSError(err error) *core.ProblemDetails {
problem := &core.ProblemDetails{Type: core.ConnectionProblem} problem := &core.ProblemDetails{Type: core.ConnectionProblem}
if netErr, ok := err.(*net.OpError); ok { 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)) return defaultDial("tcp", net.JoinHostPort(addrs[0].String(), port))
} }
var redirects []string var redirects []string
addrUsed := addrs[0]
logRedirect := func(req *http.Request, via []*http.Request) error { logRedirect := func(req *http.Request, via []*http.Request) error {
redirects = append(redirects, req.URL.String()) 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) addrs, problem := va.getAddrs(req.URL.Host)
if problem != nil { if problem != nil {
return problem 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...) challenge.ResolvedAddrs = append(challenge.ResolvedAddrs, addrs...)
redirectPort := "80" redirectPort := "80"
if va.TestMode { if va.TestMode {
@ -222,8 +224,8 @@ func (va ValidationAuthorityImpl) validateSimpleHTTP(identifier core.AcmeIdentif
challenge.Status = core.StatusInvalid challenge.Status = core.StatusInvalid
challenge.Error = &core.ProblemDetails{ challenge.Error = &core.ProblemDetails{
Type: core.UnauthorizedProblem, Type: core.UnauthorizedProblem,
Detail: fmt.Sprintf("Invalid response from %s: %d", Detail: fmt.Sprintf("Invalid response from %s [%s]: %d",
url.String(), httpResponse.StatusCode), url.String(), addrUsed, httpResponse.StatusCode),
} }
err = challenge.Error err = challenge.Error
} }

View File

@ -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") t.Logf("SIMPLESRV: Got a wait-long req\n")
time.Sleep(time.Second * 10) time.Sleep(time.Second * 10)
} else if strings.HasSuffix(r.URL.Path, "re-lookup") { } 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) http.Redirect(w, r, "http://localhost2/path", 302)
} else if strings.HasSuffix(r.URL.Path, "re-lookup-invalid") { } 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) http.Redirect(w, r, "http://invalid.super/path", 302)
} else { } else {
t.Logf("SIMPLESRV: Got a valid req\n") t.Logf("SIMPLESRV: Got a valid req\n")
@ -265,41 +265,6 @@ func TestSimpleHttp(t *testing.T) {
test.AssertNotError(t, err, chall.Path) test.AssertNotError(t, err, chall.Path)
test.AssertEquals(t, len(log.GetAllMatching(`^\[AUDIT\] `)), 1) 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() log.Clear()
chall.Path = path404 chall.Path = path404
invalidChall, err = va.validateSimpleHTTP(ident, chall) invalidChall, err = va.validateSimpleHTTP(ident, chall)
@ -346,6 +311,54 @@ func TestSimpleHttp(t *testing.T) {
test.AssertEquals(t, invalidChall.Error.Type, core.ConnectionProblem) 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) { func TestDvsni(t *testing.T) {
va := NewValidationAuthorityImpl(true) va := NewValidationAuthorityImpl(true)
va.DNSResolver = &mocks.MockDNS{} va.DNSResolver = &mocks.MockDNS{}