Skip to content

feat(policy): Add legacy keys. #2613

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.

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 6 commits into from
Aug 7, 2025
Merged

feat(policy): Add legacy keys. #2613

merged 6 commits into from
Aug 7, 2025

Conversation

c-r33d
Copy link
Contributor

@c-r33d c-r33d commented Aug 5, 2025

Proposed Changes

1.) Add legacy column to kas keys table
2.) Only allow one legacy key per registered kas
3.) Add filter to list legacy keys.
4.) Proto updates to support legacy flag.

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

@c-r33d c-r33d requested review from a team as code owners August 5, 2025 16:16
@github-actions github-actions bot added comp:db DB component comp:policy Policy Configuration ( attributes, subject mappings, resource mappings, kas registry) docs Documentation size/m labels Aug 5, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Summary of Changes

Hello @c-r33d, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

I've implemented a new feature to support "legacy" keys within the Key Access Server (KAS) policy system. This change introduces a new legacy flag for KAS keys, ensuring that only one such legacy key can exist per registered KAS. Additionally, I've added the capability to filter and list these legacy keys.

Highlights

  • API Extension: Introduced a legacy boolean field to KAS key definitions in the API, allowing keys to be marked as legacy.
  • Database Schema Update: Updated the database schema for key_access_server_keys to include a legacy column, defaulting to FALSE.
  • Data Integrity Enforcement: Implemented a unique index on the key_access_server_keys table to enforce the business rule that only one legacy key (legacy = TRUE) can be associated with a given Key Access Server ID.
  • Core Logic Enhancement: Extended the key creation, retrieval, and listing functionalities to properly handle and filter based on the new legacy attribute.
  • Testing Coverage: Added comprehensive integration tests to validate the creation of legacy keys, prevent multiple legacy keys for the same KAS, and confirm correct filtering during key listing.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.


Old keys now marked true, One per server, rule holds fast, New path, old wisdom.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request successfully introduces the legacy key feature, including the necessary database schema changes, query updates, and API modifications. The new functionality is well-tested with integration tests covering creation, listing with filters, and uniqueness constraints. I've identified a couple of areas for improvement: one is a redundant check in the database client logic, and another is the inclusion of unrelated code changes in one of the model files, which should be moved to a separate PR to maintain focus. There's also a minor formatting artifact in one of the new markdown files.

Copy link
Contributor

github-actions bot commented Aug 5, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 185.605465ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 177.799076ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 341.033927ms
Throughput 293.23 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 37.337819351s
Average Latency 371.675263ms
Throughput 133.91 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.596689943s
Average Latency 255.217844ms
Throughput 195.34 requests/second

Copy link
Member

@dmihalcik-virtru dmihalcik-virtru left a comment

Choose a reason for hiding this comment

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

An optional cleanup or two and a question about proto3 style

Copy link
Contributor

github-actions bot commented Aug 6, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 172.231491ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 97.385131ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 342.090697ms
Throughput 292.32 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.356394563s
Average Latency 361.724854ms
Throughput 137.53 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.472782892s
Average Latency 253.796192ms
Throughput 196.29 requests/second

Copy link
Contributor

github-actions bot commented Aug 6, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 177.953914ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 108.054458ms

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 345.676436ms
Throughput 289.29 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.662034599s
Average Latency 364.731229ms
Throughput 136.38 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.787414142s
Average Latency 257.175074ms
Throughput 193.89 requests/second

@c-r33d c-r33d requested a review from dmihalcik-virtru August 6, 2025 18:44
@c-r33d c-r33d enabled auto-merge August 7, 2025 13:35
@c-r33d c-r33d added this pull request to the merge queue Aug 7, 2025
Merged via the queue into main with commit 57370b0 Aug 7, 2025
41 of 47 checks passed
@c-r33d c-r33d deleted the feat/DSPX-1375-legacy-keys branch August 7, 2025 14:07
github-merge-queue bot pushed a commit that referenced this pull request Aug 9, 2025
🤖 I have created a release *beep* *boop*
---


##
[0.7.0](protocol/go/v0.6.2...protocol/go/v0.7.0)
(2025-08-08)


### ⚠ BREAKING CHANGES

* **policy:** disable kas grants in favor of key mappings
([#2220](#2220))
* **core:** Require go 1.23+
([#1979](#1979))

### Features

* add ability to retrieve policy resources by id or name
([#1901](#1901))
([deb4455](deb4455))
* **authz:** authz v2, ers v2 protos and gencode for ABAC with actions &
registered resource
([#2124](#2124))
([ea7992a](ea7992a))
* **authz:** improve v2 request proto validation
([#2357](#2357))
([f927b99](f927b99))
* **authz:** sensible request limit upper bounds
([#2526](#2526))
([b3093cc](b3093cc))
* **core:** adds bulk rewrap to sdk and service
([#1835](#1835))
([11698ae](11698ae))
* **core:** EXPERIMENTAL: EC-wrapped key support
([#1902](#1902))
([652266f](652266f))
* **core:** Require go 1.23+
([#1979](#1979))
([164c922](164c922))
* **core:** v2 ERS with proto updates
([#2210](#2210))
([a161ef8](a161ef8))
* **policy:** add enhanced standard/custom actions protos
([#2020](#2020))
([bbac53f](bbac53f))
* **policy:** Add legacy keys.
([#2613](#2613))
([57370b0](57370b0))
* **policy:** Add list key mappings rpc.
([#2533](#2533))
([fbc2724](fbc2724))
* **policy:** add obligation protos
([#2579](#2579))
([50882e1](50882e1))
* **policy:** Add validation to delete keys
([#2576](#2576))
([cc169d9](cc169d9))
* **policy:** add values to CreateObligationRequest
([#2614](#2614))
([94535cc](94535cc))
* **policy:** adds new public keys table
([#1836](#1836))
([cad5048](cad5048))
* **policy:** Allow the deletion of a key.
([#2575](#2575))
([82b96f0](82b96f0))
* **policy:** cache SubjectConditionSet selectors in dedicated column
maintained via trigger
([#2320](#2320))
([215791f](215791f))
* **policy:** Change return type for delete key proto.
([#2566](#2566))
([c1ae924](c1ae924))
* **policy:** Default Platform Keys
([#2254](#2254))
([d7447fe](d7447fe))
* **policy:** disable kas grants in favor of key mappings
([#2220](#2220))
([30f8cf5](30f8cf5))
* **policy:** DSPX-1018 NDR retrieval by FQN support
([#2131](#2131))
([0001041](0001041))
* **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-902 NDR service crud protos only (1/2)
([#2092](#2092))
([24b6cb5](24b6cb5))
* **policy:** Finish resource mapping groups
([#2224](#2224))
([5ff754e](5ff754e))
* **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:** Return KAS Key structure
([#2172](#2172))
([7f97b99](7f97b99))
* **policy:** Return Simple Kas Keys from non-Key RPCs
([#2387](#2387))
([5113e0e](5113e0e))
* **policy:** rotate keys rpc
([#2180](#2180))
([0d00743](0d00743))
* **policy:** Update key status's and UpdateKey rpc.
([#2315](#2315))
([7908db9](7908db9))
* **policy:** Update simple kas key
([#2378](#2378))
([09d8239](09d8239))


### Bug Fixes

* add pagination to list public key mappings response
([#1889](#1889))
([9898fbd](9898fbd))
* **core:** Allow 521 curve to be used
([#2485](#2485))
([aaf43dc](aaf43dc))
* **core:** Fixes protoJSON parse bug on ec rewrap
([#1943](#1943))
([9bebfd0](9bebfd0))
* **core:** Update fixtures and flattening in sdk and service
([#1827](#1827))
([d6d6a7a](d6d6a7a))
* **deps:** bump toolchain in /lib/fixtures and /examples to resolve CVE
GO-2025-3563 ([#2061](#2061))
([9c16843](9c16843))
* **policy:** protovalidate deprecated action types and removal of gRPC
gateway in subject mappings svc
([#2377](#2377))
([54a6de0](54a6de0))
* **policy:** remove gRPC gateway in policy except where needed
([#2382](#2382))
([1937acb](1937acb))
* **policy:** remove new public keys rpc's
([#1962](#1962))
([5049bab](5049bab))
* **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))
* **sdk:** Fix compatibility between bulk and non-bulk rewrap
([#1914](#1914))
([74abbb6](74abbb6))
* 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>
Co-authored-by: Krish Suchak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:db DB component comp:policy Policy Configuration ( attributes, subject mappings, resource mappings, kas registry) docs Documentation size/m
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants