From 91a8b233219e7148f706a6712af1670c6e055a09 Mon Sep 17 00:00:00 2001 From: David Mihalcik Date: Thu, 15 May 2025 13:02:58 -0400 Subject: [PATCH 01/11] fix(core): Let legacy KAOs use new trust plugins - The code path for KAOs that do not contain key identifier strings did not use the new trust.KeyIndex plugin, if one is provided - Now the code will prefer to use the index, if it is available and the 'keyring' config is not set --- service/internal/security/standard_crypto.go | 27 +++--- service/internal/security/standard_only.go | 2 +- service/kas/access/publicKey.go | 6 ++ service/kas/access/rewrap.go | 33 +++++-- service/kas/access/rewrap_test.go | 91 ++++++++++++++++++++ 5 files changed, 141 insertions(+), 18 deletions(-) diff --git a/service/internal/security/standard_crypto.go b/service/internal/security/standard_crypto.go index d386d60e3f..90105c646c 100644 --- a/service/internal/security/standard_crypto.go +++ b/service/internal/security/standard_crypto.go @@ -351,7 +351,21 @@ func (s StandardCrypto) RSAPublicKeyAsJSON(kid string) (string, error) { } func (s StandardCrypto) GenerateNanoTDFSymmetricKey(kasKID string, ephemeralPublicKeyBytes []byte, curve elliptic.Curve) ([]byte, error) { - ephemeralECDSAPublicKey, err := ocrypto.UncompressECPubKey(curve, ephemeralPublicKeyBytes) + k, ok := s.keysByID[kasKID] + if !ok { + return nil, ErrKeyPairInfoNotFound + } + ec, ok := k.(StandardECCrypto) + if !ok { + return nil, ErrKeyPairInfoMalformed + } + privateKeyPEM := []byte(ec.ecPrivateKeyPem) + + return DeriveNanoTDFSymmetricKey(curve, ephemeralPublicKeyBytes, privateKeyPEM) +} + +func DeriveNanoTDFSymmetricKey(curve elliptic.Curve, clientEphemera []byte, privateKeyPEM []byte) ([]byte, error) { + ephemeralECDSAPublicKey, err := ocrypto.UncompressECPubKey(curve, clientEphemera) if err != nil { return nil, err } @@ -366,16 +380,7 @@ func (s StandardCrypto) GenerateNanoTDFSymmetricKey(kasKID string, ephemeralPubl } ephemeralECDSAPublicKeyPEM := pem.EncodeToMemory(pemBlock) - k, ok := s.keysByID[kasKID] - if !ok { - return nil, ErrKeyPairInfoNotFound - } - ec, ok := k.(StandardECCrypto) - if !ok { - return nil, ErrKeyPairInfoMalformed - } - - symmetricKey, err := ocrypto.ComputeECDHKey([]byte(ec.ecPrivateKeyPem), ephemeralECDSAPublicKeyPEM) + symmetricKey, err := ocrypto.ComputeECDHKey(privateKeyPEM, ephemeralECDSAPublicKeyPEM) if err != nil { return nil, fmt.Errorf("ocrypto.ComputeECDHKey failed: %w", err) } diff --git a/service/internal/security/standard_only.go b/service/internal/security/standard_only.go index 2378a9c38a..6cf529c5d1 100644 --- a/service/internal/security/standard_only.go +++ b/service/internal/security/standard_only.go @@ -3,7 +3,7 @@ package security import "log/slog" type Config struct { - Type string `mapstructure:"type" json:"type" default:"standard"` + Type string `mapstructure:"type" json:"type"` // StandardConfig is the configuration for the standard key provider StandardConfig StandardConfig `mapstructure:"standard" json:"standard"` } diff --git a/service/kas/access/publicKey.go b/service/kas/access/publicKey.go index f9df99f500..ef59f48f5f 100644 --- a/service/kas/access/publicKey.go +++ b/service/kas/access/publicKey.go @@ -21,6 +21,12 @@ const ( ) func (p *Provider) lookupKid(ctx context.Context, algorithm string) (string, error) { + k, err := p.GetKeyIndex().FindKeyByAlgorithm(ctx, algorithm, false) + if err == nil { + return string(k.ID()), nil + } + p.Logger.WarnContext(ctx, "KeyIndex.FindKeyByAlgorithm failed", "err", err) + if len(p.Keyring) == 0 { p.Logger.WarnContext(ctx, "no default keys found", "algorithm", algorithm) return "", connect.NewError(connect.CodeNotFound, errors.Join(ErrConfig, errors.New("no default keys configured"))) diff --git a/service/kas/access/rewrap.go b/service/kas/access/rewrap.go index e7ca422938..309175e598 100644 --- a/service/kas/access/rewrap.go +++ b/service/kas/access/rewrap.go @@ -529,12 +529,7 @@ func (p *Provider) verifyRewrapRequests(ctx context.Context, req *kaspb.Unsigned kid := trust.KeyIdentifier(kao.GetKeyAccessObject().GetKid()) kidsToCheck = []trust.KeyIdentifier{kid} } else { - p.Logger.InfoContext(ctx, "kid free kao") - for _, k := range p.Keyring { - if k.Algorithm == security.AlgorithmRSA2048 && k.Legacy { - kidsToCheck = append(kidsToCheck, trust.KeyIdentifier(k.KID)) - } - } + kidsToCheck = p.listLegacyKeys(ctx) if len(kidsToCheck) == 0 { p.Logger.WarnContext(ctx, "failure to find legacy kids for rsa") failedKAORewrap(results, kao, err400("bad request")) @@ -603,6 +598,32 @@ func (p *Provider) verifyRewrapRequests(ctx context.Context, req *kaspb.Unsigned return policy, results, nil } +func (p *Provider) listLegacyKeys(ctx context.Context) []trust.KeyIdentifier { + var kidsToCheck []trust.KeyIdentifier + p.Logger.InfoContext(ctx, "kid free kao") + if len(p.Keyring) > 0 { + // Using deprecated 'keyring' feature for lookup + for _, k := range p.Keyring { + if k.Algorithm == security.AlgorithmRSA2048 && k.Legacy { + kidsToCheck = append(kidsToCheck, trust.KeyIdentifier(k.KID)) + } + } + return kidsToCheck + } + + k, err := p.GetKeyIndex().ListKeys(ctx) + if err != nil { + p.Logger.WarnContext(ctx, "KeyIndex.ListKeys failed", "err", err) + } else { + for _, key := range k { + if key.Algorithm() == security.AlgorithmRSA2048 && key.IsLegacy() { + kidsToCheck = append(kidsToCheck, key.ID()) + } + } + } + return kidsToCheck +} + func (p *Provider) tdf3Rewrap(ctx context.Context, requests []*kaspb.UnsignedRewrapRequest_WithPolicyRequest, clientPublicKey string, entity *entityInfo) (string, policyKAOResults) { if p.Tracer != nil { var span trace.Span diff --git a/service/kas/access/rewrap_test.go b/service/kas/access/rewrap_test.go index 2cde2952b1..61035897d4 100644 --- a/service/kas/access/rewrap_test.go +++ b/service/kas/access/rewrap_test.go @@ -8,6 +8,7 @@ import ( "encoding/hex" "encoding/json" "encoding/pem" + "errors" "log/slog" "net/http" "testing" @@ -21,6 +22,7 @@ import ( "github.com/opentdf/platform/lib/ocrypto" "github.com/opentdf/platform/service/logger" ctxAuth "github.com/opentdf/platform/service/pkg/auth" + "github.com/opentdf/platform/service/trust" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -29,6 +31,95 @@ import ( "google.golang.org/grpc/metadata" ) +type fakeKeyDetails struct { + id trust.KeyIdentifier + algorithm string + legacy bool +} + +func (f *fakeKeyDetails) ID() trust.KeyIdentifier { return f.id } +func (f *fakeKeyDetails) Algorithm() string { return f.algorithm } +func (f *fakeKeyDetails) IsLegacy() bool { return f.legacy } +func (f *fakeKeyDetails) ExportPublicKey(context.Context, trust.KeyType) (string, error) { + return "", nil +} +func (f *fakeKeyDetails) ExportCertificate(context.Context) (string, error) { return "", nil } +func (f *fakeKeyDetails) System() string { return "" } + +type fakeKeyIndex struct { + keys []trust.KeyDetails + err error +} + +func (f *fakeKeyIndex) FindKeyByAlgorithm(context.Context, string, bool) (trust.KeyDetails, error) { + return nil, nil +} + +func (f *fakeKeyIndex) FindKeyByID(context.Context, trust.KeyIdentifier) (trust.KeyDetails, error) { + return nil, nil +} +func (f *fakeKeyIndex) ListKeys(context.Context) ([]trust.KeyDetails, error) { return f.keys, f.err } + +func TestListLegacyKeys_KeyringPopulated(t *testing.T) { + testLogger := logger.CreateTestLogger() + // Simulate a Provider with Keyring containing legacy RSA keys + p := &Provider{ + Logger: testLogger, + KASConfig: KASConfig{ + Keyring: []CurrentKeyFor{ + {KID: "legacy1", Algorithm: "rsa:2048", Legacy: true}, + {KID: "notlegacy", Algorithm: "rsa:2048", Legacy: false}, + {KID: "legacy2", Algorithm: "rsa:2048", Legacy: true}, + {KID: "legacy3", Algorithm: "ec:secp256r1", Legacy: true}, // not RSA + }, + }, + } + + kids := p.listLegacyKeys(t.Context()) + assert.ElementsMatch(t, []trust.KeyIdentifier{"legacy1", "legacy2"}, kids) +} + +func TestListLegacyKeys_KeyIndexPopulated(t *testing.T) { + testLogger := logger.CreateTestLogger() + fakeKeys := []trust.KeyDetails{ + &fakeKeyDetails{id: "id1", algorithm: "rsa:2048", legacy: true}, + &fakeKeyDetails{id: "id2", algorithm: "rsa:2048", legacy: false}, + &fakeKeyDetails{id: "id3", algorithm: "ec:secp256r1", legacy: true}, + &fakeKeyDetails{id: "id4", algorithm: "rsa:2048", legacy: true}, + } + p := &Provider{ + Logger: testLogger, + KeyIndex: &fakeKeyIndex{ + keys: fakeKeys, + }, + } + kids := p.listLegacyKeys(t.Context()) + assert.ElementsMatch(t, []trust.KeyIdentifier{"id1", "id4"}, kids) +} + +func TestListLegacyKeys_Empty(t *testing.T) { + testLogger := logger.CreateTestLogger() + p := &Provider{ + Logger: testLogger, + KeyIndex: &fakeKeyIndex{}, + } + kids := p.listLegacyKeys(t.Context()) + assert.Empty(t, kids) +} + +func TestListLegacyKeys_KeyIndexError(t *testing.T) { + ctx := context.Background() + testLogger := logger.CreateTestLogger() + p := &Provider{ + Logger: testLogger, + KeyIndex: &fakeKeyIndex{ + err: errors.New("fail"), + }, + } + kids := p.listLegacyKeys(ctx) + assert.Empty(t, kids) +} + const ( ecCert = `-----BEGIN CERTIFICATE----- MIIB5DCBzQIUZsQqf2nfB0JuxsKBwrVjfCVjjmUwDQYJKoZIhvcNAQELBQAwGzEZ From 047dbd7c52ead2dd225261a8793b9d377a4fce0a Mon Sep 17 00:00:00 2001 From: David Mihalcik Date: Thu, 15 May 2025 13:09:02 -0400 Subject: [PATCH 02/11] Update rewrap_test.go --- service/kas/access/rewrap_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/service/kas/access/rewrap_test.go b/service/kas/access/rewrap_test.go index 61035897d4..45de497721 100644 --- a/service/kas/access/rewrap_test.go +++ b/service/kas/access/rewrap_test.go @@ -52,11 +52,11 @@ type fakeKeyIndex struct { } func (f *fakeKeyIndex) FindKeyByAlgorithm(context.Context, string, bool) (trust.KeyDetails, error) { - return nil, nil + return nil, errors.New("not implemented") } func (f *fakeKeyIndex) FindKeyByID(context.Context, trust.KeyIdentifier) (trust.KeyDetails, error) { - return nil, nil + return nil, errors.New("not implemented") } func (f *fakeKeyIndex) ListKeys(context.Context) ([]trust.KeyDetails, error) { return f.keys, f.err } @@ -108,7 +108,6 @@ func TestListLegacyKeys_Empty(t *testing.T) { } func TestListLegacyKeys_KeyIndexError(t *testing.T) { - ctx := context.Background() testLogger := logger.CreateTestLogger() p := &Provider{ Logger: testLogger, @@ -116,7 +115,7 @@ func TestListLegacyKeys_KeyIndexError(t *testing.T) { err: errors.New("fail"), }, } - kids := p.listLegacyKeys(ctx) + kids := p.listLegacyKeys(t.Context()) assert.Empty(t, kids) } From a081bd6c9e2fa6b1faf043be6ae17ce1e0884a0d Mon Sep 17 00:00:00 2001 From: David Mihalcik Date: Thu, 15 May 2025 13:59:14 -0400 Subject: [PATCH 03/11] Update publicKey.go --- service/kas/access/publicKey.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/service/kas/access/publicKey.go b/service/kas/access/publicKey.go index ef59f48f5f..86820a1efd 100644 --- a/service/kas/access/publicKey.go +++ b/service/kas/access/publicKey.go @@ -21,9 +21,12 @@ const ( ) func (p *Provider) lookupKid(ctx context.Context, algorithm string) (string, error) { - k, err := p.GetKeyIndex().FindKeyByAlgorithm(ctx, algorithm, false) - if err == nil { - return string(k.ID()), nil + keyIdx := p.GetKeyIndex() + if keyIdx != nil { + k, err := keyIdx.FindKeyByAlgorithm(ctx, algorithm, false) + if err == nil { + return string(k.ID()), nil + } } p.Logger.WarnContext(ctx, "KeyIndex.FindKeyByAlgorithm failed", "err", err) From 9b289b00cd7e1f87eecd04ced529e1ff60eadbf6 Mon Sep 17 00:00:00 2001 From: David Mihalcik Date: Thu, 15 May 2025 14:02:04 -0400 Subject: [PATCH 04/11] Update tdf-roundtrips.bats --- test/tdf-roundtrips.bats | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/tdf-roundtrips.bats b/test/tdf-roundtrips.bats index 4181d552cc..a721225bc1 100755 --- a/test/tdf-roundtrips.bats +++ b/test/tdf-roundtrips.bats @@ -110,6 +110,8 @@ @test "examples: legacy key support Z-TDF" { echo "[INFO] validating default key is r1" + echo "[INFO] default key result: $(grpcurl "localhost:8080" "kas.AccessService/PublicKey")" + [ "$(grpcurl "localhost:8080" "kas.AccessService/PublicKey" | jq -e -r .kid)" = r1 ] echo "[INFO] encrypting samples" From ceeac0032b9afce248af8f2cb13001199334efbb Mon Sep 17 00:00:00 2001 From: David Mihalcik Date: Thu, 15 May 2025 14:02:48 -0400 Subject: [PATCH 05/11] Update publicKey.go --- service/kas/access/publicKey.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/kas/access/publicKey.go b/service/kas/access/publicKey.go index 86820a1efd..9a561594b7 100644 --- a/service/kas/access/publicKey.go +++ b/service/kas/access/publicKey.go @@ -27,8 +27,8 @@ func (p *Provider) lookupKid(ctx context.Context, algorithm string) (string, err if err == nil { return string(k.ID()), nil } + p.Logger.WarnContext(ctx, "KeyIndex.FindKeyByAlgorithm failed", "err", err) } - p.Logger.WarnContext(ctx, "KeyIndex.FindKeyByAlgorithm failed", "err", err) if len(p.Keyring) == 0 { p.Logger.WarnContext(ctx, "no default keys found", "algorithm", algorithm) From f61b56646d871108efaf4d31577431f0e39fd2b3 Mon Sep 17 00:00:00 2001 From: David Mihalcik Date: Thu, 15 May 2025 14:46:42 -0400 Subject: [PATCH 06/11] cleanups --- service/internal/security/crypto_provider.go | 4 ++ .../internal/security/in_process_provider.go | 17 +++++- service/internal/security/standard_crypto.go | 12 ++++ service/kas/access/provider.go | 61 ++++++++++++------- service/kas/access/publicKey_test.go | 2 +- service/kas/kas.go | 4 -- 6 files changed, 71 insertions(+), 29 deletions(-) diff --git a/service/internal/security/crypto_provider.go b/service/internal/security/crypto_provider.go index 8c535fa550..bb44bd7f80 100644 --- a/service/internal/security/crypto_provider.go +++ b/service/internal/security/crypto_provider.go @@ -16,6 +16,10 @@ type CryptoProvider interface { // Gets some KID associated with a given algorithm. // Returns empty string if none are found. FindKID(alg string) string + + // Gets all KIDs associated with a given algorithm. + ListKeysByAlg(alg string) ([]string, error) + RSAPublicKey(keyID string) (string, error) RSAPublicKeyAsJSON(keyID string) (string, error) RSADecrypt(hash crypto.Hash, keyID string, keyLabel string, ciphertext []byte) ([]byte, error) diff --git a/service/internal/security/in_process_provider.go b/service/internal/security/in_process_provider.go index 5963f2b41c..9fed6834f8 100644 --- a/service/internal/security/in_process_provider.go +++ b/service/internal/security/in_process_provider.go @@ -108,6 +108,8 @@ func convertPEMToJWK(_ string) (string, error) { type InProcessProvider struct { cryptoProvider CryptoProvider logger *slog.Logger + defaultKeys []string + legacyKeys map[string]bool } // KeyDetailsAdapter adapts CryptoProvider to KeyDetails @@ -174,10 +176,17 @@ func (k *KeyDetailsAdapter) ExportCertificate(_ context.Context) (string, error) } // NewSecurityProviderAdapter creates a new adapter that implements SecurityProvider using a CryptoProvider -func NewSecurityProviderAdapter(cryptoProvider CryptoProvider) trust.KeyService { +func NewSecurityProviderAdapter(cryptoProvider CryptoProvider, defaultKeys, legacyKeys []string) trust.KeyService { + legacyKeysMap := make(map[string]bool, len(legacyKeys)) + for _, key := range legacyKeys { + legacyKeysMap[key] = true + } + return &InProcessProvider{ cryptoProvider: cryptoProvider, logger: slog.Default(), + defaultKeys: defaultKeys, + legacyKeys: legacyKeysMap, } } @@ -203,6 +212,7 @@ func (a *InProcessProvider) FindKeyByAlgorithm(_ context.Context, algorithm stri id: trust.KeyIdentifier(kid), algorithm: algorithm, cryptoProvider: a.cryptoProvider, + legacy: a.legacyKeys[kid], }, nil } @@ -216,7 +226,7 @@ func (a *InProcessProvider) FindKeyByID(_ context.Context, id trust.KeyIdentifie return &KeyDetailsAdapter{ id: id, algorithm: alg, - legacy: false, + legacy: a.legacyKeys[string(id)], cryptoProvider: a.cryptoProvider, }, nil } @@ -225,7 +235,7 @@ func (a *InProcessProvider) FindKeyByID(_ context.Context, id trust.KeyIdentifie return &KeyDetailsAdapter{ id: id, algorithm: alg, - legacy: false, + legacy: a.legacyKeys[string(id)], cryptoProvider: a.cryptoProvider, }, nil } @@ -245,6 +255,7 @@ func (a *InProcessProvider) ListKeys(_ context.Context) ([]trust.KeyDetails, err keys = append(keys, &KeyDetailsAdapter{ id: trust.KeyIdentifier(kid), algorithm: alg, + legacy: a.legacyKeys[kid], cryptoProvider: a.cryptoProvider, }) } diff --git a/service/internal/security/standard_crypto.go b/service/internal/security/standard_crypto.go index 90105c646c..af2cda867f 100644 --- a/service/internal/security/standard_crypto.go +++ b/service/internal/security/standard_crypto.go @@ -95,6 +95,18 @@ func NewStandardCrypto(cfg StandardConfig) (*StandardCrypto, error) { } } +func (s StandardCrypto) ListKeysByAlg(alg string) ([]string, error) { + k, ok := s.keysByAlg[alg] + if !ok { + return nil, fmt.Errorf("no key found with algorithm [%s]: %w", alg, ErrCertNotFound) + } + keys := make([]string, 0, len(k)) + for kid := range k { + keys = append(keys, kid) + } + return keys, nil +} + func loadKeys(ks []KeyPairInfo) (*StandardCrypto, error) { keysByAlg := make(map[string]keylist) keysByID := make(keylist) diff --git a/service/kas/access/provider.go b/service/kas/access/provider.go index 1331b10f54..f867c61b25 100644 --- a/service/kas/access/provider.go +++ b/service/kas/access/provider.go @@ -33,34 +33,53 @@ type Provider struct { trace.Tracer } -// GetSecurityProvider returns the SecurityProvider -func (p *Provider) GetSecurityProvider() trust.KeyManager { - // If SecurityProvider is explicitly set, use it - if p.KeyManager != nil { - return p.KeyManager +func (p *Provider) initSecurityProviderAdapter() { + // If the CryptoProvider is set, create a SecurityProviderAdapter + if p.CryptoProvider == nil || p.KeyManager != nil && p.KeyIndex != nil { + return } - - // Otherwise, create an adapter from CryptoProvider if available - if p.CryptoProvider != nil { - return security.NewSecurityProviderAdapter(p.CryptoProvider) + var defaults []string + var legacies []string + if p.KASConfig.Keyring != nil { + for _, key := range p.KASConfig.Keyring { + if key.Legacy { + legacies = append(legacies, key.KID) + } else { + defaults = append(defaults, key.KID) + } + } + } else { + for _, alg := range []string{security.AlgorithmECP256R1, security.AlgorithmRSA2048} { + kid := p.CryptoProvider.FindKID(alg) + if kid != "" { + defaults = append(defaults, kid) + } else { + p.Logger.Warn("no default key found for algorithm", "algorithm", alg) + } + } } - // This shouldn't happen in normal operation - p.Logger.Error("no security provider available") - return nil -} + inProcessService := security.NewSecurityProviderAdapter(p.CryptoProvider, defaults, legacies) -func (p *Provider) GetKeyIndex() trust.KeyIndex { - if p.KeyIndex != nil { - return p.KeyIndex + if p.KeyIndex == nil { + p.Logger.Warn("fallback to in-process key index") + p.KeyIndex = inProcessService } - - if p.CryptoProvider != nil { - return security.NewSecurityProviderAdapter(p.CryptoProvider) + if p.KeyManager == nil { + p.Logger.Error("fallback to in-process manager") + p.KeyManager = inProcessService } +} - p.Logger.Error("no key index available") - return nil +// GetSecurityProvider returns the SecurityProvider +func (p *Provider) GetSecurityProvider() trust.KeyManager { + p.initSecurityProviderAdapter() + return p.KeyManager +} + +func (p *Provider) GetKeyIndex() trust.KeyIndex { + p.initSecurityProviderAdapter() + return p.KeyIndex } type KASConfig struct { diff --git a/service/kas/access/publicKey_test.go b/service/kas/access/publicKey_test.go index 9544db7f09..a053641e48 100644 --- a/service/kas/access/publicKey_test.go +++ b/service/kas/access/publicKey_test.go @@ -335,7 +335,7 @@ func TestStandardCertificateHandlerEmpty(t *testing.T) { kas := Provider{ URI: *kasURI, - KeyManager: security.NewSecurityProviderAdapter(c), + KeyManager: security.NewSecurityProviderAdapter(c, nil, nil), Logger: logger.CreateTestLogger(), Tracer: noop.NewTracerProvider().Tracer(""), } diff --git a/service/kas/kas.go b/service/kas/kas.go index feee42c300..b599cef182 100644 --- a/service/kas/kas.go +++ b/service/kas/kas.go @@ -10,7 +10,6 @@ import ( "github.com/go-viper/mapstructure/v2" kaspb "github.com/opentdf/platform/protocol/go/kas" "github.com/opentdf/platform/protocol/go/kas/kasconnect" - "github.com/opentdf/platform/service/internal/security" "github.com/opentdf/platform/service/kas/access" "github.com/opentdf/platform/service/pkg/config" "github.com/opentdf/platform/service/pkg/serviceregistry" @@ -59,9 +58,6 @@ func NewRegistration() *serviceregistry.Service[kasconnect.AccessServiceHandler] if srp.OTDF.TrustKeyIndex == nil { // Set up both the legacy CryptoProvider and the new SecurityProvider - spa := security.NewSecurityProviderAdapter(srp.OTDF.CryptoProvider) - p.KeyIndex = spa - p.KeyManager = spa kasCfg.UpgradeMapToKeyring(srp.OTDF.CryptoProvider) } else { p.KeyIndex = srp.OTDF.TrustKeyIndex From 77226afdac4be24face92efa2b493662c8796a6b Mon Sep 17 00:00:00 2001 From: David Mihalcik Date: Thu, 15 May 2025 14:56:32 -0400 Subject: [PATCH 07/11] Update provider.go --- service/kas/access/provider.go | 75 +++++++++++++++++----------------- 1 file changed, 37 insertions(+), 38 deletions(-) diff --git a/service/kas/access/provider.go b/service/kas/access/provider.go index f867c61b25..586b3e065b 100644 --- a/service/kas/access/provider.go +++ b/service/kas/access/provider.go @@ -33,44 +33,6 @@ type Provider struct { trace.Tracer } -func (p *Provider) initSecurityProviderAdapter() { - // If the CryptoProvider is set, create a SecurityProviderAdapter - if p.CryptoProvider == nil || p.KeyManager != nil && p.KeyIndex != nil { - return - } - var defaults []string - var legacies []string - if p.KASConfig.Keyring != nil { - for _, key := range p.KASConfig.Keyring { - if key.Legacy { - legacies = append(legacies, key.KID) - } else { - defaults = append(defaults, key.KID) - } - } - } else { - for _, alg := range []string{security.AlgorithmECP256R1, security.AlgorithmRSA2048} { - kid := p.CryptoProvider.FindKID(alg) - if kid != "" { - defaults = append(defaults, kid) - } else { - p.Logger.Warn("no default key found for algorithm", "algorithm", alg) - } - } - } - - inProcessService := security.NewSecurityProviderAdapter(p.CryptoProvider, defaults, legacies) - - if p.KeyIndex == nil { - p.Logger.Warn("fallback to in-process key index") - p.KeyIndex = inProcessService - } - if p.KeyManager == nil { - p.Logger.Error("fallback to in-process manager") - p.KeyManager = inProcessService - } -} - // GetSecurityProvider returns the SecurityProvider func (p *Provider) GetSecurityProvider() trust.KeyManager { p.initSecurityProviderAdapter() @@ -141,6 +103,43 @@ func (kasCfg *KASConfig) UpgradeMapToKeyring(c security.CryptoProvider) { } } +func (p *Provider) initSecurityProviderAdapter() { + // If the CryptoProvider is set, create a SecurityProviderAdapter + if p.CryptoProvider == nil || p.KeyManager != nil && p.KeyIndex != nil { + return + } + var defaults []string + var legacies []string + for _, key := range p.KASConfig.Keyring { + if key.Legacy { + legacies = append(legacies, key.KID) + } else { + defaults = append(defaults, key.KID) + } + } + if len(defaults) == 0 && len(legacies) == 0 { + for _, alg := range []string{security.AlgorithmECP256R1, security.AlgorithmRSA2048} { + kid := p.CryptoProvider.FindKID(alg) + if kid != "" { + defaults = append(defaults, kid) + } else { + p.Logger.Warn("no default key found for algorithm", "algorithm", alg) + } + } + } + + inProcessService := security.NewSecurityProviderAdapter(p.CryptoProvider, defaults, legacies) + + if p.KeyIndex == nil { + p.Logger.Warn("fallback to in-process key index") + p.KeyIndex = inProcessService + } + if p.KeyManager == nil { + p.Logger.Error("fallback to in-process manager") + p.KeyManager = inProcessService + } +} + // If there exists *any* legacy keys, returns empty list. // Otherwise, create a copy with legacy=true for all values func inferLegacyKeys(keys []CurrentKeyFor) []CurrentKeyFor { From 57a3de0eaff398d3fb597c5b4d4ab980dfa46d02 Mon Sep 17 00:00:00 2001 From: David Mihalcik Date: Thu, 15 May 2025 15:38:36 -0400 Subject: [PATCH 08/11] Update kas.go --- service/kas/kas.go | 1 + 1 file changed, 1 insertion(+) diff --git a/service/kas/kas.go b/service/kas/kas.go index b599cef182..bf3c85ca29 100644 --- a/service/kas/kas.go +++ b/service/kas/kas.go @@ -59,6 +59,7 @@ func NewRegistration() *serviceregistry.Service[kasconnect.AccessServiceHandler] if srp.OTDF.TrustKeyIndex == nil { // Set up both the legacy CryptoProvider and the new SecurityProvider kasCfg.UpgradeMapToKeyring(srp.OTDF.CryptoProvider) + p.CryptoProvider = srp.OTDF.CryptoProvider } else { p.KeyIndex = srp.OTDF.TrustKeyIndex p.KeyManager = srp.OTDF.TrustKeyManager From c19affbe56bf23c9e06ad6309bf73c707b884115 Mon Sep 17 00:00:00 2001 From: David Mihalcik Date: Thu, 15 May 2025 16:27:48 -0400 Subject: [PATCH 09/11] Update in_process_provider.go --- .../internal/security/in_process_provider.go | 43 ++++++++++++------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/service/internal/security/in_process_provider.go b/service/internal/security/in_process_provider.go index 9fed6834f8..c10d60b797 100644 --- a/service/internal/security/in_process_provider.go +++ b/service/internal/security/in_process_provider.go @@ -202,18 +202,23 @@ func (a *InProcessProvider) WithLogger(logger *slog.Logger) *InProcessProvider { } // FindKeyByAlgorithm finds a key by algorithm using the underlying CryptoProvider -func (a *InProcessProvider) FindKeyByAlgorithm(_ context.Context, algorithm string, _ bool) (trust.KeyDetails, error) { +func (a *InProcessProvider) FindKeyByAlgorithm(_ context.Context, algorithm string, legacy bool) (trust.KeyDetails, error) { // Get the key ID for this algorithm - kid := a.cryptoProvider.FindKID(algorithm) - if kid == "" { + kids, err := a.cryptoProvider.ListKeysByAlg(algorithm) + if err != nil || len(kids) == 0 { return nil, ErrCertNotFound } - return &KeyDetailsAdapter{ - id: trust.KeyIdentifier(kid), - algorithm: algorithm, - cryptoProvider: a.cryptoProvider, - legacy: a.legacyKeys[kid], - }, nil + for _, kid := range kids { + if legacy != a.legacyKeys[kid] { + return &KeyDetailsAdapter{ + id: trust.KeyIdentifier(kid), + algorithm: algorithm, + cryptoProvider: a.cryptoProvider, + legacy: legacy, + }, nil + } + } + return nil, ErrCertNotFound } // FindKeyByID finds a key by ID @@ -251,13 +256,19 @@ func (a *InProcessProvider) ListKeys(_ context.Context) ([]trust.KeyDetails, err // Try to find keys for known algorithms for _, alg := range []string{AlgorithmRSA2048, AlgorithmECP256R1} { - if kid := a.cryptoProvider.FindKID(alg); kid != "" { - keys = append(keys, &KeyDetailsAdapter{ - id: trust.KeyIdentifier(kid), - algorithm: alg, - legacy: a.legacyKeys[kid], - cryptoProvider: a.cryptoProvider, - }) + if kids, err := a.cryptoProvider.ListKeysByAlg(alg); err == nil && len(kids) > 0 { + for _, kid := range kids { + keys = append(keys, &KeyDetailsAdapter{ + id: trust.KeyIdentifier(kid), + algorithm: alg, + cryptoProvider: a.cryptoProvider, + legacy: a.legacyKeys[kid], + }) + } + } else if err != nil { + if a.logger != nil { + a.logger.Warn("failed to list keys by algorithm", "algorithm", alg, "error", err) + } } } From 16898b8e65b06ee098b7b1aed892bcd3631444b9 Mon Sep 17 00:00:00 2001 From: David Mihalcik Date: Thu, 15 May 2025 16:57:16 -0400 Subject: [PATCH 10/11] fixes for defaults --- service/internal/security/in_process_provider.go | 11 ++++++++--- test/tdf-roundtrips.bats | 3 +++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/service/internal/security/in_process_provider.go b/service/internal/security/in_process_provider.go index c10d60b797..df1b9fe2b5 100644 --- a/service/internal/security/in_process_provider.go +++ b/service/internal/security/in_process_provider.go @@ -108,7 +108,7 @@ func convertPEMToJWK(_ string) (string, error) { type InProcessProvider struct { cryptoProvider CryptoProvider logger *slog.Logger - defaultKeys []string + defaultKeys map[string]bool legacyKeys map[string]bool } @@ -182,10 +182,15 @@ func NewSecurityProviderAdapter(cryptoProvider CryptoProvider, defaultKeys, lega legacyKeysMap[key] = true } + defaultKeysMap := make(map[string]bool, len(defaultKeys)) + for _, key := range defaultKeys { + defaultKeysMap[key] = true + } + return &InProcessProvider{ cryptoProvider: cryptoProvider, logger: slog.Default(), - defaultKeys: defaultKeys, + defaultKeys: defaultKeysMap, legacyKeys: legacyKeysMap, } } @@ -209,7 +214,7 @@ func (a *InProcessProvider) FindKeyByAlgorithm(_ context.Context, algorithm stri return nil, ErrCertNotFound } for _, kid := range kids { - if legacy != a.legacyKeys[kid] { + if legacy && a.legacyKeys[kid] || !legacy && a.defaultKeys[kid] { return &KeyDetailsAdapter{ id: trust.KeyIdentifier(kid), algorithm: algorithm, diff --git a/test/tdf-roundtrips.bats b/test/tdf-roundtrips.bats index a721225bc1..149557ed86 100755 --- a/test/tdf-roundtrips.bats +++ b/test/tdf-roundtrips.bats @@ -128,6 +128,7 @@ wait_for_green echo "[INFO] validating default key is r2" + echo "[INFO] default key result: $(grpcurl "localhost:8080" "kas.AccessService/PublicKey")" [ "$(grpcurl "localhost:8080" "kas.AccessService/PublicKey" | jq -e -r .kid)" = r2 ] echo "[INFO] decrypting after key rotation" @@ -137,6 +138,7 @@ @test "examples: legacy kas and service config format support" { echo "[INFO] validating default key is r1" + echo "[INFO] default key result: $(grpcurl "localhost:8080" "kas.AccessService/PublicKey")" [ "$(grpcurl "localhost:8080" "kas.AccessService/PublicKey" | jq -e -r .kid)" = r1 ] echo "[INFO] encrypting samples" @@ -153,6 +155,7 @@ wait_for_green echo "[INFO] validating default key is r1" + echo "[INFO] default key result: $(grpcurl "localhost:8080" "kas.AccessService/PublicKey")" [ $(grpcurl "localhost:8080" "kas.AccessService/PublicKey" | jq -e -r .kid) = r1 ] echo "[INFO] validating keys are correct by alg" From 0156ff6789e6edd6167460e343ab8db2c36ee60e Mon Sep 17 00:00:00 2001 From: David Mihalcik Date: Fri, 16 May 2025 09:57:50 -0400 Subject: [PATCH 11/11] remove cryptoprovider interface --- service/internal/security/crypto_provider.go | 26 ---------- .../internal/security/in_process_provider.go | 16 +++--- service/internal/security/standard_crypto.go | 50 ++----------------- service/internal/security/standard_only.go | 2 +- service/internal/server/server.go | 2 +- service/kas/access/provider.go | 4 +- service/kas/access/publicKey_test.go | 2 +- 7 files changed, 19 insertions(+), 83 deletions(-) diff --git a/service/internal/security/crypto_provider.go b/service/internal/security/crypto_provider.go index bb44bd7f80..b4c81e8ce2 100644 --- a/service/internal/security/crypto_provider.go +++ b/service/internal/security/crypto_provider.go @@ -1,34 +1,8 @@ package security -import ( - "crypto" - "crypto/elliptic" -) - const ( // Key agreement along P-256 AlgorithmECP256R1 = "ec:secp256r1" // Used for encryption with RSA of the KAO AlgorithmRSA2048 = "rsa:2048" ) - -type CryptoProvider interface { - // Gets some KID associated with a given algorithm. - // Returns empty string if none are found. - FindKID(alg string) string - - // Gets all KIDs associated with a given algorithm. - ListKeysByAlg(alg string) ([]string, error) - - RSAPublicKey(keyID string) (string, error) - RSAPublicKeyAsJSON(keyID string) (string, error) - RSADecrypt(hash crypto.Hash, keyID string, keyLabel string, ciphertext []byte) ([]byte, error) - ECDecrypt(keyID string, ephemeralPublicKey, ciphertext []byte) ([]byte, error) - - ECPublicKey(keyID string) (string, error) - ECCertificate(keyID string) (string, error) - GenerateNanoTDFSymmetricKey(kasKID string, ephemeralPublicKeyBytes []byte, curve elliptic.Curve) ([]byte, error) - GenerateEphemeralKasKeys() (any, []byte, error) - GenerateNanoTDFSessionKey(privateKeyHandle any, ephemeralPublicKey []byte) ([]byte, error) - Close() -} diff --git a/service/internal/security/in_process_provider.go b/service/internal/security/in_process_provider.go index df1b9fe2b5..1e6a83193d 100644 --- a/service/internal/security/in_process_provider.go +++ b/service/internal/security/in_process_provider.go @@ -106,7 +106,7 @@ func convertPEMToJWK(_ string) (string, error) { // InProcessProvider adapts a CryptoProvider to the SecurityProvider interface type InProcessProvider struct { - cryptoProvider CryptoProvider + cryptoProvider *StandardCrypto logger *slog.Logger defaultKeys map[string]bool legacyKeys map[string]bool @@ -117,7 +117,7 @@ type KeyDetailsAdapter struct { id trust.KeyIdentifier algorithm string legacy bool - cryptoProvider CryptoProvider + cryptoProvider *StandardCrypto } // Mode returns the mode of the key details @@ -176,7 +176,7 @@ func (k *KeyDetailsAdapter) ExportCertificate(_ context.Context) (string, error) } // NewSecurityProviderAdapter creates a new adapter that implements SecurityProvider using a CryptoProvider -func NewSecurityProviderAdapter(cryptoProvider CryptoProvider, defaultKeys, legacyKeys []string) trust.KeyService { +func NewSecurityProviderAdapter(cryptoProvider *StandardCrypto, defaultKeys, legacyKeys []string) trust.KeyService { legacyKeysMap := make(map[string]bool, len(legacyKeys)) for _, key := range legacyKeys { legacyKeysMap[key] = true @@ -206,10 +206,12 @@ func (a *InProcessProvider) WithLogger(logger *slog.Logger) *InProcessProvider { return a } -// FindKeyByAlgorithm finds a key by algorithm using the underlying CryptoProvider +// FindKeyByAlgorithm finds a key by algorithm using the underlying CryptoProvider. +// This will only return default keys if legacy is false. +// If legacy is true, it will return the first legacy key found that matches the algorithm. func (a *InProcessProvider) FindKeyByAlgorithm(_ context.Context, algorithm string, legacy bool) (trust.KeyDetails, error) { // Get the key ID for this algorithm - kids, err := a.cryptoProvider.ListKeysByAlg(algorithm) + kids, err := a.cryptoProvider.ListKIDsByAlgorithm(algorithm) if err != nil || len(kids) == 0 { return nil, ErrCertNotFound } @@ -261,7 +263,7 @@ func (a *InProcessProvider) ListKeys(_ context.Context) ([]trust.KeyDetails, err // Try to find keys for known algorithms for _, alg := range []string{AlgorithmRSA2048, AlgorithmECP256R1} { - if kids, err := a.cryptoProvider.ListKeysByAlg(alg); err == nil && len(kids) > 0 { + if kids, err := a.cryptoProvider.ListKIDsByAlgorithm(alg); err == nil && len(kids) > 0 { for _, kid := range kids { keys = append(keys, &KeyDetailsAdapter{ id: trust.KeyIdentifier(kid), @@ -302,7 +304,7 @@ func (a *InProcessProvider) Decrypt(ctx context.Context, keyID trust.KeyIdentifi if len(ephemeralPublicKey) == 0 { return nil, errors.New("ephemeral public key is required for EC decryption") } - rawKey, err = a.cryptoProvider.ECDecrypt(kid, ephemeralPublicKey, ciphertext) + rawKey, err = a.cryptoProvider.ECDecrypt(ctx, kid, ephemeralPublicKey, ciphertext) default: return nil, errors.New("unsupported key algorithm") diff --git a/service/internal/security/standard_crypto.go b/service/internal/security/standard_crypto.go index af2cda867f..401100cf73 100644 --- a/service/internal/security/standard_crypto.go +++ b/service/internal/security/standard_crypto.go @@ -95,7 +95,9 @@ func NewStandardCrypto(cfg StandardConfig) (*StandardCrypto, error) { } } -func (s StandardCrypto) ListKeysByAlg(alg string) ([]string, error) { +// ListKIDsByAlgorithm returns a list of key identifiers for the specified algorithm +// Errors if no keys are found of the requested algorithm. +func (s StandardCrypto) ListKIDsByAlgorithm(alg string) ([]string, error) { k, ok := s.keysByAlg[alg] if !ok { return nil, fmt.Errorf("no key found with algorithm [%s]: %w", alg, ErrCertNotFound) @@ -405,48 +407,6 @@ func DeriveNanoTDFSymmetricKey(curve elliptic.Curve, clientEphemera []byte, priv return key, nil } -func (s StandardCrypto) GenerateEphemeralKasKeys() (any, []byte, error) { - ephemeralKeyPair, err := ocrypto.NewECKeyPair(ocrypto.ECCModeSecp256r1) - if err != nil { - return nil, nil, fmt.Errorf("ocrypto.NewECKeyPair failed: %w", err) - } - - pubKeyInPem, err := ephemeralKeyPair.PublicKeyInPemFormat() - if err != nil { - return nil, nil, fmt.Errorf("failed to get public key in PEM format: %w", err) - } - pubKeyBytes := []byte(pubKeyInPem) - - privKey, err := ocrypto.ConvertToECDHPrivateKey(ephemeralKeyPair.PrivateKey) - if err != nil { - return nil, nil, fmt.Errorf("failed to convert provate key to ECDH: %w", err) - } - return privKey, pubKeyBytes, nil -} - -func (s StandardCrypto) GenerateNanoTDFSessionKey(privateKey any, ephemeralPublicKeyPEM []byte) ([]byte, error) { - ecdhKey, err := ocrypto.ConvertToECDHPrivateKey(privateKey) - if err != nil { - return nil, fmt.Errorf("GenerateNanoTDFSessionKey failed to ConvertToECDHPrivateKey: %w", err) - } - ephemeralECDHPublicKey, err := ocrypto.ECPubKeyFromPem(ephemeralPublicKeyPEM) - if err != nil { - return nil, fmt.Errorf("GenerateNanoTDFSessionKey failed to ocrypto.ECPubKeyFromPem: %w", err) - } - // shared secret - sessionKey, err := ecdhKey.ECDH(ephemeralECDHPublicKey) - if err != nil { - return nil, fmt.Errorf("GenerateNanoTDFSessionKey failed to ecdhKey.ECDH: %w", err) - } - - salt := versionSalt() - derivedKey, err := ocrypto.CalculateHKDF(salt, sessionKey) - if err != nil { - return nil, fmt.Errorf("ocrypto.CalculateHKDF failed:%w", err) - } - return derivedKey, nil -} - func (s StandardCrypto) Close() { } @@ -464,8 +424,8 @@ func versionSalt() []byte { } // ECDecrypt uses hybrid ECIES to decrypt the data. -func (s *StandardCrypto) ECDecrypt(keyID string, ephemeralPublicKey, ciphertext []byte) ([]byte, error) { - unwrappedKey, err := s.Decrypt(context.Background(), trust.KeyIdentifier(keyID), ciphertext, ephemeralPublicKey) +func (s *StandardCrypto) ECDecrypt(ctx context.Context, keyID string, ephemeralPublicKey, ciphertext []byte) ([]byte, error) { + unwrappedKey, err := s.Decrypt(ctx, trust.KeyIdentifier(keyID), ciphertext, ephemeralPublicKey) if err != nil { return nil, err } diff --git a/service/internal/security/standard_only.go b/service/internal/security/standard_only.go index 6cf529c5d1..11fd22415d 100644 --- a/service/internal/security/standard_only.go +++ b/service/internal/security/standard_only.go @@ -12,7 +12,7 @@ func (c Config) IsEmpty() bool { return c.Type == "" && c.StandardConfig.IsEmpty() } -func NewCryptoProvider(cfg Config) (CryptoProvider, error) { +func NewCryptoProvider(cfg Config) (*StandardCrypto, error) { switch cfg.Type { case "hsm": slog.Error("opentdf hsm mode has been removed") diff --git a/service/internal/server/server.go b/service/internal/server/server.go index 8d6eae604c..816764f288 100644 --- a/service/internal/server/server.go +++ b/service/internal/server/server.go @@ -133,7 +133,7 @@ type OpenTDFServer struct { TrustKeyManager trust.KeyManager // To Deprecate: Use the TrustKeyIndex and TrustKeyManager instead - CryptoProvider security.CryptoProvider + CryptoProvider *security.StandardCrypto logger *logger.Logger } diff --git a/service/kas/access/provider.go b/service/kas/access/provider.go index 586b3e065b..ea9f09bae2 100644 --- a/service/kas/access/provider.go +++ b/service/kas/access/provider.go @@ -26,7 +26,7 @@ type Provider struct { KeyIndex trust.KeyIndex KeyManager trust.KeyManager // Deprecated: Use SecurityProvider instead - CryptoProvider security.CryptoProvider // Kept for backward compatibility + CryptoProvider *security.StandardCrypto // Kept for backward compatibility Logger *logger.Logger Config *config.ServiceConfig KASConfig @@ -73,7 +73,7 @@ func (p *Provider) IsReady(ctx context.Context) error { return nil } -func (kasCfg *KASConfig) UpgradeMapToKeyring(c security.CryptoProvider) { +func (kasCfg *KASConfig) UpgradeMapToKeyring(c *security.StandardCrypto) { switch { case kasCfg.ECCertID != "" && len(kasCfg.Keyring) > 0: panic("invalid kas cfg: please specify keyring or eccertid, not both") diff --git a/service/kas/access/publicKey_test.go b/service/kas/access/publicKey_test.go index a053641e48..51919ee05c 100644 --- a/service/kas/access/publicKey_test.go +++ b/service/kas/access/publicKey_test.go @@ -345,7 +345,7 @@ func TestStandardCertificateHandlerEmpty(t *testing.T) { assert.Nil(t, result) } -func mustNewCryptoProvider(t *testing.T, configStandard security.Config) security.CryptoProvider { +func mustNewCryptoProvider(t *testing.T, configStandard security.Config) *security.StandardCrypto { c, err := security.NewCryptoProvider(configStandard) require.NoError(t, err) require.NotNil(t, c)