mirrors/versitygw
1
0
Fork 0
mirror of https://github.com/versity/versitygw.git synced 2026-09-22 07:54:14 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: restore IPA KRA compatibility and fix JWK parse panic

Browse Source 
`293d9f50` migrated `auth/iam_ipa.go`'s KRA session-key wrap from `rsa.EncryptPKCS1v15` to `rsa.EncryptOAEP` to silence a Go 1.26 deprecation warning. FreeIPA's KRA can be configured to unwrap session keys with either PKCS#1 v1.5 (its default) or OAEP, and the REST API has no way to query which one a given deployment uses — the same constraint FreeIPA's own client (`ipaclient/plugins/vault.py`) works around by trying one padding and falling back to the other. Hardcoding OAEP with no fallback breaks `GetUserAccount` against any KRA using the default PKCS#1 v1.5 configuration, which includes every deployment that worked before that change.

This restores compatibility by trying PKCS#1 v1.5 first, matching the pre-`293d9f50` behavior, and falling back to OAEP on failure so FIPS-mode KRAs (which reject PKCS#1 v1.5) keep working too.

Separately, `293d9f50` also changed `iamapi/internal/iamutil/webidentity.go`'s OIDC JWKS parsing to build EC public keys via `ecdsa.ParseUncompressedPublicKey` instead of setting `ecdsa.PublicKey`'s `X`/`Y` fields directly — a real improvement, since it validates the point is on the curve, which the old code never did. But it writes the JWK's `x`/`y` coordinates into a fixed-size buffer via `big.Int.FillBytes` without checking their length first, so an oversized `x` or `y` in a JWKS response panics instead of returning an error. That JWKS is fetched from the OIDC issuer configured on a role's trust policy, so a malformed or compromised response can crash request handling for `AssumeRoleWithWebIdentity`. This adds a bounds check before the `FillBytes` calls, plus `TestJwkPublicKeyEC` covering both the valid round-trip and the oversized-coordinate case, since `jwk.publicKey()`'s EC branch had no prior test coverage.
This commit is contained in:
niksis02
2026-09-09 18:31:12 +04:00
parent 7c49bb4513
commit d68efb6df9
3 changed files with 104 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files
auth/iam_ipa.go
+52 -20
View File
@@ -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
+7 -3
View File
@@ -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
+45
View File
@@ -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")
}
})
}