Provide CPU RNG Support for 64 Bit Arm CPUs #15361

otoledan · 2021-05-19T22:46:04Z

Provide support in a new engine and provider for aarch64 random number generation. The engine and provider make use of the RNDR and RNDRRS Arm assembly instructions.

Checklist

documentation is added or updated
tests are added or updated

slontis · 2021-05-19T23:03:28Z

Engines are deprecated.
Adding new engine code into master may not be a good path forward.

crypto/armcap.c

paulidale · 2021-05-19T23:11:34Z

crypto/armcap.c

+size_t OPENSSL_rndr_asm(unsigned char *buf, size_t len);
+size_t OPENSSL_rndrrs_asm(unsigned char *buf, size_t len);
+
+size_t OPENSSL_rndr_wrapper(size_t (*func)(unsigned char *, size_t), unsigned char *buf, size_t len)


I don't think that any of these functions should be public, so openssl_rndr_wrapper(...)

marked as static. fixed

_armv8_rng_probe, OPENSSL_rndr_asm, OPENSSL_rndrrs_asm(unsigned char *buf, size_t len) were made global again since the assembly function definition was not being picked up by the compiler with strict warnings enabled.

static size_t OPENSSL_rndr_wrapper has remained static since all contents definitions and references exist within this file.

crypto/armcap.c

crypto/engine/build.info

crypto/info.c

crypto/init.c

providers/implementations/rands/seeding/rand_cpu_arm64.c

providers/implementations/rands/seeding/rand_cpu_x86.c

util/missingmacro.txt

paulidale · 2021-05-20T00:30:13Z

Adding an engine to 1.1.1 would be considered a new feature and I doubt it would be accepted. It could still be distributed separately.

For 3.0, an engine is undesirable. The provider implementation is much cleaner too.

ozbenh · 2021-05-20T00:37:04Z

Adding an engine to 1.1.1 would be considered a new feature and I doubt it would be accepted. It could still be distributed separately.

For 3.0, an engine is undesirable. The provider implementation is much cleaner too.

Oops, sorry, I accidentally deleted my message, as I was going to reply to your other comment above.

I agree for 3.0 the provider is the way to go. But it will take years for 3.0 to be sufficiently widespread and we would like this to be backport'able to existing enterprise distros to exploit the HW rng. But we can indeed distribute it separately if necessary.

paulidale · 2021-05-20T01:02:22Z

The way to go would be to submit the provider code to master and raise a separate PR for 1.1.1 for the engine.

Perhaps start with an issue for the new feature in 1.1.1 and the OTC can decide to include or not.

ozbenh · 2021-05-20T01:10:25Z

The way to go would be to submit the provider code to master and raise a separate PR for 1.1.1 for the engine.

Perhaps start with an issue for the new feature in 1.1.1 and the OTC can decide to include or not.

Yup, sounds good. Thanks !

mattcaswell · 2021-05-20T07:20:56Z

Perhaps start with an issue for the new feature in 1.1.1 and the OTC can decide to include or not.

I very much doubt that OTC will approve this.

Unfortunately I think this is probably too late for inclusion in 3.0 too.

Configure

ozbenh · 2021-05-20T21:44:10Z

Perhaps start with an issue for the new feature in 1.1.1 and the OTC can decide to include or not.

I very much doubt that OTC will approve this.

Unfortunately I think this is probably too late for inclusion in 3.0 too.

I suppose it depends if you consider it a "new feature" or just ARM support for the existing "rdcpu" one :-)

crypto/armcap.c

test/rdcpu_sanitytest.c

paulidale

It would be easier for the reviewers if you used fixup commits (git --fixup ).
They get squashed as part of the merge process and they help us see what has changed.

otoledan · 2021-05-21T05:57:22Z

They get squashed as part of the merge process and they help us see what has changed.

will keep that in mind for future changes.

otoledan · 2021-05-28T16:51:23Z

Is there anything else required on my end as the developer in terms of merging this change?

I see there is now a conflict for CHANGES.md, should I wait until the status is set to approval:done or rebase periodically?

paulidale · 2021-05-28T23:45:23Z

A second reviewer's approval is required.

slontis · 2021-05-29T04:58:51Z

crypto/arm64cpuid.pl

+	b.eq	.${rdop}_done
+	mov	x3,xzr
+	mrs	x3,$rand_reg
+	b.eq	.${rdop}_done


Isnt zero a possible valid value here?

The documentation for
https://developer.arm.com/documentation/ddi0595/2021-03/AArch64-Registers/RNDR--Random-Number?lang=en

says this

If the hardware returns a genuine random number, PSTATE.NZCV is set to 0b0000.
If the instruction cannot return a genuine random number in a reasonable period of time, PSTATE.NZCV is set to 0b0100 and the data value returned is 0.

Should it be doing
mrs x4, nzcv

Yes, a zero value is possible in the x3 register, though only in a failure situation.

The mrs x3,xzr is used to clear out the register used for rng (which is not strictly necessary and used to ensure previous values were not in the register) prior to running the command for rng (mrs x3,$rand_reg). The check for the failure is done by b.eq .${rdop}_done which checks if the Zero flag is set. As quoted from the documentation "If the instruction cannot return a genuine random number in a reasonable period of time, PSTATE.NZCV is set to 0b0100". So the check for the Zero flag would be enough to validate that rng has occurred.

From my understanding of the documentation there is no need to run mrs x4, nzcv, prior to rng.

crypto/armcap.c

providers/implementations/rands/seeding/rand_cpu_arm64.c

crypto/arm64cpuid.pl

crypto/armcap.c

slontis · 2021-05-31T00:15:15Z

OTC: Should this be added to 3.0?

openssl-machine · 2021-12-16T11:00:21Z

This pull request is ready to merge

Include aarch64 asm instructions for random number generation using the RNDR and RNDRRS instructions. Provide detection functions for RNDR and RNDRRS getauxval. Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #15361)

Create new provider for RNDRRS. Modify support for rand_cpu to default to RDRAND/RDSEED on x86 and RNDRRS on aarch64. Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #15361)

Add test cases for RNDR and RNDRRS. Combine tests for RDRAND and RNDR to share common logic. Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #15361)

Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #15361)

t8m · 2021-12-16T11:41:39Z

Merged. Thank you for your contribution!

Include aarch64 asm instructions for random number generation using the RNDR and RNDRRS instructions. Provide detection functions for RNDR and RNDRRS getauxval. Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#15361) (cherry picked from commit efa1f22)

Create new provider for RNDRRS. Modify support for rand_cpu to default to RDRAND/RDSEED on x86 and RNDRRS on aarch64. Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#15361) (cherry picked from commit eb28fda)

Add test cases for RNDR and RNDRRS. Combine tests for RDRAND and RNDR to share common logic. Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#15361) (cherry picked from commit 1f8ce0c)

Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#15361) (cherry picked from commit e8b597f)

Include aarch64 asm instructions for random number generation using the RNDR and RNDRRS instructions. Provide detection functions for RNDR and RNDRRS getauxval. Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #15361) (cherry picked from commit efa1f22)

Create new provider for RNDRRS. Modify support for rand_cpu to default to RDRAND/RDSEED on x86 and RNDRRS on aarch64. Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #15361) (cherry picked from commit eb28fda)

Add test cases for RNDR and RNDRRS. Combine tests for RDRAND and RNDR to share common logic. Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #15361) (cherry picked from commit 1f8ce0c)

Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #15361) (cherry picked from commit e8b597f)

…on Linux aee5404 Add support for RNDR/RNDRRS for aarch64 on Linux (John Moffett) Pull request description: This checks whether the ARMv8.5-A optional TRNG extensions [RNDR](https://developer.arm.com/documentation/ddi0601/2022-12/AArch64-Registers/RNDR--Random-Number) and [RNDRRS](https://developer.arm.com/documentation/ddi0601/2022-12/AArch64-Registers/RNDRRS--Reseeded-Random-Number) are available and, if they are, uses them for random entropy purposes. They are nearly functionally identical to the x86 RDRAND/RDSEED extensions and are used in a similar manner. Currently, there [appears to be](https://marcin.juszkiewicz.com.pl/download/tables/arm-socs.html) only one actual hardware implementation -- the Amazon Graviton 3. (See the `rnd` column in the link.) However, future hardware implementations may become available. It's not possible to directly query for the capability in userspace, but the Linux kernel [added support](torvalds/linux@1a50ec0) for querying the extension via `getauxval` in version 5.6 (in 2020), so this is limited to Linux-only for now. Reviewers may want to launch any of the `c7g` instances from AWS to test the Graviton 3 hardware. Alternatively, QEMU emulates these opcodes for `aarch64` with CPU setting `max`. Output from Graviton 3 hardware: ``` ubuntu@ip:~/bitcoin$ src/bitcoind -regtest 2023-01-06T20:01:48Z Bitcoin Core version v24.99.0-3670266ce89a (release build) 2023-01-06T20:01:48Z Using the 'arm_shani(1way,2way)' SHA256 implementation 2023-01-06T20:01:48Z Using RNDR and RNDRRS as additional entropy sources 2023-01-06T20:01:48Z Default data directory /home/ubuntu/.bitcoin ``` Graviton 2 (doesn't support extensions): ``` ubuntu@ip:~/bitcoin$ src/bitcoind -regtest 2023-01-06T20:05:04Z Bitcoin Core version v24.99.0-3670266ce89a (release build) 2023-01-06T20:05:04Z Using the 'arm_shani(1way,2way)' SHA256 implementation 2023-01-06T20:05:04Z Default data directory /home/ubuntu/.bitcoin ``` This partially closes #26796. As noted in that issue, OpenSSL [added support](openssl/openssl#15361) for these extensions a little over a year ago. ACKs for top commit: achow101: ACK aee5404 laanwj: Tested ACK aee5404 Tree-SHA512: 1c1eb345d6690f5307a87e9bac8f06a0d1fdc7ca35db38fa22192510a44289a03252e4677dc7cbf731a27e6e3a9a4e42b6eb4149fe063bc1c905eb2536cdb1d3

aee5404 Add support for RNDR/RNDRRS for aarch64 on Linux (John Moffett) Pull request description: This checks whether the ARMv8.5-A optional TRNG extensions [RNDR](https://developer.arm.com/documentation/ddi0601/2022-12/AArch64-Registers/RNDR--Random-Number) and [RNDRRS](https://developer.arm.com/documentation/ddi0601/2022-12/AArch64-Registers/RNDRRS--Reseeded-Random-Number) are available and, if they are, uses them for random entropy purposes. They are nearly functionally identical to the x86 RDRAND/RDSEED extensions and are used in a similar manner. Currently, there [appears to be](https://marcin.juszkiewicz.com.pl/download/tables/arm-socs.html) only one actual hardware implementation -- the Amazon Graviton 3. (See the `rnd` column in the link.) However, future hardware implementations may become available. It's not possible to directly query for the capability in userspace, but the Linux kernel [added support](torvalds/linux@1a50ec0) for querying the extension via `getauxval` in version 5.6 (in 2020), so this is limited to Linux-only for now. Reviewers may want to launch any of the `c7g` instances from AWS to test the Graviton 3 hardware. Alternatively, QEMU emulates these opcodes for `aarch64` with CPU setting `max`. Output from Graviton 3 hardware: ``` ubuntu@ip:~/bitcoin$ src/bitcoind -regtest 2023-01-06T20:01:48Z Bitcoin Core version v24.99.0-3670266ce89a (release build) 2023-01-06T20:01:48Z Using the 'arm_shani(1way,2way)' SHA256 implementation 2023-01-06T20:01:48Z Using RNDR and RNDRRS as additional entropy sources 2023-01-06T20:01:48Z Default data directory /home/ubuntu/.bitcoin ``` Graviton 2 (doesn't support extensions): ``` ubuntu@ip:~/bitcoin$ src/bitcoind -regtest 2023-01-06T20:05:04Z Bitcoin Core version v24.99.0-3670266ce89a (release build) 2023-01-06T20:05:04Z Using the 'arm_shani(1way,2way)' SHA256 implementation 2023-01-06T20:05:04Z Default data directory /home/ubuntu/.bitcoin ``` This partially closes bitcoin#26796. As noted in that issue, OpenSSL [added support](openssl/openssl#15361) for these extensions a little over a year ago. ACKs for top commit: achow101: ACK aee5404 laanwj: Tested ACK aee5404 Tree-SHA512: 1c1eb345d6690f5307a87e9bac8f06a0d1fdc7ca35db38fa22192510a44289a03252e4677dc7cbf731a27e6e3a9a4e42b6eb4149fe063bc1c905eb2536cdb1d3

paulidale requested changes May 19, 2021

View reviewed changes

paulidale reviewed May 20, 2021

View reviewed changes

Configure Outdated Show resolved Hide resolved

otoledan requested a review from paulidale May 20, 2021 23:24

paulidale requested changes May 21, 2021

View reviewed changes

crypto/armcap.c Outdated Show resolved Hide resolved

test/rdcpu_sanitytest.c Outdated Show resolved Hide resolved

test/rdcpu_sanitytest.c Show resolved Hide resolved

test/rdcpu_sanitytest.c Show resolved Hide resolved

otoledan requested a review from paulidale May 21, 2021 05:25

paulidale approved these changes May 21, 2021

View reviewed changes

paulidale added approval: review pending This pull request needs review by a committer branch: master Merge to master branch labels May 21, 2021

paulidale self-assigned this May 21, 2021

otoledan requested a review from slontis May 29, 2021 01:07