Skip to content

fix(core): Let legacy KAOs use new trust plugins #2218

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

Jump to bottom
Merged
merged 11 commits into from
May 16, 2025
Merged

fix(core): Let legacy KAOs use new trust plugins #2218

merged 11 commits into from
May 16, 2025

Conversation

dmihalcik-virtru
Copy link
Member

@dmihalcik-virtru dmihalcik-virtru commented May 15, 2025 •
edited
Loading

Proposed Changes

  • 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
  • Also fixes an issue where defaulting to 'standard' in the config caused the 'in memory' crypto config (standard) to always be used, as IsEmpty would always return true

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

@dmihalcik-virtru
fix(core): Let legacy KAOs use new trust plugins
91a8b23 
- 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
@dmihalcik-virtru dmihalcik-virtru force-pushed the DSPX-1061-necessary-improvements branch from 653afea to 91a8b23 Compare May 15, 2025 17:05
@github-actions GitHub Actions
Copy link
Contributor

Benchmark results, click to expand

Benchmark Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 465.829457ms

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 379.715046ms
Throughput 263.36 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 1m18.847679443s
Average Latency 786.008127ms
Throughput 63.41 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 4930
Failed Requests 70
Concurrent Requests 50
Total Time 1m8.62506759s
Average Latency 681.592009ms
Throughput 71.84 requests/second

Error Summary:

Error Message Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error
rpc error: code = Internal desc = could not perform access		 70 occurrences

Standard Benchmark Metrics Skipped or Failed

@github-actions GitHub Actions
Copy link
Contributor

Benchmark results, click to expand

Benchmark Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 466.738526ms

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 392.578362ms
Throughput 254.73 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 1m22.791892026s
Average Latency 824.995623ms
Throughput 60.39 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 4944
Failed Requests 56
Concurrent Requests 50
Total Time 1m12.088451766s
Average Latency 716.872709ms
Throughput 68.58 requests/second

Error Summary:

Error Message Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error
rpc error: code = Internal desc = could not perform access		 56 occurrences

Standard Benchmark Metrics Skipped or Failed

@github-actions GitHub Actions
Copy link
Contributor

Benchmark results, click to expand

Benchmark Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 461.88994ms

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 376.6625ms
Throughput 265.49 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 1m20.198191389s
Average Latency 798.749753ms
Throughput 62.35 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 4943
Failed Requests 57
Concurrent Requests 50
Total Time 1m10.085211356s
Average Latency 697.070678ms
Throughput 70.53 requests/second

Error Summary:

Error Message Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error
rpc error: code = Internal desc = could not perform access		 57 occurrences

Standard Benchmark Metrics Skipped or Failed

@github-actions GitHub Actions
Copy link
Contributor

Benchmark results, click to expand

Benchmark Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 464.037854ms

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 371.72603ms
Throughput 269.02 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 1m19.721924603s
Average Latency 795.074346ms
Throughput 62.72 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 4949
Failed Requests 51
Concurrent Requests 50
Total Time 1m10.294886745s
Average Latency 699.523442ms
Throughput 70.40 requests/second

Error Summary:

Error Message Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error
rpc error: code = Internal desc = could not perform access		 51 occurrences

Standard Benchmark Metrics Skipped or Failed

@github-actions GitHub Actions
Copy link
Contributor

Benchmark results, click to expand

Benchmark Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 456.179652ms

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 366.426335ms
Throughput 272.91 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 1m17.406671297s
Average Latency 771.838735ms
Throughput 64.59 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 4948
Failed Requests 52
Concurrent Requests 50
Total Time 1m7.628000893s
Average Latency 672.221295ms
Throughput 73.16 requests/second

Error Summary:

Error Message Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error
rpc error: code = Internal desc = could not perform access		 52 occurrences

Standard Benchmark Metrics Skipped or Failed

@github-actions GitHub Actions
Copy link
Contributor

Benchmark results, click to expand

Benchmark Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 464.130709ms

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 374.058778ms
Throughput 267.34 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 1m20.794854491s
Average Latency 805.62411ms
Throughput 61.89 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 4950
Failed Requests 50
Concurrent Requests 50
Total Time 1m11.888480278s
Average Latency 712.831207ms
Throughput 68.86 requests/second

Error Summary:

Error Message Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error
rpc error: code = Internal desc = could not perform access		 50 occurrences

Standard Benchmark Metrics Skipped or Failed

@github-actions GitHub Actions
Copy link
Contributor

Benchmark results, click to expand

Benchmark Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 465.115506ms

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 377.884569ms
Throughput 264.63 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 1m22.001366107s
Average Latency 817.270827ms
Throughput 60.97 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 4942
Failed Requests 58
Concurrent Requests 50
Total Time 1m13.043576913s
Average Latency 723.406233ms
Throughput 67.66 requests/second

Error Summary:

Error Message Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error
rpc error: code = Internal desc = could not perform access		 58 occurrences

Standard Benchmark Metrics Skipped or Failed

@dmihalcik-virtru dmihalcik-virtru marked this pull request as ready for review May 16, 2025 00:14
@dmihalcik-virtru dmihalcik-virtru requested review from a team as code owners May 16, 2025 00:14
pflynn-virtru
pflynn-virtru reviewed May 16, 2025
View reviewed changes
pflynn-virtru
pflynn-virtru previously approved these changes May 16, 2025
View reviewed changes
@github-actions GitHub Actions
Copy link
Contributor

Benchmark results, click to expand

Benchmark Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 458.263345ms

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 381.02675ms
Throughput 262.45 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 1m19.140907848s
Average Latency 788.358573ms
Throughput 63.18 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 4947
Failed Requests 53
Concurrent Requests 50
Total Time 1m10.064441903s
Average Latency 695.581161ms
Throughput 70.61 requests/second

Error Summary:

Error Message Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error
rpc error: code = Internal desc = could not perform access		 53 occurrences

Standard Benchmark Metrics Skipped or Failed

elizabethhealy
elizabethhealy approved these changes May 16, 2025
View reviewed changes
Copy link
Member

@elizabethhealy elizabethhealy left a comment

Choose a reason for hiding this comment

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

do we have any unit tests for StandardCrypto?

@dmihalcik-virtru
Copy link
Member Author

do we have any unit tests for StandardCrypto?

It doesn't looks like it. We do have coverage of all of these methods (that I haven't deleted) via integration tests, but yes we should make that a priority as we refactor. I'll file a ticket

@dmihalcik-virtru dmihalcik-virtru added this pull request to the merge queue May 16, 2025
Merged via the queue into main with commit 5aa6916 May 16, 2025
29 checks passed
@dmihalcik-virtru dmihalcik-virtru deleted the DSPX-1061-necessary-improvements branch May 16, 2025 18:01
@opentdf-automation opentdf-automation bot mentioned this pull request May 16, 2025
Merged
alkalescent pushed a commit that referenced this pull request May 19, 2025
@dmihalcik-virtru @alkalescent
fix(core): Let legacy KAOs use new trust plugins (#2218)
68525c6 
### Proposed Changes

- 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
- Also fixes an issue where defaulting to 'standard' in the config
caused the 'in memory' crypto config (standard) to always be used, as
`IsEmpty` would always return true

### Checklist

- [x] 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
@opentdf-automation
chore(main): release service 0.5.3 (#2046)
66550d2 
🤖 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
Reviewers

@elizabethhealy elizabethhealy elizabethhealy approved these changes

@pflynn-virtru pflynn-virtru pflynn-virtru approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dmihalcik-virtru @elizabethhealy @pflynn-virtru