From 3d3b508ad30f31e0fd1e6d6d4bcd3a455f20cd50 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Thu, 5 Nov 2015 11:13:52 -0800 Subject: [PATCH] Check for correct algorithms in verifyPOST Fixes https://github.com/letsencrypt/boulder/issues/259 --- wfe/web-front-end.go | 39 ++++++++++ wfe/web-front-end_test.go | 152 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 191 insertions(+) diff --git a/wfe/web-front-end.go b/wfe/web-front-end.go index c63ba59b8..a2ce1227f 100644 --- a/wfe/web-front-end.go +++ b/wfe/web-front-end.go @@ -7,6 +7,7 @@ package wfe import ( "bytes" + "crypto/rsa" "crypto/x509" "encoding/json" "fmt" @@ -316,6 +317,38 @@ const ( malformedJWS = "Unable to read/verify body" ) +func algorithmForKey(key *jose.JsonWebKey) (string, error) { + // TODO(https://github.com/letsencrypt/boulder/issues/792): Support EC. + switch key.Key.(type) { + case *rsa.PublicKey: + return "RS256", nil + } + return "", core.SignatureValidationError("no signature algorithms suitable for given key type") +} + +// Check that (1) there is a suitable algorithm for the provided key based on its +// Golang type, (2) the Algorithm field on the JWK is either absent, or matches +// that algorithm, and (3) the Algorithm field on the JWK is present and matches +// that algorithm. +func checkAlgorithm(key *jose.JsonWebKey, parsedJws *jose.JsonWebSignature) (string, error) { + algorithm, err := algorithmForKey(key) + if err != nil { + return "NoAlgorithmForKey", err + } + jwsAlgorithm := parsedJws.Signatures[0].Header.Algorithm + if jwsAlgorithm != algorithm { + return "InvalidJWSAlgorithm", + core.SignatureValidationError(fmt.Sprintf( + "algorithm '%s' in JWS header not acceptable", jwsAlgorithm)) + } + if key.Algorithm != "" && key.Algorithm != algorithm { + return "InvalidAlgorithmOnKey", + core.SignatureValidationError(fmt.Sprintf( + "algorithm '%s' on JWK is unacceptable", key.Algorithm)) + } + return "", nil +} + // verifyPOST reads and parses the request body, looks up the Registration // corresponding to its JWK, verifies the JWS signature, checks that the // resource field is present and correct in the JWS protected header, and @@ -386,6 +419,7 @@ func (wfe *WebFrontEndImpl) verifyPOST(logEvent *requestEvent, request *http.Req logEvent.AddError("no signatures in POST body") return nil, nil, reg, err } + submittedKey := parsedJws.Signatures[0].Header.JsonWebKey if submittedKey == nil { err = core.SignatureValidationError("No JWK in JWS header") @@ -421,6 +455,11 @@ func (wfe *WebFrontEndImpl) verifyPOST(logEvent *requestEvent, request *http.Req logEvent.Contacts = reg.Contact } + if statName, err := checkAlgorithm(key, parsedJws); err != nil { + wfe.stats.Inc(fmt.Sprintf("WFE.Errors.%s", statName), 1, 1.0) + return nil, nil, reg, err + } + payload, header, err := parsedJws.Verify(key) if err != nil { puberr := core.SignatureValidationError("JWS verification error") diff --git a/wfe/web-front-end_test.go b/wfe/web-front-end_test.go index f1822ba0f..1665fdec6 100644 --- a/wfe/web-front-end_test.go +++ b/wfe/web-front-end_test.go @@ -7,6 +7,7 @@ package wfe import ( "bytes" + "crypto/ecdsa" "crypto/rsa" "crypto/x509" "encoding/json" @@ -1387,6 +1388,157 @@ func TestVerifyPOSTUsesStoredKey(t *testing.T) { test.AssertError(t, err, "No error returned when provided key differed from stored key.") } +func TestRejectsNone(t *testing.T) { + wfe, _ := setupWFE(t) + _, _, _, err := wfe.verifyPOST(newRequestEvent(), makePostRequest(` + { + "header": { + "alg": "none", + "jwk": { + "kty": "RSA", + "n": "vrjT", + "e": "AQAB" + } + }, + "payload": "aGkK", + "signature": "" + } + `), true, "foo") + if err == nil { + t.Fatalf("verifyPOST did not reject JWS with alg: 'none'") + } + if err.Error() != "algorithm 'none' in JWS header not acceptable" { + t.Fatalf("verifyPOST rejected JWS with alg: 'none', but for wrong reason: %s", err) + } +} + +func TestRejectsHS256(t *testing.T) { + wfe, _ := setupWFE(t) + _, _, _, err := wfe.verifyPOST(newRequestEvent(), makePostRequest(` + { + "header": { + "alg": "HS256", + "jwk": { + "kty": "RSA", + "n": "vrjT", + "e": "AQAB" + } + }, + "payload": "aGkK", + "signature": "" + } + `), true, "foo") + if err == nil { + t.Fatalf("verifyPOST did not reject JWS with alg: 'HS256'") + } + expected := "algorithm 'HS256' in JWS header not acceptable" + if err.Error() != expected { + t.Fatalf("verifyPOST rejected JWS with alg: 'none', but for wrong reason: got '%s', wanted %s", err, expected) + } +} + +func TestCheckAlgorithmES(t *testing.T) { + stat, err := checkAlgorithm(&jose.JsonWebKey{ + Algorithm: "ES256", + Key: &ecdsa.PublicKey{}, + }, &jose.JsonWebSignature{}) + expected := "no signature algorithms suitable for given key type" + if err == nil || err.Error() != expected { + t.Errorf("ES256 key: Expected error '%s', got '%s'", expected, err) + } + if stat != "NoAlgorithmForKey" { + t.Errorf("ES256 key: Expected stat '%s', got '%s'", "NoAlgorithmForKey", stat) + } +} + +func TestCheckAlgorithmHS(t *testing.T) { + stat, err := checkAlgorithm(&jose.JsonWebKey{ + Algorithm: "HS256", + }, &jose.JsonWebSignature{}) + expected := "no signature algorithms suitable for given key type" + if err == nil || err.Error() != expected { + t.Errorf("HS256 key: Expected error '%s', got '%s'", expected, err) + } + if stat != "NoAlgorithmForKey" { + t.Errorf("HS256 key: Expected stat '%s', got '%s'", "NoAlgorithmForKey", stat) + } +} + +func TestCheckAlgorithmBadJWS(t *testing.T) { + stat, err := checkAlgorithm(&jose.JsonWebKey{ + Key: &rsa.PublicKey{}, + }, &jose.JsonWebSignature{ + Signatures: []jose.Signature{ + jose.Signature{ + Header: jose.JoseHeader{ + Algorithm: "HS256", + }, + }, + }, + }) + expected := "algorithm 'HS256' in JWS header not acceptable" + if err == nil || err.Error() != expected { + t.Errorf("HS256 JWS: Expected error '%s', got '%s'", expected, err) + } + if stat != "InvalidJWSAlgorithm" { + t.Errorf("HS256 JWS: Expected stat '%s', got '%s'", "InvalidJWSAlgorithm", stat) + } +} + +func TestCheckAlgorithmBadJWK(t *testing.T) { + stat, err := checkAlgorithm(&jose.JsonWebKey{ + Algorithm: "HS256", + Key: &rsa.PublicKey{}, + }, &jose.JsonWebSignature{ + Signatures: []jose.Signature{ + jose.Signature{ + Header: jose.JoseHeader{ + Algorithm: "RS256", + }, + }, + }, + }) + expected := "algorithm 'HS256' on JWK is unacceptable" + if err == nil || err.Error() != expected { + t.Errorf("HS256 JWS: Expected error '%s', got '%s'", expected, err) + } + if stat != "InvalidAlgorithmOnKey" { + t.Errorf("HS256 JWS: Expected stat '%s', got '%s'", "InvalidAlgorithmOnKey", stat) + } +} + +func TestCheckAlgorithmSuccess(t *testing.T) { + _, err := checkAlgorithm(&jose.JsonWebKey{ + Algorithm: "RS256", + Key: &rsa.PublicKey{}, + }, &jose.JsonWebSignature{ + Signatures: []jose.Signature{ + jose.Signature{ + Header: jose.JoseHeader{ + Algorithm: "RS256", + }, + }, + }, + }) + if err != nil { + t.Errorf("RS256 key: Expected nil error, got '%s'", err) + } + _, err = checkAlgorithm(&jose.JsonWebKey{ + Key: &rsa.PublicKey{}, + }, &jose.JsonWebSignature{ + Signatures: []jose.Signature{ + jose.Signature{ + Header: jose.JoseHeader{ + Algorithm: "RS256", + }, + }, + }, + }) + if err != nil { + t.Errorf("RS256 key: Expected nil error, got '%s'", err) + } +} + func TestBadKeyCSR(t *testing.T) { wfe, _ := setupWFE(t) responseWriter := httptest.NewRecorder()