Open
Conversation
Define service operations api for promoting, and issuing CDI, ECA, and end-entity certificates as well as signing with an end entity key. Implement a message dispatcher that parses CBOR formatted service requests (see messages.cddl/messages.h) and dispatches them to a provided stateless service implementation.
Fix error handling. Properly compute required buffer sizes.
LCOV of commit
|
There was a problem hiding this comment.
Pull request overview
PR Overview
This PR adds a stateless "gnostic node" DICE service implementation — a service that has direct access to cryptographic key material (CDI secrets) and can perform cryptographic operations directly. It is stateless in that calls do not change service state except for the promote operation, which replaces the root CDI.
Changes:
- New header
include/nat20/service/gnostic.h: Definesn20_gnostic_node_state_t(holds crypto context + CDI) and declaresn20_gnostic_service_ops. - New implementation
src/service/gnostic.c: Implements the five service operations (promote, issue CDI cert, issue ECA cert, issue ECA EE cert, ECA EE sign) with path resolution for stateless CDI derivation. - New test file
src/service/test/gnostic.cpp: Comprehensive tests for all operations including NULL checks, error forwarding, key usage validation, and success paths. - Error code additions in
include/nat20/error.h(codes 30–35) and a new global CMake defineN20_WITH_COSE=1inCMakeLists.txt.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
include/nat20/service/gnostic.h |
Defines the gnostic node state struct and declares the service ops vtable |
src/service/gnostic.c |
Core implementation of all five gnostic node service operations |
src/service/test/gnostic.cpp |
GTest-based tests for all operations, error paths, and key derivation properties |
include/nat20/error.h |
Adds six new error codes (30–35) needed by the gnostic implementation |
CMakeLists.txt |
Registers new source/header files and adds an unused N20_WITH_COSE global define |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The maximum path length is no longer hard coded/set at compile time. Instead, parsing the parent path is deferred, so that the number of compressed input slices need not be known and allocated in advance.
seidelrj
requested changes
Mar 17, 2026
Fix a segfault introduced in the previous upload. Add missing doxygen comments. Some consolidation of common code. Removed check for failing key_free. There is no recovery from failing to free. It will lead to a resource leak and service degradation but there is no immediate security implication.
seidelrj
requested changes
Mar 17, 2026
Contributor
seidelrj
left a comment
seidelrj
left a comment
There was a problem hiding this comment.
just the copilot findings now.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+58
to
+61
| struct MockCryptoContext : public n20_crypto_context_t { | ||
| uint32_t err_on_zero_kdf = std::numeric_limits<uint32_t>::max(); | ||
| n20_error_t kdf_error = n20_error_ok_e; | ||
| uint32_t err_on_zero_digest = std::numeric_limits<uint32_t>::max(); |
| .subject_key_type = n20_crypto_key_type_ed25519_e, | ||
| .certificate_format = n20_certificate_format_x509_e, | ||
| }; | ||
| req.key_usage = {0, nullptr}; // empty key usage is not allowed |
Comment on lines
+176
to
+181
| if (request == NULL) { | ||
| return n20_error_unexpected_null_service_request_e; | ||
| } | ||
|
|
||
| n20_error_t error = | ||
| n20_check_node_state_and_resolve_path(node_state, &request->parent_path, &issuer_secret); |
Comment on lines
+104
to
+115
| mock_crypto_context_.digest_ctx.digest = [](n20_crypto_digest_context_t* ctx, | ||
| n20_crypto_digest_algorithm_t alg, | ||
| n20_crypto_gather_list_t const* data, | ||
| size_t msg_count, | ||
| uint8_t* out, | ||
| size_t* out_len) -> n20_error_t { | ||
| MockCryptoContext* mock_ctx = reinterpret_cast<MockCryptoContext*>(ctx); | ||
| if (!mock_ctx->err_on_zero_digest--) { | ||
| return mock_ctx->digest_error; | ||
| } | ||
| return mock_ctx->digest_fn(ctx, alg, data, msg_count, out, out_len); | ||
| }; |
|
|
||
| n20_error_t error = | ||
| n20_msg_parent_path_iterate(parent_path, n20_resolve_path_iterator_cb, &ictx); | ||
| if (error != n20_error_ok_e) { |
| error = n20_error_key_usage_not_permitted_e; | ||
| goto err_out; | ||
| } | ||
|
|
| * A nat20_crypto_context_t that the service node can use to perform cryptographic | ||
| * operations such as key derivation, digest, signing, and freeing keys. | ||
| * This context is provided by the service integrator and may be implemented | ||
| * using any crypto backend (hardware accellerated or otherwise) that conforms |
Comment on lines
207
to
+210
| extern n20_error_t n20_next_level_cdi_attest(n20_crypto_context_t *crypto_ctx, | ||
| n20_crypto_key_t current_cdi, | ||
| n20_crypto_key_t *next, | ||
| n20_compressed_input_t info); | ||
| n20_slice_t info); |
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
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.
A gnostic node is a NAT20 DICE service that has direct access to cryptographic key material and is, therefore, capable of performing cryptographic transformations directly.
It is also stateless in that service calls do not change the state of the service. The only exception is the root secret (CDI_N) of the node, which can be replaced using the promote call.