Skip to content

Commit e2dc6d8

Browse files
authored
fix(policy): reject unencrypted private keys for modes 1/2 (#3072)
## Summary - add `ocrypto.IsPEMOrDERPrivateKey` to detect PEM/DER private keys - reject unencrypted private keys in KAS CreateKey/RotateKey for modes 1/2 - add integration coverage for Create/Rotate; add ocrypto unit tests ## Testing - `gofumpt -w lib/ocrypto/key_material.go lib/ocrypto/key_material_test.go service/pkg/db/errors.go service/policy/db/key_access_server_registry.go service/integration/kas_registry_key_unencrypted_test.go` - `cd lib/ocrypto && golangci-lint run ./...` - `cd lib/ocrypto && go test ./...` - `cd service && go test ./policy/db` ## Notes - `cd service && golangci-lint run ./policy/db ./integration` fails due to existing `funcorder`/`staticcheck` findings in integration and deprecated usage (pre-existing). - `cd service && go test ./integration -run Test_CreateKasKey_PEMPrivateKey_Fail` failed due to Testcontainers/Docker socket (`/Users/strantalis/.colima/default/docker.sock`) not available in this environment. --------- Signed-off-by: strantalis <strantalis@virtru.com>
1 parent 826d857 commit e2dc6d8

File tree

5 files changed

+139
-2
lines changed

5 files changed

+139
-2
lines changed

lib/ocrypto/key_material.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package ocrypto
2+
3+
import (
4+
"crypto/x509"
5+
"encoding/pem"
6+
"strings"
7+
)
8+
9+
// IsPEMOrDERPrivateKey reports whether data appears to be an unencrypted private key
10+
// in PEM or DER format. It does not attempt decryption or key unwrapping.
11+
func IsPEMOrDERPrivateKey(data []byte) bool {
12+
for block, rest := pem.Decode(data); block != nil; block, rest = pem.Decode(rest) {
13+
if strings.Contains(block.Type, "PRIVATE KEY") {
14+
return true
15+
}
16+
}
17+
18+
if _, err := x509.ParsePKCS8PrivateKey(data); err == nil {
19+
return true
20+
}
21+
if _, err := x509.ParsePKCS1PrivateKey(data); err == nil {
22+
return true
23+
}
24+
if _, err := x509.ParseECPrivateKey(data); err == nil {
25+
return true
26+
}
27+
28+
return false
29+
}

lib/ocrypto/key_material_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
package ocrypto
2+
3+
import (
4+
"encoding/pem"
5+
"os"
6+
"path/filepath"
7+
"testing"
8+
9+
"github.com/stretchr/testify/require"
10+
)
11+
12+
func TestIsPEMOrDERPrivateKey(t *testing.T) {
13+
privateKeyFiles := []string{
14+
"sample-rsa-2048-01-private.pem",
15+
"sample-ec-secp256r1-01-private.pem",
16+
}
17+
18+
for _, filename := range privateKeyFiles {
19+
t.Run("pem-private-"+filename, func(t *testing.T) {
20+
pemData := readTestData(t, filename)
21+
require.True(t, IsPEMOrDERPrivateKey(pemData))
22+
})
23+
}
24+
25+
t.Run("pem-public", func(t *testing.T) {
26+
pemData := readTestData(t, "sample-rsa-2048-01-public.pem")
27+
require.False(t, IsPEMOrDERPrivateKey(pemData))
28+
})
29+
30+
t.Run("der-private", func(t *testing.T) {
31+
pemData := readTestData(t, "sample-rsa-2048-01-private.pem")
32+
block, _ := pem.Decode(pemData)
33+
require.NotNil(t, block)
34+
require.True(t, IsPEMOrDERPrivateKey(block.Bytes))
35+
})
36+
37+
t.Run("random-bytes", func(t *testing.T) {
38+
require.False(t, IsPEMOrDERPrivateKey([]byte("not a key")))
39+
})
40+
}
41+
42+
func readTestData(t *testing.T, filename string) []byte {
43+
t.Helper()
44+
path := filepath.Join("testdata", filename)
45+
data, err := os.ReadFile(path)
46+
require.NoError(t, err)
47+
return data
48+
}
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
package integration
2+
3+
import (
4+
"encoding/base64"
5+
6+
"github.com/opentdf/platform/protocol/go/policy"
7+
"github.com/opentdf/platform/protocol/go/policy/kasregistry"
8+
"github.com/opentdf/platform/service/pkg/db"
9+
)
10+
11+
func (s *KasRegistryKeySuite) Test_CreateKasKey_PEMPrivateKey_Fail() {
12+
pemPrivateKey := "-----BEGIN PRIVATE KEY-----\nZg==\n-----END PRIVATE KEY-----\n"
13+
encodedPem := base64.StdEncoding.EncodeToString([]byte(pemPrivateKey))
14+
req := kasregistry.CreateKeyRequest{
15+
KasId: s.kasKeys[0].KeyAccessServerID,
16+
KeyId: validKeyID1,
17+
KeyAlgorithm: policy.Algorithm_ALGORITHM_RSA_2048,
18+
KeyMode: policy.KeyMode_KEY_MODE_CONFIG_ROOT_KEY,
19+
PublicKeyCtx: &policy.PublicKeyCtx{Pem: keyCtx},
20+
PrivateKeyCtx: &policy.PrivateKeyCtx{
21+
WrappedKey: encodedPem,
22+
KeyId: validKeyID1,
23+
},
24+
}
25+
resp, err := s.db.PolicyClient.CreateKey(s.ctx, &req)
26+
s.Require().Error(err)
27+
s.Require().ErrorContains(err, db.ErrUnencryptedPrivateKey.Error())
28+
s.Nil(resp)
29+
}
30+
31+
func (s *KasRegistryKeySuite) Test_RotateKey_PEMPrivateKey_Fail() {
32+
keyMap := s.setupKeysForRotate(s.kasKeys[0].KeyAccessServerID)
33+
pemPrivateKey := "-----BEGIN PRIVATE KEY-----\nZg==\n-----END PRIVATE KEY-----\n"
34+
encodedPem := base64.StdEncoding.EncodeToString([]byte(pemPrivateKey))
35+
newKey := kasregistry.RotateKeyRequest_NewKey{
36+
KeyId: validKeyID1,
37+
Algorithm: policy.Algorithm_ALGORITHM_EC_P256,
38+
KeyMode: policy.KeyMode_KEY_MODE_CONFIG_ROOT_KEY,
39+
PublicKeyCtx: &policy.PublicKeyCtx{Pem: keyCtx},
40+
PrivateKeyCtx: &policy.PrivateKeyCtx{WrappedKey: encodedPem, KeyId: validKeyID1},
41+
}
42+
rotatedInKey, err := s.db.PolicyClient.RotateKey(s.ctx, keyMap[rotateKey], &newKey)
43+
s.Require().Error(err)
44+
s.Require().ErrorContains(err, db.ErrUnencryptedPrivateKey.Error())
45+
s.Nil(rotatedInKey)
46+
}

service/pkg/db/errors.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ var (
3333
ErrCannotUpdateToUnspecified = errors.New("ErrCannotUpdateToUnspecified: cannot update to unspecified value")
3434
ErrKeyRotationFailed = errors.New("ErrTextKeyRotationFailed: key rotation failed")
3535
ErrExpectedBase64EncodedValue = errors.New("ErrExpectedBase64EncodedValue: expected base64 encoded value")
36+
ErrUnencryptedPrivateKey = errors.New("ErrUnencryptedPrivateKey: unencrypted private key not allowed")
3637
ErrMarshalValueFailed = errors.New("ErrMashalValueFailed: failed to marshal value")
3738
ErrUnmarshalValueFailed = errors.New("ErrUnmarshalValueFailed: failed to unmarshal value")
3839
ErrNamespaceMismatch = errors.New("ErrNamespaceMismatch: namespace mismatch")
@@ -128,6 +129,7 @@ const (
128129
ErrorTextUpdateToUnspecified = "cannot update to unspecified value"
129130
ErrTextKeyRotationFailed = "key rotation failed"
130131
ErrorTextExpectedBase64EncodedValue = "expected base64 encoded value"
132+
ErrorTextUnencryptedPrivateKey = "unencrypted private key not allowed"
131133
ErrorTextMarshalFailed = "failed to marshal value"
132134
ErrorTextUnmarsalFailed = "failed to unmarshal value"
133135
ErrorTextNamespaceMismatch = "namespace mismatch"
@@ -188,6 +190,10 @@ func StatusifyError(ctx context.Context, l *logger.Logger, err error, fallbackEr
188190
l.ErrorContext(ctx, ErrorTextExpectedBase64EncodedValue, logs...)
189191
return connect.NewError(connect.CodeInvalidArgument, errors.New(ErrorTextExpectedBase64EncodedValue))
190192
}
193+
if errors.Is(err, ErrUnencryptedPrivateKey) {
194+
l.ErrorContext(ctx, ErrorTextUnencryptedPrivateKey, logs...)
195+
return connect.NewError(connect.CodeInvalidArgument, errors.New(ErrorTextUnencryptedPrivateKey))
196+
}
191197
if errors.Is(err, ErrMarshalValueFailed) {
192198
l.ErrorContext(ctx, ErrorTextMarshalFailed, logs...)
193199
return connect.NewError(connect.CodeInvalidArgument, errors.New(ErrorTextMarshalFailed))

service/policy/db/key_access_server_registry.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"strings"
1111

1212
"github.com/jackc/pgx/v5/pgtype"
13+
"github.com/opentdf/platform/lib/ocrypto"
1314
"github.com/opentdf/platform/protocol/go/common"
1415
"github.com/opentdf/platform/protocol/go/policy"
1516
"github.com/opentdf/platform/protocol/go/policy/attributes"
@@ -373,8 +374,15 @@ func (c PolicyDBClient) CreateKey(ctx context.Context, r *kasregistry.CreateKeyR
373374
if !isValidBase64(r.GetPublicKeyCtx().GetPem()) {
374375
return nil, errors.Join(errors.New("public key ctx"), db.ErrExpectedBase64EncodedValue)
375376
}
376-
if (mode == int32(policy.KeyMode_KEY_MODE_CONFIG_ROOT_KEY) || mode == int32(policy.KeyMode_KEY_MODE_PROVIDER_ROOT_KEY)) && !isValidBase64(r.GetPrivateKeyCtx().GetWrappedKey()) {
377-
return nil, errors.Join(errors.New("private key ctx"), db.ErrExpectedBase64EncodedValue)
377+
if mode == int32(policy.KeyMode_KEY_MODE_CONFIG_ROOT_KEY) || mode == int32(policy.KeyMode_KEY_MODE_PROVIDER_ROOT_KEY) {
378+
wrappedKey := r.GetPrivateKeyCtx().GetWrappedKey()
379+
decodedKey, err := base64.StdEncoding.DecodeString(wrappedKey)
380+
if err != nil {
381+
return nil, errors.Join(errors.New("private key ctx"), db.ErrExpectedBase64EncodedValue)
382+
}
383+
if ocrypto.IsPEMOrDERPrivateKey(decodedKey) {
384+
return nil, errors.Join(errors.New("private key ctx"), db.ErrUnencryptedPrivateKey)
385+
}
378386
}
379387

380388
// Marshal private key and public key context

0 commit comments

Comments
 (0)