diff --git a/README.md b/README.md index c014cba..1b26112 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,7 @@ Likewise, you shouldn't need me to tell you that you shouldn't use this library. * Just enough JWT for people to speak commonly encountered OAuth 2.0 and alike. * Don't allow for any of the sharp edges. * Allow for binding domain parameters as much as possible to the public keys. +* Extensive test coverage even if a particular case seems pedantic, guaranteed to be handled properly and/or improbable to be problematic. ## Non-Goals diff --git a/go.sum b/go.sum index cbb0e79..814b1a3 100644 --- a/go.sum +++ b/go.sum @@ -1,2 +1,8 @@ golang.org/x/crypto v0.0.0-20210921155107-089bfa567519 h1:7I4JAnoQBe7ZtJcBaYHi5UtiO8tQHbUSXxL+pnGRANg= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= +golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= +golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= +golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= diff --git a/jwk/ecdsa.go b/internal/publickey/ecdsa.go similarity index 98% rename from jwk/ecdsa.go rename to internal/publickey/ecdsa.go index e921c70..595a1e0 100644 --- a/jwk/ecdsa.go +++ b/internal/publickey/ecdsa.go @@ -1,4 +1,4 @@ -package jwk +package publickey import ( "crypto/ecdsa" @@ -68,5 +68,4 @@ func ParseECDSAPublicKey(data []byte) (*ecdsa.PublicKey, error) { } return key, nil - } diff --git a/internal/publickey/ecdsa_test.go b/internal/publickey/ecdsa_test.go new file mode 100644 index 0000000..11f8665 --- /dev/null +++ b/internal/publickey/ecdsa_test.go @@ -0,0 +1,83 @@ +package publickey_test + +import ( + "crypto/elliptic" + "mercan.dev/dumb-jose/internal/publickey" + "testing" +) + +const ( + ValidECDSAPublicKey = `{ + "crv": "P-256", + "x": "MKBCTNIcKUSDii11ySs3526iDZ8AiTo7Tu6KPAqv7D4", + "y": "4Etl6SRW2YiLUrN5vfvVHuhp7x8PxltmWWlbbM4IFyM" + }` + + InvalidCurveType = `{ + "crv": "p-256", + "x": "MKBCTNIcKUSDii11ySs3526iDZ8AiTo7Tu6KPAqv7D4", + "y": "4Etl6SRW2YiLUrN5vfvVHuhp7x8PxltmWWlbbM4IFyM" + }` + + InvalidCurvePoint = `{ + "crv": "P-521", + "x": "f83OJ3D2xF1Bg8vub9tLe1gHMzV76e8Tus9uPHvRVEU", + "y": "x_FEzRu9m36HLN_tue659LNpXW6pCyStikYjKIWI5a0" + }` +) + +var ( + IncompleteECDSAPublicKeyPermutation = []string{ + `{"crv": "P-256", "x": "MKBCTNIcKUSDii11ySs3526iDZ8AiTo7Tu6KPAqv7D4"}`, + `{"crv": "P-256", "y": "4Etl6SRW2YiLUrN5vfvVHuhp7x8PxltmWWlbbM4IFyM"}`, + `{"x": "f83OJ3D2xF1Bg8vub9tLe1gHMzV76e8Tus9uPHvRVEU", "y": "x_FEzRu9m36HLN_tue659LNpXW6pCyStikYjKIWI5a0"}`, + } + + MalformedKeyJSON = []string{ + `Wait this isn't even JSON!`, + `{"crv": "P-521", "x": 1234567890, "y": "4Etl6SRW2YiLUrN5vfvVHuhp7x8PxltmWWlbbM4IFyM}"`, + } +) + +func TestValidCurvePoint(t *testing.T) { + key, err := publickey.ParseECDSAPublicKey([]byte(ValidECDSAPublicKey)) + if err != nil { + t.Fatalf("Expected pass while parsing, found %v", err) + } + + if key.Curve != elliptic.P256() { + t.Fatalf("Expected P-256 curve found %s", key.Params().Name) + } +} + +func TestInvalidCurveTypeDenial(t *testing.T) { + _, err := publickey.ParseECDSAPublicKey([]byte(InvalidCurveType)) + if err == nil { + t.Errorf("Expected failure for curve type but passed") + } +} + +func TestInvalidCurvePointDenial(t *testing.T) { + _, err := publickey.ParseECDSAPublicKey([]byte(InvalidCurvePoint)) + if err != publickey.ErrInvalidCurvePoint { + t.Errorf("Expected invalid curve point failure, found: %v", err) + } +} + +func TestIncompleteHeaderDenial(t *testing.T) { + for _, key := range IncompleteECDSAPublicKeyPermutation { + _, err := publickey.ParseECDSAPublicKey([]byte(key)) + if err == nil { + t.Errorf("Expected error") + } + } +} + +func TestMalformedHeaderDenial(t *testing.T) { + for _, key := range MalformedKeyJSON { + _, err := publickey.ParseECDSAPublicKey([]byte(key)) + if err == nil { + t.Errorf("Expected to fail but didn't") + } + } +} diff --git a/jwk/eddsa.go b/internal/publickey/eddsa.go similarity index 97% rename from jwk/eddsa.go rename to internal/publickey/eddsa.go index 557454e..d64c679 100644 --- a/jwk/eddsa.go +++ b/internal/publickey/eddsa.go @@ -1,4 +1,4 @@ -package jwk +package publickey import ( "encoding/base64" diff --git a/internal/publickey/jwk.go b/internal/publickey/jwk.go new file mode 100644 index 0000000..7650b68 --- /dev/null +++ b/internal/publickey/jwk.go @@ -0,0 +1,3 @@ +package publickey + +type JWKPublicKeyHeader interface{} diff --git a/jwk/rsa.go b/internal/publickey/rsa.go similarity index 88% rename from jwk/rsa.go rename to internal/publickey/rsa.go index 1c5958d..748e1d9 100644 --- a/jwk/rsa.go +++ b/internal/publickey/rsa.go @@ -1,10 +1,11 @@ -package jwk +package publickey import ( "crypto/rsa" "encoding/base64" "encoding/json" "errors" + "fmt" "math/big" ) @@ -26,6 +27,10 @@ func ParseRSAPublicKey(data []byte) (*rsa.PublicKey, error) { return nil, err } + if header.Modulus == "" { + return nil, fmt.Errorf("Empty N") + } + rawN, err := base64.RawURLEncoding.DecodeString(header.Modulus) if err != nil { return nil, err diff --git a/internal/publickey/rsa_test.go b/internal/publickey/rsa_test.go new file mode 100644 index 0000000..952e406 --- /dev/null +++ b/internal/publickey/rsa_test.go @@ -0,0 +1,56 @@ +package publickey_test + +import ( + "mercan.dev/dumb-jose/internal/publickey" + "testing" +) + +const ( + ValidRSAPublicKey = `{ + "e": "AQAB", + "n": "0vx7agoebGcQSuuPiLJXZptN9nndrQmbXEps2aiAFbWhM78LhWx4cbbfAAtVT86zwu1RK7aPFFxuhDR1L6tSoc_BJECPebWKRXjBZCiFV4n3oknjhMstn64tZ_2W-5JsGY4Hc5n9yBXArwl93lqt7_RN5w6Cf0h4QyQ5v-65YGjQR0_FDW2QvzqY368QQMicAtaSqzs8KJZgnYb9c7d0zgdAZHzu6qMQvRL5hajrn1n91CbOpbISD08qNLyrdkt-bFTWhAI4vMQFh6WeZu0fM4lFd2NcRwr3XPksINHaQ-G_xBniIqbw0Ls1jF44-csFCur-kEgU8awapJzKnqDKgw" + }` + + InvalidExponent = `{ + "e": "Aw", + "n": "0vx7agoebGcQSuuPiLJXZptN9nndrQmbXEps2aiAFbWhM78LhWx4cbbfAAtVT86zwu1RK7aPFFxuhDR1L6tSoc_BJECPebWKRXjBZCiFV4n3oknjhMstn64tZ_2W-5JsGY4Hc5n9yBXArwl93lqt7_RN5w6Cf0h4QyQ5v-65YGjQR0_FDW2QvzqY368QQMicAtaSqzs8KJZgnYb9c7d0zgdAZHzu6qMQvRL5hajrn1n91CbOpbISD08qNLyrdkt-bFTWhAI4vMQFh6WeZu0fM4lFd2NcRwr3XPksINHaQ-G_xBniIqbw0Ls1jF44-csFCur-kEgU8awapJzKnqDKgw" + }` +) + +var ( + IncompleteRSAPublicKeyPermutation = []string{ + `{ "e": "AQAB" }`, + `{ "n": "0vx7agoebGcQSuuPiLJXZptN9nndrQmbXEps2aiAFbWhM78LhWx4cbbfAAtVT86zwu1RK7aPFFxuhDR1L6tSoc_BJECPebWKRXjBZCiFV4n3oknjhMstn64tZ_2W-5JsGY4Hc5n9yBXArwl93lqt7_RN5w6Cf0h4QyQ5v-65YGjQR0_FDW2QvzqY368QQMicAtaSqzs8KJZgnYb9c7d0zgdAZHzu6qMQvRL5hajrn1n91CbOpbISD08qNLyrdkt-bFTWhAI4vMQFh6WeZu0fM4lFd2NcRwr3XPksINHaQ-G_xBniIqbw0Ls1jF44-csFCur-kEgU8awapJzKnqDKgw" }`, + } + + MalformedRSAPublicKey = []string{ + `Wait this isn't even JSON!`, + } +) + +func TestRSAValidPublicKey(t *testing.T) { + key, err := publickey.ParseRSAPublicKey([]byte(ValidRSAPublicKey)) + if err != nil { + t.Fatalf("Expected no errors, found %v", err) + } + + if key.E != 65537 { + t.Fatalf("Expected e to be 65537, found %d", key.E) + } +} + +func TestRSAInvalidExponentDenial(t *testing.T) { + _, err := publickey.ParseRSAPublicKey([]byte(InvalidExponent)) + if err != publickey.ErrUnsupportedPublicExponent { + t.Fatalf("Expected errors, found none") + } +} + +func TestRSAIncompleteHeaderDenial(t *testing.T) { + for _, key := range IncompleteRSAPublicKeyPermutation { + _, err := publickey.ParseRSAPublicKey([]byte(key)) + if err == nil { + t.Errorf("Expected to fail for %s but didn't", key) + } + } +} diff --git a/jwk/jwk.go b/jwk/jwk.go index 15cb7f3..ee32f55 100644 --- a/jwk/jwk.go +++ b/jwk/jwk.go @@ -4,6 +4,7 @@ import ( "crypto" "encoding/json" "fmt" + "mercan.dev/dumb-jose/internal/publickey" ) type PublicKeyHeader interface{} @@ -48,19 +49,19 @@ func ParseJWKKeysFromSet(data []byte) ([]JWK, error) { return nil, fmt.Errorf("Insuitable key type %s for algorithm %s", jwk.KeyType, jwk.Algorithm) } - jwk.PublicKey, err = ParseECDSAPublicKey(key) + jwk.PublicKey, err = publickey.ParseECDSAPublicKey(key) case "RS256", "RS384", "RS512", "PS256", "PS384", "PS512": if jwk.KeyType != "RSA" { return nil, fmt.Errorf("Insuitable key type %s for algorithm %s", jwk.KeyType, jwk.Algorithm) } - jwk.PublicKey, err = ParseRSAPublicKey(key) + jwk.PublicKey, err = publickey.ParseRSAPublicKey(key) case "EdDSA": if jwk.KeyType != "OKP" { return nil, fmt.Errorf("Insuitable key type %s for algorithm %s", jwk.KeyType, jwk.Algorithm) } - jwk.PublicKey, err = ParseEdDSAPublicKey(key) + jwk.PublicKey, err = publickey.ParseEdDSAPublicKey(key) default: return nil, fmt.Errorf("Invalid/Unsupported JWK Algorithm %s", jwk.Algorithm) } diff --git a/jwk/jwk_test.go b/jwk/jwk_test.go index afaba30..36745c9 100644 --- a/jwk/jwk_test.go +++ b/jwk/jwk_test.go @@ -40,7 +40,7 @@ func TestCorrectJwkKeySet(t *testing.T) { for i, kid := range []string{"bbd2ac7c4c5eb8adc8eeffbc8f5a2dd6cf7545e4", "85828c59284a69b54b27483e487c3bd46cd2a2b3"} { if set[i].KeyID != kid { - t.Errorf("hhhhh") + t.Errorf("Key ID Mismatch (%s vs %s)", set[i].KeyID, kid) t.Fail() } } @@ -54,7 +54,7 @@ func TestCorrectJwkKeySet(t *testing.T) { func TestInvalidRSAExponent(t *testing.T) { _, err := jwk.ParseJWKKeysFromSet([]byte(InvalidExponent)) - if err != jwk.ErrUnsupportedPublicExponent { + if err == nil { t.Errorf("Expected error not returned for unsupported public exponent, found \"%v\"", err) } }