nrf_crypto_sign() fails plus not enough randomness

[Krzysztof Zaraska]

Problem: nrf_crypto_sign() fails with NRF_ERROR_INTERNAL. Observed in SDK 12.3.0 on NRF51422. This probably applies to SDK 14.0 when using micro-ecc backend, as the root cause of the bug is still there (see below).

Cause: nrf_crypto_sign() uses ecc_p256_sign() from ecc.c, which in turn calls uECC_sign() from micro-ecc library. However, uECC_sign() requires a working random number generator. NRF SDK does NOT supply the RNG to micro-ecc during initialization. uECC_sign() is thus unable to generate signature, and throws an error which is propagated up the call chain.

Solution: Tell micro-ecc to use a RNG. In nrf_crypto_init() (nrf_crypto.c), change

   ecc_init(false);

to

   ecc_init(true);

This should remove the cause of error.

Alternative solution: In ecc_p256_sign() in ecc.c replace call to uECC_sign() with a call to uECC_sign_deterministic(), which does not use RNG. I did not test this.

Further observations: In ecc.c, a function for supplying RNG output to micro-ecc is defined as follows:

static int ecc_rng(uint8_t *dest, unsigned size)
{
    nrf_drv_rng_block_rand(dest, (uint32_t) size);
    return 1;
}

This implementation is very naive. Consider the following:

nrf_crypto_init();

uint8_t available;
APP_ERROR_CHECK(nrf_drv_rng_bytes_available(&available));

_PRINTF("%d random bytes available\n", available);

uint8_t rnd[32];
for (int i=0; i<4; i++) {
    nrf_drv_rng_rand(rnd, 32);
    _print_hex("RAW ", rnd, 32);
}

Result:

0 random bytes available
RAW  (32 bytes): 12000000120000001BC701000C5000200C29002012000000120000001C500020
RAW  (32 bytes): 12000000120000001BC701000C5000200C29002012000000120000001C500020
RAW  (32 bytes): 12000000120000001BC701000C5000200C29002012000000120000001C500020
RAW  (32 bytes): 12000000120000001BC701000C5000200C29002012000000120000001C500020

Ouch.

Also, the code in ecc.c fails to call nrf_drv_rng_init(), but that works because the softdevice does the initialization anyway.

My solution is to simply block until more random data becomes available:

static bool rng_initialized = false;

void crypto_get_random_block(uint8_t *buf, const uint8_t len)
{
	if ( ! rng_initialized ) {
		APP_ERROR_CHECK(nrf_drv_rng_init(NULL));
		rng_initialized = true;
	}
	
	uint8_t available = 0; 
	uint8_t offset = 0;
	int needed = len;	
	
	_DEBUG("RNG request for %d bytes\n", len);
	
	while (1) {
		APP_ERROR_CHECK(nrf_drv_rng_bytes_available(&available));
		if ( available > 0 ) {
			uint8_t used = ( needed >= available ) ? available : needed;
			
			APP_ERROR_CHECK(nrf_drv_rng_block_rand(buf + offset, used));
			
			//_DEBUG("len=%d offset=%d used=%d available=%d needed=%d\n", len, offset, used, available, needed);
			
			offset = offset + used;
			needed = needed - used;
			
			if ( needed == 0 )
				break;
			
			if ( needed < 0 )
				_ERROR("should not happen!\n");
		} else {
			osDelay(1); // wait  (***)
		}
	}
}

The line marked (***) is for Keil RTX and should be replaced by the wait call of your choice.

Test code:

for (int i=0; i<4; i++) {
	crypto_get_random_block(rnd, 32);
	_print_hex("PROC", rnd, 32);
}

Result:

 @5015ms crypto.c:34 DEBUG  RNG request for 32 bytes
PROC (32 bytes): FAB97C56056922A5194FFA17A74F5FFE40182C05C50F34FD267485F052C04E33
 @5095ms crypto.c:34 DEBUG  RNG request for 32 bytes
PROC (32 bytes): E3918A1C576DCC353218090611ED0153437001E49DE9D8059A0D13300BA60327
 @5127ms crypto.c:34 DEBUG  RNG request for 32 bytes
PROC (32 bytes): F830C7840F9EC967492C318625682C3F9C02E332037AD43EEF4EF772ADF0D6FC
 @5160ms crypto.c:34 DEBUG  RNG request for 32 bytes
PROC (32 bytes): 389BEF03336EAC71715C4863A9E6D94C1A4FE92F5FF2E85187F088C593131BC8

Much better, albeit also much slower.

[Jørgen Holmefjord]

Hi Krzysztof,

I would say this is more of a chosen implementation, rather than a bug. This answer explains why the RNG function is not initialized by default. I do however agree that this could have been documented better in the SDK documentation.

The crypto library have been rewritten completely for SDK v13.0.0 (inherited by SDK v14.0.0), and this issue should no longer be present. By setting the config NRF_CRYPTO_BACKEND_MICRO_ECC_RNG, the RNG function will be initialized during the crypto library initialization.

Your randomness test also seems a bit off to me. The function ecc_rng() calls nrf_drv_rng_block_rand() to generate random bytes. This function will block until enough bytes are available. In your test you are using the non-blocking function nrf_drv_rng_rand() to generate random bytes, but you are not checking the error code returned by the function to make sure it is actually returning NRF_SUCCESS.

The RNG function passed to micro-ecc in SDK v14.0.0 use the non-blocking function, but it check error codes to make sure that randomness is actually generated:

do
{
    nrf_drv_rng_bytes_available(&available);
    cur_len = MIN(left, available);

    err_code = nrf_drv_rng_rand(p_target + (length - left), cur_len);
    if (err_code != NRF_SUCCESS)
    {
        return err_code;
    }

    // Remove current length of generated data
    left -= cur_len;

} while (left > 0);

As you can see from the API documentation of nrf_drv_rng_rand(), the error NRF_ERROR_NOT_FOUND will be returned if not enough bytes were available in the pool.

Hope this helps clear things up!

Best regards,

Jørgen

[Krzysztof Zaraska]

Hi Jørgen,

Thank you for your clarification regarding the blocking vs non-blocking RNG access. Looks like I got the two functions confused. My apologies. However, wouldn't it make more sense to have nrf_drv_rng_rand() assert if there is not enough randomness in the pool? As it stands, if the pool is depleted, the function succeeds, but returns very non-random data.

By the way, here is a non-blocking code which executes in fixed time. This uses DEVICEID and RNG to seed a SHA-256 generator.

static bool rng_initialized = false;
#define RNG_BUFFER_SIZE 32
uint8_t random_buffer[RNG_BUFFER_SIZE];

void crypto_random_init(void)
{
	*((uint32_t*) random_buffer) = NRF_FICR->DEVICEID[0];
	*((uint32_t*) random_buffer+4) = NRF_FICR->DEVICEID[1];

	APP_ERROR_CHECK(nrf_drv_rng_init(NULL));
	uint8_t available;
	do {
		APP_ERROR_CHECK(nrf_drv_rng_bytes_available(&available));
	} while ( available < 24 );

	nrf_drv_rng_rand(random_buffer+8, 24);
	
	rng_initialized = true;	
}

static uint8_t crypto_get_random_byte(void)
{
    sha256_context_t ctx;
	uint8_t buf[RNG_BUFFER_SIZE];
	APP_ERROR_CHECK(sha256_init(&ctx));
    APP_ERROR_CHECK(sha256_update(&ctx, random_buffer, RNG_BUFFER_SIZE));
    APP_ERROR_CHECK(sha256_final(&ctx, buf, 1));
	memcpy(random_buffer, buf, RNG_BUFFER_SIZE);
	
	return random_buffer[0];
}

void crypto_get_random_block(uint8_t *buf, const uint8_t len)
{
	for (int i=0; i<len; i++)
		buf[i] = crypto_get_random_byte();
}