From 26827e21234f0bca2f835e7307ae533988433798 Mon Sep 17 00:00:00 2001 From: strantalis Date: Tue, 29 Jul 2025 16:21:12 -0400 Subject: [PATCH 1/7] feat(ocrypto): add AES protected key interface and implementation Add ProtectedKey and Encapsulator interfaces to lib/ocrypto package, along with AESProtectedKey implementation. This exposes previously internal cryptographic functionality for external consumption. - Add interfaces.go with ProtectedKey and Encapsulator interfaces - Add protected_key.go with AESProtectedKey implementation - Add comprehensive test suite in protected_key_test.go - Use standard error types without logging dependencies --- lib/ocrypto/interfaces.go | 30 +++++ lib/ocrypto/protected_key.go | 91 +++++++++++++ lib/ocrypto/protected_key_test.go | 208 ++++++++++++++++++++++++++++++ 3 files changed, 329 insertions(+) create mode 100644 lib/ocrypto/interfaces.go create mode 100644 lib/ocrypto/protected_key.go create mode 100644 lib/ocrypto/protected_key_test.go diff --git a/lib/ocrypto/interfaces.go b/lib/ocrypto/interfaces.go new file mode 100644 index 000000000..7c2199459 --- /dev/null +++ b/lib/ocrypto/interfaces.go @@ -0,0 +1,30 @@ +package ocrypto + +import ( + "context" +) + +// Encapsulator interface for key encapsulation operations +type Encapsulator interface { + // Encrypt wraps a secret key with the encapsulation key + Encrypt(data []byte) ([]byte, error) + + // PublicKeyInPemFormat Returns public key in pem format, or the empty string if not present + PublicKeyInPemFormat() (string, error) + + // For EC schemes, this method returns the public part of the ephemeral key. + // Otherwise, it returns nil. + EphemeralKey() []byte +} + +// ProtectedKey represents a decrypted key with operations that can be performed on it +type ProtectedKey interface { + // VerifyBinding checks if the policy binding matches the given policy data + VerifyBinding(ctx context.Context, policy, binding []byte) error + + // Export returns the raw key data, optionally encrypting it with the provided encryptor + Export(encryptor Encapsulator) ([]byte, error) + + // Used to decrypt encrypted policies and metadata + DecryptAESGCM(iv []byte, body []byte, tagSize int) ([]byte, error) +} \ No newline at end of file diff --git a/lib/ocrypto/protected_key.go b/lib/ocrypto/protected_key.go new file mode 100644 index 000000000..ed21e4d85 --- /dev/null +++ b/lib/ocrypto/protected_key.go @@ -0,0 +1,91 @@ +package ocrypto + +import ( + "context" + "crypto/hmac" + "crypto/sha256" + "errors" + "fmt" +) + +var ( + // ErrEmptyKeyData is returned when the key data is empty + ErrEmptyKeyData = errors.New("key data is empty") + // ErrPolicyHMACMismatch is returned when policy binding verification fails + ErrPolicyHMACMismatch = errors.New("policy hmac mismatch") + // ErrHMACGeneration is returned when HMAC digest generation fails + ErrHMACGeneration = errors.New("failed to generate hmac digest") +) + +// AESProtectedKey implements the ProtectedKey interface with an in-memory secret key +type AESProtectedKey struct { + rawKey []byte +} + +var _ ProtectedKey = (*AESProtectedKey)(nil) + +// NewAESProtectedKey creates a new instance of AESProtectedKey +func NewAESProtectedKey(rawKey []byte) *AESProtectedKey { + return &AESProtectedKey{ + rawKey: rawKey, + } +} + +// DecryptAESGCM decrypts data using AES-GCM with the protected key +func (k *AESProtectedKey) DecryptAESGCM(iv []byte, body []byte, tagSize int) ([]byte, error) { + aesGcm, err := NewAESGcm(k.rawKey) + if err != nil { + return nil, fmt.Errorf("failed to create AES-GCM cipher: %w", err) + } + + decryptedData, err := aesGcm.DecryptWithIVAndTagSize(iv, body, tagSize) + if err != nil { + return nil, fmt.Errorf("AES-GCM decryption failed: %w", err) + } + + return decryptedData, nil +} + +// Export returns the raw key data, optionally encrypting it with the provided Encapsulator +func (k *AESProtectedKey) Export(encapsulator Encapsulator) ([]byte, error) { + if encapsulator == nil { + // Return raw key data without encryption - caller should be aware of this + return k.rawKey, nil + } + + // Encrypt the key data before returning + encryptedKey, err := encapsulator.Encrypt(k.rawKey) + if err != nil { + return nil, fmt.Errorf("failed to encrypt key data for export: %w", err) + } + + return encryptedKey, nil +} + +// VerifyBinding checks if the policy binding matches the given policy data +func (k *AESProtectedKey) VerifyBinding(ctx context.Context, policy, policyBinding []byte) error { + if len(k.rawKey) == 0 { + return ErrEmptyKeyData + } + + actualHMAC, err := k.generateHMACDigest(ctx, policy) + if err != nil { + return fmt.Errorf("unable to generate policy hmac: %w", err) + } + + if !hmac.Equal(actualHMAC, policyBinding) { + return ErrPolicyHMACMismatch + } + + return nil +} + +// generateHMACDigest is a helper to generate an HMAC digest from a message using the key +func (k *AESProtectedKey) generateHMACDigest(ctx context.Context, msg []byte) ([]byte, error) { + mac := hmac.New(sha256.New, k.rawKey) + _, err := mac.Write(msg) + if err != nil { + return nil, ErrHMACGeneration + } + return mac.Sum(nil), nil +} \ No newline at end of file diff --git a/lib/ocrypto/protected_key_test.go b/lib/ocrypto/protected_key_test.go new file mode 100644 index 000000000..f0e7d6ffd --- /dev/null +++ b/lib/ocrypto/protected_key_test.go @@ -0,0 +1,208 @@ +package ocrypto + +import ( + "context" + "crypto/rand" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewAESProtectedKey(t *testing.T) { + key := make([]byte, 32) + _, err := rand.Read(key) + require.NoError(t, err) + + protectedKey := NewAESProtectedKey(key) + assert.NotNil(t, protectedKey) + assert.Equal(t, key, protectedKey.rawKey) +} + +func TestAESProtectedKey_DecryptAESGCM(t *testing.T) { + // Generate a random 256-bit key + key := make([]byte, 32) + _, err := rand.Read(key) + require.NoError(t, err) + + protectedKey := NewAESProtectedKey(key) + + // Test data + plaintext := []byte("Hello, World!") + + // Encrypt the data first using the same key + aesGcm, err := NewAESGcm(key) + require.NoError(t, err) + + encrypted, err := aesGcm.Encrypt(plaintext) + require.NoError(t, err) + + // Extract IV and ciphertext (first 12 bytes are IV for GCM standard nonce size) + iv := encrypted[:GcmStandardNonceSize] + ciphertext := encrypted[GcmStandardNonceSize:] + + // Test decryption + decrypted, err := protectedKey.DecryptAESGCM(iv, ciphertext, 16) // 16 is standard GCM tag size + require.NoError(t, err) + assert.Equal(t, plaintext, decrypted) +} + +func TestAESProtectedKey_DecryptAESGCM_InvalidKey(t *testing.T) { + // Empty key should fail + protectedKey := NewAESProtectedKey([]byte{}) + + iv := make([]byte, 12) + ciphertext := make([]byte, 16) + + _, err := protectedKey.DecryptAESGCM(iv, ciphertext, 16) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to create AES-GCM cipher") +} + +func TestAESProtectedKey_Export_NoEncapsulator(t *testing.T) { + key := []byte("test-key-1234567890123456") // 24 bytes + protectedKey := NewAESProtectedKey(key) + + exported, err := protectedKey.Export(nil) + require.NoError(t, err) + assert.Equal(t, key, exported) +} + +func TestAESProtectedKey_Export_WithEncapsulator(t *testing.T) { + key := []byte("test-key-1234567890123456") // 24 bytes + protectedKey := NewAESProtectedKey(key) + + // Mock encapsulator + mockEncapsulator := &mockEncapsulator{ + encryptFunc: func(data []byte) ([]byte, error) { + // Simple XOR encryption for testing + result := make([]byte, len(data)) + for i, b := range data { + result[i] = b ^ 0xFF + } + return result, nil + }, + } + + exported, err := protectedKey.Export(mockEncapsulator) + require.NoError(t, err) + + // Verify it was encrypted (should be different from original) + assert.NotEqual(t, key, exported) + assert.Equal(t, len(key), len(exported)) + + // Verify we can decrypt it back + for i, b := range exported { + assert.Equal(t, key[i], b^0xFF) + } +} + +func TestAESProtectedKey_Export_EncapsulatorError(t *testing.T) { + key := []byte("test-key-1234567890123456") + protectedKey := NewAESProtectedKey(key) + + mockEncapsulator := &mockEncapsulator{ + encryptFunc: func(data []byte) ([]byte, error) { + return nil, assert.AnError + }, + } + + _, err := protectedKey.Export(mockEncapsulator) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to encrypt key data for export") +} + +func TestAESProtectedKey_VerifyBinding(t *testing.T) { + key := []byte("test-key-1234567890123456") + protectedKey := NewAESProtectedKey(key) + + policy := []byte("test-policy-data") + ctx := context.Background() + + // Generate the expected HMAC + expectedHMAC, err := protectedKey.generateHMACDigest(ctx, policy) + require.NoError(t, err) + + // Verify binding should succeed with correct HMAC + err = protectedKey.VerifyBinding(ctx, policy, expectedHMAC) + assert.NoError(t, err) +} + +func TestAESProtectedKey_VerifyBinding_Mismatch(t *testing.T) { + key := []byte("test-key-1234567890123456") + protectedKey := NewAESProtectedKey(key) + + policy := []byte("test-policy-data") + wrongBinding := []byte("wrong-binding-data") + ctx := context.Background() + + err := protectedKey.VerifyBinding(ctx, policy, wrongBinding) + assert.Error(t, err) + assert.Equal(t, ErrPolicyHMACMismatch, err) +} + +func TestAESProtectedKey_VerifyBinding_EmptyKey(t *testing.T) { + protectedKey := NewAESProtectedKey([]byte{}) + + policy := []byte("test-policy-data") + binding := []byte("some-binding") + ctx := context.Background() + + err := protectedKey.VerifyBinding(ctx, policy, binding) + assert.Error(t, err) + assert.Equal(t, ErrEmptyKeyData, err) +} + +func TestAESProtectedKey_VerifyBinding_DifferentPolicyData(t *testing.T) { + key := []byte("test-key-1234567890123456") + protectedKey := NewAESProtectedKey(key) + + ctx := context.Background() + + // Generate HMAC for first policy + policy1 := []byte("policy-data-1") + hmac1, err := protectedKey.generateHMACDigest(ctx, policy1) + require.NoError(t, err) + + // Try to verify with different policy data + policy2 := []byte("policy-data-2") + err = protectedKey.VerifyBinding(ctx, policy2, hmac1) + assert.Error(t, err) + assert.Equal(t, ErrPolicyHMACMismatch, err) +} + +func TestAESProtectedKey_InterfaceCompliance(t *testing.T) { + key := make([]byte, 32) + protectedKey := NewAESProtectedKey(key) + + // Ensure it implements the ProtectedKey interface + var _ ProtectedKey = protectedKey +} + +// Mock encapsulator for testing +type mockEncapsulator struct { + encryptFunc func([]byte) ([]byte, error) + publicKeyPEMFunc func() (string, error) + ephemeralKeyFunc func() []byte +} + +func (m *mockEncapsulator) Encrypt(data []byte) ([]byte, error) { + if m.encryptFunc != nil { + return m.encryptFunc(data) + } + return data, nil +} + +func (m *mockEncapsulator) PublicKeyInPemFormat() (string, error) { + if m.publicKeyPEMFunc != nil { + return m.publicKeyPEMFunc() + } + return "", nil +} + +func (m *mockEncapsulator) EphemeralKey() []byte { + if m.ephemeralKeyFunc != nil { + return m.ephemeralKeyFunc() + } + return nil +} \ No newline at end of file From a3e24d36e4a636de5fe2a9cc718c90f9957dc1e9 Mon Sep 17 00:00:00 2001 From: strantalis Date: Tue, 29 Jul 2025 17:41:28 -0400 Subject: [PATCH 2/7] fix(ocrypto): apply security improvements to AESProtectedKey - Add defensive copying in constructor to prevent external mutation - Add defensive copying in Export method for security - Simplify generateHMACDigest by removing unused context and error handling - Update tests to match simplified signature Addresses feedback from code review on PR #2599 --- lib/ocrypto/protected_key.go | 20 ++++++-------------- lib/ocrypto/protected_key_test.go | 10 ++++------ 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/lib/ocrypto/protected_key.go b/lib/ocrypto/protected_key.go index ed21e4d85..83c9632af 100644 --- a/lib/ocrypto/protected_key.go +++ b/lib/ocrypto/protected_key.go @@ -13,8 +13,6 @@ var ( ErrEmptyKeyData = errors.New("key data is empty") // ErrPolicyHMACMismatch is returned when policy binding verification fails ErrPolicyHMACMismatch = errors.New("policy hmac mismatch") - // ErrHMACGeneration is returned when HMAC digest generation fails - ErrHMACGeneration = errors.New("failed to generate hmac digest") ) // AESProtectedKey implements the ProtectedKey interface with an in-memory secret key @@ -27,7 +25,7 @@ var _ ProtectedKey = (*AESProtectedKey)(nil) // NewAESProtectedKey creates a new instance of AESProtectedKey func NewAESProtectedKey(rawKey []byte) *AESProtectedKey { return &AESProtectedKey{ - rawKey: rawKey, + rawKey: append([]byte{}, rawKey...), } } @@ -50,7 +48,7 @@ func (k *AESProtectedKey) DecryptAESGCM(iv []byte, body []byte, tagSize int) ([] func (k *AESProtectedKey) Export(encapsulator Encapsulator) ([]byte, error) { if encapsulator == nil { // Return raw key data without encryption - caller should be aware of this - return k.rawKey, nil + return append([]byte{}, k.rawKey...), nil } // Encrypt the key data before returning @@ -68,10 +66,7 @@ func (k *AESProtectedKey) VerifyBinding(ctx context.Context, policy, policyBindi return ErrEmptyKeyData } - actualHMAC, err := k.generateHMACDigest(ctx, policy) - if err != nil { - return fmt.Errorf("unable to generate policy hmac: %w", err) - } + actualHMAC := k.generateHMACDigest(policy) if !hmac.Equal(actualHMAC, policyBinding) { return ErrPolicyHMACMismatch @@ -81,11 +76,8 @@ func (k *AESProtectedKey) VerifyBinding(ctx context.Context, policy, policyBindi } // generateHMACDigest is a helper to generate an HMAC digest from a message using the key -func (k *AESProtectedKey) generateHMACDigest(ctx context.Context, msg []byte) ([]byte, error) { +func (k *AESProtectedKey) generateHMACDigest(msg []byte) []byte { mac := hmac.New(sha256.New, k.rawKey) - _, err := mac.Write(msg) - if err != nil { - return nil, ErrHMACGeneration - } - return mac.Sum(nil), nil + mac.Write(msg) + return mac.Sum(nil) } \ No newline at end of file diff --git a/lib/ocrypto/protected_key_test.go b/lib/ocrypto/protected_key_test.go index f0e7d6ffd..2765b60ba 100644 --- a/lib/ocrypto/protected_key_test.go +++ b/lib/ocrypto/protected_key_test.go @@ -120,11 +120,10 @@ func TestAESProtectedKey_VerifyBinding(t *testing.T) { ctx := context.Background() // Generate the expected HMAC - expectedHMAC, err := protectedKey.generateHMACDigest(ctx, policy) - require.NoError(t, err) + expectedHMAC := protectedKey.generateHMACDigest(policy) // Verify binding should succeed with correct HMAC - err = protectedKey.VerifyBinding(ctx, policy, expectedHMAC) + err := protectedKey.VerifyBinding(ctx, policy, expectedHMAC) assert.NoError(t, err) } @@ -161,12 +160,11 @@ func TestAESProtectedKey_VerifyBinding_DifferentPolicyData(t *testing.T) { // Generate HMAC for first policy policy1 := []byte("policy-data-1") - hmac1, err := protectedKey.generateHMACDigest(ctx, policy1) - require.NoError(t, err) + hmac1 := protectedKey.generateHMACDigest(policy1) // Try to verify with different policy data policy2 := []byte("policy-data-2") - err = protectedKey.VerifyBinding(ctx, policy2, hmac1) + err := protectedKey.VerifyBinding(ctx, policy2, hmac1) assert.Error(t, err) assert.Equal(t, ErrPolicyHMACMismatch, err) } From 119dbc9f05ff37cbc88af0112f507b6970cc8ee7 Mon Sep 17 00:00:00 2001 From: strantalis Date: Tue, 29 Jul 2025 18:00:56 -0400 Subject: [PATCH 3/7] cleanup lint issues --- lib/ocrypto/interfaces.go | 2 +- lib/ocrypto/protected_key.go | 4 ++-- lib/ocrypto/protected_key_test.go | 24 ++++++++++++------------ 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/ocrypto/interfaces.go b/lib/ocrypto/interfaces.go index 7c2199459..4a339a2b4 100644 --- a/lib/ocrypto/interfaces.go +++ b/lib/ocrypto/interfaces.go @@ -27,4 +27,4 @@ type ProtectedKey interface { // Used to decrypt encrypted policies and metadata DecryptAESGCM(iv []byte, body []byte, tagSize int) ([]byte, error) -} \ No newline at end of file +} diff --git a/lib/ocrypto/protected_key.go b/lib/ocrypto/protected_key.go index 83c9632af..0e7c17b1b 100644 --- a/lib/ocrypto/protected_key.go +++ b/lib/ocrypto/protected_key.go @@ -61,7 +61,7 @@ func (k *AESProtectedKey) Export(encapsulator Encapsulator) ([]byte, error) { } // VerifyBinding checks if the policy binding matches the given policy data -func (k *AESProtectedKey) VerifyBinding(ctx context.Context, policy, policyBinding []byte) error { +func (k *AESProtectedKey) VerifyBinding(_ context.Context, policy, policyBinding []byte) error { if len(k.rawKey) == 0 { return ErrEmptyKeyData } @@ -80,4 +80,4 @@ func (k *AESProtectedKey) generateHMACDigest(msg []byte) []byte { mac := hmac.New(sha256.New, k.rawKey) mac.Write(msg) return mac.Sum(nil) -} \ No newline at end of file +} diff --git a/lib/ocrypto/protected_key_test.go b/lib/ocrypto/protected_key_test.go index 2765b60ba..f427f9331 100644 --- a/lib/ocrypto/protected_key_test.go +++ b/lib/ocrypto/protected_key_test.go @@ -55,7 +55,7 @@ func TestAESProtectedKey_DecryptAESGCM_InvalidKey(t *testing.T) { ciphertext := make([]byte, 16) _, err := protectedKey.DecryptAESGCM(iv, ciphertext, 16) - assert.Error(t, err) + require.Error(t, err) assert.Contains(t, err.Error(), "failed to create AES-GCM cipher") } @@ -89,7 +89,7 @@ func TestAESProtectedKey_Export_WithEncapsulator(t *testing.T) { // Verify it was encrypted (should be different from original) assert.NotEqual(t, key, exported) - assert.Equal(t, len(key), len(exported)) + assert.Len(t, exported, len(key)) // Verify we can decrypt it back for i, b := range exported { @@ -102,13 +102,13 @@ func TestAESProtectedKey_Export_EncapsulatorError(t *testing.T) { protectedKey := NewAESProtectedKey(key) mockEncapsulator := &mockEncapsulator{ - encryptFunc: func(data []byte) ([]byte, error) { + encryptFunc: func(_ []byte) ([]byte, error) { return nil, assert.AnError }, } _, err := protectedKey.Export(mockEncapsulator) - assert.Error(t, err) + require.Error(t, err) assert.Contains(t, err.Error(), "failed to encrypt key data for export") } @@ -136,7 +136,7 @@ func TestAESProtectedKey_VerifyBinding_Mismatch(t *testing.T) { ctx := context.Background() err := protectedKey.VerifyBinding(ctx, policy, wrongBinding) - assert.Error(t, err) + require.Error(t, err) assert.Equal(t, ErrPolicyHMACMismatch, err) } @@ -148,7 +148,7 @@ func TestAESProtectedKey_VerifyBinding_EmptyKey(t *testing.T) { ctx := context.Background() err := protectedKey.VerifyBinding(ctx, policy, binding) - assert.Error(t, err) + require.Error(t, err) assert.Equal(t, ErrEmptyKeyData, err) } @@ -165,7 +165,7 @@ func TestAESProtectedKey_VerifyBinding_DifferentPolicyData(t *testing.T) { // Try to verify with different policy data policy2 := []byte("policy-data-2") err := protectedKey.VerifyBinding(ctx, policy2, hmac1) - assert.Error(t, err) + require.Error(t, err) assert.Equal(t, ErrPolicyHMACMismatch, err) } @@ -174,14 +174,14 @@ func TestAESProtectedKey_InterfaceCompliance(t *testing.T) { protectedKey := NewAESProtectedKey(key) // Ensure it implements the ProtectedKey interface - var _ ProtectedKey = protectedKey + assert.Implements(t, (*ProtectedKey)(nil), protectedKey) } // Mock encapsulator for testing type mockEncapsulator struct { - encryptFunc func([]byte) ([]byte, error) - publicKeyPEMFunc func() (string, error) - ephemeralKeyFunc func() []byte + encryptFunc func([]byte) ([]byte, error) + publicKeyPEMFunc func() (string, error) + ephemeralKeyFunc func() []byte } func (m *mockEncapsulator) Encrypt(data []byte) ([]byte, error) { @@ -203,4 +203,4 @@ func (m *mockEncapsulator) EphemeralKey() []byte { return m.ephemeralKeyFunc() } return nil -} \ No newline at end of file +} From 227c2aa86848931e1a396d0d3e5c91bbf3d8374b Mon Sep 17 00:00:00 2001 From: strantalis Date: Tue, 29 Jul 2025 18:41:44 -0400 Subject: [PATCH 4/7] gemini performance improvements recommendations --- lib/ocrypto/protected_key.go | 32 +++++++++------ lib/ocrypto/protected_key_test.go | 68 ++++++++++++++----------------- 2 files changed, 49 insertions(+), 51 deletions(-) diff --git a/lib/ocrypto/protected_key.go b/lib/ocrypto/protected_key.go index 0e7c17b1b..67aa36334 100644 --- a/lib/ocrypto/protected_key.go +++ b/lib/ocrypto/protected_key.go @@ -18,25 +18,35 @@ var ( // AESProtectedKey implements the ProtectedKey interface with an in-memory secret key type AESProtectedKey struct { rawKey []byte + aesGcm AesGcm } var _ ProtectedKey = (*AESProtectedKey)(nil) // NewAESProtectedKey creates a new instance of AESProtectedKey -func NewAESProtectedKey(rawKey []byte) *AESProtectedKey { - return &AESProtectedKey{ - rawKey: append([]byte{}, rawKey...), +func NewAESProtectedKey(rawKey []byte) (*AESProtectedKey, error) { + if len(rawKey) == 0 { + return nil, ErrEmptyKeyData } -} + // Create a defensive copy of the key + keyCopy := append([]byte{}, rawKey...) -// DecryptAESGCM decrypts data using AES-GCM with the protected key -func (k *AESProtectedKey) DecryptAESGCM(iv []byte, body []byte, tagSize int) ([]byte, error) { - aesGcm, err := NewAESGcm(k.rawKey) + // Pre-initialize the AES-GCM cipher for performance + aesGcm, err := NewAESGcm(keyCopy) if err != nil { - return nil, fmt.Errorf("failed to create AES-GCM cipher: %w", err) + return nil, fmt.Errorf("failed to initialize AES-GCM cipher: %w", err) } - decryptedData, err := aesGcm.DecryptWithIVAndTagSize(iv, body, tagSize) + return &AESProtectedKey{ + rawKey: keyCopy, + aesGcm: aesGcm, + }, nil +} + +// DecryptAESGCM decrypts data using AES-GCM with the protected key +func (k *AESProtectedKey) DecryptAESGCM(iv []byte, body []byte, tagSize int) ([]byte, error) { + // Use the pre-initialized AES-GCM cipher for better performance + decryptedData, err := k.aesGcm.DecryptWithIVAndTagSize(iv, body, tagSize) if err != nil { return nil, fmt.Errorf("AES-GCM decryption failed: %w", err) } @@ -62,10 +72,6 @@ func (k *AESProtectedKey) Export(encapsulator Encapsulator) ([]byte, error) { // VerifyBinding checks if the policy binding matches the given policy data func (k *AESProtectedKey) VerifyBinding(_ context.Context, policy, policyBinding []byte) error { - if len(k.rawKey) == 0 { - return ErrEmptyKeyData - } - actualHMAC := k.generateHMACDigest(policy) if !hmac.Equal(actualHMAC, policyBinding) { diff --git a/lib/ocrypto/protected_key_test.go b/lib/ocrypto/protected_key_test.go index f427f9331..182f2591a 100644 --- a/lib/ocrypto/protected_key_test.go +++ b/lib/ocrypto/protected_key_test.go @@ -14,7 +14,8 @@ func TestNewAESProtectedKey(t *testing.T) { _, err := rand.Read(key) require.NoError(t, err) - protectedKey := NewAESProtectedKey(key) + protectedKey, err := NewAESProtectedKey(key) + require.NoError(t, err) assert.NotNil(t, protectedKey) assert.Equal(t, key, protectedKey.rawKey) } @@ -25,7 +26,8 @@ func TestAESProtectedKey_DecryptAESGCM(t *testing.T) { _, err := rand.Read(key) require.NoError(t, err) - protectedKey := NewAESProtectedKey(key) + protectedKey, err := NewAESProtectedKey(key) + require.NoError(t, err) // Test data plaintext := []byte("Hello, World!") @@ -49,19 +51,15 @@ func TestAESProtectedKey_DecryptAESGCM(t *testing.T) { func TestAESProtectedKey_DecryptAESGCM_InvalidKey(t *testing.T) { // Empty key should fail - protectedKey := NewAESProtectedKey([]byte{}) - - iv := make([]byte, 12) - ciphertext := make([]byte, 16) - - _, err := protectedKey.DecryptAESGCM(iv, ciphertext, 16) + _, err := NewAESProtectedKey([]byte{}) require.Error(t, err) - assert.Contains(t, err.Error(), "failed to create AES-GCM cipher") + assert.ErrorIs(t, err, ErrEmptyKeyData) } func TestAESProtectedKey_Export_NoEncapsulator(t *testing.T) { - key := []byte("test-key-1234567890123456") // 24 bytes - protectedKey := NewAESProtectedKey(key) + key := []byte("test-key-12345678901234567890123") // 32 bytes + protectedKey, err := NewAESProtectedKey(key) + require.NoError(t, err) exported, err := protectedKey.Export(nil) require.NoError(t, err) @@ -69,8 +67,9 @@ func TestAESProtectedKey_Export_NoEncapsulator(t *testing.T) { } func TestAESProtectedKey_Export_WithEncapsulator(t *testing.T) { - key := []byte("test-key-1234567890123456") // 24 bytes - protectedKey := NewAESProtectedKey(key) + key := []byte("test-key-12345678901234567890123") // 32 bytes + protectedKey, err := NewAESProtectedKey(key) + require.NoError(t, err) // Mock encapsulator mockEncapsulator := &mockEncapsulator{ @@ -98,8 +97,9 @@ func TestAESProtectedKey_Export_WithEncapsulator(t *testing.T) { } func TestAESProtectedKey_Export_EncapsulatorError(t *testing.T) { - key := []byte("test-key-1234567890123456") - protectedKey := NewAESProtectedKey(key) + key := []byte("test-key-12345678901234567890123") // 32 bytes + protectedKey, err := NewAESProtectedKey(key) + require.NoError(t, err) mockEncapsulator := &mockEncapsulator{ encryptFunc: func(_ []byte) ([]byte, error) { @@ -107,14 +107,15 @@ func TestAESProtectedKey_Export_EncapsulatorError(t *testing.T) { }, } - _, err := protectedKey.Export(mockEncapsulator) + _, err = protectedKey.Export(mockEncapsulator) require.Error(t, err) assert.Contains(t, err.Error(), "failed to encrypt key data for export") } func TestAESProtectedKey_VerifyBinding(t *testing.T) { - key := []byte("test-key-1234567890123456") - protectedKey := NewAESProtectedKey(key) + key := []byte("test-key-12345678901234567890123") // 32 bytes + protectedKey, err := NewAESProtectedKey(key) + require.NoError(t, err) policy := []byte("test-policy-data") ctx := context.Background() @@ -123,38 +124,28 @@ func TestAESProtectedKey_VerifyBinding(t *testing.T) { expectedHMAC := protectedKey.generateHMACDigest(policy) // Verify binding should succeed with correct HMAC - err := protectedKey.VerifyBinding(ctx, policy, expectedHMAC) + err = protectedKey.VerifyBinding(ctx, policy, expectedHMAC) assert.NoError(t, err) } func TestAESProtectedKey_VerifyBinding_Mismatch(t *testing.T) { - key := []byte("test-key-1234567890123456") - protectedKey := NewAESProtectedKey(key) + key := []byte("test-key-12345678901234567890123") // 32 bytes + protectedKey, err := NewAESProtectedKey(key) + require.NoError(t, err) policy := []byte("test-policy-data") wrongBinding := []byte("wrong-binding-data") ctx := context.Background() - err := protectedKey.VerifyBinding(ctx, policy, wrongBinding) + err = protectedKey.VerifyBinding(ctx, policy, wrongBinding) require.Error(t, err) assert.Equal(t, ErrPolicyHMACMismatch, err) } -func TestAESProtectedKey_VerifyBinding_EmptyKey(t *testing.T) { - protectedKey := NewAESProtectedKey([]byte{}) - - policy := []byte("test-policy-data") - binding := []byte("some-binding") - ctx := context.Background() - - err := protectedKey.VerifyBinding(ctx, policy, binding) - require.Error(t, err) - assert.Equal(t, ErrEmptyKeyData, err) -} - func TestAESProtectedKey_VerifyBinding_DifferentPolicyData(t *testing.T) { - key := []byte("test-key-1234567890123456") - protectedKey := NewAESProtectedKey(key) + key := []byte("test-key-12345678901234567890123") // 32 bytes + protectedKey, err := NewAESProtectedKey(key) + require.NoError(t, err) ctx := context.Background() @@ -164,14 +155,15 @@ func TestAESProtectedKey_VerifyBinding_DifferentPolicyData(t *testing.T) { // Try to verify with different policy data policy2 := []byte("policy-data-2") - err := protectedKey.VerifyBinding(ctx, policy2, hmac1) + err = protectedKey.VerifyBinding(ctx, policy2, hmac1) require.Error(t, err) assert.Equal(t, ErrPolicyHMACMismatch, err) } func TestAESProtectedKey_InterfaceCompliance(t *testing.T) { key := make([]byte, 32) - protectedKey := NewAESProtectedKey(key) + protectedKey, err := NewAESProtectedKey(key) + require.NoError(t, err) // Ensure it implements the ProtectedKey interface assert.Implements(t, (*ProtectedKey)(nil), protectedKey) From 29609784f597ffdaa8a72be0dd8262e3886ad90c Mon Sep 17 00:00:00 2001 From: strantalis Date: Wed, 30 Jul 2025 12:25:15 -0400 Subject: [PATCH 5/7] some minor changes --- lib/ocrypto/interfaces.go | 18 +++++++++++------- lib/ocrypto/protected_key.go | 2 +- lib/ocrypto/protected_key_test.go | 20 +++++++++++++------- 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/lib/ocrypto/interfaces.go b/lib/ocrypto/interfaces.go index 4a339a2b4..ee0a48320 100644 --- a/lib/ocrypto/interfaces.go +++ b/lib/ocrypto/interfaces.go @@ -4,13 +4,17 @@ import ( "context" ) -// Encapsulator interface for key encapsulation operations +// Encapsulator enables key encapsulation with a public key type Encapsulator interface { + // Encapsulate wraps a secret key with the encapsulation key + Encapsulate(dek ProtectedKey) ([]byte, error) + // Encrypt wraps a secret key with the encapsulation key Encrypt(data []byte) ([]byte, error) - // PublicKeyInPemFormat Returns public key in pem format, or the empty string if not present - PublicKeyInPemFormat() (string, error) + // PublicKeyAsPEM exports the public key, used to encapsulate the value, in Privacy-Enhanced Mail format, + // or the empty string if not present. + PublicKeyAsPEM() (string, error) // For EC schemes, this method returns the public part of the ephemeral key. // Otherwise, it returns nil. @@ -20,11 +24,11 @@ type Encapsulator interface { // ProtectedKey represents a decrypted key with operations that can be performed on it type ProtectedKey interface { // VerifyBinding checks if the policy binding matches the given policy data - VerifyBinding(ctx context.Context, policy, binding []byte) error + VerifyBinding(ctx context.Context, policy, policyBinding []byte) error - // Export returns the raw key data, optionally encrypting it with the provided encryptor - Export(encryptor Encapsulator) ([]byte, error) + // Export returns the raw key data, optionally encrypting it with the provided encapsulator + Export(encapsulator Encapsulator) ([]byte, error) - // Used to decrypt encrypted policies and metadata + // DecryptAESGCM decrypts encrypted policies and metadata DecryptAESGCM(iv []byte, body []byte, tagSize int) ([]byte, error) } diff --git a/lib/ocrypto/protected_key.go b/lib/ocrypto/protected_key.go index 67aa36334..96a917a47 100644 --- a/lib/ocrypto/protected_key.go +++ b/lib/ocrypto/protected_key.go @@ -12,7 +12,7 @@ var ( // ErrEmptyKeyData is returned when the key data is empty ErrEmptyKeyData = errors.New("key data is empty") // ErrPolicyHMACMismatch is returned when policy binding verification fails - ErrPolicyHMACMismatch = errors.New("policy hmac mismatch") + ErrPolicyHMACMismatch = errors.New("policy HMAC mismatch") ) // AESProtectedKey implements the ProtectedKey interface with an in-memory secret key diff --git a/lib/ocrypto/protected_key_test.go b/lib/ocrypto/protected_key_test.go index 182f2591a..04120ea81 100644 --- a/lib/ocrypto/protected_key_test.go +++ b/lib/ocrypto/protected_key_test.go @@ -9,6 +9,8 @@ import ( "github.com/stretchr/testify/require" ) +const testKey = "test-key-12345678901234567890123" + func TestNewAESProtectedKey(t *testing.T) { key := make([]byte, 32) _, err := rand.Read(key) @@ -57,7 +59,7 @@ func TestAESProtectedKey_DecryptAESGCM_InvalidKey(t *testing.T) { } func TestAESProtectedKey_Export_NoEncapsulator(t *testing.T) { - key := []byte("test-key-12345678901234567890123") // 32 bytes + key := []byte(testKey) // 32 bytes protectedKey, err := NewAESProtectedKey(key) require.NoError(t, err) @@ -67,7 +69,7 @@ func TestAESProtectedKey_Export_NoEncapsulator(t *testing.T) { } func TestAESProtectedKey_Export_WithEncapsulator(t *testing.T) { - key := []byte("test-key-12345678901234567890123") // 32 bytes + key := []byte(testKey) // 32 bytes protectedKey, err := NewAESProtectedKey(key) require.NoError(t, err) @@ -97,7 +99,7 @@ func TestAESProtectedKey_Export_WithEncapsulator(t *testing.T) { } func TestAESProtectedKey_Export_EncapsulatorError(t *testing.T) { - key := []byte("test-key-12345678901234567890123") // 32 bytes + key := []byte(testKey) // 32 bytes protectedKey, err := NewAESProtectedKey(key) require.NoError(t, err) @@ -113,7 +115,7 @@ func TestAESProtectedKey_Export_EncapsulatorError(t *testing.T) { } func TestAESProtectedKey_VerifyBinding(t *testing.T) { - key := []byte("test-key-12345678901234567890123") // 32 bytes + key := []byte(testKey) // 32 bytes protectedKey, err := NewAESProtectedKey(key) require.NoError(t, err) @@ -129,7 +131,7 @@ func TestAESProtectedKey_VerifyBinding(t *testing.T) { } func TestAESProtectedKey_VerifyBinding_Mismatch(t *testing.T) { - key := []byte("test-key-12345678901234567890123") // 32 bytes + key := []byte(testKey) // 32 bytes protectedKey, err := NewAESProtectedKey(key) require.NoError(t, err) @@ -143,7 +145,7 @@ func TestAESProtectedKey_VerifyBinding_Mismatch(t *testing.T) { } func TestAESProtectedKey_VerifyBinding_DifferentPolicyData(t *testing.T) { - key := []byte("test-key-12345678901234567890123") // 32 bytes + key := []byte(testKey) // 32 bytes protectedKey, err := NewAESProtectedKey(key) require.NoError(t, err) @@ -176,6 +178,10 @@ type mockEncapsulator struct { ephemeralKeyFunc func() []byte } +func (m *mockEncapsulator) Encapsulate(dek ProtectedKey) ([]byte, error) { + return nil, nil +} + func (m *mockEncapsulator) Encrypt(data []byte) ([]byte, error) { if m.encryptFunc != nil { return m.encryptFunc(data) @@ -183,7 +189,7 @@ func (m *mockEncapsulator) Encrypt(data []byte) ([]byte, error) { return data, nil } -func (m *mockEncapsulator) PublicKeyInPemFormat() (string, error) { +func (m *mockEncapsulator) PublicKeyAsPEM() (string, error) { if m.publicKeyPEMFunc != nil { return m.publicKeyPEMFunc() } From c5bfa48caf581f89d31e7df3b35210b28c0e444a Mon Sep 17 00:00:00 2001 From: strantalis Date: Wed, 27 Aug 2025 18:55:06 -0400 Subject: [PATCH 6/7] return error if nil encapsulator on export --- lib/ocrypto/protected_key.go | 4 ++-- lib/ocrypto/protected_key_test.go | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/ocrypto/protected_key.go b/lib/ocrypto/protected_key.go index 96a917a47..5b3c0ab79 100644 --- a/lib/ocrypto/protected_key.go +++ b/lib/ocrypto/protected_key.go @@ -57,8 +57,8 @@ func (k *AESProtectedKey) DecryptAESGCM(iv []byte, body []byte, tagSize int) ([] // Export returns the raw key data, optionally encrypting it with the provided Encapsulator func (k *AESProtectedKey) Export(encapsulator Encapsulator) ([]byte, error) { if encapsulator == nil { - // Return raw key data without encryption - caller should be aware of this - return append([]byte{}, k.rawKey...), nil + // Return error if encapsulator is nil + return nil, errors.New("encapsulator cannot be nil") } // Encrypt the key data before returning diff --git a/lib/ocrypto/protected_key_test.go b/lib/ocrypto/protected_key_test.go index 04120ea81..ef44698b8 100644 --- a/lib/ocrypto/protected_key_test.go +++ b/lib/ocrypto/protected_key_test.go @@ -64,8 +64,9 @@ func TestAESProtectedKey_Export_NoEncapsulator(t *testing.T) { require.NoError(t, err) exported, err := protectedKey.Export(nil) - require.NoError(t, err) - assert.Equal(t, key, exported) + require.Error(t, err) + require.ErrorContains(t, err, "encapsulator cannot be nil") + assert.Nil(t, exported) } func TestAESProtectedKey_Export_WithEncapsulator(t *testing.T) { From c7f33b03103a6a462ee81e4b0d71ce8d76643f52 Mon Sep 17 00:00:00 2001 From: strantalis Date: Thu, 28 Aug 2025 12:01:11 -0400 Subject: [PATCH 7/7] fix lint issue --- lib/ocrypto/protected_key_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ocrypto/protected_key_test.go b/lib/ocrypto/protected_key_test.go index ef44698b8..6edf7f0bb 100644 --- a/lib/ocrypto/protected_key_test.go +++ b/lib/ocrypto/protected_key_test.go @@ -179,7 +179,7 @@ type mockEncapsulator struct { ephemeralKeyFunc func() []byte } -func (m *mockEncapsulator) Encapsulate(dek ProtectedKey) ([]byte, error) { +func (m *mockEncapsulator) Encapsulate(_ ProtectedKey) ([]byte, error) { return nil, nil }