feat: use public AES protected key from lib/ocrypto #2600

strantalis · 2025-07-29T20:24:25Z

Update service components to use the newly exposed AESProtectedKey implementation from lib/ocrypto instead of internal implementation.

Update trust/key_manager.go with type aliases for backward compatibility
Replace InProcessAESKey usage with ocrypto.NewAESProtectedKey()
Remove duplicate implementation from security package
Maintain all existing functionality and interfaces

Proposed Changes

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

gemini-code-assist

Summary of Changes

Hello @strantalis, I'm Gemini Code Assist¹! 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!

This pull request introduces a new public interface for handling protected cryptographic keys within the lib/ocrypto package. My aim is to expose previously internal AES-based key management functionality, allowing for external consumption and more robust cryptographic operations like key encapsulation, decryption, and policy binding verification.

Highlights

New Cryptographic Interfaces: I've added ProtectedKey and Encapsulator interfaces in lib/ocrypto/interfaces.go. ProtectedKey defines operations on a decrypted key, while Encapsulator specifies methods for wrapping secret keys.
AES Protected Key Implementation: I've provided AESProtectedKey in lib/ocrypto/protected_key.go, which implements the ProtectedKey interface. This includes methods for AES-GCM decryption, secure key export (with optional encapsulation), and HMAC-based policy binding verification.
Robust Error Handling: I've introduced specific error types (ErrEmptyKeyData, ErrPolicyHMACMismatch, ErrHMACGeneration) to provide clearer feedback on cryptographic operation failures.
Comprehensive Test Coverage: I've included a thorough test suite in lib/ocrypto/protected_key_test.go to ensure the correctness and reliability of the AESProtectedKey implementation, covering various scenarios including valid operations, error conditions, and interface compliance.

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.

Key, safely held,Unlocks secrets, binds with trust,Code's silent guard.

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. ↩

gemini-code-assist

Code Review

This pull request introduces a new ProtectedKey interface and its AESProtectedKey implementation. The changes are well-structured and include comprehensive tests. My review focuses on improving security by preventing key mutation, enhancing code clarity by removing unused parameters and redundant checks, and ensuring consistency in naming.

lib/ocrypto/protected_key.go

lib/ocrypto/interfaces.go

lib/ocrypto/protected_key.go

strantalis · 2025-07-29T22:44:27Z

This pr is dependent on #2599

strantalis · 2025-09-11T14:37:39Z

Needs this pr #2709

service/trust/key_manager.go

dmihalcik-virtru

I think we should be okay for the change to the interface, as the only call I can find outside of this repo is to the concrete type

https://github.com/virtru-corp/data-clean-room/blob/02f64c2af134aa854f017cfd637712bbe8ebd361/pkg/tdf-proxy/requestfilter/s3/mock_s3reader.go#L32

Add ProtectedKey and Encapsulator interfaces to lib/ocrypto package, along with AESProtectedKey implementation. This exposes previously internal cryptographic functionality for external consumption. - Add interfaces.go with ProtectedKey and Encapsulator interfaces - Add protected_key.go with AESProtectedKey implementation - Add comprehensive test suite in protected_key_test.go - Use standard error types without logging dependencies

- Add defensive copying in constructor to prevent external mutation - Add defensive copying in Export method for security - Simplify generateHMACDigest by removing unused context and error handling - Update tests to match simplified signature Addresses feedback from code review on PR opentdf#2599

Update service components to use the newly exposed AESProtectedKey implementation from lib/ocrypto instead of internal implementation. - Update trust/key_manager.go with type aliases for backward compatibility - Replace InProcessAESKey usage with ocrypto.NewAESProtectedKey() - Remove duplicate implementation from security package - Maintain all existing functionality and interfaces

🤖 I have created a release *beep* *boop* --- ## [0.10.0](service/v0.9.0...service/v0.10.0) (2025-09-17) ### ⚠ BREAKING CHANGES * **policy:** Add manager column to provider configuration for multi-instance support ([#2601](#2601)) ### Features * **authz:** add obligation policy decision point ([#2706](#2706)) ([bb2a4f8](bb2a4f8)) * **core:** add service negation for op mode ([#2680](#2680)) ([029db8c](029db8c)) * **core:** Bump default write timeout. ([#2671](#2671)) ([6a233c1](6a233c1)) * **core:** Encapsulate>Encrypt ([#2676](#2676)) ([3c5a614](3c5a614)) * **core:** Lets key manager factory take context ([#2715](#2715)) ([8d70993](8d70993)) * **policy:** add FQN of obligation definitions/values to protos ([#2703](#2703)) ([45ded0e](45ded0e)) * **policy:** Add manager column to provider configuration for multi-instance support ([#2601](#2601)) ([a5fc994](a5fc994)) * **policy:** Add obligation triggers ([#2675](#2675)) ([22d0837](22d0837)) * **policy:** add protovalidate for obligation defs + vals ([#2699](#2699)) ([af5c049](af5c049)) * **policy:** Allow creation and update of triggers on Obligation Values ([#2691](#2691)) ([b1e7ba1](b1e7ba1)) * **policy:** Allow for additional context to be added to obligation triggers ([#2705](#2705)) ([7025599](7025599)) * **policy:** Include Triggers in GET/LISTable reqs ([#2704](#2704)) ([b4381d1](b4381d1)) * **policy:** obligations + values CRUD ([#2545](#2545)) ([c194e35](c194e35)) * use public AES protected key from lib/ocrypto ([#2600](#2600)) ([75d7590](75d7590)) ### Bug Fixes * **core:** remove extraneous comment ([#2741](#2741)) ([ada8da6](ada8da6)) * **core:** return services in the order they were registered ([#2733](#2733)) ([1d661db](1d661db)) * **deps:** bump github.com/opentdf/platform/lib/ocrypto from 0.3.0 to 0.6.0 in /service ([#2714](#2714)) ([00354b3](00354b3)) * **deps:** bump github.com/opentdf/platform/protocol/go from 0.7.0 to 0.9.0 in /service ([#2726](#2726)) ([9004368](9004368)) * **deps:** bump protocol/go to 0.10.0 in service ([#2734](#2734)) ([11e6201](11e6201)) * **deps:** update protovalidate to v0.14.2 to use new buf validate MessageOneofRule ([#2698](#2698)) ([1cae18e](1cae18e)) * **policy:** Registered Resources should consider actions correctly within Decision Requests ([#2681](#2681)) ([cf264a2](cf264a2)) * sanitize db schema identifiers ([#2682](#2682)) ([0d3dd94](0d3dd94)) --- 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>

strantalis requested a review from a team as a code owner July 29, 2025 20:24

github-actions bot added comp:lib:ocrypto size/m labels Jul 29, 2025

gemini-code-assist bot reviewed Jul 29, 2025

View reviewed changes

strantalis requested a review from a team as a code owner July 29, 2025 20:26

gemini-code-assist bot reviewed Jul 29, 2025

View reviewed changes

strantalis changed the title ~~feat(ocrypto): add AES protected key interface and implementation~~ feat(service): use public AES protected key from lib/ocrypto Jul 29, 2025

strantalis changed the title ~~feat(service): use public AES protected key from lib/ocrypto~~ feat: use public AES protected key from lib/ocrypto Jul 29, 2025

strantalis force-pushed the dspx-1474/update-service-usage branch 2 times, most recently from 3ae0e40 to 1073782 Compare July 29, 2025 22:42

jentfoo previously approved these changes Jul 30, 2025

View reviewed changes

strantalis dismissed jentfoo’s stale review via f80dcb5 July 30, 2025 16:26

strantalis force-pushed the dspx-1474/update-service-usage branch from 1073782 to f80dcb5 Compare July 30, 2025 16:26

strantalis force-pushed the dspx-1474/update-service-usage branch from f80dcb5 to 29f21a0 Compare September 4, 2025 18:09

strantalis requested review from a team as code owners September 4, 2025 18:09

strantalis force-pushed the dspx-1474/update-service-usage branch from 1fa1986 to dffa474 Compare September 10, 2025 17:03

dmihalcik-virtru reviewed Sep 11, 2025

View reviewed changes

service/trust/key_manager.go Show resolved Hide resolved

dmihalcik-virtru self-requested a review September 11, 2025 15:28

dmihalcik-virtru previously approved these changes Sep 11, 2025

View reviewed changes

strantalis added 8 commits September 11, 2025 14:28

cleanup lint issues

c310c2e

gemini performance improvements recommendations

0f32163

some minor changes

9d30b81

bump ocrypto

1c1a4e5

rebase on main and update ocrypto

7db59fc

strantalis added 8 commits September 11, 2025 14:30

go mod tidy

5fd345e

remove ocrypto changes

39bd78c

minor fixes and ocrypto fix

d474728

don't expose key

f73603d

fmt

e7b5ac2

fix failing unit test

03dd47b

fmt

e4c461d

fix sum

5515f81

strantalis dismissed dmihalcik-virtru’s stale review via 5515f81 September 11, 2025 18:33

strantalis force-pushed the dspx-1474/update-service-usage branch from 0a8a4cc to 5515f81 Compare September 11, 2025 18:33

fix encapsulate issue

20123c2

dmihalcik-virtru approved these changes Sep 12, 2025

View reviewed changes

strantalis enabled auto-merge September 12, 2025 22:02

jrschumacher approved these changes Sep 12, 2025

View reviewed changes

strantalis added this pull request to the merge queue Sep 12, 2025

jrschumacher approved these changes Sep 12, 2025

View reviewed changes

Merged via the queue into opentdf:main with commit 75d7590 Sep 12, 2025
32 checks passed

strantalis deleted the dspx-1474/update-service-usage branch September 12, 2025 22:31

This was referenced Sep 12, 2025

chore(main): release service 0.10.0 #2672

Merged

chore(main): release lib/ocrypto 0.7.0 #2728

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: use public AES protected key from lib/ocrypto #2600

feat: use public AES protected key from lib/ocrypto #2600

strantalis commented Jul 29, 2025 •

edited

Loading

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

strantalis commented Jul 29, 2025

Uh oh!

strantalis commented Sep 11, 2025 •

edited

Loading

Uh oh!

Uh oh!

dmihalcik-virtru left a comment

Uh oh!

Uh oh!

Uh oh!

feat: use public AES protected key from lib/ocrypto #2600

feat: use public AES protected key from lib/ocrypto #2600

Conversation

strantalis commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Checklist

Testing Instructions

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

strantalis commented Jul 29, 2025

Uh oh!

strantalis commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

dmihalcik-virtru left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

strantalis commented Jul 29, 2025 •

edited

Loading

strantalis commented Sep 11, 2025 •

edited

Loading