feat(kas): add ml-kem key wrapping support#3156
Conversation
Signed-off-by: David Mihalcik <dmihalcik@virtru.com>
github-actions
Bot
added
comp:db
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 comprehensive support for ML-KEM 768, a post-quantum cryptographic algorithm, into the system's key wrapping and unwrapping mechanisms. This update expands the range of supported cryptographic algorithms, particularly within the Key Access Server (KAS), by integrating ML-KEM into the core cryptographic library and updating relevant service logic and protocol definitions. The change aims to enhance the system's future-readiness against advancements in quantum computing by providing a new, robust key encapsulation method. 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. Changelog
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. 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. Quantum threats loom near, ML-KEM secures the key, Future's secrets safe. Footnotes
|
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for ML-KEM key wrapping. The changes are extensive, touching many parts of the codebase from the low-level crypto library to the service and SDK layers. The core logic for ML-KEM encryption and decryption has been added to lib/ocrypto. A significant and beneficial refactoring has been done in service/internal/security to better support multiple key types through interfaces. Overall, the implementation looks good, but I've found a couple of issues that should be addressed, particularly around data encoding and the lack of unit tests for the new cryptographic functionality.
| type MLKEMDecryptor768 struct { | ||
| decap *mlkem.DecapsulationKey768 | ||
| } |
There was a problem hiding this comment.
This PR introduces significant new cryptographic functionality for ML-KEM, but it appears to be missing corresponding unit tests. For example, there are no tests for MLKEMDecryptor768 or MLKEMEncryptor768 to verify the encryption and decryption flow. Adding unit tests is critical to ensure the correctness of this new code and prevent regressions.
|
|
||
| func (e MLKEMEncryptor768) Metadata() (map[string]string, error) { | ||
| m := make(map[string]string) | ||
| m["encapsulatedKey"] = string(e.EphemeralKey()) |
There was a problem hiding this comment.
Casting a raw byte slice from e.EphemeralKey() directly to a string is unsafe. The ephemeral key (which is the ML-KEM ciphertext) is binary data, and this direct conversion can lead to data corruption if the bytes are not valid UTF-8. To safely represent this binary data as a string, it should be encoded, for example, using Base64. Other parts of the codebase, such as generateWrapKeyWithMLKEM in sdk/tdf.go, use Base64 encoding for this purpose.
| m["encapsulatedKey"] = string(e.EphemeralKey()) | |
| m["encapsulatedKey"] = string(Base64Encode(e.EphemeralKey())) |
Signed-off-by: David Mihalcik <dmihalcik@virtru.com>
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
|
Merged in #3652 |
1 participant
Signed-off-by: David Mihalcik dmihalcik@virtru.com
Proposed Changes
Checklist
Testing Instructions