ECC non-deterministic signing always not used

[tianyuan129]

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();
}

[yoursunny]

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.

[tianyuan129]

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.

[yoursunny]

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 and ndn_lite_default_ecdsa_sign_deterministic functions.
• Require RNG pointer passed to ndn_lite_default_ecc_load_backend function, which chooses correct signing variant; remove set_rng function.

ndn_lite_default_ecc_load_backend function must appear in header, so that compiler can optimize out unused signing variant.