Skip to content
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

add support for esp32c3 on-chip random generator to crypto/rand package. #2123

Draft
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

zdima
Copy link
Contributor

@zdima zdima commented Sep 17, 2021

The ESP32-C3 contains a true random number generator. This will allow utilize this for cryptographical operations, among other things.

Copy link
Member

@aykevl aykevl left a comment

Choose a reason for hiding this comment

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

I think the current implementation is not safe against predictable numbers. Specifically:

When there is noise coming from the SAR ADC, the random number generator is fed with a 2-bit entropy in one clock cycle of RTC20M_CLK (20 MHz), which is generated from an internal RC oscillator (see Chapter 6 Reset and Clock for details). Thus, it is advisable to read the RNG_DATA_REG register at a maximum rate of 1 MHz to obtain the maximum entropy.

For details on how this can be implemented, see the esp_random function:

https://github.com/espressif/esp-idf/blob/v4.3/components/esp32c3/hw_random.c

That said, adding a RNG like this seems like a good idea (if implemented correctly).

Comment on lines 49 to 50
r := esp.APB_CTRL.RND_DATA.Get()
a := (*[4]byte)(unsafe.Pointer(&r))[:]
Copy link
Member

Choose a reason for hiding this comment

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

This duplicates the r variable, which is a bit confusing. Can you rename the r variable (and possibly other single letter variables) to something more readable?

Comment on lines +33 to +34
// Enable SAR ADC
esp.SYSTEM.PERIP_CLK_EN0.SetBits(esp.SYSTEM_PERIP_CLK_EN0_APB_SARADC_CLK_EN)
Copy link
Member

Choose a reason for hiding this comment

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

It appears that enabling random number generation is a bit more complicated than this:

https://github.com/espressif/esp-idf/blob/v4.3/components/bootloader_support/src/bootloader_random_esp32c3.c

I'm worried that it is not truly random without this complete initialization.

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 move it to draft until have move details on initialization and the #2325 with #2333 are move so I would have better access to register's fields.

@aykevl aykevl mentioned this pull request Dec 7, 2021
@zdima zdima marked this pull request as draft December 8, 2021 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants