diff mbox series

[v4,3/3] synquacer: Add RNG pseudo TA

Message ID 1542869577-32435-3-git-send-email-sumit.garg@linaro.org
State New
Headers show
Series [v4,1/3] libtomcrypt: Import SHA512/256 approved hash algorithm | expand

Commit Message

Sumit Garg Nov. 22, 2018, 6:52 a.m. UTC
This platform provides 7 on-chip thermal sensors accessible from secure
world only. So, using thermal noise from these sensors we have tried to
create an entropy source as a pseudo TA.

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>

---
 core/arch/arm/plat-synquacer/main.c            |   8 +
 core/arch/arm/plat-synquacer/platform_config.h |   1 +
 core/arch/arm/plat-synquacer/rng_pta.c         | 347 +++++++++++++++++++++++++
 core/arch/arm/plat-synquacer/rng_pta.h         |  11 +
 core/arch/arm/plat-synquacer/rng_pta_client.h  |  30 +++
 core/arch/arm/plat-synquacer/sub.mk            |   1 +
 6 files changed, 398 insertions(+)
 create mode 100644 core/arch/arm/plat-synquacer/rng_pta.c
 create mode 100644 core/arch/arm/plat-synquacer/rng_pta.h
 create mode 100644 core/arch/arm/plat-synquacer/rng_pta_client.h

-- 
2.7.4

Comments

Daniel Thompson Nov. 22, 2018, 11:31 a.m. UTC | #1
Hi Sumit

You sent it again so I reviewed it again...

My advice is don't send it for a fifth pre-review. Anything still
pending at this stage can just go on github.



On Thu, Nov 22, 2018 at 12:22:57PM +0530, Sumit Garg wrote:
> + * Limitations

> + * ===========

> + *

> + * Output rate is limited to approx. 128 bytes per second.

> + *

> + * Our entropy estimation was not reached using any approved or

> + * published estimation framework such as NIST.SP.800-90B and was tests


s/tests/tested/

> + * on a very small set of physical samples (instead we have adopted what


The text in brackets can probably be a new sentence.

Don't venerate what I write in e-mail. It will not have been as
carefully constructed as a comment should be.

> + * we believe to be a conservative estimate and partnered it with a

> + * fairly agressive health check).

> + *

> + * Generating the SHA512/256 hash takes 24uS and will be run by an

> + * interrupt handler that pre-empts the normal world. The interrupt

> + * handler runs on core X.


Please specify X.

> + */

> +

> +#include <crypto/crypto.h>

> +#include <kernel/delay.h>

> +#include <kernel/pseudo_ta.h>

> +#include <kernel/spinlock.h>

> +#include <mm/core_memprot.h>

> +#include <io.h>

> +#include <string.h>

> +#include <rng_pta.h>

> +#include <rng_pta_client.h>

> +#include <timer_fiq.h>

> +

> +#define PTA_NAME "rng.pta"

> +

> +#define THERMAL_SENSOR_BASE0		0x54190800

> +#define THERMAL_SENSOR_OFFSET		0x80

> +#define NUM_SENSORS			7

> +#define NUM_SLOTS			13


(NUM_SENSORS * 2) - 1


> +#define TEMP_DATA_REG_OFFSET		0x34

> +

> +#define ENTROPY_POOL_SIZE		4096

> +

> +#define SENSOR_DATA_SIZE		128

> +#define CONDITIONER_PAYLOAD		(SENSOR_DATA_SIZE * NUM_SENSORS)

> +

> +/*

> + * Used in heatlh test to check if count of either bit flips 1-0 or 0-1 lies

> + * in 12.5% to 37.5% of 128 bytes raw data from particular sensor reading.


"The health test monitors each sensors least significant bit and counts
the number of rising and falling edges. It verifies that both counts
lie within in interval of between 12.5% and 37.5% of the samples."


> In

> + * ideal scenario (1 bit of entropy per LSB) either of bit flips should be

> + * around 25%.


"For true random data with 8 bits of entropy per byte then both counts
would be close to 25%."


> + */

> +#define MAX_BIT_FLIP_EDGE_COUNT		48


(3*SENSOR_DATA_SIZE) / 8

> +#define MIN_BIT_FLIP_EDGE_COUNT		16


SENSOR_DATA_SIZE / 8


> +

> +static uint8_t entropy_pool[ENTROPY_POOL_SIZE] = {0};

> +static uint32_t entropy_size;

> +

> +static uint8_t sensors_data[NUM_SLOTS][SENSOR_DATA_SIZE] = {0};

> +static uint8_t slot_idx;

> +static uint8_t sensor_idx;


Both the idx variables should commence "sensors_data_" to make clear
that they describe offsets into sensors_data.

> +

> +static uint32_t health_test_fail_cnt;

> +static uint32_t health_test_pass_cnt;

> +

> +static unsigned int entropy_lock = SPINLOCK_UNLOCK;

> +

> +static void pool_add_entropy(uint8_t *entropy, uint32_t size)

> +{

> +	uint32_t copy_size;

> +

> +	if (entropy_size >= ENTROPY_POOL_SIZE)

> +		return;

> +

> +	if ((ENTROPY_POOL_SIZE - entropy_size) >= size)

> +		copy_size = size;

> +	else

> +		copy_size = ENTROPY_POOL_SIZE - entropy_size;

> +

> +	memcpy((entropy_pool + entropy_size), entropy, copy_size);

> +

> +	entropy_size += copy_size;

> +}

> +

> +static void pool_get_entropy(uint8_t *buf, uint32_t size)

> +{

> +	uint32_t off;

> +

> +	if (size > entropy_size)

> +		return;

> +

> +	off = entropy_size - size;

> +

> +	memcpy(buf, &entropy_pool[off], size);

> +	entropy_size -= size;

> +}

> +

> +static bool health_test(uint8_t sensor_id)

> +{

> +	bool result = true;

> +	uint8_t bit_flip_falling = 0, bit_flip_rising = 0;


Should be "int" and better names would be "num_edge_rising" or
"rising_edge_count".

> +	uint8_t i;


Also should be "int"

> +

> +	for (i = 0; i < (SENSOR_DATA_SIZE - 1); i++) {

> +		if ((sensors_data[sensor_id][i] ^

> +		     sensors_data[sensor_id][i + 1]) & 0x1) {

> +			bit_flip_falling += sensors_data[sensor_id][i] & 0x1;

> +			bit_flip_rising += sensors_data[sensor_id][i + 1] & 0x1;

> +		}

> +	}

> +

> +	if (bit_flip_falling > bit_flip_rising) {

> +		if (bit_flip_rising < MIN_BIT_FLIP_EDGE_COUNT)

> +			result = false;

> +		if (bit_flip_falling > MAX_BIT_FLIP_EDGE_COUNT)

> +			result = false;

> +	} else {

> +		if (bit_flip_falling < MIN_BIT_FLIP_EDGE_COUNT)

> +			result = false;

> +		if (bit_flip_rising > MAX_BIT_FLIP_EDGE_COUNT)

> +			result = false;

> +	}


This is easier to read with extra variables to act as comments.

int lo_edge_count = rising_edge_count < falling_edge_count ?
			rising_edge_count : falling_edge_count;
int hi_edge_count = rising_edge_count < falling_edge_count ?
			falling_edge_count : rising_edge_count;
return lo_edge_count >= MIN_EDGE_COUNT && hi_edge_count <= MAX_EDGE_COUNT;


> +

> +	return result;

> +}

> +

> +static uint8_t pool_check_add_entropy(void)

> +{

> +	uint32_t i;

> +	uint8_t entropy_sha512_256[TEE_SHA256_HASH_SIZE];

> +	uint8_t pool_status = 0;

> +	TEE_Result res;

> +

> +	for (i = 0; i < NUM_SENSORS; i++) {

> +		/* Check if particular sensor data passes health test */

> +		if (health_test(slot_idx) == true) {

> +			health_test_pass_cnt++;


It would be better not to count passes. Instead count health tests (e.g.
health_test_cnt += NUM_SENSORS outside of the loop). This makes the 1%
make more sense...

> +			slot_idx++;

> +		} else {

> +			health_test_fail_cnt++;

> +			memmove(sensors_data[slot_idx],

> +				sensors_data[slot_idx + 1],

> +				(SENSOR_DATA_SIZE * (NUM_SENSORS - i - 1)));

> +		}

> +	}

> +

> +	/* Check if sensors_data have enough pass data for entropy */


s/entropy/conditioning/


> +	if (slot_idx >= NUM_SENSORS) {

> +		/*

> +		 * Use vetted conditioner SHA512/256 as per

> +		 * NIST.SP.800-90B to condition raw data from entropy

> +		 * source.

> +		 */

> +		res = hash_sha512_256_compute(entropy_sha512_256,

> +					      (uint8_t *)sensors_data,

> +					      CONDITIONER_PAYLOAD);

> +		if (res == TEE_SUCCESS)

> +			pool_add_entropy(entropy_sha512_256,

> +					 TEE_SHA256_HASH_SIZE);

> +

> +		slot_idx -= NUM_SENSORS;

> +		memmove(sensors_data[0],

> +			sensors_data[NUM_SENSORS],

> +			(SENSOR_DATA_SIZE * slot_idx));


Why do we need a memmove() here? Couldn't we have computed the hash of
the top seven slots?

> +	}

> +

> +	if (entropy_size >= ENTROPY_POOL_SIZE)

> +		pool_status = 1;

> +

> +	return pool_status;

> +}

> +

> +void rng_collect_entropy(void)

> +{

> +	uint8_t i, pool_full = 0;

> +	void *vaddr;

> +	uint32_t exceptions = thread_mask_exceptions(THREAD_EXCP_ALL);

> +

> +	cpu_spin_lock(&entropy_lock);

> +

> +	for (i = 0; i < NUM_SENSORS; i++) {

> +		vaddr = phys_to_virt_io(THERMAL_SENSOR_BASE0 +

> +					(THERMAL_SENSOR_OFFSET * i) +

> +					TEMP_DATA_REG_OFFSET);

> +		sensors_data[slot_idx + i][sensor_idx] =

> +				(uint8_t)read32((vaddr_t)vaddr);

> +	}

> +

> +	sensor_idx++;

> +

> +	if (sensor_idx >= SENSOR_DATA_SIZE) {

> +		pool_full = pool_check_add_entropy();

> +		sensor_idx = 0;

> +	}

> +

> +	if (pool_full)

> +		generic_timer_stop();

> +

> +	cpu_spin_unlock(&entropy_lock);

> +	thread_set_exceptions(exceptions);

> +}

> +

> +static TEE_Result rng_get_entropy(uint32_t types,

> +				  TEE_Param params[TEE_NUM_PARAMS])

> +{

> +	uint8_t *e = NULL;

> +	uint32_t pool_size = 0, rq_size = 0;

> +	uint32_t exceptions;

> +	TEE_Result res = TEE_SUCCESS;

> +

> +	if (types != TEE_PARAM_TYPES(TEE_PARAM_TYPE_MEMREF_INOUT,

> +				     TEE_PARAM_TYPE_NONE,

> +				     TEE_PARAM_TYPE_NONE,

> +				     TEE_PARAM_TYPE_NONE)) {

> +		EMSG("bad parameters types: 0x%" PRIx32, types);

> +		return TEE_ERROR_BAD_PARAMETERS;

> +	}

> +

> +	rq_size = params[0].memref.size;

> +

> +	if ((rq_size == 0) || (rq_size > ENTROPY_POOL_SIZE))

> +		return TEE_ERROR_NOT_SUPPORTED;

> +

> +	e = (uint8_t *)params[0].memref.buffer;

> +	if (!e)

> +		return TEE_ERROR_BAD_PARAMETERS;

> +

> +	exceptions = thread_mask_exceptions(THREAD_EXCP_ALL);

> +	cpu_spin_lock(&entropy_lock);

> +

> +	/*

> +	 * Report health test failure to normal world in case fail count

> +	 * exceeds 1% of pass count.

> +	 */

> +	if (health_test_pass_cnt > 100) {


Do we really need a branch here? ...

> +		if (health_test_fail_cnt > (health_test_pass_cnt / 100)) {


... if (health_test_fail_cnt > (health_test_pass_cnt + 100) / 100))

This also relaxes the check a little to permit two failures in 101
health test passes (but not two in 200) which is probably better.


> +			res = TEE_ERROR_HEALTH_TEST_FAIL;

> +			params[0].memref.size = 0;

> +			health_test_pass_cnt = 0;

> +			health_test_fail_cnt = 0;

> +			goto exit;

> +		}

> +	} else {

> +		if (health_test_fail_cnt > 1) {

> +			res = TEE_ERROR_HEALTH_TEST_FAIL;

> +			params[0].memref.size = 0;

> +			health_test_pass_cnt = 0;

> +			health_test_fail_cnt = 0;

> +			goto exit;

> +		}

> +	}

> +

> +	pool_size = entropy_size;

> +

> +	if (pool_size < rq_size) {

> +		params[0].memref.size = pool_size;

> +		pool_get_entropy(e, pool_size);

> +	} else {

> +		params[0].memref.size = rq_size;

> +		pool_get_entropy(e, rq_size);

> +	}

> +

> +exit:

> +	/* Enable timer FIQ to fetch entropy */

> +	generic_timer_start();

> +

> +	cpu_spin_unlock(&entropy_lock);

> +	thread_set_exceptions(exceptions);

> +

> +	return res;

> +}

> +

> +static TEE_Result invoke_command(void *pSessionContext __unused,

> +				 uint32_t nCommandID, uint32_t nParamTypes,

> +				 TEE_Param pParams[TEE_NUM_PARAMS])

> +{

> +	FMSG("command entry point for pseudo-TA \"%s\"", PTA_NAME);

> +

> +	switch (nCommandID) {

> +	case PTA_CMD_GET_ENTROPY:

> +		return rng_get_entropy(nParamTypes, pParams);

> +	default:

> +		break;

> +	}

> +

> +	return TEE_ERROR_NOT_IMPLEMENTED;

> +}

> +

> +pseudo_ta_register(.uuid = PTA_RNG_UUID, .name = PTA_NAME,

> +		   .flags = PTA_DEFAULT_FLAGS,

> +		   .invoke_command_entry_point = invoke_command);

> diff --git a/core/arch/arm/plat-synquacer/rng_pta.h b/core/arch/arm/plat-synquacer/rng_pta.h

> new file mode 100644

> index 0000000..8ce2afa

> --- /dev/null

> +++ b/core/arch/arm/plat-synquacer/rng_pta.h

> @@ -0,0 +1,11 @@

> +/* SPDX-License-Identifier: BSD-2-Clause */

> +/*

> + * Copyright (C) 2018, Linaro Limited

> + */

> +

> +#ifndef __RNG_PTA_H

> +#define __RNG_PTA_H

> +

> +void rng_collect_entropy(void);

> +

> +#endif /* __RNG_PTA_H */

> diff --git a/core/arch/arm/plat-synquacer/rng_pta_client.h b/core/arch/arm/plat-synquacer/rng_pta_client.h

> new file mode 100644

> index 0000000..b7986d3

> --- /dev/null

> +++ b/core/arch/arm/plat-synquacer/rng_pta_client.h

> @@ -0,0 +1,30 @@

> +/* SPDX-License-Identifier: BSD-2-Clause */

> +/*

> + * Copyright (C) 2018, Linaro Limited

> + */

> +

> +#ifndef __RNG_PTA_CLIENT_H

> +#define __RNG_PTA_CLIENT_H

> +

> +#define PTA_RNG_UUID { 0xab7a617c, 0xb8e7, 0x4d8f, \

> +		{ 0x83, 0x01, 0xd0, 0x9b, 0x61, 0x03, 0x6b, 0x64 } }

> +

> +#define TEE_ERROR_HEALTH_TEST_FAIL	0x00000001

> +

> +/*

> + * PTA_CMD_GET_ENTROPY - Get Entropy from RNG using Thermal Sensor

> + *

> + * param[0] (inout memref) - Entropy buffer memory reference

> + * param[1] unused

> + * param[2] unused

> + * param[3] unused

> + *

> + * Result:

> + * TEE_SUCCESS - Invoke command success

> + * TEE_ERROR_BAD_PARAMETERS - Incorrect input param

> + * TEE_ERROR_NOT_SUPPORTED - Requested entropy size greater than size of pool

> + * TEE_ERROR_HEALTH_TEST_FAIL - Continuous health testing failed

> + */

> +#define PTA_CMD_GET_ENTROPY		0x0

> +

> +#endif /* __RNG_PTA_CLIENT_H */

> diff --git a/core/arch/arm/plat-synquacer/sub.mk b/core/arch/arm/plat-synquacer/sub.mk

> index cfa1dc3..013e57d 100644

> --- a/core/arch/arm/plat-synquacer/sub.mk

> +++ b/core/arch/arm/plat-synquacer/sub.mk

> @@ -1,3 +1,4 @@

>  global-incdirs-y += .

>  srcs-y += main.c

> +srcs-y += rng_pta.c

>  srcs-y += timer_fiq.c

> -- 

> 2.7.4

>
Sumit Garg Nov. 22, 2018, 1:14 p.m. UTC | #2
Hi Daniel,

On Thu, 22 Nov 2018 at 17:01, Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>

> Hi Sumit

>

> You sent it again so I reviewed it again...


No worries :).

>

> My advice is don't send it for a fifth pre-review. Anything still

> pending at this stage can just go on github.

>


Thanks for your advice. Will incorporate your comments and push these
patches to github.

BTW, is it ok for you if I use your sign-off? I think you have
contributed much to this patch.

>

>

> On Thu, Nov 22, 2018 at 12:22:57PM +0530, Sumit Garg wrote:

> > + * Limitations

> > + * ===========

> > + *

> > + * Output rate is limited to approx. 128 bytes per second.

> > + *

> > + * Our entropy estimation was not reached using any approved or

> > + * published estimation framework such as NIST.SP.800-90B and was tests

>

> s/tests/tested/


Ok.

>

> > + * on a very small set of physical samples (instead we have adopted what

>

> The text in brackets can probably be a new sentence.

>

> Don't venerate what I write in e-mail. It will not have been as

> carefully constructed as a comment should be.


Ok, I assumed it to be carefully written.

>

> > + * we believe to be a conservative estimate and partnered it with a

> > + * fairly agressive health check).

> > + *

> > + * Generating the SHA512/256 hash takes 24uS and will be run by an

> > + * interrupt handler that pre-empts the normal world. The interrupt

> > + * handler runs on core X.

>

> Please specify X.


X could be any core. So would update comment as "core X (anyone of 0 - 23)."

>

> > + */

> > +

> > +#include <crypto/crypto.h>

> > +#include <kernel/delay.h>

> > +#include <kernel/pseudo_ta.h>

> > +#include <kernel/spinlock.h>

> > +#include <mm/core_memprot.h>

> > +#include <io.h>

> > +#include <string.h>

> > +#include <rng_pta.h>

> > +#include <rng_pta_client.h>

> > +#include <timer_fiq.h>

> > +

> > +#define PTA_NAME "rng.pta"

> > +

> > +#define THERMAL_SENSOR_BASE0         0x54190800

> > +#define THERMAL_SENSOR_OFFSET                0x80

> > +#define NUM_SENSORS                  7

> > +#define NUM_SLOTS                    13

>

> (NUM_SENSORS * 2) - 1

>


Ok.

>

> > +#define TEMP_DATA_REG_OFFSET         0x34

> > +

> > +#define ENTROPY_POOL_SIZE            4096

> > +

> > +#define SENSOR_DATA_SIZE             128

> > +#define CONDITIONER_PAYLOAD          (SENSOR_DATA_SIZE * NUM_SENSORS)

> > +

> > +/*

> > + * Used in heatlh test to check if count of either bit flips 1-0 or 0-1 lies

> > + * in 12.5% to 37.5% of 128 bytes raw data from particular sensor reading.

>

> "The health test monitors each sensors least significant bit and counts

> the number of rising and falling edges. It verifies that both counts

> lie within in interval of between 12.5% and 37.5% of the samples."

>

>

> > In

> > + * ideal scenario (1 bit of entropy per LSB) either of bit flips should be

> > + * around 25%.

>

> "For true random data with 8 bits of entropy per byte then both counts

> would be close to 25%."

>


Ok, will use these comments instead.

>

> > + */

> > +#define MAX_BIT_FLIP_EDGE_COUNT              48

>

> (3*SENSOR_DATA_SIZE) / 8

>


Ok.

> > +#define MIN_BIT_FLIP_EDGE_COUNT              16

>

> SENSOR_DATA_SIZE / 8

>


Ok.

>

> > +

> > +static uint8_t entropy_pool[ENTROPY_POOL_SIZE] = {0};

> > +static uint32_t entropy_size;

> > +

> > +static uint8_t sensors_data[NUM_SLOTS][SENSOR_DATA_SIZE] = {0};

> > +static uint8_t slot_idx;

> > +static uint8_t sensor_idx;

>

> Both the idx variables should commence "sensors_data_" to make clear

> that they describe offsets into sensors_data.

>


Ok.

> > +

> > +static uint32_t health_test_fail_cnt;

> > +static uint32_t health_test_pass_cnt;

> > +

> > +static unsigned int entropy_lock = SPINLOCK_UNLOCK;

> > +

> > +static void pool_add_entropy(uint8_t *entropy, uint32_t size)

> > +{

> > +     uint32_t copy_size;

> > +

> > +     if (entropy_size >= ENTROPY_POOL_SIZE)

> > +             return;

> > +

> > +     if ((ENTROPY_POOL_SIZE - entropy_size) >= size)

> > +             copy_size = size;

> > +     else

> > +             copy_size = ENTROPY_POOL_SIZE - entropy_size;

> > +

> > +     memcpy((entropy_pool + entropy_size), entropy, copy_size);

> > +

> > +     entropy_size += copy_size;

> > +}

> > +

> > +static void pool_get_entropy(uint8_t *buf, uint32_t size)

> > +{

> > +     uint32_t off;

> > +

> > +     if (size > entropy_size)

> > +             return;

> > +

> > +     off = entropy_size - size;

> > +

> > +     memcpy(buf, &entropy_pool[off], size);

> > +     entropy_size -= size;

> > +}

> > +

> > +static bool health_test(uint8_t sensor_id)

> > +{

> > +     bool result = true;

> > +     uint8_t bit_flip_falling = 0, bit_flip_rising = 0;

>

> Should be "int" and better names would be "num_edge_rising" or

> "rising_edge_count".


Ok.

>

> > +     uint8_t i;

>

> Also should be "int"

>


Ok.

> > +

> > +     for (i = 0; i < (SENSOR_DATA_SIZE - 1); i++) {

> > +             if ((sensors_data[sensor_id][i] ^

> > +                  sensors_data[sensor_id][i + 1]) & 0x1) {

> > +                     bit_flip_falling += sensors_data[sensor_id][i] & 0x1;

> > +                     bit_flip_rising += sensors_data[sensor_id][i + 1] & 0x1;

> > +             }

> > +     }

> > +

> > +     if (bit_flip_falling > bit_flip_rising) {

> > +             if (bit_flip_rising < MIN_BIT_FLIP_EDGE_COUNT)

> > +                     result = false;

> > +             if (bit_flip_falling > MAX_BIT_FLIP_EDGE_COUNT)

> > +                     result = false;

> > +     } else {

> > +             if (bit_flip_falling < MIN_BIT_FLIP_EDGE_COUNT)

> > +                     result = false;

> > +             if (bit_flip_rising > MAX_BIT_FLIP_EDGE_COUNT)

> > +                     result = false;

> > +     }

>

> This is easier to read with extra variables to act as comments.

>

> int lo_edge_count = rising_edge_count < falling_edge_count ?

>                         rising_edge_count : falling_edge_count;

> int hi_edge_count = rising_edge_count < falling_edge_count ?

>                         falling_edge_count : rising_edge_count;

> return lo_edge_count >= MIN_EDGE_COUNT && hi_edge_count <= MAX_EDGE_COUNT;

>


Ok, will use this snippet instead.

>

> > +

> > +     return result;

> > +}

> > +

> > +static uint8_t pool_check_add_entropy(void)

> > +{

> > +     uint32_t i;

> > +     uint8_t entropy_sha512_256[TEE_SHA256_HASH_SIZE];

> > +     uint8_t pool_status = 0;

> > +     TEE_Result res;

> > +

> > +     for (i = 0; i < NUM_SENSORS; i++) {

> > +             /* Check if particular sensor data passes health test */

> > +             if (health_test(slot_idx) == true) {

> > +                     health_test_pass_cnt++;

>

> It would be better not to count passes. Instead count health tests (e.g.

> health_test_cnt += NUM_SENSORS outside of the loop). This makes the 1%

> make more sense...

>


Ok.

> > +                     slot_idx++;

> > +             } else {

> > +                     health_test_fail_cnt++;

> > +                     memmove(sensors_data[slot_idx],

> > +                             sensors_data[slot_idx + 1],

> > +                             (SENSOR_DATA_SIZE * (NUM_SENSORS - i - 1)));

> > +             }

> > +     }

> > +

> > +     /* Check if sensors_data have enough pass data for entropy */

>

> s/entropy/conditioning/

>


Ok.

>

> > +     if (slot_idx >= NUM_SENSORS) {

> > +             /*

> > +              * Use vetted conditioner SHA512/256 as per

> > +              * NIST.SP.800-90B to condition raw data from entropy

> > +              * source.

> > +              */

> > +             res = hash_sha512_256_compute(entropy_sha512_256,

> > +                                           (uint8_t *)sensors_data,

> > +                                           CONDITIONER_PAYLOAD);

> > +             if (res == TEE_SUCCESS)

> > +                     pool_add_entropy(entropy_sha512_256,

> > +                                      TEE_SHA256_HASH_SIZE);

> > +

> > +             slot_idx -= NUM_SENSORS;

> > +             memmove(sensors_data[0],

> > +                     sensors_data[NUM_SENSORS],

> > +                     (SENSOR_DATA_SIZE * slot_idx));

>

> Why do we need a memmove() here? Couldn't we have computed the hash of

> the top seven slots?

>


Yes you are right. Will remove this memmove().

> > +     }

> > +

> > +     if (entropy_size >= ENTROPY_POOL_SIZE)

> > +             pool_status = 1;

> > +

> > +     return pool_status;

> > +}

> > +

> > +void rng_collect_entropy(void)

> > +{

> > +     uint8_t i, pool_full = 0;

> > +     void *vaddr;

> > +     uint32_t exceptions = thread_mask_exceptions(THREAD_EXCP_ALL);

> > +

> > +     cpu_spin_lock(&entropy_lock);

> > +

> > +     for (i = 0; i < NUM_SENSORS; i++) {

> > +             vaddr = phys_to_virt_io(THERMAL_SENSOR_BASE0 +

> > +                                     (THERMAL_SENSOR_OFFSET * i) +

> > +                                     TEMP_DATA_REG_OFFSET);

> > +             sensors_data[slot_idx + i][sensor_idx] =

> > +                             (uint8_t)read32((vaddr_t)vaddr);

> > +     }

> > +

> > +     sensor_idx++;

> > +

> > +     if (sensor_idx >= SENSOR_DATA_SIZE) {

> > +             pool_full = pool_check_add_entropy();

> > +             sensor_idx = 0;

> > +     }

> > +

> > +     if (pool_full)

> > +             generic_timer_stop();

> > +

> > +     cpu_spin_unlock(&entropy_lock);

> > +     thread_set_exceptions(exceptions);

> > +}

> > +

> > +static TEE_Result rng_get_entropy(uint32_t types,

> > +                               TEE_Param params[TEE_NUM_PARAMS])

> > +{

> > +     uint8_t *e = NULL;

> > +     uint32_t pool_size = 0, rq_size = 0;

> > +     uint32_t exceptions;

> > +     TEE_Result res = TEE_SUCCESS;

> > +

> > +     if (types != TEE_PARAM_TYPES(TEE_PARAM_TYPE_MEMREF_INOUT,

> > +                                  TEE_PARAM_TYPE_NONE,

> > +                                  TEE_PARAM_TYPE_NONE,

> > +                                  TEE_PARAM_TYPE_NONE)) {

> > +             EMSG("bad parameters types: 0x%" PRIx32, types);

> > +             return TEE_ERROR_BAD_PARAMETERS;

> > +     }

> > +

> > +     rq_size = params[0].memref.size;

> > +

> > +     if ((rq_size == 0) || (rq_size > ENTROPY_POOL_SIZE))

> > +             return TEE_ERROR_NOT_SUPPORTED;

> > +

> > +     e = (uint8_t *)params[0].memref.buffer;

> > +     if (!e)

> > +             return TEE_ERROR_BAD_PARAMETERS;

> > +

> > +     exceptions = thread_mask_exceptions(THREAD_EXCP_ALL);

> > +     cpu_spin_lock(&entropy_lock);

> > +

> > +     /*

> > +      * Report health test failure to normal world in case fail count

> > +      * exceeds 1% of pass count.

> > +      */

> > +     if (health_test_pass_cnt > 100) {

>

> Do we really need a branch here? ...

>

> > +             if (health_test_fail_cnt > (health_test_pass_cnt / 100)) {

>

> ... if (health_test_fail_cnt > (health_test_pass_cnt + 100) / 100))

>

> This also relaxes the check a little to permit two failures in 101

> health test passes (but not two in 200) which is probably better.

>


Agree, for a lower total count, relaxed health test would be better.
Will remove the branch and use this logic instead.

>

> > +                     res = TEE_ERROR_HEALTH_TEST_FAIL;

> > +                     params[0].memref.size = 0;

> > +                     health_test_pass_cnt = 0;

> > +                     health_test_fail_cnt = 0;

> > +                     goto exit;

> > +             }

> > +     } else {

> > +             if (health_test_fail_cnt > 1) {

> > +                     res = TEE_ERROR_HEALTH_TEST_FAIL;

> > +                     params[0].memref.size = 0;

> > +                     health_test_pass_cnt = 0;

> > +                     health_test_fail_cnt = 0;

> > +                     goto exit;

> > +             }

> > +     }

> > +

> > +     pool_size = entropy_size;

> > +

> > +     if (pool_size < rq_size) {

> > +             params[0].memref.size = pool_size;

> > +             pool_get_entropy(e, pool_size);

> > +     } else {

> > +             params[0].memref.size = rq_size;

> > +             pool_get_entropy(e, rq_size);

> > +     }

> > +

> > +exit:

> > +     /* Enable timer FIQ to fetch entropy */

> > +     generic_timer_start();

> > +

> > +     cpu_spin_unlock(&entropy_lock);

> > +     thread_set_exceptions(exceptions);

> > +

> > +     return res;

> > +}

> > +

> > +static TEE_Result invoke_command(void *pSessionContext __unused,

> > +                              uint32_t nCommandID, uint32_t nParamTypes,

> > +                              TEE_Param pParams[TEE_NUM_PARAMS])

> > +{

> > +     FMSG("command entry point for pseudo-TA \"%s\"", PTA_NAME);

> > +

> > +     switch (nCommandID) {

> > +     case PTA_CMD_GET_ENTROPY:

> > +             return rng_get_entropy(nParamTypes, pParams);

> > +     default:

> > +             break;

> > +     }

> > +

> > +     return TEE_ERROR_NOT_IMPLEMENTED;

> > +}

> > +

> > +pseudo_ta_register(.uuid = PTA_RNG_UUID, .name = PTA_NAME,

> > +                .flags = PTA_DEFAULT_FLAGS,

> > +                .invoke_command_entry_point = invoke_command);

> > diff --git a/core/arch/arm/plat-synquacer/rng_pta.h b/core/arch/arm/plat-synquacer/rng_pta.h

> > new file mode 100644

> > index 0000000..8ce2afa

> > --- /dev/null

> > +++ b/core/arch/arm/plat-synquacer/rng_pta.h

> > @@ -0,0 +1,11 @@

> > +/* SPDX-License-Identifier: BSD-2-Clause */

> > +/*

> > + * Copyright (C) 2018, Linaro Limited

> > + */

> > +

> > +#ifndef __RNG_PTA_H

> > +#define __RNG_PTA_H

> > +

> > +void rng_collect_entropy(void);

> > +

> > +#endif /* __RNG_PTA_H */

> > diff --git a/core/arch/arm/plat-synquacer/rng_pta_client.h b/core/arch/arm/plat-synquacer/rng_pta_client.h

> > new file mode 100644

> > index 0000000..b7986d3

> > --- /dev/null

> > +++ b/core/arch/arm/plat-synquacer/rng_pta_client.h

> > @@ -0,0 +1,30 @@

> > +/* SPDX-License-Identifier: BSD-2-Clause */

> > +/*

> > + * Copyright (C) 2018, Linaro Limited

> > + */

> > +

> > +#ifndef __RNG_PTA_CLIENT_H

> > +#define __RNG_PTA_CLIENT_H

> > +

> > +#define PTA_RNG_UUID { 0xab7a617c, 0xb8e7, 0x4d8f, \

> > +             { 0x83, 0x01, 0xd0, 0x9b, 0x61, 0x03, 0x6b, 0x64 } }

> > +

> > +#define TEE_ERROR_HEALTH_TEST_FAIL   0x00000001

> > +

> > +/*

> > + * PTA_CMD_GET_ENTROPY - Get Entropy from RNG using Thermal Sensor

> > + *

> > + * param[0] (inout memref) - Entropy buffer memory reference

> > + * param[1] unused

> > + * param[2] unused

> > + * param[3] unused

> > + *

> > + * Result:

> > + * TEE_SUCCESS - Invoke command success

> > + * TEE_ERROR_BAD_PARAMETERS - Incorrect input param

> > + * TEE_ERROR_NOT_SUPPORTED - Requested entropy size greater than size of pool

> > + * TEE_ERROR_HEALTH_TEST_FAIL - Continuous health testing failed

> > + */

> > +#define PTA_CMD_GET_ENTROPY          0x0

> > +

> > +#endif /* __RNG_PTA_CLIENT_H */

> > diff --git a/core/arch/arm/plat-synquacer/sub.mk b/core/arch/arm/plat-synquacer/sub.mk

> > index cfa1dc3..013e57d 100644

> > --- a/core/arch/arm/plat-synquacer/sub.mk

> > +++ b/core/arch/arm/plat-synquacer/sub.mk

> > @@ -1,3 +1,4 @@

> >  global-incdirs-y += .

> >  srcs-y += main.c

> > +srcs-y += rng_pta.c

> >  srcs-y += timer_fiq.c

> > --

> > 2.7.4

> >
Daniel Thompson Nov. 22, 2018, 1:26 p.m. UTC | #3
On Thu, Nov 22, 2018 at 06:44:33PM +0530, Sumit Garg wrote:
> > My advice is don't send it for a fifth pre-review. Anything still

> > pending at this stage can just go on github.

> >

> 

> Thanks for your advice. Will incorporate your comments and push these

> patches to github.

> 

> BTW, is it ok for you if I use your sign-off? I think you have

> contributed much to this patch.


Thanks... but there is no need for a sign off from me.


> > > + * we believe to be a conservative estimate and partnered it with a

> > > + * fairly agressive health check).

> > > + *

> > > + * Generating the SHA512/256 hash takes 24uS and will be run by an

> > > + * interrupt handler that pre-empts the normal world. The interrupt

> > > + * handler runs on core X.

> >

> > Please specify X.

> 

> X could be any core. So would update comment as "core X (anyone of 0 - 23)."


I see. I has assumed there was an affinity configured at the GIC. If
there is no affinity set it is best to remove this part of the comment.

> > > +     uint8_t bit_flip_falling = 0, bit_flip_rising = 0;

> >

> > Should be "int" and better names would be "num_edge_rising" or

> > "rising_edge_count".

> 

> Ok.

> 

> >

> > > +     uint8_t i;

> >

> > Also should be "int"

> >

> 

> Ok.


Thinking more about it, these could remain unsigned if you prefer but
should not be limited to 8-bit (it is a needless maintenance risk IMHO).


Daniel.
Sumit Garg Nov. 22, 2018, 1:35 p.m. UTC | #4
On Thu, 22 Nov 2018 at 18:56, Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>

> On Thu, Nov 22, 2018 at 06:44:33PM +0530, Sumit Garg wrote:

> > > My advice is don't send it for a fifth pre-review. Anything still

> > > pending at this stage can just go on github.

> > >

> >

> > Thanks for your advice. Will incorporate your comments and push these

> > patches to github.

> >

> > BTW, is it ok for you if I use your sign-off? I think you have

> > contributed much to this patch.

>

> Thanks... but there is no need for a sign off from me.

>

>

> > > > + * we believe to be a conservative estimate and partnered it with a

> > > > + * fairly agressive health check).

> > > > + *

> > > > + * Generating the SHA512/256 hash takes 24uS and will be run by an

> > > > + * interrupt handler that pre-empts the normal world. The interrupt

> > > > + * handler runs on core X.

> > >

> > > Please specify X.

> >

> > X could be any core. So would update comment as "core X (anyone of 0 - 23)."

>

> I see. I has assumed there was an affinity configured at the GIC. If

> there is no affinity set it is best to remove this part of the comment.

>


Actually secure timer interrupt in PPI specific to each core. So any
core can come in and enable corresponding interrupt. Will remove this
part of comment.

> > > > +     uint8_t bit_flip_falling = 0, bit_flip_rising = 0;

> > >

> > > Should be "int" and better names would be "num_edge_rising" or

> > > "rising_edge_count".

> >

> > Ok.

> >

> > >

> > > > +     uint8_t i;

> > >

> > > Also should be "int"

> > >

> >

> > Ok.

>

> Thinking more about it, these could remain unsigned if you prefer but

> should not be limited to 8-bit (it is a needless maintenance risk IMHO).

>


Hmm, will make it 32 bit instead.

-Sumit

>

> Daniel.
diff mbox series

Patch

diff --git a/core/arch/arm/plat-synquacer/main.c b/core/arch/arm/plat-synquacer/main.c
index 714becd..c5af85d 100644
--- a/core/arch/arm/plat-synquacer/main.c
+++ b/core/arch/arm/plat-synquacer/main.c
@@ -18,6 +18,7 @@ 
 #include <sm/optee_smc.h>
 #include <tee/entry_fast.h>
 #include <tee/entry_std.h>
+#include <rng_pta.h>
 #include <timer_fiq.h>
 
 static void main_fiq(void);
@@ -39,6 +40,7 @@  static struct pl011_data console_data;
 
 register_phys_mem(MEM_AREA_IO_NSEC, CONSOLE_UART_BASE, CORE_MMU_DEVICE_SIZE);
 register_phys_mem(MEM_AREA_IO_SEC, GIC_BASE, CORE_MMU_DEVICE_SIZE);
+register_phys_mem(MEM_AREA_IO_SEC, THERMAL_SENSOR_BASE, CORE_MMU_DEVICE_SIZE);
 
 const struct thread_handlers *generic_boot_get_handlers(void)
 {
@@ -78,6 +80,9 @@  static enum itr_return timer_itr_cb(struct itr_handler *h __unused)
 	/* Reset timer for next FIQ */
 	generic_timer_handler();
 
+	/* Collect entropy on each timer FIQ */
+	rng_collect_entropy();
+
 	return ITRR_HANDLED;
 }
 
@@ -92,6 +97,9 @@  static TEE_Result init_timer_itr(void)
 	itr_add(&timer_itr);
 	itr_enable(IT_SEC_TIMER);
 
+	/* Enable timer FIQ to fetch entropy required during boot */
+	generic_timer_start();
+
 	return TEE_SUCCESS;
 }
 driver_init(init_timer_itr);
diff --git a/core/arch/arm/plat-synquacer/platform_config.h b/core/arch/arm/plat-synquacer/platform_config.h
index f9b1b40..8a91ddb 100644
--- a/core/arch/arm/plat-synquacer/platform_config.h
+++ b/core/arch/arm/plat-synquacer/platform_config.h
@@ -19,6 +19,7 @@ 
 #define CONSOLE_UART_CLK_IN_HZ		62500000
 #define CONSOLE_BAUDRATE		115200
 
+#define THERMAL_SENSOR_BASE		0x54190000
 #define IT_SEC_TIMER			29
 
 #define DRAM0_BASE			0x80000000
diff --git a/core/arch/arm/plat-synquacer/rng_pta.c b/core/arch/arm/plat-synquacer/rng_pta.c
new file mode 100644
index 0000000..0329264
--- /dev/null
+++ b/core/arch/arm/plat-synquacer/rng_pta.c
@@ -0,0 +1,347 @@ 
+// SPDX-License-Identifier: BSD-2-Clause
+/*
+ * Copyright (C) 2018, Linaro Limited
+ */
+
+/*
+ * Developerbox doesn't provide a hardware based true random number
+ * generator. So this pseudo TA provides a good source of entropy using
+ * noise from 7 thermal sensors. Its suitable for entropy required
+ * during boot, seeding kernel entropy pool, cryptographic use etc.
+ *
+ * Assumption
+ * ==========
+ *
+ * We have assumed the entropy of the sensor is better than 8 bits per
+ * 14 sensor readings. This entropy estimate is based on our simple
+ * minimal entropy estimates done on 2.1G bytes of raw samples collected
+ * from thermal sensors.
+ *
+ * We believe our estimate to be conservative and have designed to
+ * health tests to trigger if a sensor does not achieve at least
+ * 8 bits in 16 sensor reading (we use 16 rather than 14 to prevent
+ * spurious failures on edge cases).
+ *
+ * Theory of operation
+ * ===================
+ *
+ * This routine uses secure timer interrupt to sample raw thermal sensor
+ * readings. As thermal sensor refresh rate is every 2ms, so interrupt
+ * fires every 2ms. It implements continuous health test counting rising
+ * and falling edges to report if sensors fail to provide entropy.
+ *
+ * It uses vetted conditioner as SHA512/256 (approved hash algorithm)
+ * to condense entropy. As per NIST.SP.800-90B spec, to get full entropy
+ * from vetted conditioner, we need to supply double of input entropy.
+ * According to assumption above and requirement for vetted conditioner,
+ * we need to supply 28 raw sensor readings to get 1 byte of full
+ * entropy as output. So for 32 bytes of conditioner output, we need to
+ * supply 896 bytes of raw sensor readings.
+ *
+ * Interfaces -> Input
+ * -------------------
+ *
+ * void rng_collect_entropy(void);
+ *
+ * Called as part of secure timer interrupt handler to sample raw
+ * thermal sensor readings and add entropy to the pool.
+ *
+ * Interfaces -> Output
+ * --------------------
+ *
+ * TEE_Result rng_get_entropy(uint32_t types,
+ *                            TEE_Param params[TEE_NUM_PARAMS]);
+ *
+ * Invoke command to expose an entropy interface to normal world.
+ *
+ * Testing
+ * =======
+ *
+ * Passes FIPS 140-2 rngtest.
+ *
+ * Limitations
+ * ===========
+ *
+ * Output rate is limited to approx. 128 bytes per second.
+ *
+ * Our entropy estimation was not reached using any approved or
+ * published estimation framework such as NIST.SP.800-90B and was tests
+ * on a very small set of physical samples (instead we have adopted what
+ * we believe to be a conservative estimate and partnered it with a
+ * fairly agressive health check).
+ *
+ * Generating the SHA512/256 hash takes 24uS and will be run by an
+ * interrupt handler that pre-empts the normal world. The interrupt
+ * handler runs on core X.
+ */
+
+#include <crypto/crypto.h>
+#include <kernel/delay.h>
+#include <kernel/pseudo_ta.h>
+#include <kernel/spinlock.h>
+#include <mm/core_memprot.h>
+#include <io.h>
+#include <string.h>
+#include <rng_pta.h>
+#include <rng_pta_client.h>
+#include <timer_fiq.h>
+
+#define PTA_NAME "rng.pta"
+
+#define THERMAL_SENSOR_BASE0		0x54190800
+#define THERMAL_SENSOR_OFFSET		0x80
+#define NUM_SENSORS			7
+#define NUM_SLOTS			13
+
+#define TEMP_DATA_REG_OFFSET		0x34
+
+#define ENTROPY_POOL_SIZE		4096
+
+#define SENSOR_DATA_SIZE		128
+#define CONDITIONER_PAYLOAD		(SENSOR_DATA_SIZE * NUM_SENSORS)
+
+/*
+ * Used in heatlh test to check if count of either bit flips 1-0 or 0-1 lies
+ * in 12.5% to 37.5% of 128 bytes raw data from particular sensor reading. In
+ * ideal scenario (1 bit of entropy per LSB) either of bit flips should be
+ * around 25%.
+ */
+#define MAX_BIT_FLIP_EDGE_COUNT		48
+#define MIN_BIT_FLIP_EDGE_COUNT		16
+
+static uint8_t entropy_pool[ENTROPY_POOL_SIZE] = {0};
+static uint32_t entropy_size;
+
+static uint8_t sensors_data[NUM_SLOTS][SENSOR_DATA_SIZE] = {0};
+static uint8_t slot_idx;
+static uint8_t sensor_idx;
+
+static uint32_t health_test_fail_cnt;
+static uint32_t health_test_pass_cnt;
+
+static unsigned int entropy_lock = SPINLOCK_UNLOCK;
+
+static void pool_add_entropy(uint8_t *entropy, uint32_t size)
+{
+	uint32_t copy_size;
+
+	if (entropy_size >= ENTROPY_POOL_SIZE)
+		return;
+
+	if ((ENTROPY_POOL_SIZE - entropy_size) >= size)
+		copy_size = size;
+	else
+		copy_size = ENTROPY_POOL_SIZE - entropy_size;
+
+	memcpy((entropy_pool + entropy_size), entropy, copy_size);
+
+	entropy_size += copy_size;
+}
+
+static void pool_get_entropy(uint8_t *buf, uint32_t size)
+{
+	uint32_t off;
+
+	if (size > entropy_size)
+		return;
+
+	off = entropy_size - size;
+
+	memcpy(buf, &entropy_pool[off], size);
+	entropy_size -= size;
+}
+
+static bool health_test(uint8_t sensor_id)
+{
+	bool result = true;
+	uint8_t bit_flip_falling = 0, bit_flip_rising = 0;
+	uint8_t i;
+
+	for (i = 0; i < (SENSOR_DATA_SIZE - 1); i++) {
+		if ((sensors_data[sensor_id][i] ^
+		     sensors_data[sensor_id][i + 1]) & 0x1) {
+			bit_flip_falling += sensors_data[sensor_id][i] & 0x1;
+			bit_flip_rising += sensors_data[sensor_id][i + 1] & 0x1;
+		}
+	}
+
+	if (bit_flip_falling > bit_flip_rising) {
+		if (bit_flip_rising < MIN_BIT_FLIP_EDGE_COUNT)
+			result = false;
+		if (bit_flip_falling > MAX_BIT_FLIP_EDGE_COUNT)
+			result = false;
+	} else {
+		if (bit_flip_falling < MIN_BIT_FLIP_EDGE_COUNT)
+			result = false;
+		if (bit_flip_rising > MAX_BIT_FLIP_EDGE_COUNT)
+			result = false;
+	}
+
+	return result;
+}
+
+static uint8_t pool_check_add_entropy(void)
+{
+	uint32_t i;
+	uint8_t entropy_sha512_256[TEE_SHA256_HASH_SIZE];
+	uint8_t pool_status = 0;
+	TEE_Result res;
+
+	for (i = 0; i < NUM_SENSORS; i++) {
+		/* Check if particular sensor data passes health test */
+		if (health_test(slot_idx) == true) {
+			health_test_pass_cnt++;
+			slot_idx++;
+		} else {
+			health_test_fail_cnt++;
+			memmove(sensors_data[slot_idx],
+				sensors_data[slot_idx + 1],
+				(SENSOR_DATA_SIZE * (NUM_SENSORS - i - 1)));
+		}
+	}
+
+	/* Check if sensors_data have enough pass data for entropy */
+	if (slot_idx >= NUM_SENSORS) {
+		/*
+		 * Use vetted conditioner SHA512/256 as per
+		 * NIST.SP.800-90B to condition raw data from entropy
+		 * source.
+		 */
+		res = hash_sha512_256_compute(entropy_sha512_256,
+					      (uint8_t *)sensors_data,
+					      CONDITIONER_PAYLOAD);
+		if (res == TEE_SUCCESS)
+			pool_add_entropy(entropy_sha512_256,
+					 TEE_SHA256_HASH_SIZE);
+
+		slot_idx -= NUM_SENSORS;
+		memmove(sensors_data[0],
+			sensors_data[NUM_SENSORS],
+			(SENSOR_DATA_SIZE * slot_idx));
+	}
+
+	if (entropy_size >= ENTROPY_POOL_SIZE)
+		pool_status = 1;
+
+	return pool_status;
+}
+
+void rng_collect_entropy(void)
+{
+	uint8_t i, pool_full = 0;
+	void *vaddr;
+	uint32_t exceptions = thread_mask_exceptions(THREAD_EXCP_ALL);
+
+	cpu_spin_lock(&entropy_lock);
+
+	for (i = 0; i < NUM_SENSORS; i++) {
+		vaddr = phys_to_virt_io(THERMAL_SENSOR_BASE0 +
+					(THERMAL_SENSOR_OFFSET * i) +
+					TEMP_DATA_REG_OFFSET);
+		sensors_data[slot_idx + i][sensor_idx] =
+				(uint8_t)read32((vaddr_t)vaddr);
+	}
+
+	sensor_idx++;
+
+	if (sensor_idx >= SENSOR_DATA_SIZE) {
+		pool_full = pool_check_add_entropy();
+		sensor_idx = 0;
+	}
+
+	if (pool_full)
+		generic_timer_stop();
+
+	cpu_spin_unlock(&entropy_lock);
+	thread_set_exceptions(exceptions);
+}
+
+static TEE_Result rng_get_entropy(uint32_t types,
+				  TEE_Param params[TEE_NUM_PARAMS])
+{
+	uint8_t *e = NULL;
+	uint32_t pool_size = 0, rq_size = 0;
+	uint32_t exceptions;
+	TEE_Result res = TEE_SUCCESS;
+
+	if (types != TEE_PARAM_TYPES(TEE_PARAM_TYPE_MEMREF_INOUT,
+				     TEE_PARAM_TYPE_NONE,
+				     TEE_PARAM_TYPE_NONE,
+				     TEE_PARAM_TYPE_NONE)) {
+		EMSG("bad parameters types: 0x%" PRIx32, types);
+		return TEE_ERROR_BAD_PARAMETERS;
+	}
+
+	rq_size = params[0].memref.size;
+
+	if ((rq_size == 0) || (rq_size > ENTROPY_POOL_SIZE))
+		return TEE_ERROR_NOT_SUPPORTED;
+
+	e = (uint8_t *)params[0].memref.buffer;
+	if (!e)
+		return TEE_ERROR_BAD_PARAMETERS;
+
+	exceptions = thread_mask_exceptions(THREAD_EXCP_ALL);
+	cpu_spin_lock(&entropy_lock);
+
+	/*
+	 * Report health test failure to normal world in case fail count
+	 * exceeds 1% of pass count.
+	 */
+	if (health_test_pass_cnt > 100) {
+		if (health_test_fail_cnt > (health_test_pass_cnt / 100)) {
+			res = TEE_ERROR_HEALTH_TEST_FAIL;
+			params[0].memref.size = 0;
+			health_test_pass_cnt = 0;
+			health_test_fail_cnt = 0;
+			goto exit;
+		}
+	} else {
+		if (health_test_fail_cnt > 1) {
+			res = TEE_ERROR_HEALTH_TEST_FAIL;
+			params[0].memref.size = 0;
+			health_test_pass_cnt = 0;
+			health_test_fail_cnt = 0;
+			goto exit;
+		}
+	}
+
+	pool_size = entropy_size;
+
+	if (pool_size < rq_size) {
+		params[0].memref.size = pool_size;
+		pool_get_entropy(e, pool_size);
+	} else {
+		params[0].memref.size = rq_size;
+		pool_get_entropy(e, rq_size);
+	}
+
+exit:
+	/* Enable timer FIQ to fetch entropy */
+	generic_timer_start();
+
+	cpu_spin_unlock(&entropy_lock);
+	thread_set_exceptions(exceptions);
+
+	return res;
+}
+
+static TEE_Result invoke_command(void *pSessionContext __unused,
+				 uint32_t nCommandID, uint32_t nParamTypes,
+				 TEE_Param pParams[TEE_NUM_PARAMS])
+{
+	FMSG("command entry point for pseudo-TA \"%s\"", PTA_NAME);
+
+	switch (nCommandID) {
+	case PTA_CMD_GET_ENTROPY:
+		return rng_get_entropy(nParamTypes, pParams);
+	default:
+		break;
+	}
+
+	return TEE_ERROR_NOT_IMPLEMENTED;
+}
+
+pseudo_ta_register(.uuid = PTA_RNG_UUID, .name = PTA_NAME,
+		   .flags = PTA_DEFAULT_FLAGS,
+		   .invoke_command_entry_point = invoke_command);
diff --git a/core/arch/arm/plat-synquacer/rng_pta.h b/core/arch/arm/plat-synquacer/rng_pta.h
new file mode 100644
index 0000000..8ce2afa
--- /dev/null
+++ b/core/arch/arm/plat-synquacer/rng_pta.h
@@ -0,0 +1,11 @@ 
+/* SPDX-License-Identifier: BSD-2-Clause */
+/*
+ * Copyright (C) 2018, Linaro Limited
+ */
+
+#ifndef __RNG_PTA_H
+#define __RNG_PTA_H
+
+void rng_collect_entropy(void);
+
+#endif /* __RNG_PTA_H */
diff --git a/core/arch/arm/plat-synquacer/rng_pta_client.h b/core/arch/arm/plat-synquacer/rng_pta_client.h
new file mode 100644
index 0000000..b7986d3
--- /dev/null
+++ b/core/arch/arm/plat-synquacer/rng_pta_client.h
@@ -0,0 +1,30 @@ 
+/* SPDX-License-Identifier: BSD-2-Clause */
+/*
+ * Copyright (C) 2018, Linaro Limited
+ */
+
+#ifndef __RNG_PTA_CLIENT_H
+#define __RNG_PTA_CLIENT_H
+
+#define PTA_RNG_UUID { 0xab7a617c, 0xb8e7, 0x4d8f, \
+		{ 0x83, 0x01, 0xd0, 0x9b, 0x61, 0x03, 0x6b, 0x64 } }
+
+#define TEE_ERROR_HEALTH_TEST_FAIL	0x00000001
+
+/*
+ * PTA_CMD_GET_ENTROPY - Get Entropy from RNG using Thermal Sensor
+ *
+ * param[0] (inout memref) - Entropy buffer memory reference
+ * param[1] unused
+ * param[2] unused
+ * param[3] unused
+ *
+ * Result:
+ * TEE_SUCCESS - Invoke command success
+ * TEE_ERROR_BAD_PARAMETERS - Incorrect input param
+ * TEE_ERROR_NOT_SUPPORTED - Requested entropy size greater than size of pool
+ * TEE_ERROR_HEALTH_TEST_FAIL - Continuous health testing failed
+ */
+#define PTA_CMD_GET_ENTROPY		0x0
+
+#endif /* __RNG_PTA_CLIENT_H */
diff --git a/core/arch/arm/plat-synquacer/sub.mk b/core/arch/arm/plat-synquacer/sub.mk
index cfa1dc3..013e57d 100644
--- a/core/arch/arm/plat-synquacer/sub.mk
+++ b/core/arch/arm/plat-synquacer/sub.mk
@@ -1,3 +1,4 @@ 
 global-incdirs-y += .
 srcs-y += main.c
+srcs-y += rng_pta.c
 srcs-y += timer_fiq.c