From b6a4b66899a744f8a30e40a28396deb4dcfe18cf Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Fri, 11 Sep 2015 23:13:52 -0400 Subject: [PATCH 1/9] Fix CORS headers, support OPTIONS requests. --- wfe/web-front-end.go | 63 ++++++++++++++++-- wfe/web-front-end_test.go | 130 ++++++++++++++++++++++++++------------ 2 files changed, 148 insertions(+), 45 deletions(-) diff --git a/wfe/web-front-end.go b/wfe/web-front-end.go index 23fa52a11..9dfa4e79f 100644 --- a/wfe/web-front-end.go +++ b/wfe/web-front-end.go @@ -146,11 +146,15 @@ func (mrw BodylessResponseWriter) Write(buf []byte) (int, error) { // // * Set a Replay-Nonce header. // +// * Respond to OPTIONS requests, including CORS preflight requests. +// // * Respond http.StatusMethodNotAllowed for HTTP methods other than -// those listed. +// those listed. +// +// * Set CORS headers when responding to CORS "actual" requests. // // * Never send a body in response to a HEAD request. (Anything -// written by the handler will be discarded if the method is HEAD.) +// 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 { @@ -163,7 +167,6 @@ func (wfe *WebFrontEndImpl) HandleFunc(mux *http.ServeMux, pattern string, h fun if err == nil { response.Header().Set("Replay-Nonce", nonce) } - response.Header().Set("Access-Control-Allow-Origin", "*") switch request.Method { case "HEAD": @@ -172,7 +175,8 @@ func (wfe *WebFrontEndImpl) HandleFunc(mux *http.ServeMux, pattern string, h fun // sending a body. response = BodylessResponseWriter{response} case "OPTIONS": - // TODO, #469 + wfe.Options(response, request, methodsOK) + return } if _, ok := methodsOK[request.Method]; !ok { @@ -184,6 +188,8 @@ func (wfe *WebFrontEndImpl) HandleFunc(mux *http.ServeMux, pattern string, h fun return } + wfe.setCORSHeaders(response, request, strings.Join(methods, ", ")) + // Call the wrapped handler. h(response, request) }) @@ -1182,6 +1188,55 @@ func (wfe *WebFrontEndImpl) BuildID(response http.ResponseWriter, request *http. } } +// Options responds to an HTTP OPTIONS request. +func (wfe *WebFrontEndImpl) Options(response http.ResponseWriter, request *http.Request, methodsOK map[string]bool) { + allowMethods := "" + for method := range methodsOK { + if allowMethods != "" { + allowMethods += ", " + } + allowMethods += method + } + + // Every OPTIONS request gets an Allow header with a list of supported methods. + response.Header().Set("Allow", allowMethods) + + // CORS preflight requests get additional headers. See + // http://www.w3.org/TR/cors/#resource-preflight-requests + reqMethod := request.Header.Get("Access-Control-Request-Method") + if reqMethod == "" { + reqMethod = "GET" + } + if _, ok := methodsOK[reqMethod]; ok { + wfe.setCORSHeaders(response, request, allowMethods) + } +} + +// setCORSHeaders() tells the client that CORS is acceptable for this +// request. If allowMethods == "" the request is assumed to be a CORS +// actual request and no Access-Control-Allow-Methods or -Headers +// headers will be sent. +func (wfe *WebFrontEndImpl) setCORSHeaders(response http.ResponseWriter, request *http.Request, allowMethods string) { + if request.Header.Get("Origin") == "" { + // This is not a CORS request. + return + } + if allowMethods != "" { + // For an OPTIONS request: allow all methods handled at this URL. + response.Header().Set("Access-Control-Allow-Methods", allowMethods) + + // Allow all requested headers. + if acrh, ok := request.Header["Access-Control-Request-Headers"]; ok { + for _, h := range acrh { + response.Header().Add("Access-Control-Allow-Headers", h) + } + } + } + response.Header().Set("Access-Control-Allow-Origin", "*") + response.Header().Set("Access-Control-Expose-Headers", "Link, Replay-Nonce") + response.Header().Set("Access-Control-Max-Age", "86400") +} + func (wfe *WebFrontEndImpl) logRequestDetails(logEvent *requestEvent) { logEvent.ResponseTime = time.Now() var msg string diff --git a/wfe/web-front-end_test.go b/wfe/web-front-end_test.go index b65abf259..163080d93 100644 --- a/wfe/web-front-end_test.go +++ b/wfe/web-front-end_test.go @@ -421,21 +421,22 @@ func TestHandleFunc(t *testing.T) { // Plain requests (no CORS) type testCase struct { - allowed []string - reqMethod string - shouldSucceed bool + allowed []string + reqMethod string + shouldCallStub bool + 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? + {[]string{"GET", "POST"}, "GET", true, true}, + {[]string{"GET", "POST"}, "POST", true, true}, + {[]string{"GET"}, "", false, false}, + {[]string{"GET"}, "POST", false, false}, + {[]string{"GET"}, "OPTIONS", false, true}, + {[]string{"GET"}, "MAKE-COFFEE", false, false}, // 405, or 418? } { runWrappedHandler(&http.Request{Method: c.reqMethod}, c.allowed...) - test.AssertEquals(t, stubCalled, c.shouldSucceed) + test.AssertEquals(t, stubCalled, c.shouldCallStub) if c.shouldSucceed { test.AssertEquals(t, rw.Code, http.StatusOK) } else { @@ -447,6 +448,7 @@ func TestHandleFunc(t *testing.T) { } nonce := rw.Header().Get("Replay-Nonce") test.AssertNotEquals(t, nonce, lastNonce) + test.AssertNotEquals(t, nonce, "") lastNonce = nonce } @@ -461,40 +463,86 @@ func TestHandleFunc(t *testing.T) { 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, err := wfe.Handler() - test.AssertNotError(t, err, "Problem setting up HTTP handlers") + // CORS "actual" request for disallowed method + runWrappedHandler(&http.Request{ + Method: "POST", + Header: map[string][]string{ + "Origin": {"http://example.com"}, + }, + }, "GET") + test.AssertEquals(t, stubCalled, false) + test.AssertEquals(t, rw.Code, http.StatusMethodNotAllowed) - cases := []struct { - path string - allowed []string - }{ - {wfe.NewReg, []string{"POST"}}, - {wfe.RegBase, []string{"POST"}}, - {wfe.NewAuthz, []string{"POST"}}, - {wfe.AuthzBase, []string{"GET"}}, - {wfe.ChallengeBase, []string{"GET", "POST"}}, - {wfe.NewCert, []string{"POST"}}, - {wfe.CertBase, []string{"GET"}}, - {wfe.SubscriberAgreementURL, []string{"GET"}}, - } + // CORS "actual" request for allowed method + runWrappedHandler(&http.Request{ + Method: "GET", + Header: map[string][]string{ + "Origin": {"http://example.com"}, + }, + }, "GET", "POST") + test.AssertEquals(t, stubCalled, true) + test.AssertEquals(t, rw.Code, http.StatusOK) + test.AssertEquals(t, rw.Header().Get("Access-Control-Allow-Origin"), "*") + test.AssertEquals(t, sortHeader(rw.Header().Get("Access-Control-Expose-Headers")), "Link, Replay-Nonce") - for _, c := range cases { - responseWriter := httptest.NewRecorder() - mux.ServeHTTP(responseWriter, &http.Request{ - Method: "BOGUS", - URL: mustParseURL(c.path), - }) - acao := responseWriter.Header().Get("Access-Control-Allow-Origin") - nonce := responseWriter.Header().Get("Replay-Nonce") - allow := responseWriter.Header().Get("Allow") - test.Assert(t, responseWriter.Code == http.StatusMethodNotAllowed, "Bogus method allowed") - test.Assert(t, acao == "*", "Bad CORS header") - test.Assert(t, len(nonce) > 0, "Bad Replay-Nonce header") - test.Assert(t, len(allow) > 0 && allow == strings.Join(c.allowed, ", "), "Bad Allow header") + // CORS preflight request for disallowed method + runWrappedHandler(&http.Request{ + Method: "OPTIONS", + Header: map[string][]string{ + "Origin": {"http://example.com"}, + "Access-Control-Request-Method": {"POST"}, + }, + }, "GET") + test.AssertEquals(t, stubCalled, false) + test.AssertEquals(t, rw.Code, http.StatusOK) + test.AssertEquals(t, rw.Header().Get("Allow"), "GET") + test.AssertEquals(t, rw.Header().Get("Access-Control-Allow-Origin"), "") + + // CORS preflight request for allowed method + runWrappedHandler(&http.Request{ + Method: "OPTIONS", + Header: map[string][]string{ + "Origin": {"http://example.com"}, + "Access-Control-Request-Method": {"POST"}, + "Access-Control-Request-Headers": {"X-Accept-Header1, X-Accept-Header2", "X-Accept-Header3"}, + }, + }, "GET", "POST") + test.AssertEquals(t, rw.Code, http.StatusOK) + test.AssertEquals(t, rw.Header().Get("Access-Control-Allow-Origin"), "*") + test.AssertEquals(t, rw.Header().Get("Access-Control-Max-Age"), "86400") + test.AssertEquals(t, sortHeader(rw.Header().Get("Access-Control-Allow-Methods")), "GET, POST") + test.AssertDeepEquals(t, rw.Header()["Access-Control-Allow-Headers"], []string{"X-Accept-Header1, X-Accept-Header2", "X-Accept-Header3"}) + test.AssertEquals(t, sortHeader(rw.Header().Get("Access-Control-Expose-Headers")), "Link, Replay-Nonce") + + // OPTIONS request without an Origin header (i.e., not a CORS + // preflight request) + runWrappedHandler(&http.Request{ + Method: "OPTIONS", + Header: map[string][]string{ + "Access-Control-Request-Method": {"POST"}, + }, + }, "GET", "POST") + test.AssertEquals(t, rw.Code, http.StatusOK) + test.AssertEquals(t, rw.Header().Get("Access-Control-Allow-Origin"), "") + test.AssertEquals(t, sortHeader(rw.Header().Get("Allow")), "GET, POST") + + // CORS preflight request missing optional Request-Method + // header. The "actual" request will be GET. + for _, allowedMethod := range []string{"GET", "POST"} { + runWrappedHandler(&http.Request{ + Method: "OPTIONS", + Header: map[string][]string{ + "Origin": {"http://example.com"}, + }, + }, allowedMethod) + test.AssertEquals(t, rw.Code, http.StatusOK) + if allowedMethod == "GET" { + test.AssertEquals(t, rw.Header().Get("Access-Control-Allow-Origin"), "*") + test.AssertEquals(t, rw.Header().Get("Access-Control-Allow-Methods"), "GET") + } else { + test.AssertEquals(t, rw.Header().Get("Access-Control-Allow-Origin"), "") + } } } From 62f7e6e53018ef45cc8d543ee18b530d55929cea Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Sat, 12 Sep 2015 01:39:33 -0400 Subject: [PATCH 2/9] Add config entry for allowed origins. --- cmd/boulder-wfe/main.go | 2 + cmd/shell.go | 2 + test/boulder-config.json | 1 + test/boulder-pkcs11-example-config.json | 1 + wfe/web-front-end.go | 28 +++++++++++- wfe/web-front-end_test.go | 57 ++++++++++++++++++++++--- 6 files changed, 84 insertions(+), 7 deletions(-) diff --git a/cmd/boulder-wfe/main.go b/cmd/boulder-wfe/main.go index 69ffc1c4b..689022ffd 100644 --- a/cmd/boulder-wfe/main.go +++ b/cmd/boulder-wfe/main.go @@ -112,6 +112,8 @@ func main() { wfe.Stats = stats wfe.SubscriberAgreementURL = c.SubscriberAgreementURL + wfe.AllowOrigins = c.WFE.AllowOrigins + wfe.CertCacheDuration, err = time.ParseDuration(c.WFE.CertCacheDuration) cmd.FailOnError(err, "Couldn't parse certificate caching duration") wfe.CertNoCacheExpirationWindow, err = time.ParseDuration(c.WFE.CertNoCacheExpirationWindow) diff --git a/cmd/shell.go b/cmd/shell.go index 5f50d6e09..fb10fc10d 100644 --- a/cmd/shell.go +++ b/cmd/shell.go @@ -70,6 +70,8 @@ type Config struct { BaseURL string ListenAddress string + AllowOrigins []string + CertCacheDuration string CertNoCacheExpirationWindow string IndexCacheDuration string diff --git a/test/boulder-config.json b/test/boulder-config.json index 81a207a37..0d8a88904 100644 --- a/test/boulder-config.json +++ b/test/boulder-config.json @@ -38,6 +38,7 @@ "wfe": { "listenAddress": "127.0.0.1:4000", + "allowOrigins": ["*"], "certCacheDuration": "6h", "certNoCacheExpirationWindow": "96h", "indexCacheDuration": "24h", diff --git a/test/boulder-pkcs11-example-config.json b/test/boulder-pkcs11-example-config.json index 5538d1851..c253eec6d 100644 --- a/test/boulder-pkcs11-example-config.json +++ b/test/boulder-pkcs11-example-config.json @@ -32,6 +32,7 @@ "wfe": { "listenAddress": "127.0.0.1:4000", + "allowOrigins": ["*"], "certCacheDuration": "6h", "certNoCacheExpirationWindow": "96h", "indexCacheDuration": "24h", diff --git a/wfe/web-front-end.go b/wfe/web-front-end.go index 9dfa4e79f..5841a440d 100644 --- a/wfe/web-front-end.go +++ b/wfe/web-front-end.go @@ -73,6 +73,9 @@ type WebFrontEndImpl struct { CertNoCacheExpirationWindow time.Duration IndexCacheDuration time.Duration IssuerCacheDuration time.Duration + + // CORS settings + AllowOrigins []string } func statusCodeFromError(err interface{}) int { @@ -1217,10 +1220,32 @@ func (wfe *WebFrontEndImpl) Options(response http.ResponseWriter, request *http. // actual request and no Access-Control-Allow-Methods or -Headers // headers will be sent. func (wfe *WebFrontEndImpl) setCORSHeaders(response http.ResponseWriter, request *http.Request, allowMethods string) { - if request.Header.Get("Origin") == "" { + reqOrigin := request.Header.Get("Origin") + if reqOrigin == "" { // This is not a CORS request. return } + + // Allow CORS if the current origin (or "*") is listed as an + // allowed origin in config. Otherwise, disallow by returning + // without setting any CORS headers. + allow := false + for _, ao := range wfe.AllowOrigins { + if ao == "*" { + response.Header().Set("Access-Control-Allow-Origin", "*") + allow = true + break + } else if ao == reqOrigin { + response.Header().Set("Vary", "Origin") + response.Header().Set("Access-Control-Allow-Origin", ao) + allow = true + break + } + } + if !allow { + return + } + if allowMethods != "" { // For an OPTIONS request: allow all methods handled at this URL. response.Header().Set("Access-Control-Allow-Methods", allowMethods) @@ -1232,7 +1257,6 @@ func (wfe *WebFrontEndImpl) setCORSHeaders(response http.ResponseWriter, request } } } - response.Header().Set("Access-Control-Allow-Origin", "*") response.Header().Set("Access-Control-Expose-Headers", "Link, Replay-Nonce") response.Header().Set("Access-Control-Max-Age", "86400") } diff --git a/wfe/web-front-end_test.go b/wfe/web-front-end_test.go index 163080d93..bfcd87a2f 100644 --- a/wfe/web-front-end_test.go +++ b/wfe/web-front-end_test.go @@ -464,11 +464,14 @@ func TestHandleFunc(t *testing.T) { test.AssertEquals(t, rw.Body.String(), "") test.AssertEquals(t, sortHeader(rw.Header().Get("Allow")), "GET, POST") + wfe.AllowOrigins = []string{"*"} + testOrigin := "https://example.com" + // CORS "actual" request for disallowed method runWrappedHandler(&http.Request{ Method: "POST", Header: map[string][]string{ - "Origin": {"http://example.com"}, + "Origin": {testOrigin}, }, }, "GET") test.AssertEquals(t, stubCalled, false) @@ -478,7 +481,7 @@ func TestHandleFunc(t *testing.T) { runWrappedHandler(&http.Request{ Method: "GET", Header: map[string][]string{ - "Origin": {"http://example.com"}, + "Origin": {testOrigin}, }, }, "GET", "POST") test.AssertEquals(t, stubCalled, true) @@ -490,7 +493,7 @@ func TestHandleFunc(t *testing.T) { runWrappedHandler(&http.Request{ Method: "OPTIONS", Header: map[string][]string{ - "Origin": {"http://example.com"}, + "Origin": {testOrigin}, "Access-Control-Request-Method": {"POST"}, }, }, "GET") @@ -503,7 +506,7 @@ func TestHandleFunc(t *testing.T) { runWrappedHandler(&http.Request{ Method: "OPTIONS", Header: map[string][]string{ - "Origin": {"http://example.com"}, + "Origin": {testOrigin}, "Access-Control-Request-Method": {"POST"}, "Access-Control-Request-Headers": {"X-Accept-Header1, X-Accept-Header2", "X-Accept-Header3"}, }, @@ -533,7 +536,7 @@ func TestHandleFunc(t *testing.T) { runWrappedHandler(&http.Request{ Method: "OPTIONS", Header: map[string][]string{ - "Origin": {"http://example.com"}, + "Origin": {testOrigin}, }, }, allowedMethod) test.AssertEquals(t, rw.Code, http.StatusOK) @@ -544,6 +547,50 @@ func TestHandleFunc(t *testing.T) { test.AssertEquals(t, rw.Header().Get("Access-Control-Allow-Origin"), "") } } + + // No CORS headers are given when configuration does not list + // "*" or the client-provided origin. + for _, wfe.AllowOrigins = range [][]string{ + {}, + {"http://example.com", "https://other.example"}, + {""}, // Invalid origin is never matched + } { + runWrappedHandler(&http.Request{ + Method: "OPTIONS", + Header: map[string][]string{ + "Origin": {testOrigin}, + "Access-Control-Request-Method": {"POST"}, + }, + }, "POST") + test.AssertEquals(t, rw.Code, http.StatusOK) + for _, h := range []string{ + "Access-Control-Allow-Methods", + "Access-Control-Allow-Origin", + "Access-Control-Expose-Headers", + "Access-Control-Request-Headers", + } { + test.AssertEquals(t, rw.Header().Get(h), "") + } + } + + // CORS headers are offered when configuration lists "*" or + // the client-provided origin. + for _, wfe.AllowOrigins = range [][]string{ + {testOrigin, "http://example.org", "*"}, + {"", "http://example.org", testOrigin}, // Invalid origin is harmless + } { + runWrappedHandler(&http.Request{ + Method: "OPTIONS", + Header: map[string][]string{ + "Origin": {testOrigin}, + "Access-Control-Request-Method": {"POST"}, + }, + }, "POST") + test.AssertEquals(t, rw.Code, http.StatusOK) + test.AssertEquals(t, rw.Header().Get("Access-Control-Allow-Origin"), testOrigin) + // http://www.w3.org/TR/cors/ section 6.4: + test.AssertEquals(t, rw.Header().Get("Vary"), "Origin") + } } func TestIndexPOST(t *testing.T) { From 9eca9f0805557c5f107d9bb18dc7a9c6e3fccbba Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Sat, 12 Sep 2015 17:09:07 -0400 Subject: [PATCH 3/9] golint --- wfe/web-front-end.go | 2 +- wfe/web-front-end_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/wfe/web-front-end.go b/wfe/web-front-end.go index 5841a440d..127277d90 100644 --- a/wfe/web-front-end.go +++ b/wfe/web-front-end.go @@ -865,7 +865,7 @@ func (wfe *WebFrontEndImpl) prepChallengeForDisplay(authz core.Authorization, ch // display to the client by clearing its ID and RegistrationID fields, and // preparing all its challenges. func (wfe *WebFrontEndImpl) prepAuthorizationForDisplay(authz *core.Authorization) { - for i, _ := range authz.Challenges { + for i := range authz.Challenges { wfe.prepChallengeForDisplay(*authz, &authz.Challenges[i]) } authz.ID = "" diff --git a/wfe/web-front-end_test.go b/wfe/web-front-end_test.go index bfcd87a2f..6cb472197 100644 --- a/wfe/web-front-end_test.go +++ b/wfe/web-front-end_test.go @@ -174,8 +174,8 @@ func (sa *MockSA) GetAuthorization(id string) (core.Authorization, error) { return core.Authorization{}, nil } -func (sa *MockSA) GetLatestValidAuthorization(registrationId int64, identifier core.AcmeIdentifier) (authz core.Authorization, err error) { - if registrationId == 1 && identifier.Type == "dns" { +func (sa *MockSA) GetLatestValidAuthorization(registrationID int64, identifier core.AcmeIdentifier) (authz core.Authorization, err error) { + if registrationID == 1 && identifier.Type == "dns" { if sa.authorizedDomains[identifier.Value] || identifier.Value == "not-an-example.com" { exp := time.Now().AddDate(100, 0, 0) return core.Authorization{Status: core.StatusValid, RegistrationID: 1, Expires: &exp, Identifier: identifier}, nil @@ -1396,12 +1396,12 @@ func assertCsrLogged(t *testing.T, mockLog *mocks.MockSyslogWriter) { } func TestLogCsrPem(t *testing.T) { - const certificateRequestJson = `{ + const certificateRequestJSON = `{ "csr": "MIICWTCCAUECAQAwFDESMBAGA1UEAwwJbG9jYWxob3N0MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAycX3ca-fViOuRWF38mssORISFxbJvspDfhPGRBZDxJ63NIqQzupB-6dp48xkcX7Z_KDaRJStcpJT2S0u33moNT4FHLklQBETLhExDk66cmlz6Xibp3LGZAwhWuec7wJoEwIgY8oq4rxihIyGq7HVIJoq9DqZGrUgfZMDeEJqbphukQOaXGEop7mD-eeu8-z5EVkB1LiJ6Yej6R8MAhVPHzG5fyOu6YVo6vY6QgwjRLfZHNj5XthxgPIEETZlUbiSoI6J19GYHvLURBTy5Ys54lYAPIGfNwcIBAH4gtH9FrYcDY68R22rp4iuxdvkf03ZWiT0F2W1y7_C9B2jayTzvQIDAQABoAAwDQYJKoZIhvcNAQELBQADggEBAHd6Do9DIZ2hvdt1GwBXYjsqprZidT_DYOMfYcK17KlvdkFT58XrBH88ulLZ72NXEpiFMeTyzfs3XEyGq_Bbe7TBGVYZabUEh-LOskYwhgcOuThVN7tHnH5rhN-gb7cEdysjTb1QL-vOUwYgV75CB6PE5JVYK-cQsMIVvo0Kz4TpNgjJnWzbcH7h0mtvub-fCv92vBPjvYq8gUDLNrok6rbg05tdOJkXsF2G_W-Q6sf2Fvx0bK5JeH4an7P7cXF9VG9nd4sRt5zd-L3IcyvHVKxNhIJXZVH0AOqh_1YrKI9R0QKQiZCEy0xN1okPlcaIVaFhb7IKAHPxTI3r5f72LXY" }` wfe := setupWFE(t) var certificateRequest core.CertificateRequest - err := json.Unmarshal([]byte(certificateRequestJson), &certificateRequest) + err := json.Unmarshal([]byte(certificateRequestJSON), &certificateRequest) test.AssertNotError(t, err, "Unable to parse certificateRequest") mockSA := MockSA{} From fa30debe150a98052254d84786bfb0c873583a5a Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Tue, 29 Sep 2015 00:49:41 -0700 Subject: [PATCH 4/9] De-duplicate uses of strings.Join() --- wfe/web-front-end.go | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/wfe/web-front-end.go b/wfe/web-front-end.go index 5f8794f88..d076fbdaa 100644 --- a/wfe/web-front-end.go +++ b/wfe/web-front-end.go @@ -165,9 +165,10 @@ func (mrw BodylessResponseWriter) Write(buf []byte) (int, error) { // * 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) + methodsStr := strings.Join(methods, ", ") + methodsMap := make(map[string]bool) for _, m := range methods { - methodsOK[m] = true + methodsMap[m] = true } mux.HandleFunc(pattern, func(response http.ResponseWriter, request *http.Request) { // We do not propagate errors here, because (1) they should be @@ -184,20 +185,20 @@ func (wfe *WebFrontEndImpl) HandleFunc(mux *http.ServeMux, pattern string, h fun // sending a body. response = BodylessResponseWriter{response} case "OPTIONS": - wfe.Options(response, request, methodsOK) + wfe.Options(response, request, methodsStr, methodsMap) return } - if _, ok := methodsOK[request.Method]; !ok { + if !methodsMap[request.Method] { logEvent := wfe.populateRequestEvent(request) defer wfe.logRequestDetails(&logEvent) logEvent.Error = "Method not allowed" - response.Header().Set("Allow", strings.Join(methods, ", ")) + response.Header().Set("Allow", methodsStr) wfe.sendError(response, logEvent.Error, request.Method, http.StatusMethodNotAllowed) return } - wfe.setCORSHeaders(response, request, strings.Join(methods, ", ")) + wfe.setCORSHeaders(response, request, methodsStr) // Call the wrapped handler. h(response, request) @@ -1196,17 +1197,9 @@ func (wfe *WebFrontEndImpl) BuildID(response http.ResponseWriter, request *http. } // Options responds to an HTTP OPTIONS request. -func (wfe *WebFrontEndImpl) Options(response http.ResponseWriter, request *http.Request, methodsOK map[string]bool) { - allowMethods := "" - for method := range methodsOK { - if allowMethods != "" { - allowMethods += ", " - } - allowMethods += method - } - +func (wfe *WebFrontEndImpl) Options(response http.ResponseWriter, request *http.Request, methodsStr string, methodsMap map[string]bool) { // Every OPTIONS request gets an Allow header with a list of supported methods. - response.Header().Set("Allow", allowMethods) + response.Header().Set("Allow", methodsStr) // CORS preflight requests get additional headers. See // http://www.w3.org/TR/cors/#resource-preflight-requests @@ -1214,8 +1207,8 @@ func (wfe *WebFrontEndImpl) Options(response http.ResponseWriter, request *http. if reqMethod == "" { reqMethod = "GET" } - if _, ok := methodsOK[reqMethod]; ok { - wfe.setCORSHeaders(response, request, allowMethods) + if methodsMap[reqMethod] { + wfe.setCORSHeaders(response, request, methodsStr) } } From 9898aec7e7c579209e41155ca3696da81ed0ecb6 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Wed, 30 Sep 2015 11:10:18 -0700 Subject: [PATCH 5/9] Test status==405 and body=="" on disallowed HEAD. --- wfe/web-front-end_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/wfe/web-front-end_test.go b/wfe/web-front-end_test.go index c1aedb1a9..b07449142 100644 --- a/wfe/web-front-end_test.go +++ b/wfe/web-front-end_test.go @@ -325,8 +325,10 @@ func TestHandleFunc(t *testing.T) { // HEAD doesn't work with POST-only endpoints runWrappedHandler(&http.Request{Method: "HEAD"}, "POST") test.AssertEquals(t, stubCalled, false) + test.AssertEquals(t, rw.Code, http.StatusMethodNotAllowed) test.AssertEquals(t, rw.Header().Get("Content-Type"), "application/problem+json") test.AssertEquals(t, rw.Header().Get("Allow"), "POST") + test.AssertEquals(t, rw.Body.String(), "") wfe.AllowOrigins = []string{"*"} testOrigin := "https://example.com" From 05a142c6b7a682464266336baa13bdbf61d50bd9 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Wed, 30 Sep 2015 13:34:36 -0700 Subject: [PATCH 6/9] Fix accidentally sending preflight headers with "actual" responses. --- wfe/web-front-end.go | 2 +- wfe/web-front-end_test.go | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/wfe/web-front-end.go b/wfe/web-front-end.go index 9b5bc7460..85ea5c08f 100644 --- a/wfe/web-front-end.go +++ b/wfe/web-front-end.go @@ -204,7 +204,7 @@ func (wfe *WebFrontEndImpl) HandleFunc(mux *http.ServeMux, pattern string, h fun return } - wfe.setCORSHeaders(response, request, methodsStr) + wfe.setCORSHeaders(response, request, "") // Call the wrapped handler. h(response, request) diff --git a/wfe/web-front-end_test.go b/wfe/web-front-end_test.go index b07449142..3b9bed727 100644 --- a/wfe/web-front-end_test.go +++ b/wfe/web-front-end_test.go @@ -352,6 +352,8 @@ func TestHandleFunc(t *testing.T) { }, "GET", "POST") test.AssertEquals(t, stubCalled, true) test.AssertEquals(t, rw.Code, http.StatusOK) + test.AssertEquals(t, rw.Header().Get("Access-Control-Allow-Methods"), "") + test.AssertEquals(t, rw.Header().Get("Access-Control-Allow-Headers"), "") test.AssertEquals(t, rw.Header().Get("Access-Control-Allow-Origin"), "*") test.AssertEquals(t, sortHeader(rw.Header().Get("Access-Control-Expose-Headers")), "Link, Replay-Nonce") From 41fcec2db5eb39fcc92d84f2e18b7782d4fe284e Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Wed, 30 Sep 2015 18:23:02 -0700 Subject: [PATCH 7/9] Remove unneeded Access-Control-Allow-Headers header. --- wfe/web-front-end.go | 7 ------- wfe/web-front-end_test.go | 2 -- 2 files changed, 9 deletions(-) diff --git a/wfe/web-front-end.go b/wfe/web-front-end.go index 85ea5c08f..7ad3a3d55 100644 --- a/wfe/web-front-end.go +++ b/wfe/web-front-end.go @@ -1252,13 +1252,6 @@ func (wfe *WebFrontEndImpl) setCORSHeaders(response http.ResponseWriter, request if allowMethods != "" { // For an OPTIONS request: allow all methods handled at this URL. response.Header().Set("Access-Control-Allow-Methods", allowMethods) - - // Allow all requested headers. - if acrh, ok := request.Header["Access-Control-Request-Headers"]; ok { - for _, h := range acrh { - response.Header().Add("Access-Control-Allow-Headers", h) - } - } } response.Header().Set("Access-Control-Expose-Headers", "Link, Replay-Nonce") response.Header().Set("Access-Control-Max-Age", "86400") diff --git a/wfe/web-front-end_test.go b/wfe/web-front-end_test.go index 3b9bed727..178af73fc 100644 --- a/wfe/web-front-end_test.go +++ b/wfe/web-front-end_test.go @@ -353,7 +353,6 @@ func TestHandleFunc(t *testing.T) { test.AssertEquals(t, stubCalled, true) test.AssertEquals(t, rw.Code, http.StatusOK) test.AssertEquals(t, rw.Header().Get("Access-Control-Allow-Methods"), "") - test.AssertEquals(t, rw.Header().Get("Access-Control-Allow-Headers"), "") test.AssertEquals(t, rw.Header().Get("Access-Control-Allow-Origin"), "*") test.AssertEquals(t, sortHeader(rw.Header().Get("Access-Control-Expose-Headers")), "Link, Replay-Nonce") @@ -383,7 +382,6 @@ func TestHandleFunc(t *testing.T) { test.AssertEquals(t, rw.Header().Get("Access-Control-Allow-Origin"), "*") test.AssertEquals(t, rw.Header().Get("Access-Control-Max-Age"), "86400") test.AssertEquals(t, sortHeader(rw.Header().Get("Access-Control-Allow-Methods")), "GET, HEAD, POST") - test.AssertDeepEquals(t, rw.Header()["Access-Control-Allow-Headers"], []string{"X-Accept-Header1, X-Accept-Header2", "X-Accept-Header3"}) test.AssertEquals(t, sortHeader(rw.Header().Get("Access-Control-Expose-Headers")), "Link, Replay-Nonce") // OPTIONS request without an Origin header (i.e., not a CORS From 72538dcd7614883fd3133453a9e6c705d537ec5e Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Thu, 1 Oct 2015 10:29:08 -0700 Subject: [PATCH 8/9] Update comment. A-C-A-Headers is never sent at all any more. --- wfe/web-front-end.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wfe/web-front-end.go b/wfe/web-front-end.go index 7ad3a3d55..a00ddb518 100644 --- a/wfe/web-front-end.go +++ b/wfe/web-front-end.go @@ -1220,8 +1220,8 @@ func (wfe *WebFrontEndImpl) Options(response http.ResponseWriter, request *http. // setCORSHeaders() tells the client that CORS is acceptable for this // request. If allowMethods == "" the request is assumed to be a CORS -// actual request and no Access-Control-Allow-Methods or -Headers -// headers will be sent. +// actual request and no Access-Control-Allow-Methods header will be +// sent. func (wfe *WebFrontEndImpl) setCORSHeaders(response http.ResponseWriter, request *http.Request, allowMethods string) { reqOrigin := request.Header.Get("Origin") if reqOrigin == "" { From 6b89b69712951e027d5079a2aeeb9b8492ee3446 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Fri, 2 Oct 2015 15:19:27 -0700 Subject: [PATCH 9/9] Remove pkcs11-example-config.json. We haven't been keeping this config up to date, and it doesn't make sense to keep a whole separate test config just as an example of how to configure PKCS11. --- test/boulder-pkcs11-example-config.json | 162 ------------------------ 1 file changed, 162 deletions(-) delete mode 100644 test/boulder-pkcs11-example-config.json diff --git a/test/boulder-pkcs11-example-config.json b/test/boulder-pkcs11-example-config.json deleted file mode 100644 index c253eec6d..000000000 --- a/test/boulder-pkcs11-example-config.json +++ /dev/null @@ -1,162 +0,0 @@ -{ - "syslog": { - "network": "", - "server": "", - "tag": "boulder" - }, - - "amqp": { - "server": "amqp://guest:guest@localhost:5672", - "RA": { - "client": "RA.client", - "server": "RA.server" - }, - "VA": { - "client": "VA.client", - "server": "VA.server" - }, - "SA": { - "client": "SA.client", - "server": "SA.server" - }, - "CA": { - "client": "CA.client", - "server": "CA.server" - } - }, - - "statsd": { - "server": "localhost:8125", - "prefix": "Boulder" - }, - - "wfe": { - "listenAddress": "127.0.0.1:4000", - "allowOrigins": ["*"], - "certCacheDuration": "6h", - "certNoCacheExpirationWindow": "96h", - "indexCacheDuration": "24h", - "issuerCacheDuration": "48h", - "debugAddr": "localhost:8000" - }, - - "ca": { - "serialPrefix": 255, - "profile": "ee", - "dbConnect": "mysql+tcp://boulder@localhost:3306/boulder_test", - "debugAddr": "localhost:8001", - "testMode": true, - "_comment": "This should only be present in testMode. In prod use an HSM.", - "expiry": "2160h", - "lifespanOCSP": "96h", - "maxNames": 1000, - "Key": { - "PKCS11": { - "_comment": "On OS X, the module will likely be /Library/OpenSC/lib/opensc-pkcs11.so", - "Module": "/usr/lib/x86_64-linux-gnu/opensc-pkcs11.so", - "Token": "Yubico Yubikey NEO OTP+CCID 00 00", - "Label": "SIGN key", - "PIN": "1234" - } - }, - "cfssl": { - "signing": { - "profiles": { - "ee": { - "usages": [ - "digital signature", - "key encipherment", - "server auth", - "client auth" - ], - "backdate": "1h", - "is_ca": false, - "issuer_urls": [ - "http://int-x1.letsencrypt.org/cert" - ], - "ocsp_url": "http://int-x1.letsencrypt.org/ocsp", - "crl_url": "http://int-x1.letsencrypt.org/crl", - "policies": [ - { - "ID": "2.23.140.1.2.1" - }, - ], - "expiry": "8760h", - "CSRWhitelist": { - "PublicKeyAlgorithm": true, - "PublicKey": true, - "SignatureAlgorithm": true - }, - "UseSerialSeq": true - } - }, - "default": { - "usages": [ - "digital signature" - ], - "expiry": "8760h" - } - } - } - }, - - "ra": { - "debugAddr": "localhost:8002" - }, - - "sa": { - "dbConnect": "mysql+tcp://boulder@localhost:3306/boulder_test", - "debugAddr": "localhost:8003" - }, - - "va": { - "userAgent": "boulder", - "debugAddr": "localhost:8004" - }, - - "sql": { - "SQLDebug": true - }, - - "revoker": { - "dbConnect": "mysql+tcp://boulder@localhost:3306/boulder_test" - }, - - "ocspResponder": { - "dbConnect": "mysql+tcp://boulder@localhost:3306/boulder_test", - "debugAddr": "localhost:8004", - "path": "/", - "listenAddress": "localhost:4001" - }, - - "ocspUpdater": { - "dbConnect": "mysql+tcp://boulder@localhost:3306/boulder_test", - "debugAddr": "localhost:8005", - "minTimeToExpiry": "72h" - }, - - "activityMonitor": { - "debugAddr": "localhost:8006" - }, - - "mailer": { - "server": "mail.example.com", - "port": "25", - "username": "cert-master@example.com", - "password": "password", - "dbConnect": "mysql+tcp://boulder@localhost:3306/boulder_test", - "messageLimit": 0, - "nagTimes": ["24h", "72h", "168h", "336h"], - "emailTemplate": "test/example-expiration-template", - "debugAddr": "localhost:8004" - }, - - "common": { - "baseURL": "http://localhost:4000", - "issuerCert": "test/test-ca.pem", - "dnsResolver": "8.8.8.8:53", - "dnsTimeout": "10s" - }, - - "subscriberAgreementURL": "http://localhost:4000/terms" -}