Add ML-KEM agreement - #91
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds ML-KEM (MLKEM1024) support to the node enrollment protocol, making it the default key agreement mechanism while preserving an option to force X25519. It extends the wire/storage schema to carry ML-KEM parameters, and refactors encryption key derivation to use a shared KeyProducer interface so both X25519 and ML-KEM can be handled uniformly.
Changes:
- Add
MLKEM1024as a supportedKEYTYPEand introduceMLKEMParametersacross relevant protobuf messages (node info/creds, fetch request/response, server-led activation token). - Implement ML-KEM handling in node-led and server-led registration flows, including storage wrapping/unwrapping of ML-KEM parameters.
- Introduce
KeyProducer+EncryptionKeyMaterialto generalize encrypt/decrypt operations beyond X25519, and update types/tests accordingly.
Reviewed changes
Copilot reviewed 17 out of 20 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| types/server_led_activation_token.go | Wrap/unwrap ML-KEM parameters for stored activation tokens. |
| types/node_information.go | Persist ML-KEM parameters and add shared-key derivation APIs for encryption/decryption. |
| types/node_information_test.go | Add tests for new shared-key derivation API. |
| types/node_credentials.go | Add ML-KEM key generation, fetch request/response handling, and shared-key derivation APIs. |
| types/node_credentials_test.go | Extend tests to cover both ML-KEM and X25519 variants. |
| types/github.com.hashicorp.nodeenrollment.types.v1.proto | Schema changes: MLKEM key type + MLKEM parameters + updated encryption key fields. |
| types/github.com.hashicorp.nodeenrollment.types.v1.pb.go | Regenerated protobuf output for the updated schema. |
| types/common.go | Add shared-key derivation dispatcher and refactor X25519 derivation helper. |
| types/common_test.go | Add tests for shared-key derivation dispatcher. |
| registration/register_server_led.go | Generate ML-KEM or X25519 server-led activation token key material and validate ML-KEM flows. |
| registration/register_server_led_test.go | Update server-led registration tests for ML-KEM default. |
| registration/register_node_led.go | Update node-led fetch response shape for ML-KEM and support key-type-specific response fields. |
| registration/register_node_led_test.go | Expand node-led tests to cover ML-KEM and X25519. |
| registration/authorize.go | Authorize nodes using either X25519 or ML-KEM key material (including from options). |
| registration/authorize_test.go | Expand authorize tests to cover ML-KEM and X25519. |
| options.go | Add options for encryption private key type/material and ML-KEM parameters. |
| encryption.go | Introduce KeyProducer-based encryption/decryption and keep X25519 producer compatibility. |
| encryption_test.go | Add tests for key selection and previous-key fallback under the new abstraction. |
| multihop/*.pb.go | Protobuf regeneration version bump only. |
Files not reviewed (3)
- multihop/github.com.hashicorp.nodeenrollment.multihop.v1.pb.go: Language not supported
- multihop/github.com.hashicorp.nodeenrollment.multihop.v1_grpc.pb.go: Language not supported
- types/github.com.hashicorp.nodeenrollment.types.v1.pb.go: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 20 changed files in this pull request and generated 8 comments.
Files not reviewed (3)
- multihop/github.com.hashicorp.nodeenrollment.multihop.v1.pb.go: Language not supported
- multihop/github.com.hashicorp.nodeenrollment.multihop.v1_grpc.pb.go: Language not supported
- types/github.com.hashicorp.nodeenrollment.types.v1.pb.go: Language not supported
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 21 changed files in this pull request and generated 2 comments.
Files not reviewed (3)
- multihop/github.com.hashicorp.nodeenrollment.multihop.v1.pb.go: Language not supported
- multihop/github.com.hashicorp.nodeenrollment.multihop.v1_grpc.pb.go: Language not supported
- types/github.com.hashicorp.nodeenrollment.types.v1.pb.go: Language not supported
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 21 changed files in this pull request and generated 9 comments.
Files not reviewed (3)
- multihop/github.com.hashicorp.nodeenrollment.multihop.v1.pb.go: Language not supported
- multihop/github.com.hashicorp.nodeenrollment.multihop.v1_grpc.pb.go: Language not supported
- types/github.com.hashicorp.nodeenrollment.types.v1.pb.go: Language not supported
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 21 changed files in this pull request and generated 5 comments.
Files not reviewed (3)
- multihop/github.com.hashicorp.nodeenrollment.multihop.v1.pb.go: Language not supported
- multihop/github.com.hashicorp.nodeenrollment.multihop.v1_grpc.pb.go: Language not supported
- types/github.com.hashicorp.nodeenrollment.types.v1.pb.go: Language not supported
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 21 changed files in this pull request and generated 3 comments.
Files not reviewed (3)
- multihop/github.com.hashicorp.nodeenrollment.multihop.v1.pb.go: Language not supported
- multihop/github.com.hashicorp.nodeenrollment.multihop.v1_grpc.pb.go: Language not supported
- types/github.com.hashicorp.nodeenrollment.types.v1.pb.go: Language not supported
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 21 changed files in this pull request and generated 3 comments.
Files not reviewed (3)
- multihop/github.com.hashicorp.nodeenrollment.multihop.v1.pb.go: Language not supported
- multihop/github.com.hashicorp.nodeenrollment.multihop.v1_grpc.pb.go: Language not supported
- types/github.com.hashicorp.nodeenrollment.types.v1.pb.go: Language not supported
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 21 changed files in this pull request and generated 2 comments.
Files not reviewed (3)
- multihop/github.com.hashicorp.nodeenrollment.multihop.v1.pb.go: Language not supported
- multihop/github.com.hashicorp.nodeenrollment.multihop.v1_grpc.pb.go: Language not supported
- types/github.com.hashicorp.nodeenrollment.types.v1.pb.go: Language not supported
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 21 changed files in this pull request and generated 5 comments.
Files not reviewed (3)
- multihop/github.com.hashicorp.nodeenrollment.multihop.v1.pb.go: Language not supported
- multihop/github.com.hashicorp.nodeenrollment.multihop.v1_grpc.pb.go: Language not supported
- types/github.com.hashicorp.nodeenrollment.types.v1.pb.go: Language not supported
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 21 changed files in this pull request and generated 1 comment.
Files not reviewed (3)
- multihop/github.com.hashicorp.nodeenrollment.multihop.v1.pb.go: Language not supported
- multihop/github.com.hashicorp.nodeenrollment.multihop.v1_grpc.pb.go: Language not supported
- types/github.com.hashicorp.nodeenrollment.types.v1.pb.go: Language not supported
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 21 changed files in this pull request and generated 2 comments.
Files not reviewed (3)
- multihop/github.com.hashicorp.nodeenrollment.multihop.v1.pb.go: Language not supported
- multihop/github.com.hashicorp.nodeenrollment.multihop.v1_grpc.pb.go: Language not supported
- types/github.com.hashicorp.nodeenrollment.types.v1.pb.go: Language not supported
Comments suppressed due to low confidence (1)
encryption.go:149
- DecryptMessage now requires KeyProducer, which is a breaking API change for callers that previously passed an X25519KeyProducer. To preserve compatibility (and avoid forcing downstream types to add new methods), consider accepting either interface and dispatching via a type switch.
func DecryptMessage(ctx context.Context, ct []byte, keySource KeyProducer, result proto.Message, _ ...Option) error {
const op = "nodeenrollment.DecryptMessage"
switch {
case len(ct) == 0:
return fmt.Errorf("(%s) incoming ciphertext is empty", op)
case IsNil(keySource):
42c3cf7 to
89e6312
Compare
The server can support old nodes with node-led flow but nodes will have to be updated to accept new server activation tokens for server-led flow.
irenarindos
left a comment
There was a problem hiding this comment.
Going to take another review pass at this, but here are some nits around documentation and testing
Description
This adds support for ML-KEM (1024 bit variant, which might be overkill but is not expensive here). ML-KEM is used as the default, with an option that can be used to force X25519 instead.
From a backwards-compatibility standpoint, as far as I am aware the only issue is if the server is updated but the node is not, the node will be unable to use new server activation tokens until it is updated.
PCI review checklist
I have documented a clear reason for, and description of, the change I am making.
If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
If applicable, I've documented the impact of any changes to security controls.
Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.