What changed, and why it matters
This commit removes an old C source file called keystore.c and moves two small cryptographic helper functions directly into the Rust part of the project. The functions themselves still do exactly the same secp256k1 signing and nonce-commitment work as before; they are just called from Rust instead of going through a thin C wrapper. There is no indication this fixes or introduces a security bug.
No security action required. Treat as routine code cleanup. If reviewing for correctness, verify the Rust FFI argument order matches the secp256k1-zkp function signatures and that MaybeUninit values are not read before initialization.
Security signals we found
Refactor only: no change to cryptographic algorithm or parameters
Anti-Exfil/s2c protocol remains in use
FFI allowlist updated to expose lower-level secp256k1 functions to Rust
MaybeUninit used for uninitialized secp256k1 output structs
Evidence from the diff
The patch deletes src/keystore.c and removes its two functions (keystore_secp256k1_nonce_commit and keystore_secp256k1_sign) from the build and header. The same secp256k1-zkp Anti-Exfil operations are now invoked directly from src/rust/bitbox02/src/secp256k1.rs via FFI bindings added in build.rs. The Rust code replicates the original C logic: secp256k1_ecdsa_anti_exfil_signer_commit + secp256k1_ecdsa_s2c_opening_serialize for nonce commitment, and secp256k1_anti_exfil_sign + secp256k1_ecdsa_signature_serialize_compact for signing. It switches output buffers to MaybeUninit for the intermediate secp256k1 structures, which is idiomatic Rust for FFI out-parameters.
Changed components
src/keystore.csrc/keystore.hsrc/rust/bitbox02/src/secp256k1.rssrc/rust/bitbox02-sys/build.rssrc/CMakeLists.txtInspect captured patch +50 / −147
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 7fc4a6e..44e98fc 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -17,7 +17,6 @@
set(DBB-FIRMWARE-SOURCES
${CMAKE_SOURCE_DIR}/src/firmware_main_loop.c
${CMAKE_SOURCE_DIR}/src/delay.c
- ${CMAKE_SOURCE_DIR}/src/keystore.c
${CMAKE_SOURCE_DIR}/src/random.c
${CMAKE_SOURCE_DIR}/src/hardfault.c
${CMAKE_SOURCE_DIR}/src/util.c
diff --git a/src/keystore.c b/src/keystore.c
deleted file mode 100644
index 1ca9557..0000000
--- a/src/keystore.c
+++ /dev/null
@@ -1,65 +0,0 @@
-// Copyright 2019 Shift Cryptosecurity AG
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#include <string.h>
-
-#include "hardfault.h"
-#include "keystore.h"
-#include "memory/bitbox02_smarteeprom.h"
-#include "memory/memory.h"
-#include "reset.h"
-#include "salt.h"
-#include "securechip/securechip.h"
-#include "util.h"
-#include <usb/usb_processing.h>
-
-#include <secp256k1_ecdsa_s2c.h>
-
-bool keystore_secp256k1_nonce_commit(
- const secp256k1_context* ctx,
- const uint8_t* private_key,
- const uint8_t* msg32,
- const uint8_t* host_commitment,
- uint8_t* signer_commitment_out)
-{
- secp256k1_ecdsa_s2c_opening signer_commitment;
- if (!secp256k1_ecdsa_anti_exfil_signer_commit(
- ctx, &signer_commitment, msg32, private_key, host_commitment)) {
- return false;
- }
-
- if (!secp256k1_ecdsa_s2c_opening_serialize(ctx, signer_commitment_out, &signer_commitment)) {
- return false;
- }
- return true;
-}
-
-bool keystore_secp256k1_sign(
- const secp256k1_context* ctx,
- const uint8_t* private_key,
- const uint8_t* msg32,
- const uint8_t* host_nonce32,
- uint8_t* sig_compact_out,
- int* recid_out)
-{
- secp256k1_ecdsa_signature secp256k1_sig = {0};
- if (!secp256k1_anti_exfil_sign(
- ctx, &secp256k1_sig, msg32, private_key, host_nonce32, recid_out)) {
- return false;
- }
- if (!secp256k1_ecdsa_signature_serialize_compact(ctx, sig_compact_out, &secp256k1_sig)) {
- return false;
- }
- return true;
-}
diff --git a/src/keystore.h b/src/keystore.h
index 4773062..6e83152 100644
--- a/src/keystore.h
+++ b/src/keystore.h
@@ -17,68 +17,9 @@
#include "compiler_util.h"
-#include <stdbool.h>
-#include <stddef.h>
-#include <stdint.h>
-
-#include <secp256k1.h>
-
#define KEYSTORE_U2F_SEED_LENGTH SHA256_LEN
// Max. length of an xpub string, including the null terminator.
#define XPUB_ENCODED_LEN 113
-/**
- * Get a commitment to the original nonce before tweaking it with the host nonce. This is part of
- * the ECDSA Anti-Klepto Protocol. For more details, check the docs of
- * `secp256k1_ecdsa_anti_exfil_signer_commit`.
- * @param[in] ctx secp256k1 context
- * @param[in] private_key 32 byte private key
- * @param[in] msg32 32 byte message which will be signed by `keystore_secp256k1_sign`.
- * @param[in] host_commitment must be `sha256(sha256(tag)||shas256(tag)||host_nonce)` where
- * host_nonce is passed to `keystore_secp256k1_sign()`. See
- * `secp256k1_ecdsa_anti_exfil_host_commit()`.
- * @param[out] client_commitment_out EC_PUBLIC_KEY_LEN bytes compressed signer nonce pubkey.
- */
-USE_RESULT bool keystore_secp256k1_nonce_commit(
- const secp256k1_context* ctx,
- const uint8_t* private_key,
- const uint8_t* msg32,
- const uint8_t* host_commitment,
- uint8_t* client_commitment_out);
-
-// clang-format off
-/**
- * Sign message with private key using the given private key.
- *
- * Details about `host_nonce32`, the host nonce contribution.
- * Instead of using plain rfc6979 to generate the nonce in this signature, the following formula is used:
- * r = rfc6979(..., additional_data=Hash_d(host_nonce32))
- * R=r*G (pubkey to secret r)
- * nonce = r + Hash_p(R, host_nonce32)
- * `Hash_d(msg)` and `Hash_p(msg)` are tagged hashes: `sha256(sha256(tag)||shas256(tag)||msg)`.
- * Tag for `Hash_d`: "s2c/ecdsa/data".
- * Tag for `Hash_p`: "s2c/ecdsa/point".
- * This is part of the ECSDA Anti-Klepto protocol, preventing this function to leak any secrets via
- * the signatures (see the ecdsa-s2c module in secp256k1-zpk for more details).
- *
- * @param[in] ctx secp256k1 context
- * @param[in] private_key 32 byte private key
- * @param[in] msg32 32 byte message to sign
- * @param[in] host_nonce32 32 byte nonce contribution. Cannot be NULL.
- * Intended to be a contribution by the host. If there is none available, use 32 zero bytes.
- * @param[out] sig_compact_out resulting signature in compact format. Must be 64 bytes.
- * @param[out] recid recoverable id. Can be NULL if not needed.
- * Parse with secp256k1_ecdsa_signature_serialize_compact().
- * @return true on success, false if the keystore is locked.
- */
-// clang-format on
-USE_RESULT bool keystore_secp256k1_sign(
- const secp256k1_context* ctx,
- const uint8_t* private_key,
- const uint8_t* msg32,
- const uint8_t* host_nonce32,
- uint8_t* sig_compact_out,
- int* recid_out);
-
#endif
diff --git a/src/rust/bitbox02-sys/build.rs b/src/rust/bitbox02-sys/build.rs
index 9f09f82..a761f82 100644
--- a/src/rust/bitbox02-sys/build.rs
+++ b/src/rust/bitbox02-sys/build.rs
@@ -91,8 +91,6 @@ const ALLOWLIST_FNS: &[&str] = &[
"hww_setup",
"keystore_bip39_mnemonic_to_seed",
"keystore_get_bip39_word",
- "keystore_secp256k1_nonce_commit",
- "keystore_secp256k1_sign",
"label_create",
"memory_add_noise_remote_static_pubkey",
"memory_ble_enable",
@@ -163,9 +161,13 @@ const ALLOWLIST_FNS: &[&str] = &[
"sd_load_bin",
"sd_write_bin",
"sdcard_create",
+ "secp256k1_anti_exfil_sign",
"secp256k1_anti_exfil_host_verify",
+ "secp256k1_ecdsa_anti_exfil_signer_commit",
"secp256k1_ecdsa_anti_exfil_host_commit",
+ "secp256k1_ecdsa_s2c_opening_serialize",
"secp256k1_ecdsa_s2c_opening_parse",
+ "secp256k1_ecdsa_signature_serialize_compact",
"securechip_attestation_sign",
"securechip_init_new_password",
"securechip_kdf",
@@ -221,7 +223,6 @@ const BITBOX02_SOURCES: &[&str] = &[
"src/hardfault.c",
"src/hww.c",
"src/i2c_ecc.c",
- "src/keystore.c",
"src/memory/bitbox02_smarteeprom.c",
"src/memory/memory_shared.c",
"src/memory/memory_spi.c",
diff --git a/src/rust/bitbox02/src/secp256k1.rs b/src/rust/bitbox02/src/secp256k1.rs
index a2e4170..5138961 100644
--- a/src/rust/bitbox02/src/secp256k1.rs
+++ b/src/rust/bitbox02/src/secp256k1.rs
@@ -17,6 +17,7 @@ use bitcoin::secp256k1::ffi::CPtr;
use bitcoin::secp256k1::{All, Secp256k1};
use alloc::vec::Vec;
+use core::mem::MaybeUninit;
/// Length of a compressed secp256k1 pubkey.
pub const EC_PUBLIC_KEY_LEN: usize = 33;
@@ -32,24 +33,37 @@ pub fn _secp256k1_sign(
msg: &[u8; 32],
host_nonce: &[u8; 32],
) -> Result<SignResult, ()> {
- let mut signature = [0u8; 64];
+ let mut sig = MaybeUninit::<bitbox02_sys::secp256k1_ecdsa_signature>::uninit();
let mut recid: core::ffi::c_int = 0;
- match unsafe {
- bitbox02_sys::keystore_secp256k1_sign(
+ if unsafe {
+ bitbox02_sys::secp256k1_anti_exfil_sign(
secp.ctx().as_ptr().cast(),
- private_key.as_ptr(),
+ sig.as_mut_ptr(),
msg.as_ptr(),
+ private_key.as_ptr(),
host_nonce.as_ptr(),
- signature.as_mut_ptr(),
&mut recid,
)
- } {
- true => Ok(SignResult {
- signature,
- recid: recid.try_into().unwrap(),
- }),
- false => Err(()),
+ } != 1
+ {
+ return Err(());
+ }
+
+ let mut signature = [0u8; 64];
+ if unsafe {
+ bitbox02_sys::secp256k1_ecdsa_signature_serialize_compact(
+ secp.ctx().as_ptr().cast(),
+ signature.as_mut_ptr(),
+ sig.as_ptr(),
+ )
+ } != 1
+ {
+ return Err(());
}
+ Ok(SignResult {
+ signature,
+ recid: recid.try_into().unwrap(),
+ })
}
pub fn _secp256k1_nonce_commit(
@@ -58,19 +72,32 @@ pub fn _secp256k1_nonce_commit(
msg: &[u8; 32],
host_commitment: &[u8; 32],
) -> Result<[u8; EC_PUBLIC_KEY_LEN], ()> {
- let mut signer_commitment = [0u8; EC_PUBLIC_KEY_LEN];
- match unsafe {
- bitbox02_sys::keystore_secp256k1_nonce_commit(
+ let mut signer_commitment = MaybeUninit::<bitbox02_sys::secp256k1_ecdsa_s2c_opening>::uninit();
+ if unsafe {
+ bitbox02_sys::secp256k1_ecdsa_anti_exfil_signer_commit(
secp.ctx().as_ptr().cast(),
- private_key.as_ptr(),
+ signer_commitment.as_mut_ptr(),
msg.as_ptr(),
+ private_key.as_ptr(),
host_commitment.as_ptr(),
- signer_commitment.as_mut_ptr(),
)
- } {
- true => Ok(signer_commitment),
- false => Err(()),
+ } != 1
+ {
+ return Err(());
+ }
+
+ let mut out = [0u8; EC_PUBLIC_KEY_LEN];
+ if unsafe {
+ bitbox02_sys::secp256k1_ecdsa_s2c_opening_serialize(
+ secp.ctx().as_ptr().cast(),
+ out.as_mut_ptr(),
+ signer_commitment.as_ptr(),
+ )
+ } != 1
+ {
+ return Err(());
}
+ Ok(out)
}
pub fn ecdsa_anti_exfil_host_commit(secp: &Secp256k1<All>, rand32: &[u8]) -> Result<Vec<u8>, ()> {
Why this scored 12/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.