internal/publickey: move from public jwk add tests

Moving algorithm specific jwk code to an internal module.
Not caring about it should be fine(?) to the end user as long
as the library remains vigilantic about it. Also, the crypto.PublicKey
in JWK isn't meant to be used directly anyway.

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
 

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=

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
-
 }

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")
+		}
+	}
+}

internal/publickey/eddsa.go

@@ -1,4 +1,4 @@
-package jwk
+package publickey
 
 import (
 	"encoding/base64"

internal/publickey/jwk.go

@@ -0,0 +1,3 @@
+package publickey
+
+type JWKPublicKeyHeader interface{}

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

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)
+		}
+	}
+}

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)
 		}

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)
 	}
 }