Merge pull request #2379 from versity/sis/restore-ipa-kra-compat-jwk-panic-fix

fix: restore IPA KRA compatibility and fix JWK parse panic

auth/iam_ipa.go

@@ -169,27 +169,19 @@ func (ipa *IpaIAMService) GetUserAccount(access string) (Account, error) {
 		return account, fmt.Errorf("ipa cannot generate session key: %w", err)
 	}
 
-	encryptedKey, err := rsa.EncryptOAEP(sha256.New(), rand.Reader, ipa.kraTransportKey, session_key, nil)
+	// FreeIPA's KRA can be configured to unwrap session keys with either
+	// RSA PKCS#1 v1.5 (its default) or RSA-OAEP (used under FIPS mode), and
+	// the REST API gives no way to tell which one a given deployment uses.
+	// FreeIPA's own client hits the same wall - see the use_oaep fallback in
+	// ipaclient/plugins/vault.py's _do_internal - so try the default first
+	// and fall back to OAEP so both KRA configurations keep working.
+	data, err := ipa.retrieveVaultSecret(access, session_key, false)
 	if err != nil {
-		return account, fmt.Errorf("ipa vault secret retrieval: %w", err)
-	}
-
-	req, err = ipa.newRequest("vault_retrieve_internal/1", []string{ipa.vaultName},
-		map[string]any{"username": access,
-			"session_key":   Base64EncodedWrapped(encryptedKey),
-			"wrapping_algo": "aes-128-cbc"})
-	if err != nil {
-		return Account{}, fmt.Errorf("ipa vault_retrieve_internal: %w", err)
-	}
-
-	data := struct {
-		Vault_data Base64EncodedWrapped
-		Nonce      Base64EncodedWrapped
-	}{}
-
-	err = ipa.rpc(req, &data)
-	if err != nil {
-		return account, err
+		debuglogger.IAMLogf("ipa vault_retrieve_internal with PKCS1v15 session key wrap failed, retrying with OAEP: %v", err)
+		data, err = ipa.retrieveVaultSecret(access, session_key, true)
+		if err != nil {
+			return account, err
+		}
 	}
 
 	aes, err := aes.NewCipher(session_key)
@@ -212,6 +204,46 @@ func (ipa *IpaIAMService) GetUserAccount(access string) (Account, error) {
 	return account, nil
 }
 
+type vaultSecretData struct {
+	Vault_data Base64EncodedWrapped
+	Nonce      Base64EncodedWrapped
+}
+
+// retrieveVaultSecret RSA-wraps sessionKey with the KRA transport key and
+// calls vault_retrieve_internal to fetch the account secret it protects.
+// useOAEP selects RSA-OAEP wrapping instead of the PKCS#1 v1.5 default.
+func (ipa *IpaIAMService) retrieveVaultSecret(access string, sessionKey []byte, useOAEP bool) (vaultSecretData, error) {
+	var (
+		encryptedKey []byte
+		err          error
+	)
+	if useOAEP {
+		encryptedKey, err = rsa.EncryptOAEP(sha256.New(), rand.Reader, ipa.kraTransportKey, sessionKey, nil)
+	} else {
+		//lint:ignore SA1019 Reason: PKCS#1 v1.5 is the FreeIPA KRA's default
+		//session key wrapping scheme and required for protocol compatibility
+		//with it; see the fallback this feeds in GetUserAccount.
+		encryptedKey, err = rsa.EncryptPKCS1v15(rand.Reader, ipa.kraTransportKey, sessionKey)
+	}
+	if err != nil {
+		return vaultSecretData{}, fmt.Errorf("ipa vault secret retrieval: %w", err)
+	}
+
+	req, err := ipa.newRequest("vault_retrieve_internal/1", []string{ipa.vaultName},
+		map[string]any{"username": access,
+			"session_key":   Base64EncodedWrapped(encryptedKey),
+			"wrapping_algo": "aes-128-cbc"})
+	if err != nil {
+		return vaultSecretData{}, fmt.Errorf("ipa vault_retrieve_internal: %w", err)
+	}
+
+	var data vaultSecretData
+	if err := ipa.rpc(req, &data); err != nil {
+		return vaultSecretData{}, err
+	}
+	return data, nil
+}
+
 // ResolveAccounts returns the subset of accessKeyIDs that do not exist.
 func (ipa *IpaIAMService) ResolveAccounts(accessKeyIDs []string) ([]string, error) {
 	return resolveAccountsByLookup(accessKeyIDs, ipa.GetUserAccount)

iamapi/internal/iamutil/webidentity.go

@@ -543,12 +543,16 @@ func (k jwk) publicKey() (any, error) {
 		if err != nil {
 			return nil, fmt.Errorf("decode EC y: %w", err)
 		}
+		byteLen := (curve.Params().BitSize + 7) / 8
+		if len(xb) > byteLen || len(yb) > byteLen {
+			return nil, fmt.Errorf("EC coordinate too large for curve %q", k.Crv)
+		}
 		x := new(big.Int).SetBytes(xb)
 		y := new(big.Int).SetBytes(yb)
-		keyBytes := make([]byte, 1+(curve.Params().BitSize+7)/8*2)
+		keyBytes := make([]byte, 1+byteLen*2)
 		keyBytes[0] = 0x04
-		xBytes := x.FillBytes(make([]byte, (curve.Params().BitSize+7)/8))
-		yBytes := y.FillBytes(make([]byte, (curve.Params().BitSize+7)/8))
+		xBytes := x.FillBytes(make([]byte, byteLen))
+		yBytes := y.FillBytes(make([]byte, byteLen))
 		copy(keyBytes[1:1+len(xBytes)], xBytes)
 		copy(keyBytes[1+len(xBytes):], yBytes)
 		return ecdsa.ParseUncompressedPublicKey(curve, keyBytes)

iamapi/internal/iamutil/webidentity_test.go

@@ -16,6 +16,8 @@ package iamutil
 
 import (
 	"context"
+	"crypto/ecdsa"
+	"crypto/elliptic"
 	"crypto/rand"
 	"crypto/rsa"
 	"crypto/tls"
@@ -627,3 +629,46 @@ func TestForceRefreshJWKSCacheGatesFailedAttempts(t *testing.T) {
 		t.Errorf("forceRefreshJWKSCache re-attempted a fetch within jwksMinForcedRefreshInterval: lastForcedRefresh changed from %v to %v", before, after)
 	}
 }
+
+func TestJwkPublicKeyEC(t *testing.T) {
+	priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Fatalf("generate EC key: %v", err)
+	}
+	byteLen := (priv.Curve.Params().BitSize + 7) / 8
+	// Uncompressed point: 0x04 || X || Y. Derived via PublicKey.Bytes
+	// rather than the deprecated X/Y fields directly.
+	pointBytes, err := priv.PublicKey.Bytes()
+	if err != nil {
+		t.Fatalf("encode EC public key: %v", err)
+	}
+	validX := base64.RawURLEncoding.EncodeToString(pointBytes[1 : 1+byteLen])
+	validY := base64.RawURLEncoding.EncodeToString(pointBytes[1+byteLen:])
+
+	t.Run("valid coordinates round-trip to the same key", func(t *testing.T) {
+		k := jwk{Kty: "EC", Crv: "P-256", X: validX, Y: validY}
+		pub, err := k.publicKey()
+		if err != nil {
+			t.Fatalf("publicKey() error = %v", err)
+		}
+		ecPub, ok := pub.(*ecdsa.PublicKey)
+		if !ok {
+			t.Fatalf("publicKey() returned %T, want *ecdsa.PublicKey", pub)
+		}
+		if !ecPub.Equal(&priv.PublicKey) {
+			t.Fatalf("publicKey() returned a key that doesn't match the source key")
+		}
+	})
+
+	// Regression test: an oversized "x"/"y" JWK field (no length check
+	// existed before) used to panic in big.Int.FillBytes instead of
+	// returning an error, which took down the request instead of failing
+	// cleanly with an "invalid identity token" style error.
+	t.Run("oversized coordinate errors instead of panicking", func(t *testing.T) {
+		oversized := base64.RawURLEncoding.EncodeToString(make([]byte, byteLen+1))
+		k := jwk{Kty: "EC", Crv: "P-256", X: oversized, Y: validY}
+		if _, err := k.publicKey(); err == nil {
+			t.Fatal("publicKey() error = nil, want an error for an oversized x coordinate")
+		}
+	})
+}