Skip to content

Conversation

dmihalcik-virtru
Copy link
Member

@dmihalcik-virtru dmihalcik-virtru commented Apr 23, 2025

Proposed Changes

  • Adds service/trust package, which includes an abstraction or core key lookup (KeyIndex) and runtime cryptographic operations (KeyManager).
  • Adds functional options for starting a platform service with custom providers of these services.

Checklist

  • I have added or updated unit tests
  • I have added or updated integration tests (if appropriate)
  • I have added or updated documentation

Testing Instructions

Copy link
Contributor

Standard Benchmark Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 0
Failed Decrypts 100
Total Time 21.397512ms
Throughput 0.00 requests/second

Error Summary

Error Message Occurrences
bulk rewrap failed: error making rewrap request to kas: error making rewrap request: rpc error: code = Internal desc = stream terminated by RST_STREAM with error code: INTERNAL_ERROR 1 occurrences

@dmihalcik-virtru dmihalcik-virtru force-pushed the DSPX-494-key-provider-framework branch from 3564c73 to d6e1613 Compare April 23, 2025 17:31
Copy link
Contributor

Standard Benchmark Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 0
Failed Decrypts 100
Total Time 23.107319ms
Throughput 0.00 requests/second

Error Summary

Error Message Occurrences
bulk rewrap failed: error making rewrap request to kas: error making rewrap request: rpc error: code = Internal desc = stream terminated by RST_STREAM with error code: INTERNAL_ERROR 1 occurrences

Copy link
Contributor

Standard Benchmark Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 0
Failed Decrypts 100
Total Time 22.210591ms
Throughput 0.00 requests/second

Error Summary

Error Message Occurrences
bulk rewrap failed: error making rewrap request to kas: error making rewrap request: rpc error: code = Internal desc = stream terminated by RST_STREAM with error code: INTERNAL_ERROR 1 occurrences

@dmihalcik-virtru dmihalcik-virtru changed the title WIP: Novel Key and Security Providers feat(core): Novel Key and Security Providers Apr 23, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces new key and security provider functionality that integrates a new SecurityProvider interface while maintaining backward compatibility with the legacy CryptoProvider. Key changes include switching decryption and key export functions to use the new SecurityProvider adapter, updating tracing calls to p.Start(ctx, …), and modifying various methods to handle salted key operations.

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file
File Description
service/kas/kas.go Added SecurityProvider initialization alongside legacy CryptoProvider.
service/kas/access/rewrap.go Migrated decryption and policy binding verification to use SecurityProvider.
service/kas/access/publicKey.go Updated key lookup and export to use the new SecurityProvider.
service/kas/access/provider.go Introduced SecurityProvider field and accessor methods for backward compatibility.
service/internal/security/* Added new functions for salted decryption and key derivation.
sdk/* and lib/ocrypto/* Updated tests and encryption/decryption calls to use the new salted APIs.
Comments suppressed due to low confidence (2)

service/kas/access/provider.go:25

  • [nitpick] Consider renaming the deprecated GetCryptoProvider() method to GetLegacyCryptoProvider() so that the distinction between the new SecurityProvider and legacy CryptoProvider is clearer.
SecurityProvider security.KeyManager

service/kas/access/rewrap.go:516

  • [nitpick] Ensure that converting the key identifier using security.KeyIdentifier is consistently applied across the codebase and that GetKid() reliably returns a valid identifier string.
kid := security.KeyIdentifier(kao.GetKeyAccessObject().GetKid())

Copy link
Contributor

Standard Benchmark Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 0
Failed Decrypts 100
Total Time 23.076062ms
Throughput 0.00 requests/second

Error Summary

Error Message Occurrences
bulk rewrap failed: error making rewrap request to kas: error making rewrap request: rpc error: code = Internal desc = stream terminated by RST_STREAM with error code: INTERNAL_ERROR 1 occurrences

Copy link
Contributor

Standard Benchmark Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 0
Failed Decrypts 100
Total Time 25.959617ms
Throughput 0.00 requests/second

Error Summary

Error Message Occurrences
bulk rewrap failed: error making rewrap request to kas: error making rewrap request: rpc error: code = Internal desc = stream terminated by RST_STREAM with error code: INTERNAL_ERROR 1 occurrences

Copy link
Contributor

Standard Benchmark Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 366.262598ms
Throughput 273.03 requests/second

Copy link
Contributor

Standard Benchmark Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 385.183968ms
Throughput 259.62 requests/second

Copy link
Contributor

Standard Benchmark Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 361.325183ms
Throughput 276.76 requests/second

Copy link
Contributor

Standard Benchmark Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 376.694634ms
Throughput 265.47 requests/second

@opentdf opentdf deleted a comment from github-actions bot Apr 25, 2025
@opentdf opentdf deleted a comment from github-actions bot Apr 25, 2025
@dmihalcik-virtru dmihalcik-virtru force-pushed the DSPX-494-key-provider-framework branch from 00bc170 to b5735fe Compare April 25, 2025 19:05
Copy link
Contributor

Standard Benchmark Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 373.163617ms
Throughput 267.98 requests/second

Copy link
Contributor

Benchmark results, click to expand

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 358.854726ms
Throughput 278.66 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 1m15.803605122s
Average Latency 756.879345ms
Throughput 65.96 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 4943
Failed Requests 57
Concurrent Requests 50
Total Time 1m5.567645416s
Average Latency 651.203756ms
Throughput 75.39 requests/second

Error Summary:

Error Message Occurrences
ReadNanoTDF error: rewrap failed: ocrypto.ComputeECDHKey failed :ocrypto.ECPubKeyFromPem failed: failed to parse PEM formatted public key 57 occurrences

Standard Benchmark Metrics Skipped or Failed

Copy link
Contributor

Benchmark results, click to expand

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 399.797459ms
Throughput 250.13 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 1m23.076319481s
Average Latency 828.928604ms
Throughput 60.19 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 4947
Failed Requests 53
Concurrent Requests 50
Total Time 1m10.810887755s
Average Latency 704.05484ms
Throughput 69.86 requests/second

Error Summary:

Error Message Occurrences
ReadNanoTDF error: rewrap failed: ocrypto.ComputeECDHKey failed :ocrypto.ECPubKeyFromPem failed: failed to parse PEM formatted public key 53 occurrences

Standard Benchmark Metrics Skipped or Failed

Copy link
Contributor

github-actions bot commented May 2, 2025

Benchmark results, click to expand

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 367.322456ms
Throughput 272.24 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 1m16.859713629s
Average Latency 766.439403ms
Throughput 65.05 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 4946
Failed Requests 54
Concurrent Requests 50
Total Time 1m7.107842116s
Average Latency 666.343484ms
Throughput 73.70 requests/second

Error Summary:

Error Message Occurrences
ReadNanoTDF error: rewrap failed: ocrypto.ComputeECDHKey failed :ocrypto.ECPubKeyFromPem failed: failed to parse PEM formatted public key 54 occurrences

Standard Benchmark Metrics Skipped or Failed

i'm still used to reentrant locks
Copy link
Contributor

github-actions bot commented May 2, 2025

Benchmark results, click to expand

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 368.425268ms
Throughput 271.43 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 1m18.794959289s
Average Latency 785.347737ms
Throughput 63.46 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 4942
Failed Requests 58
Concurrent Requests 50
Total Time 1m8.978322581s
Average Latency 685.281942ms
Throughput 71.65 requests/second

Error Summary:

Error Message Occurrences
ReadNanoTDF error: rewrap failed: ocrypto.ComputeECDHKey failed :ocrypto.ECPubKeyFromPem failed: failed to parse PEM formatted public key 58 occurrences

Standard Benchmark Metrics Skipped or Failed

Copy link
Contributor

github-actions bot commented May 2, 2025

Benchmark results, click to expand

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 383.645287ms
Throughput 260.66 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 1m20.924878566s
Average Latency 806.721709ms
Throughput 61.79 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 4948
Failed Requests 52
Concurrent Requests 50
Total Time 1m10.396985259s
Average Latency 700.85045ms
Throughput 70.29 requests/second

Error Summary:

Error Message Occurrences
ReadNanoTDF error: rewrap failed: ocrypto.ComputeECDHKey failed :ocrypto.ECPubKeyFromPem failed: failed to parse PEM formatted public key 52 occurrences

Standard Benchmark Metrics Skipped or Failed

@dmihalcik-virtru dmihalcik-virtru marked this pull request as ready for review May 2, 2025 17:43
@dmihalcik-virtru dmihalcik-virtru requested review from a team as code owners May 2, 2025 17:43
c-r33d
c-r33d previously approved these changes May 2, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces novel key and security provider capabilities by adding new interfaces (Encapsulator, ProtectedKey, KeyManager, etc.) and updating service logic to support trust‐based key management. Key changes include:

  • Implementing new trust package interfaces and updating key management abstractions.
  • Refactoring server startup and KAS provider logic to conditionally use the trust key index/manager over the legacy crypto provider.
  • Updating tests and provider adapter layers to align with the new key management semantics.

Reviewed Changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
service/trust/key_manager.go Introduced new trust interfaces for key encryption and decryption.
service/trust/key_index.go Added types and methods for key identification and export.
service/trust/delegating_key_service.go Implemented delegation logic for key managers with caching.
service/pkg/server/start.go Updated server startup to enforce exclusive use of trust provider.
service/kas/* Refactored key decryption and public key export to use trust APIs.
service/internal/security/* Updated crypto provider adapters to wrap legacy implementations.
Files not reviewed (1)
  • service/go.mod: Language not supported
Comments suppressed due to low confidence (1)

service/trust/delegating_key_service.go:67

  • [nitpick] The method name '_defKM' uses an unconventional underscore prefix. Consider renaming it to 'getDefaultKeyManager' for improved clarity.
func (d *DelegatingKeyService) _defKM() (KeyManager, error) {

Copy link
Contributor

github-actions bot commented May 2, 2025

Benchmark results, click to expand

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 374.444479ms
Throughput 267.06 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 1m19.568204837s
Average Latency 793.719243ms
Throughput 62.84 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 4961
Failed Requests 39
Concurrent Requests 50
Total Time 1m9.449850598s
Average Latency 691.602994ms
Throughput 71.43 requests/second

Error Summary:

Error Message Occurrences
ReadNanoTDF error: rewrap failed: ocrypto.ComputeECDHKey failed :ocrypto.ECPubKeyFromPem failed: failed to parse PEM formatted public key 39 occurrences

Standard Benchmark Metrics Skipped or Failed

@dmihalcik-virtru dmihalcik-virtru changed the title feat(core): Novel Key and Security Providers feat(core): New Key Index and Manager Plugin SPI May 2, 2025
@dmihalcik-virtru dmihalcik-virtru added this pull request to the merge queue May 5, 2025
Merged via the queue into main with commit eb446fc May 5, 2025
26 of 27 checks passed
@dmihalcik-virtru dmihalcik-virtru deleted the DSPX-494-key-provider-framework branch May 5, 2025 20:12
jakedoublev pushed a commit that referenced this pull request May 6, 2025
### Proposed Changes

* Adds service/trust package, which includes an abstraction or core key
lookup (`KeyIndex`) and runtime cryptographic operations (`KeyManager`).
* Adds functional options for starting a platform service with custom
providers of these services.

### Checklist

- [ ] I have added or updated unit tests
- [ ] I have added or updated integration tests (if appropriate)
- [ ] I have added or updated documentation

### Testing Instructions
github-merge-queue bot pushed a commit that referenced this pull request May 22, 2025
🤖 I have created a release *beep* *boop*
---


##
[0.5.3](service/v0.5.2...service/v0.5.3)
(2025-05-22)


### Features

* **authz:** authz v2 versioning implementation
([#2173](#2173))
([557fc21](557fc21))
* **authz:** authz v2, ers v2 protos and gencode for ABAC with actions &
registered resource
([#2124](#2124))
([ea7992a](ea7992a))
* **authz:** export entity id prefix constant from entity instead of
authorization service v1
([#2261](#2261))
([94079a9](94079a9))
* **authz:** subject mapping plugin support for ABAC with actions
([#2223](#2223))
([d08b939](d08b939))
* bulk keycloak provisioning
([#2205](#2205))
([59e4485](59e4485))
* **core:** add otel to opentdf services
([#1858](#1858))
([53a7aa0](53a7aa0))
* **core:** Adds EC withSalt options
([#2126](#2126))
([67b6fb8](67b6fb8))
* **core:** enhance db configuration options
([#2285](#2285))
([ed9ff59](ed9ff59))
* **core:** New Key Index and Manager Plugin SPI
([#2095](#2095))
([eb446fc](eb446fc))
* **core:** support onConfigUpdate hook when registering services
([#1992](#1992))
([366d4dc](366d4dc))
* **core:** v2 ERS with proto updates
([#2210](#2210))
([a161ef8](a161ef8))
* **policy:** actions crud service endpoints and proto validation
([#2037](#2037))
([e933fa9](e933fa9))
* **policy:** actions service RPCs should actually hit storage layer
CRUD ([#2063](#2063))
([da4faf5](da4faf5))
* **policy:** add enhanced standard/custom actions protos
([#2020](#2020))
([bbac53f](bbac53f))
* **policy:** Add platform key indexer.
([#2189](#2189))
([861ef8d](861ef8d))
* **policy:** consume lib/identifier parse function
([#2181](#2181))
([1cef22b](1cef22b))
* **policy:** DSPX-1018 NDR retrieval by FQN support
([#2131](#2131))
([0001041](0001041))
* **policy:** DSPX-1057 registered resource action attribute values (DB
+ Service implementation)
([#2191](#2191))
([6bf1b2e](6bf1b2e))
* **policy:** DSPX-1057 registered resource action attribute values
(protos only) ([#2217](#2217))
([6375596](6375596))
* **policy:** DSPX-893 NDR define crud protos
([#2056](#2056))
([55a5c27](55a5c27))
* **policy:** DSPX-898 NDR database schema
([#2055](#2055))
([2a10a6a](2a10a6a))
* **policy:** DSPX-901 NDR database crud
([#2071](#2071))
([20e0a5f](20e0a5f))
* **policy:** DSPX-902 NDR service crud implementation (2/2)
([#2066](#2066))
([030ad33](030ad33))
* **policy:** DSPX-902 NDR service crud protos only (1/2)
([#2092](#2092))
([24b6cb5](24b6cb5))
* **policy:** Finish resource mapping groups
([#2224](#2224))
([5ff754e](5ff754e))
* **policy:** GetMatchedSubjectMappings should provide value FQN
([#2151](#2151))
([ad80044](ad80044))
* **policy:** key management crud
([#2110](#2110))
([4c3d53d](4c3d53d))
* **policy:** Key management proto
([#2115](#2115))
([561f853](561f853))
* **policy:** Modify get request to search for keys by kasid with keyid.
([#2147](#2147))
([780d2e4](780d2e4))
* **policy:** Restrict KAS deletion when tied to Key
([#2144](#2144))
([4c4ab13](4c4ab13))
* **policy:** Return KAS Key structure
([#2172](#2172))
([7f97b99](7f97b99))
* **policy:** rotate keys rpc
([#2180](#2180))
([0d00743](0d00743))
* **policy:** stored enhanced actions database migration, CRUD queries,
SM updates ([#2040](#2040))
([e6b7c79](e6b7c79))
* **sdk:** Add a KAS allowlist
([#2085](#2085))
([d7cfdf3](d7cfdf3))
* **sdk:** add nanotdf plaintext policy
([#2182](#2182))
([e5c56db](e5c56db))
* **sdk:** Use ConnectRPC in the go client
([#2200](#2200))
([fc34ee6](fc34ee6))


### Bug Fixes

* **core:** access pdp cleanup before actions in ABAC decisioning
([#2123](#2123))
([9b38a3c](9b38a3c))
* **core:** Autobump service
([#2080](#2080))
([006c724](006c724))
* **core:** Autobump service
([#2104](#2104))
([1f72cc7](1f72cc7))
* **core:** Autobump service
([#2108](#2108))
([be5b7d7](be5b7d7))
* **core:** bump to go 1.24 and bump service proto module dependencies
([#2064](#2064))
([94891a0](94891a0))
* **core:** Fix DPoP with grpc-gateway
([#2044](#2044))
([4483ef2](4483ef2))
* **core:** fix service go.mod
([#2141](#2141))
([3b98f6d](3b98f6d))
* **core:** Improves errors when under heavy load
([#2132](#2132))
([4490a14](4490a14))
* **core:** Let legacy KAOs use new trust plugins
([#2218](#2218))
([5aa6916](5aa6916))
* **core:** migrate from mitchellh/mapstructure to go-viper/mapstructure
([#2087](#2087))
([0a3a82e](0a3a82e))
* **core:** update viper to 1.20.1
([#2088](#2088))
([09099e9](09099e9))
* **core:** Updates vulnerable dep go/x/net
([#2072](#2072))
([11c02cd](11c02cd))
* **deps:** bump github.com/creasty/defaults from 1.7.0 to 1.8.0 in
/service ([#2242](#2242))
([86a9b46](86a9b46))
* **deps:** bump github.com/jackc/pgx/v5 from 5.5.5 to 5.7.5 in /service
([#2249](#2249))
([d8f3b67](d8f3b67))
* **deps:** bump the internal group across 1 directory with 2 updates
([#2296](#2296))
([7f92c70](7f92c70))
* **deps:** bump toolchain in /lib/fixtures and /examples to resolve CVE
GO-2025-3563 ([#2061](#2061))
([9c16843](9c16843))
* handle empty private and public key ctx structs
([#2272](#2272))
([f3fc647](f3fc647))
* **policy:** remove predefined rules in actions protos
([#2069](#2069))
([060f059](060f059))
* **policy:** return kas uri on keys for definition, namespace and
values ([#2186](#2186))
([6c55fb8](6c55fb8))
* update key_mode to provide more context
([#2226](#2226))
([44d0805](44d0805))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants