Skip to content

LPC546XX: Add TRNG support #6221

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 18, 2018
Merged

LPC546XX: Add TRNG support #6221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion targets/TARGET_NXP/TARGET_MCUXpresso_MCUS/TARGET_LPC/objects.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,18 @@ struct spi_s {
uint8_t bits;
};

#if DEVICE_FLASH
#if defined(DEVICE_FLASH)
struct flash_s {
uint8_t dummy;
};
#endif

#if defined(DEVICE_TRNG)
struct trng_s {
uint8_t dummy;
};
#endif

#include "gpio_object.h"

#ifdef __cplusplus
Expand Down
59 changes: 59 additions & 0 deletions targets/TARGET_NXP/TARGET_MCUXpresso_MCUS/TARGET_LPC/trng_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/* mbed Microcontroller Library
* Copyright (c) 2018 ARM Limited
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include "trng_api.h"

#if defined(DEVICE_TRNG)
#include "fsl_rng.h"

void trng_init(trng_t *obj)
{
}

void trng_free(trng_t *obj)
{
}

int trng_get_bytes(trng_t *obj, uint8_t *output, size_t length, size_t *output_length)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please suppress unused argument compiler warning for obj.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mmahadevan108 Could you address that comment, too, when you're fixing the zeroization below?

{
uint32_t skip;
uint32_t data;
size_t idx = 0;
int i;

/* Get Random data */
while (idx < length) {

data = RNG_GetRandomData();

for (i = 0; ((i < 4) && (idx < length)); i++) {
output[idx++] = (data >> (i * 8)) & 0xFF;
}

/* Skip next 32 random numbers for better entropy */
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is that guaranteeing better entropy? Is there some known correlation between subsequent bytes returned from the RNG?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is done per what is recommended in the reference manual.

To constitute one 128 bit number, a 32 bit random number is read, then the next 32
numbers are read but not used. The next 32 bit number is read and used and so on. Thus
32 32-bit random numbers are skipped between two 32-bit numbers that are used.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code doesn't implement this but instead skips 32*32 bits per 32 bits of entropy. Could you correct that, to avoid unnecessary workload in the device?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For every 32 bit random number read, it is recommended that the next 32 32-bit random numbers are skipped. This is what the code is doing.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should have read that properly - apologies, my fault!

for (skip = 0; skip < 32; skip++) {
RNG_GetRandomData();
}
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please reliably zeroize data before returning to avoid leakage of entropy on the stack.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will zeroize data and update this commit

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mmahadevan108 Please zeroize data reliably, i.e. avoiding compiler-optimizations to remove it. For example, you could do *((volatile uint32_t*) &data)=0 at the end of the function (it's not necessary to do the clearing in every iteration).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the time the while loop ends data contains a 32bit random number that has been skipped. Is it something we need to zeroize?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mazimkhan Yes, it is zeroized before returning from the function, isn't it?

*output_length = idx;

/* Zeroize to avoid leakage of entropy on the stack. Also ensure this is not removed by compiler optimizations */
*((volatile uint32_t*) &data) = 0;

return (idx == length ? 0 : -1);
}
#endif
2 changes: 1 addition & 1 deletion targets/targets.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,7 @@
"is_disk_virtual": true,
"macros": ["CPU_LPC54628J512ET180", "FSL_RTOS_MBED"],
"inherits": ["Target"],
"device_has": ["ANALOGIN", "I2C", "I2CSLAVE", "INTERRUPTIN", "PORTIN", "PORTINOUT", "PORTOUT", "RTC", "SERIAL", "SLEEP", "SPI", "SPISLAVE", "STDIO_MESSAGES", "FLASH"],
"device_has": ["ANALOGIN", "I2C", "I2CSLAVE", "INTERRUPTIN", "PORTIN", "PORTINOUT", "PORTOUT", "RTC", "SERIAL", "SLEEP", "SPI", "SPISLAVE", "STDIO_MESSAGES", "FLASH", "TRNG"],
"features": ["LWIP"],
"device_name" : "LPC54628J512ET180"
},
Expand Down