briansmith / ring

Safe, fast, small crypto using Rust
Other
3.68k stars 693 forks source link

Make less-safe work on espidf #1944

Closed coder0xff closed 3 months ago

coder0xff commented 6 months ago

The espidf platforms don't have an (always working) hardware RNG. https://github.com/briansmith/ring/issues/1787 plans to work around this by implementing a software CSPRNG to support such platforms. In the meantime, however, this PR enables the "less-safe-getrandom-custom-or-rdrand" feature to be used with espidf.

briansmith commented 6 months ago

I think we should change this to have its own feature, less-safe-getrandom-espidf. We should also add documentation for the feature about why it is "less safe": based on the espidf documentation, it isn't clear that the espidf PRNG is strong enough to consider a CSPRNG.

coder0xff commented 6 months ago

I think we should change this to have its own feature, less-safe-getrandom-espidf. We should also add documentation for the feature about why it is "less safe": based on the espidf documentation, it isn't clear that the espidf PRNG is strong enough to consider a CSPRNG.

I've updated the PR with your corrections

briansmith commented 6 months ago

Great, thanks! How can I install the tools needed to cross-compile for this target so I can test that this at least builds?

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 96.30%. Comparing base (ee1e217) to head (f9840d6).

:exclamation: Current head f9840d6 differs from pull request most recent head 67a2379. Consider uploading reports for the commit 67a2379 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1944 +/- ## ========================================== - Coverage 96.32% 96.30% -0.03% ========================================== Files 137 135 -2 Lines 20704 20663 -41 Branches 226 226 ========================================== - Hits 19943 19899 -44 - Misses 728 730 +2 - Partials 33 34 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

briansmith commented 5 months ago

@coder0xff Could you please rebase this, cargo fmt to fix the CI error, and then squash the commits into one? Then I can merge this. Thanks!

coder0xff commented 5 months ago

I'll make some time this weekend. Thanks for your patience!

On Wed, Feb 28, 2024, 7:30 PM Brian Smith @.***> wrote:

@coder0xff https://github.com/coder0xff Could you please rebase this, cargo fmt to fix the CI error, and then squash the commits into one? Then I can merge this. Thanks!

— Reply to this email directly, view it on GitHub https://github.com/briansmith/ring/pull/1944#issuecomment-1970331976, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARIHPM4JZLSPB7L4XNJG2DYV7Y4LAVCNFSM6AAAAABDD3KPFKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZQGMZTCOJXGY . You are receiving this because you were mentioned.Message ID: @.***>

coder0xff commented 5 months ago

@coder0xff Could you please rebase this, cargo fmt to fix the CI error, and then squash the commits into one? Then I can merge this. Thanks!

Rebased and formatted. Does GitHub have an option to squash on merge? I couldn't find it.

Great, thanks! How can I install the tools needed to cross-compile for this target so I can test that this at least builds?

I used the guide at https://docs.esp-rs.org/book/installation/riscv-and-xtensa.html

I would also appreciate your help in getting this target building (mk/cargo.sh test --norun, at least) in GitHub Actions, if you can.

I don't have any experience with GitHub actions. I do have a simple test project that I've been using to build for the esp32s3 target.

Directory Structure

.
├── build.rs
├── .cargo
│   └── config.toml
├── Cargo.toml
├── .gitignore
├── rust-toolchain.toml
├── sdkconfig.defaults
└── src
    └── main.rs

2 directories, 7 files

Contents of: build.rs

fn main() {
    embuild::espidf::sysenv::output();
}

Contents of: .cargo/config.toml

[build]
target = "xtensa-esp32s3-espidf"

[target.xtensa-esp32s3-espidf]
linker = "ldproxy"
# runner = "espflash --monitor" # Select this runner for espflash v1.x.x
runner = "espflash flash --monitor" # Select this runner for espflash v2.x.x
rustflags = [ "--cfg",  "espidf_time64"] # Extending time_t for ESP IDF 5: https://github.com/esp-rs/rust/issues/110

[unstable]
build-std = ["std", "panic_abort"]

[env]
MCU="esp32s3"
# Note: this variable is not used by the pio builder (`cargo build --features pio`)
ESP_IDF_VERSION = "v5.1.2"

Contents of: Cargo.toml

[package]
name = "esp32s3-ring-sandbox"
version = "0.1.0"
authors = ["coder0xff <coder0xff@gmail.com>"]
edition = "2021"
resolver = "2"
rust-version = "1.71"

[profile.release]
opt-level = "s"

[profile.dev]
debug = true    # Symbols are nice and they don't increase the size on Flash
opt-level = "z"

[features]
default = ["std", "embassy", "esp-idf-svc/native"]

pio = ["esp-idf-svc/pio"]
std = ["alloc", "esp-idf-svc/binstart", "esp-idf-svc/std"]
alloc = ["esp-idf-svc/alloc"]
nightly = ["esp-idf-svc/nightly"]
experimental = ["esp-idf-svc/experimental"]
embassy = ["esp-idf-svc/embassy-sync", "esp-idf-svc/critical-section", "esp-idf-svc/embassy-time-driver"]

[dependencies]
log = { version = "0.4", default-features = false }
esp-idf-svc = { version = "0.48", default-features = false }
ring = { features = ["alloc", "less-safe-getrandom-espidf"] }

[patch.crates-io]
ring = { path = "../ring" }

[build-dependencies]
embuild = "0.31.3"

Contents of: .gitignore

/.vscode
/.embuild
/target
/Cargo.lock

Contents of: rust-toolchain.toml

[toolchain]
channel = "esp"

Contents of: sdkconfig.defaults

# Rust often needs a bit of an extra main task stack size compared to C (the default is 3K)
CONFIG_ESP_MAIN_TASK_STACK_SIZE=8000

# Use this to set FreeRTOS kernel tick frequency to 1000 Hz (100 Hz by default).
# This allows to use 1 ms granuality for thread sleeps (10 ms by default).
#CONFIG_FREERTOS_HZ=1000

# Workaround for https://github.com/espressif/esp-idf/issues/7631
#CONFIG_MBEDTLS_CERTIFICATE_BUNDLE=n
#CONFIG_MBEDTLS_CERTIFICATE_BUNDLE_DEFAULT_FULL=n

Contents of: src/main.rs

fn main() {
    // It is necessary to call this function once. Otherwise some patches to the runtime
    // implemented by esp-idf-sys might not link properly. See https://github.com/esp-rs/esp-idf-template/issues/71
    esp_idf_svc::sys::link_patches();

    // Bind the log crate to the ESP Logging facilities
    esp_idf_svc::log::EspLogger::initialize_default();

    log::info!("Hello, world!");
}
coder0xff commented 4 months ago

@briansmith Does everything look good?

coder0xff commented 4 months ago

Hi. Would you please trigger the workflows?