letsencrypt
/
boulder
mirror of https://github.com/letsencrypt/boulder.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #467 from tomclegg/master 

Dry up "method not allowed" error handling.
This commit is contained in:
Jacob Hoffman-Andrews 2015-07-20 13:31:49 -07:00
parent d41737ecaf ff491962b9
commit 8e94f9feb1
3 changed files with 172 additions and 168 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
test/test-tools.go
View File

@ -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)

215
wfe/web-front-end.go
View File

@ -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(`<html>
<body>
This is an <a href="https://github.com/letsencrypt/acme-spec/">ACME</a>
@ -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())

123
wfe/web-front-end_test.go
View File

@ -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"),