diff --git a/test/test-tools.go b/test/test-tools.go index 8229c137f..fd050bfa3 100644 --- a/test/test-tools.go +++ b/test/test-tools.go @@ -54,7 +54,7 @@ func AssertError(t *testing.T, err error, message string) { } } -// AssertEquals uses the equality operator (=) to measure one and two +// AssertEquals uses the equality operator (==) to measure one and two func AssertEquals(t *testing.T, one interface{}, two interface{}) { if one != two { t.Errorf("%s [%v] != [%v]", caller(), one, two) diff --git a/wfe/web-front-end.go b/wfe/web-front-end.go index 313cc59c2..e3b4a3189 100644 --- a/wfe/web-front-end.go +++ b/wfe/web-front-end.go @@ -121,6 +121,65 @@ func NewWebFrontEndImpl() (WebFrontEndImpl, error) { }, nil } +// BodylessResponseWriter wraps http.ResponseWriter, discarding +// anything written to the body. +type BodylessResponseWriter struct { + http.ResponseWriter +} + +func (mrw BodylessResponseWriter) Write(buf []byte) (int, error) { + return len(buf), nil +} + +// HandleFunc registers a handler at the given path. It's +// http.HandleFunc(), but with a wrapper around the handler that +// provides some generic per-request functionality: +// +// * Set a Replay-Nonce header. +// +// * Respond http.StatusMethodNotAllowed for HTTP methods other than +// those listed. +// +// * Never send a body in response to a HEAD request. (Anything +// written by the handler will be discarded if the method is HEAD.) +func (wfe *WebFrontEndImpl) HandleFunc(mux *http.ServeMux, pattern string, h func(http.ResponseWriter, *http.Request), methods ...string) { + methodsOK := make(map[string]bool) + for _, m := range methods { + methodsOK[m] = true + } + mux.HandleFunc(pattern, func(response http.ResponseWriter, request *http.Request) { + // We do not propagate errors here, because (1) they should be + // transient, and (2) they fail closed. + nonce, err := wfe.nonceService.Nonce() + if err == nil { + response.Header().Set("Replay-Nonce", nonce) + } + response.Header().Set("Access-Control-Allow-Origin", "*") + + switch request.Method { + case "HEAD": + // We'll be sending an error anyway, but we + // should still comply with HTTP spec by not + // sending a body. + response = BodylessResponseWriter{response} + case "OPTIONS": + // TODO, #469 + } + + if _, ok := methodsOK[request.Method]; !ok { + logEvent := wfe.populateRequestEvent(request) + defer wfe.logRequestDetails(&logEvent) + logEvent.Error = "Method not allowed" + response.Header().Set("Allow", strings.Join(methods, ", ")) + wfe.sendError(response, logEvent.Error, request.Method, http.StatusMethodNotAllowed) + return + } + + // Call the wrapped handler. + h(response, request) + }) +} + // Handler returns an http.Handler that uses various functions for // various ACME-specified paths. func (wfe *WebFrontEndImpl) Handler() http.Handler { @@ -132,17 +191,17 @@ func (wfe *WebFrontEndImpl) Handler() http.Handler { wfe.CertBase = wfe.BaseURL + CertPath m := http.NewServeMux() - m.HandleFunc("/", wfe.Index) - m.HandleFunc(NewRegPath, wfe.NewRegistration) - m.HandleFunc(NewAuthzPath, wfe.NewAuthorization) - m.HandleFunc(NewCertPath, wfe.NewCertificate) - m.HandleFunc(RegPath, wfe.Registration) - m.HandleFunc(AuthzPath, wfe.Authorization) - m.HandleFunc(CertPath, wfe.Certificate) - m.HandleFunc(RevokeCertPath, wfe.RevokeCertificate) - m.HandleFunc(TermsPath, wfe.Terms) - m.HandleFunc(IssuerPath, wfe.Issuer) - m.HandleFunc(BuildIDPath, wfe.BuildID) + wfe.HandleFunc(m, "/", wfe.Index, "GET") + wfe.HandleFunc(m, NewRegPath, wfe.NewRegistration, "POST") + wfe.HandleFunc(m, NewAuthzPath, wfe.NewAuthorization, "POST") + wfe.HandleFunc(m, NewCertPath, wfe.NewCertificate, "POST") + wfe.HandleFunc(m, RegPath, wfe.Registration, "POST") + wfe.HandleFunc(m, AuthzPath, wfe.Authorization, "GET", "POST") + wfe.HandleFunc(m, CertPath, wfe.Certificate, "GET", "POST") + wfe.HandleFunc(m, RevokeCertPath, wfe.RevokeCertificate, "POST") + wfe.HandleFunc(m, TermsPath, wfe.Terms, "GET") + wfe.HandleFunc(m, IssuerPath, wfe.Issuer, "GET") + wfe.HandleFunc(m, BuildIDPath, wfe.BuildID, "GET") return m } @@ -153,8 +212,6 @@ func (wfe *WebFrontEndImpl) Index(response http.ResponseWriter, request *http.Re logEvent := wfe.populateRequestEvent(request) defer wfe.logRequestDetails(&logEvent) - wfe.sendStandardHeaders(response) - // http://golang.org/pkg/net/http/#example_ServeMux_Handle // The "/" pattern matches everything, so we need to check // that we're at the root here. @@ -164,13 +221,6 @@ func (wfe *WebFrontEndImpl) Index(response http.ResponseWriter, request *http.Re return } - if request.Method != "GET" { - logEvent.Error = "Method not allowed" - sendAllow(response, "GET") - wfe.sendError(response, logEvent.Error, request.Method, http.StatusMethodNotAllowed) - return - } - tmpl := template.Must(template.New("body").Parse(` This is an ACME @@ -189,21 +239,6 @@ func parseIDFromPath(path string) string { return re.ReplaceAllString(path, "") } -func sendAllow(response http.ResponseWriter, methods ...string) { - response.Header().Set("Allow", strings.Join(methods, ", ")) -} - -func (wfe *WebFrontEndImpl) sendStandardHeaders(response http.ResponseWriter) { - // We do not propagate errors here, because (1) they should be - // transient, and (2) they fail closed. - nonce, err := wfe.nonceService.Nonce() - if err == nil { - response.Header().Set("Replay-Nonce", nonce) - } - - response.Header().Set("Access-Control-Allow-Origin", "*") -} - const ( unknownKey = "No registration exists matching provided key" malformedJWS = "Unable to read/verify body" @@ -329,15 +364,6 @@ func (wfe *WebFrontEndImpl) NewRegistration(response http.ResponseWriter, reques logEvent := wfe.populateRequestEvent(request) defer wfe.logRequestDetails(&logEvent) - wfe.sendStandardHeaders(response) - - if request.Method != "POST" { - logEvent.Error = "Method not allowed" - sendAllow(response, "POST") - wfe.sendError(response, logEvent.Error, "", http.StatusMethodNotAllowed) - return - } - body, key, _, err := wfe.verifyPOST(request, false) if err != nil { logEvent.Error = err.Error() @@ -406,15 +432,6 @@ func (wfe *WebFrontEndImpl) NewAuthorization(response http.ResponseWriter, reque logEvent := wfe.populateRequestEvent(request) defer wfe.logRequestDetails(&logEvent) - wfe.sendStandardHeaders(response) - - if request.Method != "POST" { - logEvent.Error = "Method not allowed" - sendAllow(response, "POST") - wfe.sendError(response, logEvent.Error, request.Method, http.StatusMethodNotAllowed) - return - } - body, _, currReg, err := wfe.verifyPOST(request, true) if err != nil { logEvent.Error = err.Error() @@ -484,15 +501,6 @@ func (wfe *WebFrontEndImpl) RevokeCertificate(response http.ResponseWriter, requ logEvent := wfe.populateRequestEvent(request) defer wfe.logRequestDetails(&logEvent) - wfe.sendStandardHeaders(response) - - if request.Method != "POST" { - logEvent.Error = "Method not allowed" - sendAllow(response, "POST") - wfe.sendError(response, logEvent.Error, request.Method, http.StatusMethodNotAllowed) - return - } - // We don't ask verifyPOST to verify there is a correponding registration, // because anyone with the right private key can revoke a certificate. body, requestKey, registration, err := wfe.verifyPOST(request, false) @@ -585,15 +593,6 @@ func (wfe *WebFrontEndImpl) NewCertificate(response http.ResponseWriter, request logEvent := wfe.populateRequestEvent(request) defer wfe.logRequestDetails(&logEvent) - wfe.sendStandardHeaders(response) - - if request.Method != "POST" { - logEvent.Error = "Method not allowed" - sendAllow(response, "POST") - wfe.sendError(response, logEvent.Error, request.Method, http.StatusMethodNotAllowed) - return - } - body, key, reg, err := wfe.verifyPOST(request, true) if err != nil { logEvent.Error = err.Error() @@ -673,15 +672,6 @@ func (wfe *WebFrontEndImpl) NewCertificate(response http.ResponseWriter, request } func (wfe *WebFrontEndImpl) challenge(authz core.Authorization, response http.ResponseWriter, request *http.Request, logEvent requestEvent) requestEvent { - wfe.sendStandardHeaders(response) - - if request.Method != "GET" && request.Method != "POST" { - logEvent.Error = "Method not allowed" - sendAllow(response, "GET", "POST") - wfe.sendError(response, "Method not allowed", request.Method, http.StatusMethodNotAllowed) - return logEvent - } - // Check that the requested challenge exists within the authorization found := false var challengeIndex int @@ -701,12 +691,6 @@ func (wfe *WebFrontEndImpl) challenge(authz core.Authorization, response http.Re } switch request.Method { - default: - logEvent.Error = "Method not allowed" - sendAllow(response, "GET", "POST") - wfe.sendError(response, logEvent.Error, "", http.StatusMethodNotAllowed) - return logEvent - case "GET": challenge := authz.Challenges[challengeIndex] jsonReply, err := json.Marshal(challenge) @@ -810,15 +794,6 @@ func (wfe *WebFrontEndImpl) Registration(response http.ResponseWriter, request * logEvent := wfe.populateRequestEvent(request) defer wfe.logRequestDetails(&logEvent) - wfe.sendStandardHeaders(response) - - if request.Method != "POST" { - logEvent.Error = "Method not allowed" - sendAllow(response, "POST") - wfe.sendError(response, logEvent.Error, request.Method, http.StatusMethodNotAllowed) - return - } - body, _, currReg, err := wfe.verifyPOST(request, true) if err != nil { logEvent.Error = err.Error() @@ -898,15 +873,6 @@ func (wfe *WebFrontEndImpl) Authorization(response http.ResponseWriter, request logEvent := wfe.populateRequestEvent(request) defer wfe.logRequestDetails(&logEvent) - wfe.sendStandardHeaders(response) - - if request.Method != "GET" && request.Method != "POST" { - logEvent.Error = "Method not allowed" - sendAllow(response, "GET", "POST") - wfe.sendError(response, logEvent.Error, request.Method, http.StatusMethodNotAllowed) - return - } - // Requests to this handler should have a path that leads to a known authz id := parseIDFromPath(request.URL.Path) authz, err := wfe.SA.GetAuthorization(id) @@ -929,12 +895,6 @@ func (wfe *WebFrontEndImpl) Authorization(response http.ResponseWriter, request } switch request.Method { - default: - logEvent.Error = "Method not allowed" - sendAllow(response, "GET", "POST") - wfe.sendError(response, logEvent.Error, request.Method, http.StatusMethodNotAllowed) - return - case "GET": // Blank out ID and regID authz.ID = "" @@ -965,14 +925,6 @@ func (wfe *WebFrontEndImpl) Certificate(response http.ResponseWriter, request *h logEvent := wfe.populateRequestEvent(request) defer wfe.logRequestDetails(&logEvent) - wfe.sendStandardHeaders(response) - - if request.Method != "GET" && request.Method != "POST" { - logEvent.Error = "Method not allowed" - sendAllow(response, "GET", "POST") - wfe.sendError(response, logEvent.Error, request.Method, http.StatusMethodNotAllowed) - } - path := request.URL.Path switch request.Method { case "GET": @@ -1025,15 +977,6 @@ func (wfe *WebFrontEndImpl) Terms(response http.ResponseWriter, request *http.Re logEvent := wfe.populateRequestEvent(request) defer wfe.logRequestDetails(&logEvent) - wfe.sendStandardHeaders(response) - - if request.Method != "GET" { - logEvent.Error = "Method not allowed" - sendAllow(response, "GET") - wfe.sendError(response, logEvent.Error, request.Method, http.StatusMethodNotAllowed) - return - } - http.Redirect(response, request, wfe.SubscriberAgreementURL, http.StatusFound) } @@ -1042,15 +985,6 @@ func (wfe *WebFrontEndImpl) Issuer(response http.ResponseWriter, request *http.R logEvent := wfe.populateRequestEvent(request) defer wfe.logRequestDetails(&logEvent) - wfe.sendStandardHeaders(response) - - if request.Method != "GET" { - logEvent.Error = "Method not allowed" - sendAllow(response, "GET") - wfe.sendError(response, "Method not allowed", request.Method, http.StatusMethodNotAllowed) - return - } - // TODO Content negotiation response.Header().Set("Content-Type", "application/pkix-cert") response.WriteHeader(http.StatusOK) @@ -1065,15 +999,6 @@ func (wfe *WebFrontEndImpl) BuildID(response http.ResponseWriter, request *http. logEvent := wfe.populateRequestEvent(request) defer wfe.logRequestDetails(&logEvent) - wfe.sendStandardHeaders(response) - - if request.Method != "GET" { - logEvent.Error = "Method not allowed" - sendAllow(response, "GET") - wfe.sendError(response, "Method not allowed", request.Method, http.StatusMethodNotAllowed) - return - } - response.Header().Set("Content-Type", "text/plain") response.WriteHeader(http.StatusOK) detailsString := fmt.Sprintf("Boulder=(%s %s) Golang=(%s) BuildHost=(%s)", core.GetBuildID(), core.GetBuildTime(), runtime.Version(), core.GetBuildHost()) diff --git a/wfe/web-front-end_test.go b/wfe/web-front-end_test.go index aeaf88832..1087f4f55 100644 --- a/wfe/web-front-end_test.go +++ b/wfe/web-front-end_test.go @@ -18,6 +18,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "sort" "strings" "testing" "time" @@ -331,30 +332,103 @@ func setupWFE(t *testing.T) WebFrontEndImpl { return wfe } +func mustParseURL(s string) *url.URL { + if u, err := url.Parse(s); err != nil { + panic("Cannot parse URL " + s) + } else { + return u + } +} + +func sortHeader(s string) string { + a := strings.Split(s, ", ") + sort.Sort(sort.StringSlice(a)) + return strings.Join(a, ", ") +} + +func TestHandleFunc(t *testing.T) { + wfe := setupWFE(t) + var mux *http.ServeMux + var rw *httptest.ResponseRecorder + var stubCalled bool + runWrappedHandler := func(req *http.Request, allowed ...string) { + mux = http.NewServeMux() + rw = httptest.NewRecorder() + stubCalled = false + wfe.HandleFunc(mux, "/test", func(http.ResponseWriter, *http.Request) { + stubCalled = true + }, allowed...) + req.URL = mustParseURL("/test") + mux.ServeHTTP(rw, req) + } + + // Plain requests (no CORS) + type testCase struct { + allowed []string + reqMethod string + shouldSucceed bool + } + var lastNonce string + for _, c := range []testCase{ + {[]string{"GET", "POST"}, "GET", true}, + {[]string{"GET", "POST"}, "POST", true}, + {[]string{"GET"}, "", false}, + {[]string{"GET"}, "POST", false}, + {[]string{"GET"}, "OPTIONS", false}, // TODO, #469 + {[]string{"GET"}, "MAKE-COFFEE", false}, // 405, or 418? + } { + runWrappedHandler(&http.Request{Method: c.reqMethod}, c.allowed...) + test.AssertEquals(t, stubCalled, c.shouldSucceed) + if c.shouldSucceed { + test.AssertEquals(t, rw.Code, http.StatusOK) + } else { + test.AssertEquals(t, rw.Code, http.StatusMethodNotAllowed) + test.AssertEquals(t, sortHeader(rw.Header().Get("Allow")), strings.Join(c.allowed, ", ")) + test.AssertEquals(t, + rw.Body.String(), + `{"type":"urn:acme:error:malformed","detail":"Method not allowed"}`) + } + nonce := rw.Header().Get("Replay-Nonce") + test.AssertNotEquals(t, nonce, lastNonce) + lastNonce = nonce + } + + // Disallowed method returns error JSON in body + runWrappedHandler(&http.Request{Method: "PUT"}, "GET", "POST") + test.AssertEquals(t, rw.Header().Get("Content-Type"), "application/problem+json") + test.AssertEquals(t, rw.Body.String(), `{"type":"urn:acme:error:malformed","detail":"Method not allowed"}`) + test.AssertEquals(t, sortHeader(rw.Header().Get("Allow")), "GET, POST") + + // Disallowed method special case: response to HEAD has got no body + runWrappedHandler(&http.Request{Method: "HEAD"}, "GET", "POST") + test.AssertEquals(t, stubCalled, false) + test.AssertEquals(t, rw.Body.String(), "") + test.AssertEquals(t, sortHeader(rw.Header().Get("Allow")), "GET, POST") +} + func TestStandardHeaders(t *testing.T) { wfe := setupWFE(t) + mux := wfe.Handler() cases := []struct { path string - handler func(http.ResponseWriter, *http.Request) allowed []string }{ - {"/", wfe.Index, []string{"GET"}}, - {wfe.NewReg, wfe.NewRegistration, []string{"POST"}}, - {wfe.RegBase, wfe.Registration, []string{"POST"}}, - {wfe.NewAuthz, wfe.NewAuthorization, []string{"POST"}}, - {wfe.AuthzBase, wfe.Authorization, []string{"GET", "POST"}}, - {wfe.NewCert, wfe.NewCertificate, []string{"POST"}}, - {wfe.CertBase, wfe.Certificate, []string{"GET", "POST"}}, - {wfe.SubscriberAgreementURL, wfe.Terms, []string{"GET"}}, + {"/", []string{"GET"}}, + {wfe.NewReg, []string{"POST"}}, + {wfe.RegBase, []string{"POST"}}, + {wfe.NewAuthz, []string{"POST"}}, + {wfe.AuthzBase, []string{"GET", "POST"}}, + {wfe.NewCert, []string{"POST"}}, + {wfe.CertBase, []string{"GET", "POST"}}, + {wfe.SubscriberAgreementURL, []string{"GET"}}, } for _, c := range cases { responseWriter := httptest.NewRecorder() - url, _ := url.Parse(c.path) - c.handler(responseWriter, &http.Request{ + mux.ServeHTTP(responseWriter, &http.Request{ Method: "BOGUS", - URL: url, + URL: mustParseURL(c.path), }) acao := responseWriter.Header().Get("Access-Control-Allow-Origin") nonce := responseWriter.Header().Get("Replay-Nonce") @@ -394,6 +468,7 @@ func TestIndex(t *testing.T) { // - RA returns with a failure func TestIssueCertificate(t *testing.T) { wfe := setupWFE(t) + mux := wfe.Handler() // TODO: Use a mock RA so we can test various conditions of authorized, not authorized, etc. ra := ra.NewRegistrationAuthorityImpl() @@ -405,8 +480,9 @@ func TestIssueCertificate(t *testing.T) { responseWriter := httptest.NewRecorder() // GET instead of POST should be rejected - wfe.NewCertificate(responseWriter, &http.Request{ + mux.ServeHTTP(responseWriter, &http.Request{ Method: "GET", + URL: mustParseURL(NewCertPath), }) test.AssertEquals(t, responseWriter.Body.String(), @@ -573,6 +649,7 @@ func TestChallenge(t *testing.T) { func TestNewRegistration(t *testing.T) { wfe := setupWFE(t) + mux := wfe.Handler() wfe.RA = &MockRegistrationAuthority{} wfe.SA = &MockSA{} @@ -581,8 +658,9 @@ func TestNewRegistration(t *testing.T) { responseWriter := httptest.NewRecorder() // GET instead of POST should be rejected - wfe.NewRegistration(responseWriter, &http.Request{ + mux.ServeHTTP(responseWriter, &http.Request{ Method: "GET", + URL: mustParseURL(NewRegPath), }) test.AssertEquals(t, responseWriter.Body.String(), "{\"type\":\"urn:acme:error:malformed\",\"detail\":\"Method not allowed\"}") @@ -815,6 +893,7 @@ func TestRevokeCertificateAlreadyRevoked(t *testing.T) { func TestAuthorization(t *testing.T) { wfe := setupWFE(t) + mux := wfe.Handler() wfe.RA = &MockRegistrationAuthority{} wfe.SA = &MockSA{} @@ -822,8 +901,9 @@ func TestAuthorization(t *testing.T) { responseWriter := httptest.NewRecorder() // GET instead of POST should be rejected - wfe.NewAuthorization(responseWriter, &http.Request{ + mux.ServeHTTP(responseWriter, &http.Request{ Method: "GET", + URL: mustParseURL(NewAuthzPath), }) test.AssertEquals(t, responseWriter.Body.String(), "{\"type\":\"urn:acme:error:malformed\",\"detail\":\"Method not allowed\"}") @@ -898,6 +978,7 @@ func TestAuthorization(t *testing.T) { func TestRegistration(t *testing.T) { wfe := setupWFE(t) + mux := wfe.Handler() wfe.RA = &MockRegistrationAuthority{} wfe.SA = &MockSA{} @@ -906,11 +987,10 @@ func TestRegistration(t *testing.T) { responseWriter := httptest.NewRecorder() // Test invalid method - path, _ := url.Parse("/1") - wfe.Registration(responseWriter, &http.Request{ + mux.ServeHTTP(responseWriter, &http.Request{ Method: "MAKE-COFFEE", + URL: mustParseURL(RegPath), Body: makeBody("invalid"), - URL: path, }) test.AssertEquals(t, responseWriter.Body.String(), @@ -918,10 +998,9 @@ func TestRegistration(t *testing.T) { responseWriter.Body.Reset() // Test GET proper entry returns 405 - path, _ = url.Parse("/1") - wfe.Registration(responseWriter, &http.Request{ + mux.ServeHTTP(responseWriter, &http.Request{ Method: "GET", - URL: path, + URL: mustParseURL(RegPath), }) test.AssertEquals(t, responseWriter.Body.String(), @@ -929,7 +1008,7 @@ func TestRegistration(t *testing.T) { responseWriter.Body.Reset() // Test POST invalid JSON - path, _ = url.Parse("/2") + path, _ := url.Parse("/2") wfe.Registration(responseWriter, &http.Request{ Method: "POST", Body: makeBody("invalid"),