Merge commit from fork

milosgajdos · web-flow · commit 5ea9aa028db6 · 2025-02-10T19:50:35.000-08:00
Fix registry token authentication bug
diff --git a/registry/auth/token/token.go b/registry/auth/token/token.go
@@ -212,18 +212,18 @@ func verifyCertChain(header jose.Header, roots *x509.CertPool) (signingKey crypt
 	return
 }
 
-func verifyJWK(header jose.Header, verifyOpts VerifyOptions) (signingKey crypto.PublicKey, err error) {
+func verifyJWK(header jose.Header, verifyOpts VerifyOptions) (crypto.PublicKey, error) {
 	jwk := header.JSONWebKey
-	signingKey = jwk.Key
 
 	// Check to see if the key includes a certificate chain.
 	if len(jwk.Certificates) == 0 {
 		// The JWK should be one of the trusted root keys.
-		if _, trusted := verifyOpts.TrustedKeys[jwk.KeyID]; !trusted {
+		key, trusted := verifyOpts.TrustedKeys[jwk.KeyID]
+		if !trusted {
 			return nil, errors.New("untrusted JWK with no certificate chain")
 		}
 		// The JWK is one of the trusted keys.
-		return
+		return key, nil
 	}
 
 	opts := x509.VerifyOptions{
@@ -245,9 +245,8 @@ func verifyJWK(header jose.Header, verifyOpts VerifyOptions) (signingKey crypto.
 	if err != nil {
 		return nil, err
 	}
-	signingKey = getCertPubKey(chains)
 
-	return
+	return getCertPubKey(chains), nil
 }
 
 func getCertPubKey(chains [][]*x509.Certificate) crypto.PublicKey {
diff --git a/registry/auth/token/token_test.go b/registry/auth/token/token_test.go
@@ -646,3 +646,57 @@ func TestNewAccessControllerPemBlock(t *testing.T) {
 		t.Fatal("accessController has the wrong number of certificates")
 	}
 }
+
+// This test makes sure the untrusted key can not be used in token verification.
+func TestVerifyJWKWithTrustedKey(t *testing.T) {
+	// Generate a test key pair
+	privKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	pubKey := privKey.Public()
+
+	// Create a JWK with no certificates
+	jwk := &jose.JSONWebKey{
+		Key:       privKey,
+		KeyID:     "test-key-id",
+		Use:       "sig",
+		Algorithm: string(jose.ES256),
+	}
+
+	// Create verify options with our public key as trusted
+	verifyOpts := VerifyOptions{
+		TrustedKeys: map[string]crypto.PublicKey{
+			"test-key-id": pubKey,
+		},
+	}
+
+	// Create test header
+	header := jose.Header{
+		JSONWebKey: jwk,
+	}
+
+	// Test the verifyJWK function
+	returnedKey, err := verifyJWK(header, verifyOpts)
+	if err != nil {
+		t.Fatalf("Expected no error, got: %v", err)
+	}
+
+	// Verify the returned key matches our trusted key
+	if returnedKey != pubKey {
+		t.Error("Returned key does not match the trusted key")
+	}
+
+	// Test with untrusted key
+	verifyOpts.TrustedKeys = map[string]crypto.PublicKey{
+		"different-key-id": pubKey,
+	}
+
+	_, err = verifyJWK(header, verifyOpts)
+	if err == nil {
+		t.Error("Expected error for untrusted key, got none")
+	}
+	if err.Error() != "untrusted JWK with no certificate chain" {
+		t.Errorf("Expected 'untrusted JWK with no certificate chain' error, got: %v", err)
+	}
+}

Original file line number	Diff line number	Diff line change
`@@ -212,18 +212,18 @@ func verifyCertChain(header jose.Header, roots *x509.CertPool) (signingKey crypt`
`212`	`212`	`return`
`213`	`213`	`}`
`214`	`214`
`215`		`-func verifyJWK(header jose.Header, verifyOpts VerifyOptions) (signingKey crypto.PublicKey, err error) {`
	`215`	`+func verifyJWK(header jose.Header, verifyOpts VerifyOptions) (crypto.PublicKey, error) {`
`216`	`216`	`jwk := header.JSONWebKey`
`217`		`- signingKey = jwk.Key`
`218`	`217`
`219`	`218`	`// Check to see if the key includes a certificate chain.`
`220`	`219`	`if len(jwk.Certificates) == 0 {`
`221`	`220`	`// The JWK should be one of the trusted root keys.`
`222`		`- if _, trusted := verifyOpts.TrustedKeys[jwk.KeyID]; !trusted {`
	`221`	`+ key, trusted := verifyOpts.TrustedKeys[jwk.KeyID]`
	`222`	`+ if !trusted {`
`223`	`223`	`return nil, errors.New("untrusted JWK with no certificate chain")`
`224`	`224`	`}`
`225`	`225`	`// The JWK is one of the trusted keys.`
`226`		`- return`
	`226`	`+ return key, nil`
`227`	`227`	`}`
`228`	`228`
`229`	`229`	`opts := x509.VerifyOptions{`
`@@ -245,9 +245,8 @@ func verifyJWK(header jose.Header, verifyOpts VerifyOptions) (signingKey crypto.`
`245`	`245`	`if err != nil {`
`246`	`246`	`return nil, err`
`247`	`247`	`}`
`248`		`- signingKey = getCertPubKey(chains)`
`249`	`248`
`250`		`- return`
	`249`	`+ return getCertPubKey(chains), nil`
`251`	`250`	`}`
`252`	`251`
`253`	`252`	`func getCertPubKey(chains [][]*x509.Certificate) crypto.PublicKey {`