← Watch feed
refactor(crypto): return remote static key from `noise_xxpsk3_*_handle_*()`
Public commit record
62/100 · Adequate
What the developer wrote
Authored by Ondřej Vejpustek
refactor(crypto): return remote static key from `noise_xxpsk3_*_handle_*()`
[no changelog]
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Uses a recognizable type or scope! No meaningful explanatory body
What changed, and why it matters
This commit is a code cleanup (refactor) for the cryptographic handshake code used in Trezor devices. It changes how the other party's long-term public key is returned to the caller: instead of storing it inside an internal state structure, the function now writes it into a buffer supplied by the caller. The commit message explicitly calls it a refactor and includes '[no changelog]', indicating it is not being presented as a security fix. There is no direct evidence in the diff that this change fixes an active vulnerability, but it does reduce the amount of sensitive key material kept in internal state and gives callers explicit control over the output buffer, which is generally a defensive improvement.
Recommended action
Treat as a routine defensive refactor rather than an urgent security patch. Review downstream callers of `noise_xxpsk3_initiator_handle_response1()` and `noise_xxpsk3_responder_handle_request2()` to ensure they pass a valid, adequately sized buffer and handle failures by treating the output buffer as potentially sensitive. Continue normal regression testing; no incident response is warranted based solely on this commit.
Security signals we found
01
Removal of long-term public key storage from internal handshake state
02
Caller-supplied output buffer for remote static public key reduces internal secret retention
03
Error-path memzero of returned key material on failure
04
No changelog entry; commit labeled 'refactor' and '[no changelog]'
05
No explicit security fix language in commit message or diff comments
Evidence from the diff
The patch modifies the Noise XXpsk3 handshake implementation in crypto/noise_xxpsk3.c/h. It removes the has_remote_static_public boolean and the remote_static_public[NOISE_XXPSK3_DHLEN] field from noise_xxpsk3_handshake_state_t, and instead adds an output parameter remote_static_public_key[NOISE_XXPSK3_DHLEN] to noise_xxpsk3_initiator_handle_response1() and noise_xxpsk3_responder_handle_request2(). The decrypted remote static public key is written directly into the caller-supplied buffer, used immediately for the ECDH step, and zeroed on error paths. Test code is updated to pass buffers and assert the returned keys match the expected Curve25519 public keys. No changelog entry is recorded. No independent security disclosure or CVE is referenced in the materials.
Changed components
crypto/noise_xxpsk3.ccrypto/noise_xxpsk3.hcrypto/tests/test_check.cInspect captured patch +79 / −42
diff --git a/crypto/noise_xxpsk3.c b/crypto/noise_xxpsk3.c
index 8e313ded..c6d605f8 100644
--- a/crypto/noise_xxpsk3.c
+++ b/crypto/noise_xxpsk3.c
@@ -376,7 +376,6 @@ static bool noise_xxpsk3_init_state(
state->has_ephemeral_private = false;
state->has_remote_ephemeral_public = false;
- state->has_remote_static_public = false;
return true;
}
@@ -541,7 +540,8 @@ cleanup:
bool noise_xxpsk3_responder_handle_request2(
noise_xxpsk3_responder_t *rspn, const uint8_t *request, size_t request_len,
- uint8_t *payload, size_t max_payload_size, size_t *payload_size) {
+ uint8_t remote_static_public_key[NOISE_XXPSK3_DHLEN], uint8_t *payload,
+ size_t max_payload_size, size_t *payload_size) {
if (rspn == NULL) {
return false;
}
@@ -549,6 +549,7 @@ bool noise_xxpsk3_responder_handle_request2(
noise_xxpsk3_handshake_state_t *state = &rspn->handshake_state;
if (!rspn->initialized || request == NULL ||
+ remote_static_public_key == NULL ||
rspn->handshake_stage != NOISE_XXPSK3_RSPN_WAITING_FOR_REQUEST2 ||
!state->has_ephemeral_private ||
(payload == NULL && max_payload_size != 0)) {
@@ -562,14 +563,13 @@ bool noise_xxpsk3_responder_handle_request2(
if (!ss_decrypt_and_hash(&state->symmetric_state, request,
NOISE_XXPSK3_DHLEN + NOISE_TAG_SIZE_BYTES,
- state->remote_static_public)) {
+ remote_static_public_key)) {
goto cleanup;
}
- state->has_remote_static_public = true;
-
uint8_t input_key_material[NOISE_XXPSK3_DHLEN] = {0};
- dh(&state->ephemeral_private, &state->remote_static_public,
+ dh(&state->ephemeral_private,
+ (const uint8_t (*)[NOISE_XXPSK3_DHLEN])remote_static_public_key,
&input_key_material);
ss_mix_key(&state->symmetric_state, &input_key_material);
memzero(input_key_material, sizeof(input_key_material));
@@ -603,6 +603,9 @@ bool noise_xxpsk3_responder_handle_request2(
return true;
cleanup:
+ if (remote_static_public_key != NULL) {
+ memzero(remote_static_public_key, NOISE_XXPSK3_DHLEN);
+ }
noise_xxpsk3_responder_deinit(rspn);
return false;
}
@@ -693,19 +696,18 @@ cleanup:
return false;
}
-bool noise_xxpsk3_initiator_handle_response1(noise_xxpsk3_initiator_t *intr,
- const uint8_t *response,
- size_t response_len,
- uint8_t *payload,
- size_t max_payload_size,
- size_t *payload_size) {
+bool noise_xxpsk3_initiator_handle_response1(
+ noise_xxpsk3_initiator_t *intr, const uint8_t *response,
+ size_t response_len, uint8_t remote_static_public_key[NOISE_XXPSK3_DHLEN],
+ uint8_t *payload, size_t max_payload_size, size_t *payload_size) {
if (intr == NULL) {
return false;
}
if (!intr->initialized ||
intr->handshake_stage != NOISE_XXPSK3_INTR_WAITING_FOR_RESPONSE1 ||
- response == NULL || (payload == NULL && max_payload_size != 0)) {
+ response == NULL || remote_static_public_key == NULL ||
+ (payload == NULL && max_payload_size != 0)) {
goto cleanup;
}
@@ -730,12 +732,12 @@ bool noise_xxpsk3_initiator_handle_response1(noise_xxpsk3_initiator_t *intr,
if (!ss_decrypt_and_hash(&state->symmetric_state,
response + NOISE_XXPSK3_DHLEN,
NOISE_XXPSK3_DHLEN + NOISE_TAG_SIZE_BYTES,
- state->remote_static_public)) {
+ remote_static_public_key)) {
goto cleanup;
}
- state->has_remote_static_public = true;
- dh(&state->ephemeral_private, &state->remote_static_public,
+ dh(&state->ephemeral_private,
+ (const uint8_t (*)[NOISE_XXPSK3_DHLEN])remote_static_public_key,
&input_key_material);
ss_mix_key(&state->symmetric_state, &input_key_material);
memzero(input_key_material, sizeof(input_key_material));
@@ -763,6 +765,9 @@ bool noise_xxpsk3_initiator_handle_response1(noise_xxpsk3_initiator_t *intr,
return true;
cleanup:
+ if (remote_static_public_key != NULL) {
+ memzero(remote_static_public_key, NOISE_XXPSK3_DHLEN);
+ }
noise_xxpsk3_initiator_deinit(intr);
return false;
}
diff --git a/crypto/noise_xxpsk3.h b/crypto/noise_xxpsk3.h
index ad1098d3..971a3c7f 100644
--- a/crypto/noise_xxpsk3.h
+++ b/crypto/noise_xxpsk3.h
@@ -61,9 +61,6 @@ typedef struct {
bool has_remote_ephemeral_public;
uint8_t remote_ephemeral_public[NOISE_XXPSK3_DHLEN];
- bool has_remote_static_public;
- uint8_t remote_static_public[NOISE_XXPSK3_DHLEN];
-
} noise_xxpsk3_handshake_state_t;
#ifdef USE_NOISE_XXPSK3_RESPONDER
@@ -187,17 +184,17 @@ bool noise_xxpsk3_initiator_create_request1(
* @param intr Pointer to the initiator structure
* @param response Incoming response buffer
* @param response_len Length of the incoming response buffer
+ * @param remote_static_public_key Output buffer for the responder's static
+ * public key (32 bytes)
* @param payload Output buffer for the decrypted payload
* @param max_payload_size Size of the output buffer
* @param payload_size Set to the number of decrypted payload bytes
* @return true if the response was handled correctly, false otherwise
*/
-bool noise_xxpsk3_initiator_handle_response1(noise_xxpsk3_initiator_t *intr,
- const uint8_t *response,
- size_t response_len,
- uint8_t *payload,
- size_t max_payload_size,
- size_t *payload_size);
+bool noise_xxpsk3_initiator_handle_response1(
+ noise_xxpsk3_initiator_t *intr, const uint8_t *response,
+ size_t response_len, uint8_t remote_static_public_key[NOISE_XXPSK3_DHLEN],
+ uint8_t *payload, size_t max_payload_size, size_t *payload_size);
/**
* @brief Create request2, the third handshake message from the initiator.
@@ -329,6 +326,8 @@ bool noise_xxpsk3_responder_create_response1(
* @param rspn Pointer to the responder structure
* @param request Incoming request buffer
* @param request_len Length of the incoming request buffer
+ * @param remote_static_public_key Output buffer for the initiator's static
+ * public key (32 bytes)
* @param payload Output buffer for the decrypted payload
* @param max_payload_size Size of the output buffer
* @param payload_size Set to the number of decrypted payload bytes
@@ -336,7 +335,8 @@ bool noise_xxpsk3_responder_create_response1(
*/
bool noise_xxpsk3_responder_handle_request2(
noise_xxpsk3_responder_t *rspn, const uint8_t *request, size_t request_len,
- uint8_t *payload, size_t max_payload_size, size_t *payload_size);
+ uint8_t remote_static_public_key[NOISE_XXPSK3_DHLEN], uint8_t *payload,
+ size_t max_payload_size, size_t *payload_size);
#endif /* USE_NOISE_XXPSK3_RESPONDER */
diff --git a/crypto/tests/test_check.c b/crypto/tests/test_check.c
index 549605a6..e10c583e 100644
--- a/crypto/tests/test_check.c
+++ b/crypto/tests/test_check.c
@@ -11714,16 +11714,23 @@ START_TEST(test_noise_xxpsk3) {
random_buffer(initiator_private_key, sizeof(initiator_private_key));
random_buffer(responder_private_key, sizeof(responder_private_key));
+ uint8_t initiator_public_key[32] = {0};
+ uint8_t responder_public_key[32] = {0};
+ curve25519_scalarmult_basepoint(initiator_public_key, initiator_private_key);
+ curve25519_scalarmult_basepoint(responder_public_key, responder_private_key);
+
noise_xxpsk3_initiator_t initiator = {0};
noise_xxpsk3_responder_t responder = {0};
bool ret = false;
// Initialize initiator and responder
- ret = noise_xxpsk3_initiator_init(&initiator, psk, initiator_private_key);
+ ret = noise_xxpsk3_initiator_init(&initiator, psk, initiator_private_key,
+ initiator_public_key);
ck_assert_int_eq(ret, true);
- ret = noise_xxpsk3_responder_init(&responder, psk, responder_private_key);
+ ret = noise_xxpsk3_responder_init(&responder, psk, responder_private_key,
+ responder_public_key);
ck_assert_int_eq(ret, true);
// --- Handshake ---
@@ -11752,9 +11759,13 @@ START_TEST(test_noise_xxpsk3) {
ck_assert_int_eq(response1_size, 32 + 48 + 16);
// Initiator handles response1
- ret = noise_xxpsk3_initiator_handle_response1(&initiator, response1,
- response1_size, NULL, 0, NULL);
+ uint8_t received_responder_public_key[32] = {0};
+ ret = noise_xxpsk3_initiator_handle_response1(
+ &initiator, response1, response1_size, received_responder_public_key,
+ NULL, 0, NULL);
ck_assert_int_eq(ret, true);
+ ck_assert_mem_eq(received_responder_public_key, responder_public_key,
+ sizeof(responder_public_key));
// Initiator creates request2
uint8_t request2[256] = {0};
@@ -11766,9 +11777,13 @@ START_TEST(test_noise_xxpsk3) {
ck_assert_int_eq(request2_size, 48 + 16);
// Responder handles request2 — handshake complete
- ret = noise_xxpsk3_responder_handle_request2(&responder, request2,
- request2_size, NULL, 0, NULL);
+ uint8_t received_initiator_public_key[32] = {0};
+ ret = noise_xxpsk3_responder_handle_request2(
+ &responder, request2, request2_size, received_initiator_public_key, NULL,
+ 0, NULL);
ck_assert_int_eq(ret, true);
+ ck_assert_mem_eq(received_initiator_public_key, initiator_public_key,
+ sizeof(initiator_public_key));
// --- Transport phase: both directions ---
@@ -11851,10 +11866,12 @@ START_TEST(test_noise_xxpsk3) {
ck_assert_int_eq(ret, false);
// --- Double-init should fail ---
- ret = noise_xxpsk3_initiator_init(&initiator, psk, initiator_private_key);
+ ret = noise_xxpsk3_initiator_init(&initiator, psk, initiator_private_key,
+ initiator_public_key);
ck_assert_int_eq(ret, false);
- ret = noise_xxpsk3_responder_init(&responder, psk, responder_private_key);
+ ret = noise_xxpsk3_responder_init(&responder, psk, responder_private_key,
+ responder_public_key);
ck_assert_int_eq(ret, false);
// Both sides must have the same handshake hash
@@ -11993,6 +12010,13 @@ START_TEST(test_noise_xxpsk3_vectors) {
random_buffer(initiator_private_key, sizeof(initiator_private_key));
random_buffer(responder_private_key, sizeof(responder_private_key));
+ uint8_t initiator_public_key[32] = {0};
+ uint8_t responder_public_key[32] = {0};
+ curve25519_scalarmult_basepoint(initiator_public_key,
+ initiator_private_key);
+ curve25519_scalarmult_basepoint(responder_public_key,
+ responder_private_key);
+
size_t req1_plen = strlen(vectors[v].req1_payload) / 2;
size_t rsp1_plen = strlen(vectors[v].rsp1_payload) / 2;
size_t req2_plen = strlen(vectors[v].req2_payload) / 2;
@@ -12012,9 +12036,11 @@ START_TEST(test_noise_xxpsk3_vectors) {
noise_xxpsk3_responder_t responder = {0};
bool ret;
- ret = noise_xxpsk3_initiator_init(&initiator, psk, initiator_private_key);
+ ret = noise_xxpsk3_initiator_init(&initiator, psk, initiator_private_key,
+ initiator_public_key);
ck_assert_int_eq(ret, true);
- ret = noise_xxpsk3_responder_init(&responder, psk, responder_private_key);
+ ret = noise_xxpsk3_responder_init(&responder, psk, responder_private_key,
+ responder_public_key);
ck_assert_int_eq(ret, true);
uint8_t req1[512] = {0};
@@ -12042,10 +12068,13 @@ START_TEST(test_noise_xxpsk3_vectors) {
uint8_t rsp1_dec[512] = {0};
size_t rsp1_dec_size = 0;
- ret = noise_xxpsk3_initiator_handle_response1(&initiator, rsp1, rsp1_size,
- rsp1_dec, sizeof(rsp1_dec),
- &rsp1_dec_size);
+ uint8_t received_responder_public_key[32] = {0};
+ ret = noise_xxpsk3_initiator_handle_response1(
+ &initiator, rsp1, rsp1_size, received_responder_public_key, rsp1_dec,
+ sizeof(rsp1_dec), &rsp1_dec_size);
ck_assert_int_eq(ret, true);
+ ck_assert_mem_eq(received_responder_public_key, responder_public_key,
+ sizeof(responder_public_key));
ck_assert_int_eq(rsp1_dec_size, rsp1_plen);
if (rsp1_plen) ck_assert_mem_eq(rsp1_dec, rsp1_payload, rsp1_plen);
@@ -12058,10 +12087,13 @@ START_TEST(test_noise_xxpsk3_vectors) {
uint8_t req2_dec[512] = {0};
size_t req2_dec_size = 0;
- ret = noise_xxpsk3_responder_handle_request2(&responder, req2, req2_size,
- req2_dec, sizeof(req2_dec),
- &req2_dec_size);
+ uint8_t received_initiator_public_key[32] = {0};
+ ret = noise_xxpsk3_responder_handle_request2(
+ &responder, req2, req2_size, received_initiator_public_key, req2_dec,
+ sizeof(req2_dec), &req2_dec_size);
ck_assert_int_eq(ret, true);
+ ck_assert_mem_eq(received_initiator_public_key, initiator_public_key,
+ sizeof(initiator_public_key));
ck_assert_int_eq(req2_dec_size, req2_plen);
if (req2_plen) ck_assert_mem_eq(req2_dec, req2_payload, req2_plen);
Risk score
Our methodology →Why this scored 18/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.