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.

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

LPC546XX: Add TRNG support #6221

Merged
merged 1 commit into from Apr 18, 2018

Conversation

@mmahadevan108
Contributor

mmahadevan108 commented Feb 26, 2018

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

This comment has been minimized.

Contributor

mmahadevan108 commented Feb 26, 2018

@cmonr cmonr added the needs: review label Feb 27, 2018

@cmonr cmonr requested review from 0xc0170 and maclobdell Feb 27, 2018

@0xc0170 0xc0170 requested review from mazimkhan and hanno-arm Feb 27, 2018

{
}

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

This comment has been minimized.

@mazimkhan

mazimkhan Feb 27, 2018

Contributor

Please suppress unused argument compiler warning for obj.

This comment has been minimized.

@hanno-arm

hanno-arm Mar 13, 2018

Contributor

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

@ciarmcom ciarmcom added the mirrored label Feb 27, 2018

@ciarmcom

This comment has been minimized.

Member

ciarmcom commented Feb 27, 2018

ARM Internal Ref: IOTSSL-2111

RNG_GetRandomData();
}
}

This comment has been minimized.

@hanno-arm

hanno-arm Mar 1, 2018

Contributor

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

This comment has been minimized.

@mmahadevan108

mmahadevan108 Mar 2, 2018

Contributor

I will zeroize data and update this commit

This comment has been minimized.

@hanno-arm

hanno-arm Mar 13, 2018

Contributor

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

This comment has been minimized.

@mazimkhan

mazimkhan Apr 3, 2018

Contributor

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

This comment has been minimized.

@hanno-arm

hanno-arm Apr 3, 2018

Contributor

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

output[idx++] = (data >> (i * 8)) & 0xFF;
}

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

This comment has been minimized.

@hanno-arm

hanno-arm Mar 1, 2018

Contributor

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

This comment has been minimized.

@mmahadevan108

mmahadevan108 Mar 2, 2018

Contributor

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.


This comment has been minimized.

@hanno-arm

hanno-arm Mar 21, 2018

Contributor

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?

This comment has been minimized.

@mmahadevan108

mmahadevan108 Mar 21, 2018

Contributor

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.

This comment has been minimized.

@hanno-arm

hanno-arm Mar 21, 2018

Contributor

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

@hanno-arm

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

@mmahadevan108 mmahadevan108 force-pushed the NXPmicro:Add_RNG_LPC54XXX branch from 066430a to c12c72d Mar 2, 2018

@mmahadevan108

This comment has been minimized.

Contributor

mmahadevan108 commented Mar 2, 2018

PR has been updated per review comments

@cmonr

This comment has been minimized.

Contributor

cmonr commented Mar 6, 2018

/morph build

@cmonr

This comment has been minimized.

Contributor

cmonr commented Mar 6, 2018

@hanno-arm Mind doing a re-review?

@mbed-ci

This comment has been minimized.

mbed-ci commented Mar 6, 2018

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

@cmonr

This comment has been minimized.

Contributor

cmonr commented Mar 6, 2018

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

This comment has been minimized.

Contributor

mmahadevan108 commented Mar 6, 2018

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

@mmahadevan108 mmahadevan108 force-pushed the NXPmicro:Add_RNG_LPC54XXX branch from c12c72d to 0eb06fe Mar 6, 2018

@mmahadevan108

This comment has been minimized.

Contributor

mmahadevan108 commented Mar 6, 2018

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

@hanno-arm

Looks good to me.

@hanno-arm

This comment has been minimized.

Contributor

hanno-arm commented Apr 3, 2018

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

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 3, 2018

/morph build

@0xc0170

0xc0170 approved these changes Apr 3, 2018

@mbed-ci

This comment has been minimized.

mbed-ci commented Apr 3, 2018

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

This comment has been minimized.

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

@mbed-ci

This comment has been minimized.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Apr 3, 2018

NRF52 Flash cache test issue...
/morph test

@mbed-ci

This comment has been minimized.

@mmahadevan108

This comment has been minimized.

Contributor

mmahadevan108 commented Apr 9, 2018

Anything needed for this PR?

@cmonr

This comment has been minimized.

Contributor

cmonr commented Apr 10, 2018

@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

This comment has been minimized.

Member

0xc0170 commented Apr 17, 2018

Green 🎊 PR head was fixed

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 17, 2018

I'll rerun CI to get the latest results

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Apr 17, 2018

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

This comment has been minimized.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Apr 17, 2018

java.io.EOFException

ಠ_ಠ

/morph export-build

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

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

11 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 9307 cycles
Details
travis-ci/littlefs Passed, code size is 10092B
Details
travis-ci/tools Local tools testing has passed
Details

@mmahadevan108 mmahadevan108 deleted the NXPmicro:Add_RNG_LPC54XXX branch Apr 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment