Skip to content

ESP-IDF: Enable unconditionally#583

Merged
newpavlov merged 1 commit into
masterfrom
espidf
Jan 13, 2025
Merged

ESP-IDF: Enable unconditionally#583
newpavlov merged 1 commit into
masterfrom
espidf

Conversation

@josephlr

@josephlr josephlr commented Jan 10, 2025

Copy link
Copy Markdown
Member

After reviewing the updated ESP-IDF random documentation:

Given that ESP-IDF provides binding of getrandom/getentropy to esp_fill_random, it's clear they intend for
cryptographic libraries (such as their fork of WolfSSL) to use them. Furthermore, it seems like the only time the Hardware RNG would lack sufficient entropy is during early boot, so I added a section to our "Early boot" documentation noting this issue.

Also note that Rust's standard library unconditionally supports ESP-IDF for both hash seed generation and generating cryptographic random bytes, see https://github.com/rust-lang/rust/blob/62bf38fa600f4beb878d61c537837729d4ee689e/library/std/src/sys/random/espidf.rs#L7 I don't think there's a good reason to deviate from the standard library here.

@josephlr josephlr added this to the v0.3 milestone Jan 10, 2025
@josephlr josephlr requested a review from newpavlov January 10, 2025 05:39
After reviewing the updated ESP-IDF random documentation:
  - https://docs.espressif.com/projects/esp-idf/en/stable/esp32/api-reference/system/random.html
  - https://www.espressif.com/sites/default/files/documentation/esp32_technical_reference_manual_en.pdf#rng

I think we should enable this backend by default.

Given that ESP-IDF provides such bindings, it's clear they intend for
cryptographic libraries (such as their fork of WolfSSL) to use them.
Furthurmore, it seems like the only time the Hardware RNG wouldn't be
seeded with sufficient entropy is during early boot, so I added a
section to our "Early boot" documentation noting this issue.

Also note that Rust's standard library unconditonally supports ESP-IDF
for both hash seed generation and generating cryptographic random bytes,
see https://github.com/rust-lang/rust/blob/62bf38fa600f4beb878d61c537837729d4ee689e/library/std/src/sys/random/espidf.rs#L7

Signed-off-by: Joe Richey <joerichey@google.com>
@josephlr josephlr changed the title ESP-IDF: Enable by default and use getrandom() ESP-IDF: Enable unconditionally Jan 10, 2025
@josephlr

Copy link
Copy Markdown
Member Author

@newpavlov the first version of this PR used the getrandom bindings provided by ESP-IDF, but using those would require binding to ESP-IDF's errno, which seems unnecessarily complicated, so I just kept the implementation as-is.

@newpavlov

newpavlov commented Jan 10, 2025

Copy link
Copy Markdown
Member

it's clear they intend for cryptographic libraries (such as their fork of WolfSSL) to use them

It's not so clear for me, considering the following remark:

If none of the above conditions are true, the output of the RNG should be considered as pseudo-random only.

In other words, without a proper hardware setup users can get potentially predictable entropy.

Also note that Rust's standard library unconditionally supports ESP-IDF for both hash seed generation and generating cryptographic random bytes

IIRC std uses entropy sources only for seeding hash maps (well, technically it also uses it for things like thread ID generation on Windows, but it's not relevant), so it's less strict about entropy quality (e.g. it tries to use GRND_INSECURE on Linux). Meanwhile in our case we strive to always provide good quality entropy even during early boot.

I would be fine to use esp_fill_random for "insecure" functions (see #546), but I am hesitant to use them by default for "secure" functions without some kind of a check function, so to be conservative I think it makes sense to ask users to make a conscious decision.

See #397 for the earlier discussion. I also raised these concerns in espressif/esp-idf#8725, but, unfortunately, there are no updates on it.

cc @briansmith

@newpavlov newpavlov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On second thought, I guess we can simply say that it's a potential deficiency of the target and we did our best given the provided platform capabilities. In the issue they wrote the following:

Note that esp_random_fill() can be used to seed a software CSPRNG, which is done in mbedtls, as far as I know. I.e., esp_random_fill() is similar to the libc getentropy() function.

While I don't think that a getentropy()-like function should ever return potentially predictable entropy, it may be fine to defer to the intention of ESP IDF maintainers.

@newpavlov newpavlov merged commit 2648e60 into master Jan 13, 2025
@newpavlov newpavlov deleted the espidf branch January 13, 2025 14:43
takumi-earth pushed a commit to earthlings-dev/getrandom that referenced this pull request Jan 27, 2026
After reviewing the updated ESP-IDF random documentation:
-
https://docs.espressif.com/projects/esp-idf/en/stable/esp32/api-reference/system/random.html
-
https://www.espressif.com/sites/default/files/documentation/esp32_technical_reference_manual_en.pdf#rng

Given that ESP-IDF provides binding of `getrandom`/`getentropy` to
`esp_fill_random`, it's clear they intend for
cryptographic libraries (such as their fork of WolfSSL) to use them.
Furthermore, it seems like the only time the Hardware RNG would lack
sufficient entropy is during early boot, so I added a section to our
"Early boot" documentation noting this issue.

Also note that Rust's standard library unconditionally supports ESP-IDF
for both hash seed generation and generating cryptographic random bytes,
see
https://github.com/rust-lang/rust/blob/62bf38fa600f4beb878d61c537837729d4ee689e/library/std/src/sys/random/espidf.rs#L7
I don't think there's a good reason to deviate from the standard library
here.
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