ECC non-deterministic signing always not used
tianyuan129 opened this issue · 3 comments
Current ECC backend uses macro FEATURE_PERIPH_HWRNG
to decide whether to use deterministic signing.
#ifndef FEATURE_PERIPH_HWRNG
uint8_t tmp[NDN_SEC_ECC_SECP256R1_PRIVATE_KEY_SIZE +
NDN_SEC_ECC_SECP256R1_PRIVATE_KEY_SIZE +
NDN_SEC_ECC_SECP256R1_PUBLIC_KEY_SIZE];
uECC_SHA256_HashContext HashContext;
uECC_SHA256_HashContext* ctx = &HashContext;
ctx->uECC.init_hash = &_init_sha256;
ctx->uECC.update_hash = &_update_sha256;
ctx->uECC.finish_hash = &_finish_sha256;
ctx->uECC.block_size = NDN_SEC_ECC_SECP256R1_PUBLIC_KEY_SIZE;
ctx->uECC.result_size = NDN_SEC_ECC_SECP256R1_PRIVATE_KEY_SIZE;
ctx->uECC.tmp = tmp;
ecc_sign_result = uECC_sign_deterministic(abs_key->key_value, input_value, input_size,
&ctx->uECC, output_value, curve);
#else
ecc_sign_result = uECC_sign(abs_key->key_value, input_value, input_size,
output_value, curve);
#endif
It's a copy from RIOT and shouldn't have been here. Also since non-RIOT package (e.g., POSIX) never uses this macro, each ndn_ecdsa_sign
will fall into uECC_sign_deterministic
even if they already set a RNG (e.g., ndn_lite_startup
in POSIX package).
void
ndn_lite_startup()
{
register_platform_security_init(ndn_lite_posix_rng_load_backend);
ndn_key_storage_init();
ndn_security_init();
ndn_forwarder_init();
}
The adaptation needs to pass -DFEATURE_PERIPH_HWRNG
to compiler, so that normal signing is selected at compile time.
It would be a bad idea to select code path at runtime using if-else, because it increases binary code size.
Yes, and I'm not saying to use runtime if-else. I think I missed something here. No documentation have ever mentioned about FEATURE_PERIPH_HWRNG
and how to use it, so if you simply follow instructions on README, normal signing is never used. One can only see a simple statement below:
/**
* Sign a buffer using ECDSA algorithm. This function will automatically use
* deterministic signing when no hardware pseudo-random number generator is available.
* The signature generated will be in ASN.1 DER format.
What I wonder is should adaptation define the FEATURE_PERIPH_HWRNG
in code, otherwise we should improve the documentation.
esp8266ndn defines FEATURE_PERIPH_HWRNG
in code:
https://github.com/yoursunny/esp8266ndn/blob/087ebd7a4a9c83766decf941b13c43af86de96e7/src/ndn-lite/security/default-backend/ndn-lite-default-ecc-impl.c#L1
but this isn't the best approach.
It should be in Makefile, but Arduino doesn't allow libraries to set compiler flags.
Given security-frontend invokes security-backend via function pointer, one option is:
- Have separate
ndn_lite_default_ecdsa_sign_randomized
andndn_lite_default_ecdsa_sign_deterministic
functions. - Require RNG pointer passed to
ndn_lite_default_ecc_load_backend
function, which chooses correct signing variant; removeset_rng
function.
ndn_lite_default_ecc_load_backend
function must appear in header, so that compiler can optimize out unused signing variant.