From e89dddd4af67d34e441ec1733bdb22ce725d621c Mon Sep 17 00:00:00 2001
From: Monis Khan <i@monis.app>
Date: Tue, 27 Feb 2024 12:40:59 -0500
Subject: [PATCH] jwt: strictly support compact serialization only

Signed-off-by: Monis Khan <mok@microsoft.com>
---
 pkg/serviceaccount/jwt.go                     |  8 +++
 pkg/serviceaccount/jwt_test.go                | 67 ++++++++++++++++---
 .../pkg/authenticator/token/oidc/oidc.go      |  3 +
 3 files changed, 67 insertions(+), 11 deletions(-)

diff --git a/pkg/serviceaccount/jwt.go b/pkg/serviceaccount/jwt.go
index 57ae0a59b99..45bf3828074 100644
--- a/pkg/serviceaccount/jwt.go
+++ b/pkg/serviceaccount/jwt.go
@@ -291,6 +291,11 @@ func (j *jwtTokenAuthenticator) AuthenticateToken(ctx context.Context, tokenData
 		return nil, false, utilerrors.NewAggregate(errlist)
 	}
 
+	// sanity check issuer since we parsed it out before signature validation
+	if !j.issuers[public.Issuer] {
+		return nil, false, fmt.Errorf("token issuer %q is invalid", public.Issuer)
+	}
+
 	tokenAudiences := authenticator.Audiences(public.Audience)
 	if len(tokenAudiences) == 0 {
 		// only apiserver audiences are allowed for legacy tokens
@@ -330,6 +335,9 @@ func (j *jwtTokenAuthenticator) AuthenticateToken(ctx context.Context, tokenData
 // Note: go-jose currently does not allow access to unverified JWS payloads.
 // See https://github.com/square/go-jose/issues/169
 func (j *jwtTokenAuthenticator) hasCorrectIssuer(tokenData string) bool {
+	if strings.HasPrefix(strings.TrimSpace(tokenData), "{") {
+		return false
+	}
 	parts := strings.Split(tokenData, ".")
 	if len(parts) != 3 {
 		return false
diff --git a/pkg/serviceaccount/jwt_test.go b/pkg/serviceaccount/jwt_test.go
index 629bb8496ab..f8e41079eb4 100644
--- a/pkg/serviceaccount/jwt_test.go
+++ b/pkg/serviceaccount/jwt_test.go
@@ -18,6 +18,8 @@ package serviceaccount_test
 
 import (
 	"context"
+	"encoding/base64"
+	"encoding/json"
 	"fmt"
 	"reflect"
 	"strings"
@@ -200,23 +202,16 @@ func TestTokenGenerateAndValidate(t *testing.T) {
 	checkJSONWebSignatureHasKeyID(t, invalidAutoSecretToken, rsaKeyID)
 
 	// Generate the ECDSA token
-	ecdsaGenerator, err := serviceaccount.JWTTokenGenerator(serviceaccount.LegacyIssuer, getPrivateKey(ecdsaPrivateKey))
-	if err != nil {
-		t.Fatalf("error making generator: %v", err)
-	}
-	ecdsaToken, err := ecdsaGenerator.GenerateToken(serviceaccount.LegacyClaims(*serviceAccount, *ecdsaSecret))
-	if err != nil {
-		t.Fatalf("error generating token: %v", err)
-	}
-	if len(ecdsaToken) == 0 {
-		t.Fatalf("no token generated")
-	}
+	ecdsaToken := generateECDSAToken(t, serviceaccount.LegacyIssuer, serviceAccount, ecdsaSecret)
+
 	ecdsaSecret.Data = map[string][]byte{
 		"token": []byte(ecdsaToken),
 	}
 
 	checkJSONWebSignatureHasKeyID(t, ecdsaToken, ecdsaKeyID)
 
+	ecdsaTokenMalformedIss := generateECDSATokenWithMalformedIss(t, serviceAccount, ecdsaSecret)
+
 	// Generate signer with same keys as RSA signer but different unrecognized issuer
 	badIssuerGenerator, err := serviceaccount.JWTTokenGenerator("foo", getPrivateKey(rsaPrivateKey))
 	if err != nil {
@@ -356,6 +351,13 @@ func TestTokenGenerateAndValidate(t *testing.T) {
 			Keys:        []interface{}{getPublicKey(rsaPublicKey)},
 			ExpectedErr: true,
 		},
+		"malformed iss": {
+			Token:       ecdsaTokenMalformedIss,
+			Client:      nil,
+			Keys:        []interface{}{getPublicKey(ecdsaPublicKey)},
+			ExpectedErr: false,
+			ExpectedOK:  false,
+		},
 	}
 
 	for k, tc := range testCases {
@@ -457,3 +459,46 @@ func (f *fakeIndexer) GetByKey(key string) (interface{}, bool, error) {
 	obj, err := f.get(namespace, name)
 	return obj, err == nil, err
 }
+
+func generateECDSAToken(t *testing.T, iss string, serviceAccount *v1.ServiceAccount, ecdsaSecret *v1.Secret) string {
+	t.Helper()
+
+	ecdsaGenerator, err := serviceaccount.JWTTokenGenerator(iss, getPrivateKey(ecdsaPrivateKey))
+	if err != nil {
+		t.Fatalf("error making generator: %v", err)
+	}
+	ecdsaToken, err := ecdsaGenerator.GenerateToken(serviceaccount.LegacyClaims(*serviceAccount, *ecdsaSecret))
+	if err != nil {
+		t.Fatalf("error generating token: %v", err)
+	}
+	if len(ecdsaToken) == 0 {
+		t.Fatalf("no token generated")
+	}
+
+	return ecdsaToken
+}
+
+func generateECDSATokenWithMalformedIss(t *testing.T, serviceAccount *v1.ServiceAccount, ecdsaSecret *v1.Secret) string {
+	t.Helper()
+
+	ecdsaToken := generateECDSAToken(t, "panda", serviceAccount, ecdsaSecret)
+
+	ecdsaTokenJWS, err := jose.ParseSigned(ecdsaToken)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	dataFullSerialize := map[string]any{}
+	if err := json.Unmarshal([]byte(ecdsaTokenJWS.FullSerialize()), &dataFullSerialize); err != nil {
+		t.Fatal(err)
+	}
+
+	dataFullSerialize["malformed_iss"] = "." + base64.RawURLEncoding.EncodeToString([]byte(`{"iss":"bar"}`)) + "."
+
+	out, err := json.Marshal(dataFullSerialize)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	return string(out)
+}
diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go
index 0b1f682c530..b0aa1f5c8af 100644
--- a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go
+++ b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go
@@ -342,6 +342,9 @@ func New(opts Options) (*Authenticator, error) {
 // or returns an error if the token can not be parsed.  Since the JWT is not
 // verified, the returned issuer should not be trusted.
 func untrustedIssuer(token string) (string, error) {
+	if strings.HasPrefix(strings.TrimSpace(token), "{") {
+		return "", fmt.Errorf("token is not compact JWT")
+	}
 	parts := strings.Split(token, ".")
 	if len(parts) != 3 {
 		return "", fmt.Errorf("malformed token")