Merge pull request #788 from tomclegg/469-fix-cors-headers

Fix cors headers

cmd/boulder-wfe/main.go

@@ -83,6 +83,8 @@ func main() {
 		wfe.SA = &sac
 		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)

cmd/shell.go

@@ -74,6 +74,8 @@ type Config struct {
 		BaseURL       string
 		ListenAddress string
 
+		AllowOrigins []string
+
 		CertCacheDuration           string
 		CertNoCacheExpirationWindow string
 		IndexCacheDuration          string

test/boulder-config.json

@@ -42,6 +42,7 @@
 
   "wfe": {
     "listenAddress": "127.0.0.1:4000",
+    "allowOrigins": ["*"],
     "certCacheDuration": "6h",
     "certNoCacheExpirationWindow": "96h",
     "indexCacheDuration": "24h",

test/boulder-pkcs11-example-config.json

@@ -32,6 +32,7 @@
 
   "wfe": {
     "listenAddress": "127.0.0.1:4000",
+    "allowOrigins": ["*"],
     "certCacheDuration": "6h",
     "certNoCacheExpirationWindow": "96h",
     "indexCacheDuration": "24h",

wfe/web-front-end.go

@@ -73,7 +73,10 @@ type WebFrontEndImpl struct {
 	IndexCacheDuration          time.Duration
 	IssuerCacheDuration         time.Duration
 
-	// Gracefull shutdown settings
+	// CORS settings
+	AllowOrigins []string
+
+	// Graceful shutdown settings
 	ShutdownStopTimeout time.Duration
 	ShutdownKillTimeout time.Duration
 }
@@ -153,17 +156,27 @@ 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. Also, all
+// written by the handler will be discarded if the method is HEAD.
-//   handlers that accept GET automatically accept HEAD.
+// Also, all handlers that accept GET automatically accept HEAD.
 func (wfe *WebFrontEndImpl) HandleFunc(mux *http.ServeMux, pattern string, h func(http.ResponseWriter, *http.Request), methods ...string) {
-	methodsOK := make(map[string]bool)
+	methodsMap := make(map[string]bool)
 	for _, m := range methods {
-		methodsOK[m] = true
+		methodsMap[m] = true
 	}
+	if methodsMap["GET"] && !methodsMap["HEAD"] {
+		// Allow HEAD for any resource that allows GET
+		methods = append(methods, "HEAD")
+		methodsMap["HEAD"] = true
+	}
+	methodsStr := strings.Join(methods, ", ")
 	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.
@@ -171,27 +184,29 @@ 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", "*")
 
-		// Return a bodyless response to HEAD for any resource that allows GET.
+		switch request.Method {
-		if _, ok := methodsOK["GET"]; ok && request.Method == "HEAD" {
+		case "HEAD":
-			// We'll be sending an error anyway, but we
+			// Whether or not we're sending a 405 error,
-			// should still comply with HTTP spec by not
+			// we should comply with HTTP spec by not
 			// sending a body.
 			response = BodylessResponseWriter{response}
-			h(response, request)
+		case "OPTIONS":
+			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, "")
+
 		// Call the wrapped handler.
 		h(response, request)
 	})
@@ -885,7 +900,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 = ""
@@ -1211,6 +1226,61 @@ 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, methodsStr string, methodsMap map[string]bool) {
+	// Every OPTIONS request gets an Allow header with a list of supported methods.
+	response.Header().Set("Allow", methodsStr)
+
+	// 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 methodsMap[reqMethod] {
+		wfe.setCORSHeaders(response, request, methodsStr)
+	}
+}
+
+// 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 header will be
+// sent.
+func (wfe *WebFrontEndImpl) setCORSHeaders(response http.ResponseWriter, request *http.Request, allowMethods string) {
+	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)
+	}
+	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

wfe/web-front-end_test.go

@@ -253,6 +253,15 @@ func sortHeader(s string) string {
 	return strings.Join(a, ", ")
 }
 
+func addHeadIfGet(s []string) []string {
+	for _, a := range s {
+		if a == "GET" {
+			return append(s, "HEAD")
+		}
+	}
+	return s
+}
+
 func TestHandleFunc(t *testing.T) {
 	wfe := setupWFE(t)
 	var mux *http.ServeMux
@@ -271,32 +280,34 @@ func TestHandleFunc(t *testing.T) {
 
 	// Plain requests (no CORS)
 	type testCase struct {
-		allowed       []string
+		allowed        []string
-		reqMethod     string
+		reqMethod      string
-		shouldSucceed bool
+		shouldCallStub bool
+		shouldSucceed  bool
 	}
 	var lastNonce string
 	for _, c := range []testCase{
-		{[]string{"GET", "POST"}, "GET", true},
+		{[]string{"GET", "POST"}, "GET", true, true},
-		{[]string{"GET", "POST"}, "POST", true},
+		{[]string{"GET", "POST"}, "POST", true, true},
-		{[]string{"GET"}, "", false},
+		{[]string{"GET"}, "", false, false},
-		{[]string{"GET"}, "POST", false},
+		{[]string{"GET"}, "POST", false, false},
-		{[]string{"GET"}, "OPTIONS", false},     // TODO, #469
+		{[]string{"GET"}, "OPTIONS", false, true},
-		{[]string{"GET"}, "MAKE-COFFEE", false}, // 405, or 418?
+		{[]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 {
 			test.AssertEquals(t, rw.Code, http.StatusMethodNotAllowed)
-			test.AssertEquals(t, sortHeader(rw.Header().Get("Allow")), strings.Join(c.allowed, ", "))
+			test.AssertEquals(t, sortHeader(rw.Header().Get("Allow")), sortHeader(strings.Join(addHeadIfGet(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)
+		test.AssertNotEquals(t, nonce, "")
 		lastNonce = nonce
 	}
 
@@ -304,7 +315,7 @@ func TestHandleFunc(t *testing.T) {
 	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")
+	test.AssertEquals(t, sortHeader(rw.Header().Get("Allow")), "GET, HEAD, POST")
 
 	// Disallowed method special case: response to HEAD has got no body
 	runWrappedHandler(&http.Request{Method: "HEAD"}, "GET", "POST")
@@ -314,43 +325,137 @@ 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.Body.String(), `{"type":"urn:acme:error:malformed","detail":"Method not allowed"}`)
 	test.AssertEquals(t, rw.Header().Get("Allow"), "POST")
-}
+	test.AssertEquals(t, rw.Body.String(), "")
 
-func TestStandardHeaders(t *testing.T) {
+	wfe.AllowOrigins = []string{"*"}
-	wfe := setupWFE(t)
+	testOrigin := "https://example.com"
-	mux, err := wfe.Handler()
-	test.AssertNotError(t, err, "Problem setting up HTTP handlers")
 
-	cases := []struct {
+	// CORS "actual" request for disallowed method
-		path    string
+	runWrappedHandler(&http.Request{
-		allowed []string
+		Method: "POST",
-	}{
+		Header: map[string][]string{
-		{wfe.NewReg, []string{"POST"}},
+			"Origin": {testOrigin},
-		{wfe.RegBase, []string{"POST"}},
+		},
-		{wfe.NewAuthz, []string{"POST"}},
+	}, "GET")
-		{wfe.AuthzBase, []string{"GET"}},
+	test.AssertEquals(t, stubCalled, false)
-		{wfe.ChallengeBase, []string{"GET", "POST"}},
+	test.AssertEquals(t, rw.Code, http.StatusMethodNotAllowed)
-		{wfe.NewCert, []string{"POST"}},
+
-		{wfe.CertBase, []string{"GET"}},
+	// CORS "actual" request for allowed method
-		{wfe.SubscriberAgreementURL, []string{"GET"}},
+	runWrappedHandler(&http.Request{
+		Method: "GET",
+		Header: map[string][]string{
+			"Origin": {testOrigin},
+		},
+	}, "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-Origin"), "*")
+	test.AssertEquals(t, sortHeader(rw.Header().Get("Access-Control-Expose-Headers")), "Link, Replay-Nonce")
+
+	// CORS preflight request for disallowed method
+	runWrappedHandler(&http.Request{
+		Method: "OPTIONS",
+		Header: map[string][]string{
+			"Origin":                        {testOrigin},
+			"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, HEAD")
+	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":                         {testOrigin},
+			"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, HEAD, POST")
+	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, HEAD, 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": {testOrigin},
+			},
+		}, 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, HEAD")
+		} else {
+			test.AssertEquals(t, rw.Header().Get("Access-Control-Allow-Origin"), "")
+		}
 	}
 
-	for _, c := range cases {
+	// No CORS headers are given when configuration does not list
-		responseWriter := httptest.NewRecorder()
+	// "*" or the client-provided origin.
-		mux.ServeHTTP(responseWriter, &http.Request{
+	for _, wfe.AllowOrigins = range [][]string{
-			Method: "BOGUS",
+		{},
-			URL:    mustParseURL(c.path),
+		{"http://example.com", "https://other.example"},
-		})
+		{""}, // Invalid origin is never matched
-		acao := responseWriter.Header().Get("Access-Control-Allow-Origin")
+	} {
-		nonce := responseWriter.Header().Get("Replay-Nonce")
+		runWrappedHandler(&http.Request{
-		allow := responseWriter.Header().Get("Allow")
+			Method: "OPTIONS",
-		test.Assert(t, responseWriter.Code == http.StatusMethodNotAllowed, "Bogus method allowed")
+			Header: map[string][]string{
-		test.Assert(t, acao == "*", "Bad CORS header")
+				"Origin":                        {testOrigin},
-		test.Assert(t, len(nonce) > 0, "Bad Replay-Nonce header")
+				"Access-Control-Request-Method": {"POST"},
-		test.Assert(t, len(allow) > 0 && allow == strings.Join(c.allowed, ", "), "Bad Allow header")
+			},
+		}, "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")
 	}
 }
 
@@ -1183,12 +1288,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 := mocks.MockSA{}