factorysetup: move secp256k1 verification to Rust
What the developer wrote
Authored by benma's agent
factorysetup: move secp256k1 verification to Rust
The last remaining secp256k1 C library use. After this, we can
consolidate all sec256k1 things into a new bitbox-secp256k1 crate.
What changed, and why it matters
This commit rewrites a small piece of the BitBox02 factory-setup code from C to Rust. The code verifies a cryptographic signature on the device during factory setup. The change itself is a routine refactoring: it removes the old C secp256k1 library call and replaces it with an equivalent Rust call. There is no indication in the commit that this fixes a known security bug, and the new Rust code includes tests for invalid inputs. On its own, this looks like a defensive hardening/cleanup change rather than a vulnerability fix.
Treat as a normal code-quality/cleanup commit. Review the new Rust wrapper for FFI safety (Bytes lifetime assumptions), ensure the global SECP256K1 context is initialized securely, and confirm that returning false on parse failure does not break factory-setup protocol error reporting. No urgent security response is warranted based solely on this commit.
Security signals we found
Refactor of cryptographic verification code from C to Rust
Removal of manual secp256k1 context creation/destruction in C
Addition of Rust unit tests for invalid signature, message, public key, and mismatch cases
No mention of vulnerability, CVE, or security bug in commit message or diff
Evidence from the diff
The patch moves ECDSA secp256k1 signature verification in factorysetup.c from the C secp256k1 library to a new Rust wrapper, rust_secp256k1_verify, exposed via FFI. The C code previously created a secp256k1 context, parsed a compact signature, parsed each root public key, and called secp256k1_ecdsa_verify. The new Rust implementation uses bitcoin::secp256k1 to parse the signature, message digest, and public key, then verifies with bitbox02_rust::secp256k1::SECP256K1. Error handling changes slightly: invalid signature/public-key/message parsing now returns false instead of setting ERR_INVALID_INPUT for a bad signature. The commit message frames this as the last step before consolidating secp256k1 usage into a new crate. No CVE, advisory, or security disclosure is referenced.
Changed components
src/factorysetup.csrc/rust/bitbox02-rust-c/src/secp256k1.rs (new)src/rust/bitbox02-rust-c/src/lib.rssrc/rust/bitbox02-rust-c/Cargo.tomlInspect captured patch +119 / −25
diff --git a/src/factorysetup.c b/src/factorysetup.c
index 9b4e8ee..1becf41 100644
--- a/src/factorysetup.c
+++ b/src/factorysetup.c
@@ -21,7 +21,6 @@
#include "usb/usb_packet.h"
#include "usb/usb_processing.h"
#include "utils_ringbuffer.h"
-#include <secp256k1.h>
#include <ui/oled/oled.h>
#define BUFFER_SIZE_DOWN 1024
@@ -294,14 +293,6 @@ static void _free(uint8_t** buf)
*buf = NULL;
}
-static void _destroy(secp256k1_context** ctx)
-{
- if (*ctx) {
- secp256k1_context_destroy(*ctx);
- *ctx = NULL;
- }
-}
-
/**
* Computes the hash which is signed by the root key.
* @param[in] attestation_device_pubkey 64 bytes P-256 pubkey.
@@ -365,27 +356,15 @@ static void _api_msg(const uint8_t* input, size_t in_len, uint8_t* output, size_
const uint8_t* root_pubkey_identifier = input + 1 + pubkey_size + certificate_size;
// Verify sig
-
- secp256k1_context* __attribute__((__cleanup__(_destroy))) ctx =
- secp256k1_context_create(SECP256K1_CONTEXT_NONE);
-
- secp256k1_ecdsa_signature sig = {0};
- if (!secp256k1_ecdsa_signature_parse_compact(ctx, &sig, certificate)) {
- result = ERR_INVALID_INPUT;
- break;
- }
uint8_t msg32[32] = {0};
_attestation_sighash(attestation_device_pubkey, msg32);
bool matches_a_root_pubkey = false;
for (size_t pubkey_idx = 0; pubkey_idx < sizeof(_root_pubkey_bytes) / ROOT_PUBKEY_SIZE;
pubkey_idx++) {
- secp256k1_pubkey pubkey;
- if (!secp256k1_ec_pubkey_parse(
- ctx, &pubkey, _root_pubkey_bytes[pubkey_idx], ROOT_PUBKEY_SIZE)) {
- Abort("Invalid root pubkey");
- }
-
- if (secp256k1_ecdsa_verify(ctx, &sig, msg32, &pubkey)) {
+ if (rust_secp256k1_verify(
+ rust_util_bytes(certificate, certificate_size),
+ rust_util_bytes(msg32, sizeof(msg32)),
+ rust_util_bytes(_root_pubkey_bytes[pubkey_idx], ROOT_PUBKEY_SIZE))) {
matches_a_root_pubkey = true;
break;
}
diff --git a/src/rust/bitbox02-rust-c/Cargo.toml b/src/rust/bitbox02-rust-c/Cargo.toml
index c0a6756..d91069f 100644
--- a/src/rust/bitbox02-rust-c/Cargo.toml
+++ b/src/rust/bitbox02-rust-c/Cargo.toml
@@ -43,8 +43,10 @@ target-firmware = ["firmware", "platform-bitbox02", "app-bitcoin", "app-litecoin
target-firmware-btc = ["firmware", "platform-bitbox02", "app-bitcoin"]
target-factory-setup = [
# enable these features
+ "factory-setup",
"firmware",
"platform-bitbox02",
+ "dep:bitcoin",
]
# add Rust features which are called in the C unit tests (currently there is only one target for C tests).
target-c-unit-tests = [
@@ -113,4 +115,6 @@ app-cardano = [
"bitbox02-rust/app-cardano",
]
+factory-setup = []
+
rtt = [ "util/rtt" ]
diff --git a/src/rust/bitbox02-rust-c/src/lib.rs b/src/rust/bitbox02-rust-c/src/lib.rs
index 62d0697..b629d77 100644
--- a/src/rust/bitbox02-rust-c/src/lib.rs
+++ b/src/rust/bitbox02-rust-c/src/lib.rs
@@ -13,6 +13,8 @@ mod alloc;
pub mod async_usb;
#[cfg(feature = "firmware")]
mod der;
+#[cfg(feature = "factory-setup")]
+mod secp256k1;
#[cfg(feature = "firmware")]
pub mod workflow;
diff --git a/src/rust/bitbox02-rust-c/src/secp256k1.rs b/src/rust/bitbox02-rust-c/src/secp256k1.rs
new file mode 100644
index 0000000..23c4fcb
--- /dev/null
+++ b/src/rust/bitbox02-rust-c/src/secp256k1.rs
@@ -0,0 +1,109 @@
+// SPDX-License-Identifier: Apache-2.0
+
+use bitcoin::secp256k1::{Message, PublicKey};
+
+#[unsafe(no_mangle)]
+pub extern "C" fn rust_secp256k1_verify(
+ signature_compact: util::bytes::Bytes,
+ msg32: util::bytes::Bytes,
+ pubkey: util::bytes::Bytes,
+) -> bool {
+ let Ok(signature) =
+ bitcoin::secp256k1::ecdsa::Signature::from_compact(signature_compact.as_ref())
+ else {
+ return false;
+ };
+ let Ok(message) = Message::from_digest_slice(msg32.as_ref()) else {
+ return false;
+ };
+ let Ok(public_key) = PublicKey::from_slice(pubkey.as_ref()) else {
+ return false;
+ };
+ bitbox02_rust::secp256k1::SECP256K1
+ .verify_ecdsa(&message, &signature, &public_key)
+ .is_ok()
+}
+
+#[cfg(test)]
+mod tests {
+ use super::rust_secp256k1_verify;
+
+ use bitcoin::secp256k1::{Message, PublicKey, Secp256k1, SecretKey};
+
+ fn verify(signature_compact: &[u8], msg32: &[u8], pubkey: &[u8]) -> bool {
+ rust_secp256k1_verify(
+ unsafe {
+ util::bytes::rust_util_bytes(signature_compact.as_ptr(), signature_compact.len())
+ },
+ unsafe { util::bytes::rust_util_bytes(msg32.as_ptr(), msg32.len()) },
+ unsafe { util::bytes::rust_util_bytes(pubkey.as_ptr(), pubkey.len()) },
+ )
+ }
+
+ #[test]
+ fn test_rust_secp256k1_verify() {
+ let secp = Secp256k1::new();
+ let sk = SecretKey::from_slice(&[0x11u8; 32]).unwrap();
+ let pk = PublicKey::from_secret_key(&secp, &sk);
+
+ let msg32 = [0x22u8; 32];
+ let msg = Message::from_digest_slice(&msg32).unwrap();
+ let sig64 = secp.sign_ecdsa(&msg, &sk).serialize_compact();
+
+ assert!(verify(&sig64, &msg32, &pk.serialize_uncompressed()));
+ assert!(verify(&sig64, &msg32, &pk.serialize()));
+ }
+
+ #[test]
+ fn test_rust_secp256k1_verify_invalid_signature() {
+ let secp = Secp256k1::new();
+ let sk = SecretKey::from_slice(&[0x11u8; 32]).unwrap();
+ let pk = PublicKey::from_secret_key(&secp, &sk);
+
+ let msg32 = [0x22u8; 32];
+ let msg = Message::from_digest_slice(&msg32).unwrap();
+ let sig64 = secp.sign_ecdsa(&msg, &sk).serialize_compact();
+
+ assert!(!verify(&sig64[..63], &msg32, &pk.serialize_uncompressed()));
+ }
+
+ #[test]
+ fn test_rust_secp256k1_verify_invalid_message() {
+ let secp = Secp256k1::new();
+ let sk = SecretKey::from_slice(&[0x11u8; 32]).unwrap();
+ let pk = PublicKey::from_secret_key(&secp, &sk);
+
+ let msg32 = [0x22u8; 32];
+ let msg = Message::from_digest_slice(&msg32).unwrap();
+ let sig64 = secp.sign_ecdsa(&msg, &sk).serialize_compact();
+
+ assert!(!verify(&sig64, &msg32[..31], &pk.serialize_uncompressed()));
+ }
+
+ #[test]
+ fn test_rust_secp256k1_verify_invalid_pubkey() {
+ let secp = Secp256k1::new();
+ let sk = SecretKey::from_slice(&[0x11u8; 32]).unwrap();
+
+ let msg32 = [0x22u8; 32];
+ let msg = Message::from_digest_slice(&msg32).unwrap();
+ let sig64 = secp.sign_ecdsa(&msg, &sk).serialize_compact();
+
+ assert!(!verify(&sig64, &msg32, &[0u8; 65]));
+ }
+
+ #[test]
+ fn test_rust_secp256k1_verify_mismatch() {
+ let secp = Secp256k1::new();
+ let sk = SecretKey::from_slice(&[0x11u8; 32]).unwrap();
+ let pk = PublicKey::from_secret_key(&secp, &sk);
+
+ let msg32 = [0x22u8; 32];
+ let msg = Message::from_digest_slice(&msg32).unwrap();
+ let sig64 = secp.sign_ecdsa(&msg, &sk).serialize_compact();
+
+ let mut other_msg32 = msg32;
+ other_msg32[0] ^= 1;
+ assert!(!verify(&sig64, &other_msg32, &pk.serialize_uncompressed()));
+ }
+}
Why this scored 20/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.