Skip to content

feat(policy): Update simple kas key #2378

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 5, 2025
Merged

feat(policy): Update simple kas key #2378

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f859fd2
feat(policy): Simple kas key algorithm should be enum.
c-r33d Jun 4, 2025
5cd07e7
refactor.
c-r33d Jun 4, 2025
aacf38b
refactor.
c-r33d Jun 4, 2025
dc7783d
merge conflicts.
c-r33d Jun 5, 2025
4690015
unremove changelog.
c-r33d Jun 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/grpc/index.html
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion docs/openapi/policy/kasregistry/key_access_server_registry.swagger.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 0 additions & 30 deletions protocol/go/CHANGELOG.md
View file
Open in desktop

This file was deleted.

378 changes: 190 additions & 188 deletions protocol/go/policy/kasregistry/key_access_server_registry.pb.go
View file
Open in desktop

Large diffs are not rendered by default.

6 changes: 5 additions & 1 deletion service/integration/kas_registry_key_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const (
validKeyID3 = "key_id_3"
keyID4 = "key_id_4"
notFoundKasUUID = "123e4567-e89b-12d3-a456-426614174000"
keyCtx = `eyJrZXkiOiJ2YWx1ZSJ9Cg==`
keyCtx = `YS1wZW0K`
providerConfigID = "123e4567-e89b-12d3-a456-426614174000"
rotateKey = "rotate_key"
nonRotateKey = "non_rotate_key"
Expand Down Expand Up @@ -903,6 +903,10 @@ func (s *KasRegistryKeySuite) Test_SetBaseKey_Insert_Success() {
s.NotNil(newBaseKey)
s.Nil(newBaseKey.GetPreviousBaseKey())
s.Equal(key.GetKasKey().GetKey().GetKeyId(), newBaseKey.GetNewBaseKey().GetPublicKey().GetKid())
s.Equal(key.GetKasKey().GetKey().GetKeyAlgorithm(), newBaseKey.GetNewBaseKey().GetPublicKey().GetAlgorithm())
decodedKeyCtx, err := base64.StdEncoding.DecodeString(keyCtx)
s.Require().NoError(err)
s.Equal(string(decodedKeyCtx), newBaseKey.GetNewBaseKey().GetPublicKey().GetPem())
}

func (s *KasRegistryKeySuite) Test_SetBaseKey_CannotSetPublicKeyOnlyKey_Fails() {
Expand Down
22 changes: 1 addition & 21 deletions service/pkg/db/marshalHelpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
package db

import (
"encoding/base64"
"encoding/json"
"errors"
"fmt"
"strconv"

"github.com/opentdf/platform/protocol/go/common"
"github.com/opentdf/platform/protocol/go/policy"
Expand Down Expand Up @@ -125,7 +123,7 @@ func KasKeysProtoJSON(keysJSON []byte) ([]*policy.KasKey, error) {
return keys, nil
}

func formatAlg(alg policy.Algorithm) (string, error) {
func FormatAlg(alg policy.Algorithm) (string, error) {
switch alg {
case policy.Algorithm_ALGORITHM_RSA_2048:
return "rsa:2048", nil
Expand All @@ -151,24 +149,6 @@ func UnmarshalSimpleKasKey(keysJSON []byte) (*kasregistry.SimpleKasKey, error) {
if err := protojson.Unmarshal(keysJSON, key); err != nil {
return nil, err
}

// In the db, this is stored as an integer, which is parsed to a string
// and then converted to the correct algorithm format.
alg, err := strconv.ParseInt(key.GetPublicKey().GetAlgorithm(), 10, 32)
if err != nil {
return nil, err
}
algorithm, err := formatAlg(policy.Algorithm(alg))
if err != nil {
return nil, err
}
// The pem should always be present and base64 encoded, as it is required for creating a key.
pem, err := base64.StdEncoding.DecodeString(key.GetPublicKey().GetPem())
if err != nil {
return nil, err
}
key.PublicKey.Pem = string(pem)
key.PublicKey.Algorithm = algorithm
}
return key, nil
}
13 changes: 13 additions & 0 deletions service/policy/db/key_access_server_registry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -782,6 +782,19 @@
return err
}

if baseKey != nil {
algorithm, err := db.FormatAlg(baseKey.GetPublicKey().GetAlgorithm())
if err != nil {
return fmt.Errorf("failed to format algorithm: %w", err)
}
publicKey, ok := keyMap["public_key"].(map[string]any)
if !ok {
return fmt.Errorf("failed to cast public_key")

Check failure on line 792 in service/policy/db/key_access_server_registry.go

View workflow job for this annotation

GitHub Actions / go (service) 

error-format: fmt.Errorf can be replaced with errors.New (perfsprint)
}
publicKey["algorithm"] = algorithm
keyMap["public_key"] = publicKey
}

wellknownconfiguration.UpdateConfigurationBaseKey(keyMap)
return nil
}
Expand Down
9 changes: 5 additions & 4 deletions service/policy/db/models.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions service/policy/db/query.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1700,9 +1700,9 @@ SELECT
DISTINCT JSONB_BUILD_OBJECT(
'kas_uri', kas.uri,
'public_key', JSONB_BUILD_OBJECT(
'algorithm', kask.key_algorithm::TEXT,
'algorithm', kask.key_algorithm::INTEGER,
'kid', kask.key_id,
'pem', kask.public_key_ctx ->> 'pem'
'pem', CONVERT_FROM(DECODE(kask.public_key_ctx ->> 'pem', 'base64'), 'UTF8')
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The change to decode the PEM from base64 directly in the SQL query using CONVERT_FROM(DECODE(kask.public_key_ctx ->> 'pem', 'base64'), 'UTF8') is a good simplification for the Go unmarshalling logic.

Could you confirm if kask.public_key_ctx ->> 'pem' is always guaranteed to contain a valid base64 encoded string as per data insertion/update logic? If it could ever be non-base64 or malformed, the DECODE function might cause the query to error out.

Assuming the data integrity ensures it's always valid base64 (as suggested by the PublicKeyCtx proto definition which states the pem field is base64 encoded), this approach is robust.

Copy link
Contributor Author

@c-r33d c-r33d Jun 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No we have a check in the key creation RPC that verifies the public key is base64.

)
) AS base_keys
FROM base_keys bk
Expand Down
8 changes: 4 additions & 4 deletions service/policy/db/query.sql.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion service/policy/kasregistry/key_access_server_registry.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@
}

message SimpleKasPublicKey {
string algorithm = 1;
Algorithm algorithm = 1;

Check failure on line 608 in service/policy/kasregistry/key_access_server_registry.proto

View workflow job for this annotation

GitHub Actions / Protocol Buffer Lint and Gencode Up-to-date check 

Field "1" with name "algorithm" on message "SimpleKasPublicKey" changed type from "string" to "enum".

Check failure on line 608 in service/policy/kasregistry/key_access_server_registry.proto

View workflow job for this annotation

GitHub Actions / Protocol Buffer Lint and Gencode Up-to-date check 

Field "1" with name "algorithm" on message "SimpleKasPublicKey" changed type from "string" to "enum". See https://developers.google.com/protocol-buffers/docs/proto3#updating for wire compatibility rules.

Check failure on line 608 in service/policy/kasregistry/key_access_server_registry.proto

View workflow job for this annotation

GitHub Actions / Protocol Buffer Lint and Gencode Up-to-date check 

Field "1" with name "algorithm" on message "SimpleKasPublicKey" changed type from "string" to "enum". See https://developers.google.com/protocol-buffers/docs/proto3#updating for wire compatibility rules and https://developers.google.com/protocol-buffers/docs/proto3#json for JSON compatibility rules.
string kid = 2;
string pem = 3;
}
Expand Down
Loading