From 70e0d8850c91d4f165728f904b5d47df636fed09 Mon Sep 17 00:00:00 2001
From: Milos Gajdos <milosthegajdos@gmail.com>
Date: Sun, 21 Jul 2024 10:48:55 +0100
Subject: [PATCH] auth: fix token verification chain

There was a small regression introduced in
https://github.com/distribution/distribution/pull/4349.

Specifically, if the certificate chain verification succeeds we should
return immediately instead of following up with further token verification
checks.

This commit fixes that: we only follow up with further token
verifications if x5c header is missing.

We've also refactored this method so it's hopefully clearer.

Co-authored-by: Kyle Squizzato <ksquizz@gmail.com>
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
---
 registry/auth/token/token.go | 37 ++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/registry/auth/token/token.go b/registry/auth/token/token.go
index 5a0810698..55d0b7f6f 100644
--- a/registry/auth/token/token.go
+++ b/registry/auth/token/token.go
@@ -162,7 +162,7 @@ func (t *Token) Verify(verifyOpts VerifyOptions) (*ClaimSet, error) {
 }
 
 // VerifySigningKey attempts to verify and return the signing key which was used to sign the token.
-func (t *Token) VerifySigningKey(verifyOpts VerifyOptions) (signingKey crypto.PublicKey, err error) {
+func (t *Token) VerifySigningKey(verifyOpts VerifyOptions) (crypto.PublicKey, error) {
 	if len(t.JWT.Headers) == 0 {
 		return nil, ErrInvalidToken
 	}
@@ -172,26 +172,27 @@ func (t *Token) VerifySigningKey(verifyOpts VerifyOptions) (signingKey crypto.Pu
 	// verifying the first one in the list only at the moment.
 	header := t.JWT.Headers[0]
 
-	signingKey, err = verifyCertChain(header, verifyOpts.Roots)
-	// NOTE(milosgajdos): if the x5c header is missing
-	// the token may have been signed by a JWKS.
-	if err != nil && err != jose.ErrMissingX5cHeader {
-		return
-	}
-
-	switch {
-	case header.JSONWebKey != nil:
-		signingKey, err = verifyJWK(header, verifyOpts)
-	case len(header.KeyID) > 0:
-		signingKey = verifyOpts.TrustedKeys[header.KeyID]
-		if signingKey == nil {
-			err = fmt.Errorf("token signed by untrusted key with ID: %q", header.KeyID)
+	signingKey, err := verifyCertChain(header, verifyOpts.Roots)
+	if err != nil {
+		// NOTE(milosgajdos): if the x5c header is missing
+		// the token may have been signed by a JWKS.
+		if errors.Is(err, jose.ErrMissingX5cHeader) {
+			switch {
+			case header.JSONWebKey != nil:
+				return verifyJWK(header, verifyOpts)
+			case header.KeyID != "":
+				if signingKey, ok := verifyOpts.TrustedKeys[header.KeyID]; ok {
+					return signingKey, nil
+				}
+				return nil, fmt.Errorf("token signed by untrusted key with ID: %q", header.KeyID)
+			default:
+				return nil, ErrInvalidToken
+			}
 		}
-	default:
-		err = ErrInvalidToken
+		return nil, err
 	}
 
-	return
+	return signingKey, nil
 }
 
 func verifyCertChain(header jose.Header, roots *x509.CertPool) (signingKey crypto.PublicKey, err error) {