LPC546XX: Add TRNG support #6221

mmahadevan108 · 2018-02-26T21:12:00Z

LPC546XX: Add TRNG support

| LPC546XX-ARM | LPC546XX | mbed-os-tests-mbedtls-multi |
Crypto: sha256_multi | 1 | 0 | OK
| 0.82 |
| LPC546XX-ARM | LPC546XX | mbed-os-tests-mbedtls-multi |
Crypto: sha256_split | 1 | 0 | OK
| 0.28 |
| LPC546XX-ARM | LPC546XX | mbed-os-tests-mbedtls-selftest |
mbedtls_entropy_self_test | 1 | 0 | OK
| 0.12 |
| LPC546XX-ARM | LPC546XX | mbed-os-tests-mbedtls-selftest |
mbedtls_sha256_self_test | 1 | 0 | OK
| 0.98 |
| LPC546XX-ARM | LPC546XX | mbed-os-tests-mbedtls-selftest |
mbedtls_sha512_self_test | 1 | 0 | OK
| 2.1 |

mmahadevan108 · 2018-02-26T21:12:17Z

cc @0xc0170 @maclobdell

mazimkhan · 2018-02-27T10:19:29Z

targets/TARGET_NXP/TARGET_MCUXpresso_MCUS/TARGET_LPC/trng_api.c

+{
+}
+
+int trng_get_bytes(trng_t *obj, uint8_t *output, size_t length, size_t *output_length)


Please suppress unused argument compiler warning for obj.

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

ciarmcom · 2018-02-27T12:50:24Z

ARM Internal Ref: IOTSSL-2111

hanno-becker · 2018-03-01T10:39:09Z

targets/TARGET_NXP/TARGET_MCUXpresso_MCUS/TARGET_LPC/trng_api.c

+            RNG_GetRandomData();
+        }
+    }
+


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

I will zeroize data and update this commit

@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).

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

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

hanno-becker · 2018-03-01T10:47:45Z

targets/TARGET_NXP/TARGET_MCUXpresso_MCUS/TARGET_LPC/trng_api.c

+            output[idx++] = (data >> (i * 8)) & 0xFF;
+        }
+
+        /* Skip next 32 random numbers for better entropy */


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

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.

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?

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.

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

hanno-becker

The stack variable holding the most recent 32 bits of entropy should be reliably zeroized before returning from trng_get_bytes.

mmahadevan108 · 2018-03-02T16:31:59Z

PR has been updated per review comments

cmonr · 2018-03-06T04:32:50Z

/morph build

cmonr · 2018-03-06T04:33:12Z

@hanno-arm Mind doing a re-review?

mbed-ci · 2018-03-06T05:20:31Z

Build : FAILURE

Build number : 1355
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6221/

cmonr · 2018-03-06T17:25:38Z

From one of the error files:

        [DEBUG] Compile: /usr/local/ARM_Compiler_5.06u3/bin/armcc -c --gnu -Otime --split_sections --apcs=interwork --brief_diagnostics --restrict --multibyte_chars -O3 --cpu=Cortex-M4.fp --md --no_depend_system_headers --c99 -D__ASSERT_MSG -DTARGET_LPC54114 -DMBED_STACK_STATS_ENABLED=1 -D__MBED__=1 -DDEVICE_I2CSLAVE=1 -D__FPU_PRESENT=1 -DTARGET_NXP -DDEVICE_PORTINOUT=1 -DTARGET_RTOS_M4_M7 -DMBED_TRAP_ERRORS_ENABLED=1 -DDEVICE_RTC=1 -DTOOLCHAIN_object -D__CMSIS_RTOS -DFSL_RTOS_MBED -DCPU_LPC54114J256BD64_cm4 -DTARGET_CORTEX_M -DTARGET_LPC -DTARGET_LIKE_CORTEX_M4 -DTARGET_M4 -DTARGET_UVISOR_UNSUPPORTED -DDEVICE_ANALOGIN=1 -DMBED_HEAP_STATS_ENABLED=1 -DMBED_BUILD_TIMESTAMP=1520313317.47 -DDEVICE_INTERRUPTIN=1 -DTARGET_CORTEX -DDEVICE_I2C=1 -DDEVICE_PORTOUT=1 -D__CORTEX_M4 -DDEVICE_STDIO_MESSAGES=1 -DTARGET_LIKE_MBED -DTARGET_FF_ARDUINO -DDEVICE_PORTIN=1 -DTARGET_RELEASE -D__MBED_CMSIS_RTOS_CM -DDEVICE_SLEEP=1 -DTARGET_MCUXpresso_MCUS -DDEVICE_SPI=1 -DTOOLCHAIN_ARM_STD -DDEVICE_SPISLAVE=1 -DTARGET_LPCXpresso -DTARGET_LPC54114_M4 -DDEVICE_SERIAL=1 -DTOOLCHAIN_ARM -DARM_MATH_CM4 --via /builds/ws/mbed-os-build-matrix/1355/LPC54114_ARM/sources/mbed-os/BUILD/tests/LPC54114/ARM/.includes_ec80e9c611099007aa03df32d4461a39.txt --preinclude=/builds/ws/mbed-os-build-matrix/1355/LPC54114_ARM/sources/mbed-os/BUILD/tests/LPC54114/ARM/mbed_config.h --depend BUILD/tests/LPC54114/ARM/targets/TARGET_NXP/TARGET_MCUXpresso_MCUS/TARGET_LPC/trng_api.d -o BUILD/tests/LPC54114/ARM/targets/TARGET_NXP/TARGET_MCUXpresso_MCUS/TARGET_LPC/trng_api.o /builds/ws/mbed-os-build-matrix/1355/LPC54114_ARM/sources/mbed-os/targets/TARGET_NXP/TARGET_MCUXpresso_MCUS/TARGET_LPC/trng_api.c
        [Error] trng_api.c@18,0:  #5: cannot open source input file "fsl_rng.h": No such file or directory
        [DEBUG] Return: 1
        [DEBUG] Output: "/builds/ws/mbed-os-build-matrix/1355/LPC54114_ARM/sources/mbed-os/targets/TARGET_NXP/TARGET_MCUXpresso_MCUS/TARGET_LPC/trng_api.c", line 18: Error:  #5: cannot open source input file "fsl_rng.h": No such file or directory
        [DEBUG] Output: /builds/ws/mbed-os-build-matrix/1355/LPC54114_ARM/sources/mbed-os/targets/TARGET_NXP/TARGET_MCUXpresso_MCUS/TARGET_LPC/trng_api.c: 0 warnings, 1 error

mmahadevan108 · 2018-03-06T18:11:44Z

I don't see this build error. The fsl_rng.h file is available under the LPC546XX/drivers folder.

mmahadevan108 · 2018-03-06T19:09:57Z

@cmonr I updated the PR to fix the build error seen on LPC54114.

hanno-becker

Looks good to me.

hanno-becker · 2018-04-03T09:26:30Z

@mazimkhan Could you re-review this PR? It has changed slightly since your last approval.

0xc0170 · 2018-04-03T09:32:24Z

/morph build

mbed-ci · 2018-04-03T09:57:39Z

Build : SUCCESS

Build number : 1644
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6221/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

mbed-ci · 2018-04-03T15:35:48Z

Exporter Build : SUCCESS

Build number : 1276
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/6221/

mbed-ci · 2018-04-03T18:29:06Z

Test : FAILURE

Build number : 1433
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/6221/1433

cmonr · 2018-04-03T18:31:11Z

NRF52 Flash cache test issue...
/morph test

mbed-ci · 2018-04-04T05:34:10Z

Test : SUCCESS

Build number : 1442
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/6221/1442

mmahadevan108 · 2018-04-09T20:50:05Z

Anything needed for this PR?

cmonr · 2018-04-10T22:28:57Z

@mmahadevan108 We're waiting on a resolution on a GitHub issue that's affecting pr-head. Once it's resolved, we'll relaunch pr-head and hopefully merge this PR in.

0xc0170 · 2018-04-17T10:26:37Z

Green 🎊 PR head was fixed

0xc0170 · 2018-04-17T10:27:14Z

I'll rerun CI to get the latest results

/morph build

mbed-ci · 2018-04-17T11:10:53Z

Build : SUCCESS

Build number : 1771
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6221/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

mbed-ci · 2018-04-17T18:07:59Z

Exporter Build : FAILURE

Build number : 1411
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/6221/

cmonr · 2018-04-17T20:59:11Z

java.io.EOFException

ಠ_ಠ

/morph export-build

mbed-ci · 2018-04-17T22:42:59Z

Test : SUCCESS

Build number : 1581
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/6221/1581

mbed-ci · 2018-04-18T03:34:44Z

Exporter Build : SUCCESS

Build number : 1419
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/6221/

cmonr added the needs: review label Feb 27, 2018

cmonr requested review from 0xc0170 and maclobdell February 27, 2018 05:07

0xc0170 requested review from mazimkhan and hanno-becker February 27, 2018 09:42

0xc0170 added tracking component: security labels Feb 27, 2018

0xc0170 previously approved these changes Feb 27, 2018

View reviewed changes

mazimkhan reviewed Feb 27, 2018

View reviewed changes

mazimkhan approved these changes Feb 27, 2018

View reviewed changes

ciarmcom added the mirrored label Feb 27, 2018

hanno-becker reviewed Mar 1, 2018

View reviewed changes

hanno-becker suggested changes Mar 1, 2018

View reviewed changes

mmahadevan108 dismissed 0xc0170’s stale review via c12c72d March 2, 2018 16:30

mmahadevan108 force-pushed the Add_RNG_LPC54XXX branch from 066430a to c12c72d Compare March 2, 2018 16:30

0xc0170 previously approved these changes Mar 5, 2018

View reviewed changes

cmonr added needs: work and removed needs: review labels Mar 6, 2018

mmahadevan108 dismissed 0xc0170’s stale review via 0eb06fe March 6, 2018 19:09

mmahadevan108 force-pushed the Add_RNG_LPC54XXX branch from c12c72d to 0eb06fe Compare March 6, 2018 19:09

mmahadevan108 force-pushed the Add_RNG_LPC54XXX branch from 4a48754 to 76c8a1b Compare March 30, 2018 19:22

hanno-becker approved these changes Apr 3, 2018

View reviewed changes

0xc0170 approved these changes Apr 3, 2018

View reviewed changes

mazimkhan approved these changes Apr 3, 2018

View reviewed changes

cmonr added needs: CI and removed needs: review labels Apr 3, 2018

0xc0170 added ready for merge release-version: 5.8.3 and removed needs: CI labels Apr 18, 2018

0xc0170 merged commit cba28cc into ARMmbed:master Apr 18, 2018

0xc0170 removed the ready for merge label Apr 18, 2018

TeroJaasko mentioned this pull request Apr 18, 2018

Macro definitions in mbed_app.json are not passed to IAR assembler #6670

Closed

ciarmcom added the closed_in_jira label Apr 18, 2018

mmahadevan108 deleted the Add_RNG_LPC54XXX branch April 19, 2018 14:57

LPC546XX: Add TRNG support #6221

LPC546XX: Add TRNG support #6221

Uh oh!

Conversation

mmahadevan108 commented Feb 26, 2018

Uh oh!

mmahadevan108 commented Feb 26, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ciarmcom commented Feb 27, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hanno-becker left a comment

Choose a reason for hiding this comment

Uh oh!

mmahadevan108 commented Mar 2, 2018

Uh oh!

cmonr commented Mar 6, 2018

Uh oh!

cmonr commented Mar 6, 2018

Uh oh!

mbed-ci commented Mar 6, 2018

Uh oh!

cmonr commented Mar 6, 2018

Uh oh!

mmahadevan108 commented Mar 6, 2018

Uh oh!

mmahadevan108 commented Mar 6, 2018

Uh oh!

hanno-becker left a comment

Choose a reason for hiding this comment

Uh oh!

hanno-becker commented Apr 3, 2018

Uh oh!

0xc0170 commented Apr 3, 2018

Uh oh!

mbed-ci commented Apr 3, 2018

Triggering tests

Uh oh!

mbed-ci commented Apr 3, 2018

Uh oh!

mbed-ci commented Apr 3, 2018

Uh oh!

cmonr commented Apr 3, 2018

Uh oh!

mbed-ci commented Apr 4, 2018

Uh oh!

mmahadevan108 commented Apr 9, 2018

Uh oh!

cmonr commented Apr 10, 2018

Uh oh!

0xc0170 commented Apr 17, 2018

Uh oh!

0xc0170 commented Apr 17, 2018

Uh oh!

mbed-ci commented Apr 17, 2018

Triggering tests

Uh oh!