Handle mismatched URLs in key rollover. (letsencrypt#4752)

jsha · web-flow · commit 1278679afb2e · 2020-04-07T19:21:02.000-07:00
Fixes letsencrypt#4751
diff --git a/wfe2/verify.go b/wfe2/verify.go
@@ -725,7 +725,7 @@ func (wfe *WebFrontEndImpl) validKeyRollover(
 	// payload already.
 
 	// Verify that the outer and inner JWS protected URL headers match
-	if wfe.matchJWSURLs(outerJWS, innerJWS) != nil {
+	if prob := wfe.matchJWSURLs(outerJWS, innerJWS); prob != nil {
 		return nil, prob
 	}
 
diff --git a/wfe2/verify_test.go b/wfe2/verify_test.go
@@ -67,7 +67,7 @@ func pubKeyForKey(t *testing.T, privKey interface{}) interface{} {
 	return nil
 }
 
-// signRequestEmbed creates a JWS for aa given request body with an embedded JWK
+// signRequestEmbed creates a JWS for a given request body with an embedded JWK
 // corresponding to the private key provided. The URL and nonce extra headers
 // are set based on the additional arguments. A computed JWS, the corresponding
 // embedded JWK and the JWS in serialized string form are returned.
diff --git a/wfe2/wfe_test.go b/wfe2/wfe_test.go
@@ -2521,6 +2521,26 @@ func TestKeyRollover(t *testing.T) {
 	}
 }
 
+func TestKeyRolloverMismatchedJWSURLs(t *testing.T) {
+	responseWriter := httptest.NewRecorder()
+	wfe, _ := setupWFE(t)
+
+	newKeyBytes, err := ioutil.ReadFile("../test/test-key-5.der")
+	test.AssertNotError(t, err, "Failed to read ../test/test-key-5.der")
+	newKeyPriv, err := x509.ParsePKCS1PrivateKey(newKeyBytes)
+	test.AssertNotError(t, err, "Failed parsing private key")
+
+	_, _, inner := signRequestEmbed(t, newKeyPriv, "http://localhost/wrong-url", "{}", wfe.nonceService)
+	_, _, outer := signRequestKeyID(t, 1, nil, "http://localhost/key-change", inner, wfe.nonceService)
+	wfe.KeyRollover(ctx, newRequestEvent(), responseWriter, makePostRequestWithPath("key-change", outer))
+	test.AssertUnmarshaledEquals(t, responseWriter.Body.String(), `
+		{
+			"type": "urn:ietf:params:acme:error:malformed",
+			"detail": "Outer JWS 'url' value \"http://localhost/key-change\" does not match inner JWS 'url' value \"http://localhost/wrong-url\"",
+			"status": 400
+		}`)
+}
+
 func TestGetOrder(t *testing.T) {
 	wfe, _ := setupWFE(t)
 

Original file line number	Diff line number	Diff line change
`@@ -725,7 +725,7 @@ func (wfe *WebFrontEndImpl) validKeyRollover(`
`725`	`725`	`// payload already.`
`726`	`726`
`727`	`727`	`// Verify that the outer and inner JWS protected URL headers match`
`728`		`- if wfe.matchJWSURLs(outerJWS, innerJWS) != nil {`
	`728`	`+ if prob := wfe.matchJWSURLs(outerJWS, innerJWS); prob != nil {`
`729`	`729`	`return nil, prob`
`730`	`730`	`}`
`731`	`731`
Original file line number	Diff line number	Diff line change
`@@ -67,7 +67,7 @@ func pubKeyForKey(t *testing.T, privKey interface{}) interface{} {`
`67`	`67`	`return nil`
`68`	`68`	`}`
`69`	`69`
`70`		`-// signRequestEmbed creates a JWS for aa given request body with an embedded JWK`
	`70`	`+// signRequestEmbed creates a JWS for a given request body with an embedded JWK`
`71`	`71`	`// corresponding to the private key provided. The URL and nonce extra headers`
`72`	`72`	`// are set based on the additional arguments. A computed JWS, the corresponding`
`73`	`73`	`// embedded JWK and the JWS in serialized string form are returned.`