From f4a500caf68169dccb0b54cb90523e68ee1ac2be Mon Sep 17 00:00:00 2001
From: Milos Gajdos <milosthegajdos@gmail.com>
Date: Sat, 1 Feb 2025 15:30:18 -0800
Subject: [PATCH] Fix registry token authentication bug

When a JWT contains a JWK header without a certificate chain,
the original code only checked if the KeyID (kid) matches one of the trusted keys,
but doesn't verify that the actual key material matches.

As a result, if an attacker guesses the kid, they can inject an
untrusted key which would then be used to grant access to protected
data.

This fixes the issue such as only the trusted key is verified.

Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
---
 registry/auth/token/token.go      |  5 +--
 registry/auth/token/token_test.go | 54 +++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/registry/auth/token/token.go b/registry/auth/token/token.go
index 55d0b7f6f..e1660d176 100644
--- a/registry/auth/token/token.go
+++ b/registry/auth/token/token.go
@@ -219,11 +219,12 @@ func verifyJWK(header jose.Header, verifyOpts VerifyOptions) (signingKey crypto.
 	// 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 {
+		trustedKey, 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 trustedKey, nil
 	}
 
 	opts := x509.VerifyOptions{
diff --git a/registry/auth/token/token_test.go b/registry/auth/token/token_test.go
index b982ab14b..ba9f073cb 100644
--- 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)
+	}
+}