feat(core): add hybrid NIST EC + ML-KEM key wrapping support#3276
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds three hybrid post‑quantum key algorithms (X‑Wing, P‑256+ML‑KEM‑768, P‑384+ML‑KEM‑1024) across protobuf/OpenAPI/docs, implements X‑Wing and hybrid NIST crypto in ocrypto, integrates hybrid wrapping/unwrapping into SDK and service layers, and updates configs and extensive tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SDK as SDK<br/>(tdf.go)
participant Crypto as Cryptography<br/>(ocrypto)
participant KAS as KAS Service<br/>(security)
participant DB as Database
rect rgba(100, 150, 200, 0.5)
Note over Client,SDK: Client creates TDF using hybrid KAS public key
Client->>SDK: CreateTDF(plaintext, policy, KASInfo)
SDK->>SDK: createKeyAccess(hybrid algorithm, kasPublicKey)
SDK->>Crypto: Encapsulate(publicKey)
Crypto-->>SDK: sharedSecret, ciphertext
SDK->>Crypto: CalculateHKDF(tdfSalt, sharedSecret)
Crypto-->>SDK: wrapKey
SDK->>Crypto: AES‑GCM encrypt(wrapKey, DEK)
Crypto-->>SDK: encryptedDEK
SDK->>SDK: ASN.1 marshal(HybridWrapped)
SDK-->>Client: TDF(..., KeyAccess{keyType:"hybrid-wrapped", wrappedKey})
end
rect rgba(150, 100, 200, 0.5)
Note over KAS,DB: KAS unwraps wrapped DEK
KAS->>KAS: Decrypt(keyDetails, wrappedKey)
KAS->>Crypto: Decapsulate(ciphertext, privKey)
Crypto-->>KAS: sharedSecret
KAS->>Crypto: CalculateHKDF(tdfSalt, sharedSecret) → wrapKey
KAS->>Crypto: AES‑GCM decrypt(wrapKey, encryptedDEK)
Crypto-->>KAS: DEK
KAS-->>Client: return DecryptedKey(DEK)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Summary of ChangesHello, 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! This pull request introduces support for hybrid post-quantum cryptographic key wrapping within the TDF ecosystem. By combining classical elliptic curve cryptography with NIST-standardized ML-KEM algorithms, the changes enhance the platform's security posture against future quantum threats. The implementation includes new cryptographic primitives, updated key management logic, and necessary schema adjustments to support these hybrid schemes across the SDK and KAS services. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe 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 by creating a comment using either
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 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Quantum threats are on the way, Hybrid keys will save the day. EC and KEM now intertwined, Security for all mankind. Footnotes
|
github-actions
Bot
added
comp:db
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces support for hybrid post-quantum cryptography (PQC) schemes, specifically X-Wing and NIST hybrid (EC + ML-KEM) algorithms, across the ocrypto library, SDK, and KAS service. The changes include new key generation and wrapping logic, protocol buffer updates, and service-level integration for the hybrid-wrapped key type. Review feedback identifies a critical bug in the experimental TDF implementation where all hybrid keys are incorrectly processed as X-Wing, along with a similar issue in the test environment's fake KAS. Additionally, improvements are suggested for documentation accuracy regarding supported hybrid schemes and the modularity of shared cryptographic helper functions.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
service/pkg/db/marshalHelpers.go (1)
160-165:⚠️ Potential issue | 🟠 Major
FormatAlgstill misses the two ML-KEM algorithm cases.Line 160 introduces X-Wing, but
policy.Algorithm_ALGORITHM_HPQT_SECP256R1_MLKEM768andpolicy.Algorithm_ALGORITHM_HPQT_SECP384R1_MLKEM1024still hit the unsupported path.Proposed fix
case policy.Algorithm_ALGORITHM_HPQT_XWING: return "hpqt:xwing", nil + case policy.Algorithm_ALGORITHM_HPQT_SECP256R1_MLKEM768: + return "hpqt:secp256r1-mlkem768", nil + case policy.Algorithm_ALGORITHM_HPQT_SECP384R1_MLKEM1024: + return "hpqt:secp384r1-mlkem1024", nil case policy.Algorithm_ALGORITHM_UNSPECIFIED: fallthrough🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/pkg/db/marshalHelpers.go` around lines 160 - 165, FormatAlg is missing cases for the two ML-KEM algorithms so those enums fall through to the default unsupported path; add explicit case branches for policy.Algorithm_ALGORITHM_HPQT_SECP256R1_MLKEM768 and policy.Algorithm_ALGORITHM_HPQT_SECP384R1_MLKEM1024 in the switch (alongside ALGORITHM_HPQT_XWING) and return the correct canonical string identifiers (e.g., the ML-KEM names) with nil error so they no longer trigger the default unsupported-algorithm error in FormatAlg.service/kas/kas.go (1)
195-201:⚠️ Potential issue | 🟠 MajorFallback key discovery is incomplete for hybrid ML-KEM variants.
Line 195 only adds
security.AlgorithmHPQTXWing. If deployment keys are onlyhpqt:secp256r1-mlkem768orhpqt:secp384r1-mlkem1024, this fallback path may fail to seed defaults.Proposed fix
- for _, alg := range []string{security.AlgorithmECP256R1, security.AlgorithmRSA2048, security.AlgorithmHPQTXWing} { + for _, alg := range []string{ + security.AlgorithmECP256R1, + security.AlgorithmRSA2048, + security.AlgorithmHPQTXWing, + security.AlgorithmHPQTSecp256r1MLKEM768, + security.AlgorithmHPQTSecp384r1MLKEM1024, + } {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/kas/kas.go` around lines 195 - 201, The fallback loop only checks security.AlgorithmHPQTXWing and misses hybrid ML-KEM variants, causing defaults to be empty when only hybrid keys like hpqt:secp256r1-mlkem768 or hpqt:secp384r1-mlkem1024 exist; update the loop around cryptoProvider.FindKID to also try the hybrid variant identifiers (e.g., the hpqt ML-KEM names used in your deployment) in addition to security.AlgorithmHPQTXWing, appending any found KIDs to defaults and retaining the existing l.Warn for misses so cryptoProvider.FindKID, defaults, and l.Warn continue to be used consistently.service/policy/db/grant_mappings.go (1)
25-30:⚠️ Potential issue | 🟠 MajorAdd missing ML-KEM algorithm mappings in the switch.
Line 25 adds X-Wing, but algorithms for the NIST EC + ML-KEM variants still fall through to
...UNSPECIFIED, which can silently degrade grant key metadata.Proposed fix
case policy.Algorithm_ALGORITHM_HPQT_XWING: return policy.KasPublicKeyAlgEnum_KAS_PUBLIC_KEY_ALG_ENUM_HPQT_XWING + case policy.Algorithm_ALGORITHM_HPQT_SECP256R1_MLKEM768: + return policy.KasPublicKeyAlgEnum_KAS_PUBLIC_KEY_ALG_ENUM_HPQT_SECP256R1_MLKEM768 + case policy.Algorithm_ALGORITHM_HPQT_SECP384R1_MLKEM1024: + return policy.KasPublicKeyAlgEnum_KAS_PUBLIC_KEY_ALG_ENUM_HPQT_SECP384R1_MLKEM1024 case policy.Algorithm_ALGORITHM_UNSPECIFIED: return policy.KasPublicKeyAlgEnum_KAS_PUBLIC_KEY_ALG_ENUM_UNSPECIFIED🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/policy/db/grant_mappings.go` around lines 25 - 30, The switch mapping on policy.Algorithm (seen handling policy.Algorithm_ALGORITHM_HPQT_XWING -> policy.KasPublicKeyAlgEnum_KAS_PUBLIC_KEY_ALG_ENUM_HPQT_XWING) is missing cases for the NIST EC + ML-KEM algorithm variants, causing them to fall through to KAS_PUBLIC_KEY_ALG_ENUM_UNSPECIFIED; add explicit case branches for each ML-KEM algorithm constant (the policy.Algorithm_... ML-KEM variants) and return the corresponding policy.KasPublicKeyAlgEnum_KAS_PUBLIC_KEY_ALG_ENUM_... ML-KEM enum values instead of letting them default to UNSPECIFIED so grant key metadata is preserved. Ensure each new case mirrors the existing case style used for ALGORITHM_HPQT_XWING and the default remains unchanged.docs/openapi/policy/obligations/obligations.openapi.yaml (1)
511-522:⚠️ Potential issue | 🟠 MajorOpenAPI enum is missing the two ML‑KEM hybrid values
policy.Algorithmandpolicy.KasPublicKeyAlgEnuminclude...HPQT_XWINGbut still omit...HPQT_SECP256R1_MLKEM768and...HPQT_SECP384R1_MLKEM1024. That leaves this API contract out of sync with the supported algorithm set and can break generated clients.Suggested patch
policy.Algorithm: type: string title: Algorithm enum: - ALGORITHM_UNSPECIFIED - ALGORITHM_RSA_2048 - ALGORITHM_RSA_4096 - ALGORITHM_EC_P256 - ALGORITHM_EC_P384 - ALGORITHM_EC_P521 - ALGORITHM_HPQT_XWING + - ALGORITHM_HPQT_SECP256R1_MLKEM768 + - ALGORITHM_HPQT_SECP384R1_MLKEM1024 @@ policy.KasPublicKeyAlgEnum: type: string title: KasPublicKeyAlgEnum enum: - KAS_PUBLIC_KEY_ALG_ENUM_UNSPECIFIED - KAS_PUBLIC_KEY_ALG_ENUM_RSA_2048 - KAS_PUBLIC_KEY_ALG_ENUM_RSA_4096 - KAS_PUBLIC_KEY_ALG_ENUM_EC_SECP256R1 - KAS_PUBLIC_KEY_ALG_ENUM_EC_SECP384R1 - KAS_PUBLIC_KEY_ALG_ENUM_EC_SECP521R1 - KAS_PUBLIC_KEY_ALG_ENUM_HPQT_XWING + - KAS_PUBLIC_KEY_ALG_ENUM_HPQT_SECP256R1_MLKEM768 + - KAS_PUBLIC_KEY_ALG_ENUM_HPQT_SECP384R1_MLKEM1024Also applies to: 538-549
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/openapi/policy/obligations/obligations.openapi.yaml` around lines 511 - 522, The OpenAPI enums policy.Algorithm and policy.KasPublicKeyAlgEnum are missing the two ML‑KEM hybrid values; update both enum definitions to include the entries ...HPQT_SECP256R1_MLKEM768 and ...HPQT_SECP384R1_MLKEM1024 (match casing/style of existing entries like ALGORITHM_HPQT_XWING) so the API contract reflects the full supported algorithm set and stays in sync with generated clients.docs/openapi/policy/actions/actions.openapi.yaml (1)
196-207:⚠️ Potential issue | 🟠 MajorAdd the two ML-KEM enum values here as well.
policy.Algorithmandpolicy.KasPublicKeyAlgEnumstill stop at X-Wing, but this PR addsALGORITHM_HPQT_SECP256R1_MLKEM768andALGORITHM_HPQT_SECP384R1_MLKEM1024elsewhere. That leaves this spec unable to describe validkasKeys[].publicKey.algorithm/KasPublicKey.algvalues, and generated clients may reject legitimate server responses.Also applies to: 223-234
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/openapi/policy/actions/actions.openapi.yaml` around lines 196 - 207, The OpenAPI enum definitions for policy.Algorithm and policy.KasPublicKeyAlgEnum are missing the two new ML-KEM values; update both enums to include ALGORITHM_HPQT_SECP256R1_MLKEM768 and ALGORITHM_HPQT_SECP384R1_MLKEM1024 so that kasKeys[].publicKey.algorithm / KasPublicKey.alg can represent the new algorithms; locate the enum blocks named policy.Algorithm and policy.KasPublicKeyAlgEnum in the YAML and append those two identifiers to their enum lists (also mirror the same changes around the second block noted at lines 223-234).docs/openapi/policy/resourcemapping/resource_mapping.openapi.yaml (1)
406-417:⚠️ Potential issue | 🟠 MajorThese shared policy enums are still missing the two ML-KEM variants.
Only X-Wing was added here, but the shared
policy.Algorithm/policy.KasPublicKeyAlgEnumdefinitions also needALGORITHM_HPQT_SECP256R1_MLKEM768andALGORITHM_HPQT_SECP384R1_MLKEM1024. Otherwise this OpenAPI document still understates the supported key algorithms for embeddedkasKeys/public-key payloads.Also applies to: 433-444
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/openapi/policy/resourcemapping/resource_mapping.openapi.yaml` around lines 406 - 417, The shared policy enum definitions are missing two ML-KEM variants; update the policy.Algorithm and policy.KasPublicKeyAlgEnum enum lists to include ALGORITHM_HPQT_SECP256R1_MLKEM768 and ALGORITHM_HPQT_SECP384R1_MLKEM1024 so the OpenAPI file accurately reflects supported key algorithms for kasKeys/public-key payloads, and mirror the same additions in the other identical enum block referenced (the second occurrence around the kas public-key enums).docs/openapi/policy/kasregistry/key_access_server_registry.openapi.yaml (1)
527-549:⚠️ Potential issue | 🟠 MajorKAS registry OpenAPI still blocks the two new NIST hybrid algorithms.
The schema enums stop at X-Wing, and the
CreateKey,ListKeys, andRotateKey.NewKeyCEL snippets still only admit up to6. Since the authoritativepolicy.Algorithmenum now includes values7and8for the ML-KEM variants, this spec still prevents clients from describing or submitting those new key types.Also applies to: 1146-1155, 1669-1678, 1931-1940
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/openapi/policy/kasregistry/key_access_server_registry.openapi.yaml` around lines 527 - 549, The OpenAPI spec and CEL validation are missing the two new ML-KEM algorithm values (numeric values 7 and 8) causing clients to be blocked; update the policy.Algorithm and policy.KasPublicKeyAlgEnum enums to include the two ML-KEM entries (matching the authoritative enum names for the new ML-KEM variants) and update any CEL expressions used in CreateKey, ListKeys, and RotateKey.NewKey (and the corresponding enum definitions at the other locations noted) to allow values 7 and 8 instead of stopping at 6 so the new key types are accepted.service/kas/access/publicKey.go (1)
79-86: 🧹 Nitpick | 🔵 TrivialMisleading comment: "For RSA keys" now also handles HPQT algorithms.
The comment on line 81 states "For RSA keys, return the public key in PKCS8 format" but this case now handles HPQT hybrid algorithms as well. Consider updating the comment to reflect this.
📝 Suggested comment update
case security.AlgorithmRSA2048, security.AlgorithmHPQTXWing, security.AlgorithmHPQTSecp256r1MLKEM768, security.AlgorithmHPQTSecp384r1MLKEM1024, "": - // For RSA keys, return the public key in PKCS8 format + // For RSA and HPQT keys, return the public key in PKCS8 format pem, err = keyDetails.ExportPublicKey(ctx, trust.KeyTypePKCS8)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/kas/access/publicKey.go` around lines 79 - 86, Update the misleading comment in the switch case that handles security.AlgorithmRSA2048, security.AlgorithmHPQTXWing, security.AlgorithmHPQTSecp256r1MLKEM768, security.AlgorithmHPQTSecp384r1MLKEM1024 and "" to accurately reflect that both RSA and HPQT/hybrid algorithms are covered; edit the comment above the case (near keyDetails.ExportPublicKey usage) to say something like "For RSA and HPQT/hybrid keys, return the public key in PKCS8 format" so the comment matches the handled algorithms and refers to keyDetails.ExportPublicKey for clarity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/grpc/index.html`:
- Line 6291: Update the description of the 'hybrid-wrapped' value to be
algorithm-agnostic (or list all supported hybrid variants) instead of implying
X-Wing-only; locate the occurrences of the literal 'hybrid-wrapped' in the docs
(e.g., the HTML lines that currently read "hybrid-wrapped (experimental
X-Wing-wrapped)") and replace that phrasing with either a generic description
like "hybrid-wrapped (experimental hybrid KEMs combining classical and
post-quantum/ECDH schemes; algorithm-specific variants may include NIST EC +
ML-KEM, X-Wing, etc.)" or enumerate the supported hybrid variants so integrators
are not misled.
- Around line 4147-4151: The generated docs are missing the ML-KEM enum entries
for KasPublicKeyAlgEnum; add entries for KAS_PUBLIC_KEY_ALG_ENUM_MLKEM768 = 11
and KAS_PUBLIC_KEY_ALG_ENUM_MLKEM1024 = 12 alongside the existing
KAS_PUBLIC_KEY_ALG_ENUM_HPQT_XWING (10) in the KasPublicKeyAlgEnum section so
the documentation matches service/policy/objects.proto (symbols:
KasPublicKeyAlgEnum, KAS_PUBLIC_KEY_ALG_ENUM_HPQT_XWING,
KAS_PUBLIC_KEY_ALG_ENUM_MLKEM768, KAS_PUBLIC_KEY_ALG_ENUM_MLKEM1024).
In `@docs/openapi/authorization/v2/authorization.openapi.yaml`:
- Line 178: The OpenAPI enums that currently list ALGORITHM_HPQT_XWING need the
two ML-KEM variants added so API contracts cover the full hybrid family; update
both enums that contain ALGORITHM_HPQT_XWING to also include
ALGORITHM_HPQT_MLKEM and ALGORITHM_HPQT_MLKEM_AES (or your repository’s exact
ML-KEM symbol names) so NIST EC + ML-KEM variants are present alongside
ALGORITHM_HPQT_XWING.
In `@sdk/basekey_test.go`:
- Around line 96-100: Add test cases for the two new ML-KEM mappings by updating
the table-driven tests in TestGetKasKeyAlg and TestFormatAlg and the round-trip
cases so they include entries for HybridSecp256r1MLKEM768Key and
HybridSecp384r1MLKEM1024Key; specifically, add rows mapping
string(HybridSecp256r1MLKEM768Key) ->
policy.Algorithm_ALGORITHM_ML_KEM_SECP256R1_768 (or the exact enum used) and
string(HybridSecp384r1MLKEM1024Key) ->
policy.Algorithm_ALGORITHM_ML_KEM_SECP384R1_1024 in the same style as the
existing "hybrid xwing" entry, and include them in the round-trip assertion
table so TestGetKasKeyAlg, TestFormatAlg and the round-trip checks validate the
new conversions.
In `@sdk/granter.go`:
- Around line 505-510: FormatAlg is missing cases for
ALGORITHM_HPQT_SECP256R1_MLKEM768 and ALGORITHM_HPQT_SECP384R1_MLKEM1024,
causing those algs to fall through to the default error; add explicit switch
cases in FormatAlg that return the corresponding ocrypto constants
(ocrypto.HybridSecp256r1MLKEM768Key and ocrypto.HybridSecp384r1MLKEM1024Key)
similar to the existing HPQT_XWING case, and update reverseAlgMap in
service/pkg/db/marshalHelpers_test.go to include mappings from
ALGORITHM_HPQT_SECP256R1_MLKEM768 -> ocrypto.HybridSecp256r1MLKEM768Key and
ALGORITHM_HPQT_SECP384R1_MLKEM1024 -> ocrypto.HybridSecp384r1MLKEM1024Key so
tests cover these new variants.
In `@sdk/schema/manifest-lax.schema.json`:
- Line 55: The enum that currently lists ["wrapped", "hybrid-wrapped", "remote"]
must include "ec-wrapped" to preserve compatibility with existing EC manifests;
update the enum array in sdk/schema/manifest-lax.schema.json (the enum
containing "wrapped" / "hybrid-wrapped" / "remote") to also include "ec-wrapped"
so the schema accepts EC-style manifests used by SDK key access generation.
In `@sdk/tdf_test.go`:
- Around line 2919-2920: The two KAS fixture entries for "https://d.kas/" and
"https://e.kas/" advertise "rsa:2048" while their key material variables
(mockECPrivateKey1/mockECPublicKey1 and mockECPrivateKey2/mockECPublicKey2) are
EC keys; update those two entries in sdk/tdf_test.go to the correct EC algorithm
identifier (e.g., "ec:secp256r1" or the project's canonical EC string) so the
advertised algorithm matches the PEM, and re-run tests to ensure no other
fixtures rely on "rsa:2048"; this keeps KASInfo.Algorithm consistent with the
key material.
- Around line 3198-3230: The hybrid rewrap branch creates an asymmetric
encryptor via ocrypto.FromPublicPEM(bodyData.GetClientPublicKey()) and encrypts
the symmetricKey into entityWrappedKey but never sets resp.SessionPublicKey;
update this branch to mirror the ec-wrapped path by extracting the
ephemeral/session public key from the asymmetric encryptor (the same value the
ec-wrapped branch sets) and assign it to resp.SessionPublicKey after creating
asymEncrypt and before returning, so EC client keys receive the session public
key needed to unwrap the returned DEK.
In `@sdk/tdf.go`:
- Line 46: The comment notes duplicated hybrid wrap logic between kHybridWrapped
handling in createKeyAccess/generateWrapKeyWithHybrid and wrapKeyWithHybrid in
sdk/experimental/tdf/key_access.go; refactor by extracting the shared hybrid
key-wrap logic into a single helper function (e.g., NewHybridWrapKey or
wrapKeyHybridShared) placed in a common package accessible to both stable and
experimental code, update createKeyAccess to call that new helper instead of
generateWrapKeyWithHybrid, remove or forward the duplicate wrapKeyWithHybrid to
the shared helper, and ensure kHybridWrapped constant and error handling are
preserved so behavior remains unchanged.
In `@service/internal/security/standard_crypto.go`:
- Around line 502-543: The cases for StandardXWingCrypto and
StandardHybridCrypto risk confusing variable shadowing around err and
privateKey; extract each decryption path into small helper methods (e.g.,
decryptXWing using XWingPrivateKeyFromPem + XWingUnwrapDEK, decryptP256MLKEM768
using P256MLKEM768PrivateKeyFromPem + P256MLKEM768UnwrapDEK,
decryptP384MLKEM1024 using P384MLKEM1024PrivateKeyFromPem +
P384MLKEM1024UnwrapDEK) and call those from the switch, so parsing and unwrap
happen in isolated functions and no local err shadowing occurs in
StandardXWingCrypto / StandardHybridCrypto.
In `@service/policy/kasregistry/key_access_server_registry.proto`:
- Around line 403-407: The CEL validators for the key_algorithm field (validator
id "key_algorithm_defined") restrict allowed values to [1..6], but the Algorithm
enum now includes values 7 and 8 (ALGORITHM_HPQT_SECP256R1_MLKEM768 and
ALGORITHM_HPQT_SECP384R1_MLKEM1024), causing Create/List/Rotate key requests
with ML‑KEM algorithms to fail; update the expression in the key_algorithm
validators (the field named key_algorithm in key_access_server_registry.proto)
to include 7 and 8 (e.g., change "this in [1, 2, 3, 4, 5, 6]" to "this in [1, 2,
3, 4, 5, 6, 7, 8]") for all occurrences (including the other two validator
instances referenced in the comment) so the new enum values pass validation.
---
Outside diff comments:
In `@docs/openapi/policy/actions/actions.openapi.yaml`:
- Around line 196-207: The OpenAPI enum definitions for policy.Algorithm and
policy.KasPublicKeyAlgEnum are missing the two new ML-KEM values; update both
enums to include ALGORITHM_HPQT_SECP256R1_MLKEM768 and
ALGORITHM_HPQT_SECP384R1_MLKEM1024 so that kasKeys[].publicKey.algorithm /
KasPublicKey.alg can represent the new algorithms; locate the enum blocks named
policy.Algorithm and policy.KasPublicKeyAlgEnum in the YAML and append those two
identifiers to their enum lists (also mirror the same changes around the second
block noted at lines 223-234).
In `@docs/openapi/policy/kasregistry/key_access_server_registry.openapi.yaml`:
- Around line 527-549: The OpenAPI spec and CEL validation are missing the two
new ML-KEM algorithm values (numeric values 7 and 8) causing clients to be
blocked; update the policy.Algorithm and policy.KasPublicKeyAlgEnum enums to
include the two ML-KEM entries (matching the authoritative enum names for the
new ML-KEM variants) and update any CEL expressions used in CreateKey, ListKeys,
and RotateKey.NewKey (and the corresponding enum definitions at the other
locations noted) to allow values 7 and 8 instead of stopping at 6 so the new key
types are accepted.
In `@docs/openapi/policy/obligations/obligations.openapi.yaml`:
- Around line 511-522: The OpenAPI enums policy.Algorithm and
policy.KasPublicKeyAlgEnum are missing the two ML‑KEM hybrid values; update both
enum definitions to include the entries ...HPQT_SECP256R1_MLKEM768 and
...HPQT_SECP384R1_MLKEM1024 (match casing/style of existing entries like
ALGORITHM_HPQT_XWING) so the API contract reflects the full supported algorithm
set and stays in sync with generated clients.
In `@docs/openapi/policy/resourcemapping/resource_mapping.openapi.yaml`:
- Around line 406-417: The shared policy enum definitions are missing two ML-KEM
variants; update the policy.Algorithm and policy.KasPublicKeyAlgEnum enum lists
to include ALGORITHM_HPQT_SECP256R1_MLKEM768 and
ALGORITHM_HPQT_SECP384R1_MLKEM1024 so the OpenAPI file accurately reflects
supported key algorithms for kasKeys/public-key payloads, and mirror the same
additions in the other identical enum block referenced (the second occurrence
around the kas public-key enums).
In `@service/kas/access/publicKey.go`:
- Around line 79-86: Update the misleading comment in the switch case that
handles security.AlgorithmRSA2048, security.AlgorithmHPQTXWing,
security.AlgorithmHPQTSecp256r1MLKEM768,
security.AlgorithmHPQTSecp384r1MLKEM1024 and "" to accurately reflect that both
RSA and HPQT/hybrid algorithms are covered; edit the comment above the case
(near keyDetails.ExportPublicKey usage) to say something like "For RSA and
HPQT/hybrid keys, return the public key in PKCS8 format" so the comment matches
the handled algorithms and refers to keyDetails.ExportPublicKey for clarity.
In `@service/kas/kas.go`:
- Around line 195-201: The fallback loop only checks security.AlgorithmHPQTXWing
and misses hybrid ML-KEM variants, causing defaults to be empty when only hybrid
keys like hpqt:secp256r1-mlkem768 or hpqt:secp384r1-mlkem1024 exist; update the
loop around cryptoProvider.FindKID to also try the hybrid variant identifiers
(e.g., the hpqt ML-KEM names used in your deployment) in addition to
security.AlgorithmHPQTXWing, appending any found KIDs to defaults and retaining
the existing l.Warn for misses so cryptoProvider.FindKID, defaults, and l.Warn
continue to be used consistently.
In `@service/pkg/db/marshalHelpers.go`:
- Around line 160-165: FormatAlg is missing cases for the two ML-KEM algorithms
so those enums fall through to the default unsupported path; add explicit case
branches for policy.Algorithm_ALGORITHM_HPQT_SECP256R1_MLKEM768 and
policy.Algorithm_ALGORITHM_HPQT_SECP384R1_MLKEM1024 in the switch (alongside
ALGORITHM_HPQT_XWING) and return the correct canonical string identifiers (e.g.,
the ML-KEM names) with nil error so they no longer trigger the default
unsupported-algorithm error in FormatAlg.
In `@service/policy/db/grant_mappings.go`:
- Around line 25-30: The switch mapping on policy.Algorithm (seen handling
policy.Algorithm_ALGORITHM_HPQT_XWING ->
policy.KasPublicKeyAlgEnum_KAS_PUBLIC_KEY_ALG_ENUM_HPQT_XWING) is missing cases
for the NIST EC + ML-KEM algorithm variants, causing them to fall through to
KAS_PUBLIC_KEY_ALG_ENUM_UNSPECIFIED; add explicit case branches for each ML-KEM
algorithm constant (the policy.Algorithm_... ML-KEM variants) and return the
corresponding policy.KasPublicKeyAlgEnum_KAS_PUBLIC_KEY_ALG_ENUM_... ML-KEM enum
values instead of letting them default to UNSPECIFIED so grant key metadata is
preserved. Ensure each new case mirrors the existing case style used for
ALGORITHM_HPQT_XWING and the default remains unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 19596a14-4638-47ad-be5a-be8062ddd675
⛔ Files ignored due to path filters (4)
lib/ocrypto/go.sumis excluded by!**/*.sumprotocol/go/kas/kas.pb.gois excluded by!**/*.pb.goprotocol/go/policy/kasregistry/key_access_server_registry.pb.gois excluded by!**/*.pb.goprotocol/go/policy/objects.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (50)
docs/grpc/index.htmldocs/openapi/authorization/authorization.openapi.yamldocs/openapi/authorization/v2/authorization.openapi.yamldocs/openapi/kas/kas.openapi.yamldocs/openapi/policy/actions/actions.openapi.yamldocs/openapi/policy/attributes/attributes.openapi.yamldocs/openapi/policy/kasregistry/key_access_server_registry.openapi.yamldocs/openapi/policy/namespaces/namespaces.openapi.yamldocs/openapi/policy/objects.openapi.yamldocs/openapi/policy/obligations/obligations.openapi.yamldocs/openapi/policy/registeredresources/registered_resources.openapi.yamldocs/openapi/policy/resourcemapping/resource_mapping.openapi.yamldocs/openapi/policy/subjectmapping/subject_mapping.openapi.yamldocs/openapi/policy/unsafe/unsafe.openapi.yamllib/ocrypto/asym_decryption.golib/ocrypto/asym_encryption.golib/ocrypto/ec_key_pair.golib/ocrypto/go.modlib/ocrypto/hybrid_nist.golib/ocrypto/hybrid_nist_test.golib/ocrypto/xwing.golib/ocrypto/xwing_test.gosdk/basekey.gosdk/basekey_test.gosdk/experimental/tdf/key_access.gosdk/experimental/tdf/key_access_test.gosdk/experimental/tdf/keysplit/attributes.gosdk/experimental/tdf/keysplit/attributes_test.gosdk/granter.gosdk/schema/manifest-lax.schema.jsonsdk/schema/manifest.schema.jsonsdk/tdf.gosdk/tdf_hybrid_test.gosdk/tdf_test.goservice/internal/security/basic_manager.goservice/internal/security/crypto_provider.goservice/internal/security/in_process_provider.goservice/internal/security/standard_crypto.goservice/kas/access/provider.goservice/kas/access/publicKey.goservice/kas/access/rewrap.goservice/kas/kas.goservice/kas/kas.protoservice/kas/key_indexer.goservice/kas/key_indexer_test.goservice/pkg/db/marshalHelpers.goservice/pkg/db/marshalHelpers_test.goservice/policy/db/grant_mappings.goservice/policy/kasregistry/key_access_server_registry.protoservice/policy/objects.proto
👮 Files not reviewed due to content moderation or server errors (8)
- service/kas/access/rewrap.go
- docs/openapi/policy/unsafe/unsafe.openapi.yaml
- service/kas/key_indexer_test.go
- docs/openapi/policy/namespaces/namespaces.openapi.yaml
- service/kas/key_indexer.go
- docs/openapi/policy/subjectmapping/subject_mapping.openapi.yaml
- service/internal/security/crypto_provider.go
- docs/openapi/authorization/authorization.openapi.yaml
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdk/experimental/tdf/key_access.go (1)
221-233:⚠️ Potential issue | 🟠 MajorEC key wrapping uses XOR instead of AES-GCM - breaks interoperability with stable SDK.
The experimental
wrapKeyWithECfunction uses XOR-based key wrapping (line 230:wrapped[i] = symKey[i] ^ wrapKey[i]), while the stable SDK'sgenerateWrapKeyWithECuses AES-GCM encryption. TDFs created with the experimental SDK using EC keys will not be decryptable by the stable SDK, and vice versa.The hybrid key wrapping implementation correctly uses AES-GCM (line 298), matching the stable SDK pattern. Align
wrapKeyWithECto use AES-GCM for consistency and interoperability:Expected change
Replace the XOR wrapping logic with AES-GCM:
gcm, err := ocrypto.NewAESGcm(wrapKey) if err != nil { return "", "", "", fmt.Errorf("failed to create AES-GCM: %w", err) } wrapped, err := gcm.Encrypt(symKey) if err != nil { return "", "", "", fmt.Errorf("failed to encrypt: %w", err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/experimental/tdf/key_access.go` around lines 221 - 233, The wrapKeyWithEC function currently XORs symKey with wrapKey (wrapped[i] = symKey[i] ^ wrapKey[i]) which breaks interoperability with the stable SDK; replace that XOR logic in wrapKeyWithEC with AES-GCM encryption like the hybrid path and the stable generateWrapKeyWithEC uses: create an AES-GCM instance from wrapKey (using ocrypto.NewAESGcm or equivalent), call its Encrypt on symKey to produce wrapped, and propagate any errors with wrapped/ephemeralPubKey error returns (e.g., wrap failure -> return "", "", "", fmt.Errorf("failed to create AES-GCM: %w", err) or "failed to encrypt: %w"). Ensure the function still returns Base64Encode(wrapped), the same wrapper type "eccWrapped", and the ephemeralPubKey on success.
♻️ Duplicate comments (1)
sdk/tdf.go (1)
774-836: 🧹 Nitpick | 🔵 TrivialImplementation is correct; consider consolidating with experimental package.
The hybrid wrapping logic follows sound cryptographic practices:
- KEM encapsulation → HKDF key derivation → AES-GCM encryption → ASN.1 marshaling
However, this function is structurally identical to
wrapKeyWithHybridinsdk/experimental/tdf/key_access.go. When the experimental package stabilizes, extract the shared logic into a common helper to reduce maintenance burden.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/tdf.go` around lines 774 - 836, The generateWrapKeyWithHybrid function duplicates logic found in wrapKeyWithHybrid (sdk/experimental/tdf/key_access.go); consolidate by extracting the common KEM→HKDF→AES-GCM→ASN.1 flow into a shared helper (e.g., shared package or util function) that both generateWrapKeyWithHybrid and wrapKeyWithHybrid call, keeping unique parsing/algorithm-dispatching in each caller (referencing generateWrapKeyWithHybrid, wrapKeyWithHybrid, and the ocrypto encapsulate/KDF/AESGcm/ASN.1 steps) so maintenance happens in one place once the experimental package stabilizes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/ocrypto/hybrid_nist.go`:
- Around line 97-105: Add a short clarifying comment around the mlkemDecapsulate
implementation (the anonymous function assigned to mlkemDecapsulate that calls
sk.DecapsulateTo) stating that CIRCL's DecapsulateTo implements "implicit
rejection" and therefore does not return an error on incorrect ciphertext, and
that ciphertext authenticity is provided by the subsequent AES-GCM decryption
step rather than by ML-KEM; place the comment next to the DecapsulateTo call so
future readers understand the design choice and where tampering is detected.
---
Outside diff comments:
In `@sdk/experimental/tdf/key_access.go`:
- Around line 221-233: The wrapKeyWithEC function currently XORs symKey with
wrapKey (wrapped[i] = symKey[i] ^ wrapKey[i]) which breaks interoperability with
the stable SDK; replace that XOR logic in wrapKeyWithEC with AES-GCM encryption
like the hybrid path and the stable generateWrapKeyWithEC uses: create an
AES-GCM instance from wrapKey (using ocrypto.NewAESGcm or equivalent), call its
Encrypt on symKey to produce wrapped, and propagate any errors with
wrapped/ephemeralPubKey error returns (e.g., wrap failure -> return "", "", "",
fmt.Errorf("failed to create AES-GCM: %w", err) or "failed to encrypt: %w").
Ensure the function still returns Base64Encode(wrapped), the same wrapper type
"eccWrapped", and the ephemeralPubKey on success.
---
Duplicate comments:
In `@sdk/tdf.go`:
- Around line 774-836: The generateWrapKeyWithHybrid function duplicates logic
found in wrapKeyWithHybrid (sdk/experimental/tdf/key_access.go); consolidate by
extracting the common KEM→HKDF→AES-GCM→ASN.1 flow into a shared helper (e.g.,
shared package or util function) that both generateWrapKeyWithHybrid and
wrapKeyWithHybrid call, keeping unique parsing/algorithm-dispatching in each
caller (referencing generateWrapKeyWithHybrid, wrapKeyWithHybrid, and the
ocrypto encapsulate/KDF/AESGcm/ASN.1 steps) so maintenance happens in one place
once the experimental package stabilizes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7e652f77-7128-4434-a346-54a1bb0cd243
📒 Files selected for processing (7)
lib/ocrypto/ec_key_pair.golib/ocrypto/hybrid_nist.golib/ocrypto/hybrid_nist_test.golib/ocrypto/xwing.golib/ocrypto/xwing_test.gosdk/experimental/tdf/key_access.gosdk/tdf.go
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/ocrypto/hybrid_nist_test.go`:
- Around line 197-227: The tests TestP256MLKEM768Encapsulate and
TestP384MLKEM1024Encapsulate currently only check hybridCt is non-empty; change
those assertions to validate the exact ciphertext length using the protocol
constants (P256MLKEM768CiphertextSize and P384MLKEM1024CiphertextSize) against
the hybridCt slice (and prefer require/ assert that checks size, e.g.,
require.Equal/ assert.Len) so the tests fail on size regressions instead of just
non-empty values.
- Around line 87-111: The tests TestP256MLKEM768WrapUnwrapWrongKeyFails and
TestP384MLKEM1024WrapUnwrapWrongKeyFails currently accept any error; update each
to assert the specific unwrap failure by replacing the generic require.Error
check with a check that the returned error from P256MLKEM768UnwrapDEK /
P384MLKEM1024UnwrapDEK contains the string "AES-GCM decrypt failed" (e.g., use
require.ErrorContains(t, err, "AES-GCM decrypt failed") or an equivalent
assertion) so the wrong-key path is validated explicitly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6213bab2-4c80-4aae-9340-efd652fbb3d5
📒 Files selected for processing (2)
lib/ocrypto/hybrid_nist_test.golib/ocrypto/xwing_test.go
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
lib/ocrypto/hybrid_nist.go (1)
97-104: 🧹 Nitpick | 🔵 TrivialClarify CIRCL's implicit-rejection behavior next to
DecapsulateTo.
DecapsulateTointentionally does not fail on bad ciphertext, so a short note that tampering is detected by the later AES-GCM decrypt would prevent future “missing error handling” cleanups here.Also applies to: 128-135
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ocrypto/hybrid_nist.go` around lines 97 - 104, Add a short clarifying comment next to the call to sk.DecapsulateTo in mlkemDecapsulate (and the analogous block at lines 128–135) stating that CIRCL's DecapsulateTo intentionally does not return an error on malformed ciphertext (implicit-rejection) and that tampering is detected later by the AES-GCM decryption step; reference mlkem768.PrivateKey, DecapsulateTo and the subsequent AES-GCM decrypt to make it clear why no error is returned here and to prevent future reviewers from trying to add error handling at this callsite.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/ocrypto/hybrid_nist.go`:
- Around line 307-320: Rename the scheme-specific helper defaultXWingSalt() to a
scheme-agnostic defaultHybridSalt() and update all callers (e.g.,
NewP256MLKEM768Decryptor, NewSaltedP256MLKEM768Decryptor,
NewP384MLKEM1024Decryptor, NewSaltedP384MLKEM1024Decryptor and the other
occurrences noted) to call defaultHybridSalt() instead; keep the implementation
identical (or make defaultHybridSalt() delegate to defaultXWingSalt() and then
remove the old name) so behavior is unchanged but the helper name is generic for
all hybrid NIST constructors.
In `@sdk/experimental/tdf/writer_test.go`:
- Around line 892-980: The tests testHybridXWingFlow, testHybridP256MLKEM768Flow
and testHybridP384MLKEM1024Flow currently only assert a non-empty WrappedKey;
update each to call validateManifestSchema(t, result.Manifest) and then perform
a full round-trip unwrap using the generated keyPair's private key (from
ocrypto.NewXWingKeyPair / NewP256MLKEM768KeyPair / NewP384MLKEM1024KeyPair) and
the unwrap implementation in lib/ocrypto/hybrid_nist.go to decrypt
result.Manifest.KeyAccessObjs[0].WrappedKey, asserting the unwrapped CEK equals
the expected content-encryption key used by the writer (or that decrypting the
encrypted segment recovers the original bytes); keep NewWriter, WriteSegment and
Finalize usage but add these schema and unwrap assertions to ensure writer and
hybrid_nist unwrap are compatible.
- Around line 883-890: The helper createTestAttributeWithAlgorithm currently
no-ops when the nested grants/KAS-keys are missing which hides test setup bugs;
change its signature to accept t *testing.T (e.g.,
createTestAttributeWithAlgorithm(t *testing.T, fqn, kasURL, kid string, alg
policy.Algorithm, pem string)) and replace the silent branch with a fail-fast
assertion (t.Fatalf or t.Fatalff) if value.GetGrants() is empty or
value.GetGrants()[0].GetKasKeys() is empty, then proceed to set
PublicKey.Algorithm and PublicKey.Pem; update all callers to pass the testing.T.
---
Duplicate comments:
In `@lib/ocrypto/hybrid_nist.go`:
- Around line 97-104: Add a short clarifying comment next to the call to
sk.DecapsulateTo in mlkemDecapsulate (and the analogous block at lines 128–135)
stating that CIRCL's DecapsulateTo intentionally does not return an error on
malformed ciphertext (implicit-rejection) and that tampering is detected later
by the AES-GCM decryption step; reference mlkem768.PrivateKey, DecapsulateTo and
the subsequent AES-GCM decrypt to make it clear why no error is returned here
and to prevent future reviewers from trying to add error handling at this
callsite.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ddf776b3-70e8-4011-b2bd-5a5f433f2483
📒 Files selected for processing (6)
lib/ocrypto/hybrid_nist.gosdk/basekey_test.gosdk/experimental/tdf/writer_test.goservice/pkg/db/marshalHelpers.goservice/pkg/db/marshalHelpers_test.goservice/policy/kasregistry/key_access_server_registry.proto
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/grpc/index.html (1)
6315-6315:⚠️ Potential issue | 🟠 MajorBroaden
hybrid-wrappedwording beyond X-Wing-only phrasing.Line 6315 and Line 6700 still describe hybrid wrapping as X-Wing-specific, which is inaccurate now that NIST EC + ML-KEM variants are supported too. Please make this algorithm-agnostic (or enumerate all supported hybrid variants).
Because this HTML is generated, the source comments inservice/kas/kas.protoshould be updated and docs regenerated.📝 Suggested wording update
-Values: 'wrapped' (RSA-wrapped for ZTDF), 'ec-wrapped' (experimental ECDH-wrapped), 'hybrid-wrapped' (experimental X-Wing-wrapped) +Values: 'wrapped' (RSA-wrapped for ZTDF), 'ec-wrapped' (experimental ECDH-wrapped), 'hybrid-wrapped' (experimental hybrid PQ-wrapped, e.g., X-Wing and NIST EC+ML-KEM variants)-Optional: Empty for RSA-based or X-Wing-based ZTDF (key_type="wrapped" or key_type="hybrid-wrapped") +Optional: Empty for RSA-based or hybrid PQ-wrapped ZTDF (key_type="wrapped" or key_type="hybrid-wrapped")Also applies to: 6700-6700
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/grpc/index.html` at line 6315, The docs still describe the 'hybrid-wrapped' value as X-Wing-specific; update the source comment in service/kas/kas.proto where the enum/value 'hybrid-wrapped' (and any surrounding description for the Values list) is documented to make the wording algorithm-agnostic (e.g., "hybrid-wrapped (hybrid KEM combining classical EC and PQ/ML-KEM variants such as X-Wing, NIST EC+ML-KEM, etc.)" or enumerate supported hybrids), then regenerate the HTML/docs so lines referencing 'hybrid-wrapped' (shown in generated docs at the two occurrences) reflect the new, broadened wording.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/grpc/index.html`:
- Line 6315: The docs still describe the 'hybrid-wrapped' value as
X-Wing-specific; update the source comment in service/kas/kas.proto where the
enum/value 'hybrid-wrapped' (and any surrounding description for the Values
list) is documented to make the wording algorithm-agnostic (e.g.,
"hybrid-wrapped (hybrid KEM combining classical EC and PQ/ML-KEM variants such
as X-Wing, NIST EC+ML-KEM, etc.)" or enumerate supported hybrids), then
regenerate the HTML/docs so lines referencing 'hybrid-wrapped' (shown in
generated docs at the two occurrences) reflect the new, broadened wording.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7ff68288-35c0-4752-945c-722dde3d092b
📒 Files selected for processing (13)
docs/grpc/index.htmldocs/openapi/authorization/authorization.openapi.yamldocs/openapi/authorization/v2/authorization.openapi.yamldocs/openapi/policy/actions/actions.openapi.yamldocs/openapi/policy/attributes/attributes.openapi.yamldocs/openapi/policy/kasregistry/key_access_server_registry.openapi.yamldocs/openapi/policy/namespaces/namespaces.openapi.yamldocs/openapi/policy/objects.openapi.yamldocs/openapi/policy/obligations/obligations.openapi.yamldocs/openapi/policy/registeredresources/registered_resources.openapi.yamldocs/openapi/policy/resourcemapping/resource_mapping.openapi.yamldocs/openapi/policy/subjectmapping/subject_mapping.openapi.yamldocs/openapi/policy/unsafe/unsafe.openapi.yaml
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
The determineKeyType method only accepts a key ID parameter, not a context. Update test calls to match the function signature. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Contributor
|
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
dmihalcik-virtru
approved these changes
May 21, 2026
strantalis
approved these changes
May 21, 2026
jrschumacher
approved these changes
May 21, 2026
This was referenced May 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed Changes
(hpqt:secp384r1-mlkem1024)
Checklist
Testing Instructions
Summary by CodeRabbit
New Features
Documentation
Configuration
Tests