Add stateless gnostic node implementation by werwurm · Pull Request #89 · aurora-opensource/libnat20

werwurm · 2026-03-10T00:01:02Z

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.

github-actions · 2026-03-10T00:02:44Z

LCOV of commit `fc2a2d1` during lcov-test-coverage-report #109

Summary coverage rate:
  lines......: 95.6% (2940 of 3075 lines)
  functions..: 99.1% (221 of 223 functions)
  branches...: 86.9% (1603 of 1845 branches)

Files changed coverage rate: n/a

Copilot

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: Defines n20_gnostic_node_state_t (holds crypto context + CDI) and declares n20_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 define N20_WITH_COSE=1 in CMakeLists.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.

Copilot · 2026-03-10T00:11:15Z

src/service/gnostic.c

+    error = node_state->crypto_context->key_free(node_state->crypto_context, *min_cdi);
+    if (error != n20_error_ok_e) {
+        return error;
+    }


In n20_gnostic_promote, when key_free at line 86 returns an error, the function returns immediately at line 88 without freeing the newly derived next key handle. This leaks the next key. Additionally, if the crypto implementation internally freed the old key even though it returned an error, *min_cdi (which is node_state->min_cdi) is left pointing to a freed handle, while next (the valid new key) is lost. Before returning the key_free error, next must be freed by also calling key_free on it.

Copilot · 2026-03-10T00:11:15Z

src/service/test/gnostic.cpp

+    // Set the mock context to return an error on the next kdf call, which will be made during
+    // ECA signing.


The comment "which will be made during ECA signing" is incorrect for this test. This test is for CDI certificate issuance (n20_srv_issue_cdi_certificate), not ECA signing. The comment should say something like "which will be made during CDI certificate issuance".

Copilot · 2026-03-10T00:11:15Z

src/service/test/gnostic.cpp

+    // Set the mock context to return an error on the next kdf call, which will be made during
+    // ECA signing.


The comment "which will be made during ECA signing" is incorrect for this test. This test is for ECA certificate issuance (n20_srv_issue_eca_certificate), not ECA signing. The comment should say something like "which will be made during ECA certificate issuance".

Copilot · 2026-03-10T00:11:15Z

include/nat20/service/gnostic.h

+#pragma once
+
+#include <nat20/crypto.h>
+#include <nat20/functionality.h>
+#include <nat20/service/service.h>
+#include <nat20/types.h>
+


All other public service headers in the same directory (messages.h, service.h, service_message_dispatch.h) begin with a /** @file */ Doxygen comment immediately after the license block (at line 38). The new gnostic.h is missing this comment, which deviates from the established convention. A /** @file */ comment (or more descriptive /** @file gnostic.h */ with a brief description) should be added after the license block.

Copilot · 2026-03-10T00:11:16Z

src/service/test/gnostic.cpp

+    ASSERT_EQ(sz_first, sz_second);
+    EXPECT_EQ(0, std::memcmp(first_cert.data(), cert_buffer_.data(), sz_first));


The determinism check at line 781 is incorrect. The stream writes certificates at the END of the provided buffer (certificate + buffer_size - bytes_written), as documented in service.h and confirmed by the n20_stream_s implementation. The comparison should use first_cert.data() + first_cert.size() - sz_first and cert_buffer_.data() + cert_buffer_.size() - sz_second as start pointers, not first_cert.data() and cert_buffer_.data(). As currently written, the comparison compares zero-initialized bytes at the beginning of both arrays, so it always passes regardless of the actual certificate content, making this test ineffective at verifying determinism.

Copilot · 2026-03-10T00:11:16Z

include/nat20/error.h

+    /**
+     * @brief Key usage not permitted.
+     *
+     * This error is returned when an ECA end-enty key is used


"end-enty" is a typo and should be "end-entity".

Copilot · 2026-03-10T00:11:17Z

src/service/gnostic.c

+    /* TODO: FIX compressed input type mismatch. */
+    error = n20_next_level_cdi_attest(
+        node_state->crypto_context, *min_cdi, &next, (uint8_t*)request->compressed_context.buffer);


The TODO: FIX compressed input type mismatch comment indicates a known correctness issue: request->compressed_context.buffer is cast to uint8_t* and passed directly to n20_next_level_cdi_attest as n20_compressed_input_t, but no size validation is performed. If the n20_slice_t.size differs from N20_FUNC_COMPRESSED_INPUT_SIZE, the function will silently read the wrong number of bytes (potentially out-of-bounds read). This should be validated before the cast, and n20_error_incompatible_compressed_input_size_e should be returned if the sizes don't match (the error code for exactly this scenario already exists in error.h at value 22). The same unchecked cast also appears in n20_resolve_path at lines 112 and 121.

Copilot · 2026-03-10T00:11:17Z

CMakeLists.txt

    set(CMAKE_CXX_STANDARD 17)
 endif()

+add_definitions(-DN20_WITH_COSE=1)


add_definitions(-DN20_WITH_COSE=1) is added unconditionally but the macro N20_WITH_COSE is not referenced anywhere in the codebase (no source or header file checks for it). This is dead configuration that adds noise. Additionally, add_definitions() is a CMake anti-pattern because it applies to all targets in the current directory and subdirectories without scoping. The idiomatic CMake approach is to use target_compile_definitions() on the specific targets that need the definition (e.g., as is already done for N20_STATELESS_MAX_PATH_LENGTH on nat20_service and nat20_service_coverage targets). Either the define should be removed (if unused) or moved to target_compile_definitions() on the specific target(s) that consume it.

Copilot · 2026-03-10T00:11:17Z

src/service/test/gnostic.cpp

+
+    EXPECT_EQ(n20_error_crypto_implementation_specific_e,
+              n20_gnostic_service_ops.n20_srv_promote(&state_, &req));
+    mock_crypto_context_.err_on_zero_kdf = 0;


Line 310 (mock_crypto_context_.err_on_zero_kdf = 0;) is dead code — it is immediately overwritten on line 312 (mock_crypto_context_.err_on_zero_kdf = std::numeric_limits<uint32_t>::max();). This assignment has no observable effect and is likely a leftover from an earlier version of the test. It should be removed.

Copilot · 2026-03-10T00:11:17Z

src/service/gnostic.c

+    n20_error_t error = n20_next_level_cdi_attest(
+        crypto_ctx, current_secret, &next, (uint8_t*)parent_path[i].buffer);
+    if (error != n20_error_ok_e) {
+        return error;
+    }
+    current_secret = next;
+    ++i;
+
+    while (i < parent_path_size) {
+        error = n20_next_level_cdi_attest(
+            crypto_ctx, current_secret, &next, (uint8_t*)parent_path[i].buffer);


The same unvalidated cast from n20_slice_t.buffer to n20_compressed_input_t (uint8_t*) appears here without checking parent_path[i].size against N20_FUNC_COMPRESSED_INPUT_SIZE. If a path element has a different size, this will silently read out-of-bounds or use incomplete data. The n20_error_incompatible_compressed_input_size_e error code (value 22 in error.h) already exists for this scenario and should be returned when validation fails.

Add gnostic node implementation

c94fa4a

werwurm requested a review from Copilot March 10, 2026 00:01

Copilot started reviewing on behalf of werwurm March 10, 2026 00:01 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

fix compressed input type mismatch.

fc2a2d1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add stateless gnostic node implementation#89

Add stateless gnostic node implementation#89
werwurm wants to merge 2 commits intowerwurm/message_dispatcherfrom
werwurm/gnostic_node2

werwurm commented Mar 10, 2026

Uh oh!

github-actions bot commented Mar 10, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// Set the mock context to return an error on the next kdf call, which will be made during
		// ECA signing.

		ASSERT_EQ(sz_first, sz_second);
		EXPECT_EQ(0, std::memcmp(first_cert.data(), cert_buffer_.data(), sz_first));

Conversation

werwurm commented Mar 10, 2026

Uh oh!

github-actions bot commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

LCOV of commit fc2a2d1 during lcov-test-coverage-report #109

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

PR Overview

Reviewed changes

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Mar 10, 2026 •

edited

Loading

LCOV of commit `fc2a2d1` during lcov-test-coverage-report #109