Skip to content

no_std is broken, doesn't play with sgx #31

Description

@brenzi

I'm trying to use this lib within sgx using

https://github.com/baidu/rust-sgx-sdk

It seems that #[no_std] is broken as std gets pulled in in places (like context.rs).
Also alloc is not imported

@elichai commented the following: apache/teaclave-sgx-sdk#93 (comment)

I'll need schnorrkel to support sgx for substraTEE

Activity

  1. brenzi commented on Apr 29, 2019

    @brenzi
    Author

    When I build this crate with

    cargo build --release --no-default-features --features "u64_backend alloc"
    

    errors explode.

    the low hanging fruit:

    diff --git a/src/context.rs b/src/context.rs
    index 44d9ab3..ec56bc1 100644
    --- a/src/context.rs
    +++ b/src/context.rs
    @@ -9,7 +9,7 @@
     
     //! ### Schnorr signature contexts and configuration, adaptable to most Schnorr signature schemes.
     
    -use std::cell::RefCell;
    +use core::cell::RefCell;
     
     use rand::prelude::*;  // {RngCore,thread_rng};
    
    

    But that doesn't do the trick by far

  2. burdges commented on Apr 30, 2019

    @burdges
    Collaborator

    I've pushed a fix that passes your build command. It suffices? Thanks!

    I'll publish a new version soon, but we've several compatibility breaks already in master and..

    I'd love to resolve #9 along with the other compatibility breaks, but so far nothing conclusive arose. I'll make a decision there asap. :)

  3. burdges commented on Apr 30, 2019

    @burdges
    Collaborator

    Appears ed25519-dalek lacks this extern crate alloc; that I needed, so not sure if I fixed those errors correctly or not.

  4. brenzi commented on Apr 30, 2019

    @brenzi
    Author

    Cheers for your attempt to fix this issue.

    I now get

    error[E0658]: use of unstable library feature 'alloc': this library is unlikely to be stabilized in its current form or name (see issue #27783)
       --> /home/abrenzikofer/.cargo/git/checkouts/schnorrkel-0d699dd19a5d19bc/bc1cf8e/src/lib.rs:262:1
        |
    262 | extern crate alloc;
        | ^^^^^^^^^^^^^^^^^^^
        |
        = help: add #![feature(alloc)] to the crate attributes to enable
    
    error: aborting due to previous error
    

    So I must guess this issue isn't fixed just yet - not only because of upstream crate ed25519-dalek

  5. brenzi commented on Apr 30, 2019

    @brenzi
    Author

    edit: sorry, this issue should be fixed by updating rust nightly

    There are further issues when building without alloc feature:

    > cargo build --release --no-default-features --features "u64_backend"
    error[E0432]: unresolved import `crate::sign::verify_batch`
       --> src/lib.rs:308:50
        |
    308 | pub use crate::sign::{Signature,SIGNATURE_LENGTH,verify_batch};
        |                                                  ^^^^^^^^^^^^ no `verify_batch` in `sign`
    
    error[E0433]: failed to resolve: use of undeclared type or module `BTreeMap`
       --> src/musig.rs:373:22
        |
    373 |         let mut Rs = BTreeMap::new();
        |                      ^^^^^^^^ use of undeclared type or module `BTreeMap`
    
    ...
    
  6. brenzi commented on Apr 30, 2019

    @brenzi
    Author

    Doesn't build with rustsgx-sdk. Compiler still complains:

    error: duplicate lang item in crate `std`: `f32_runtime`.
      |
      = note: first defined in crate `sgx_tstd`.
    
    error: duplicate lang item in crate `std`: `f64_runtime`.
      |
      = note: first defined in crate `sgx_tstd`.
    
    error: duplicate lang item in crate `std`: `panic_impl`.
      |
      = note: first defined in crate `sgx_tstd`.
    
    error: duplicate lang item in crate `std`: `oom`.
      |
      = note: first defined in crate `sgx_trts`.
    

    It looks like one or more of your dependencies don't support no_std: merlin ?

  7. burdges commented on Apr 30, 2019

    @burdges
    Collaborator

    If I understand, the #![feature(alloc)] error comes form your rust being outdated, yes?

    I've fixed the issues I understood and pushed the changes, in part by omitting musig without alloc or std. I even made cargo test .. work with various combinations, which eve dalek does not do.

    I still do not understand why ed25519-dalek does not require extern crate alloc; like schnorrkel seemingly does. An edition setting in Cargo.toml maybe?

    I also do not understand the no_std: merlin issue. I know merlin's tests use strobe-rs, which requires Vec, but outside tests I'm unsure about the problem. Can you pin down the actual problem?

  8. burdges commented on Apr 30, 2019

    @burdges
    Collaborator

    There is a serious dependency flaw caused by the rand crate being chopped up into too many poinless subcrates: rust-random/rand#738 Is that the problem? I'm afraid merlin does create an Rng, but some workaround exist I think.

  9. burdges commented on May 1, 2019

    @burdges
    Collaborator

    Or maybe my Cargo.toml does not pass through some feature flags correctly?

  10. brenzi commented on May 1, 2019

    @brenzi
    Author

    right now we're trying to use ed25519 signatures with our node as a workaround. I'll revert when I find time for further debugging

  11. burdges commented on May 1, 2019

    @burdges
    Collaborator

    I've found the problem maybe.. merlin lacks any support for no_std, like you said.

    I believe https://github.com/dalek-cryptography/merlin/blob/master/src/strobe.rs#L4 might be the only problematic line in merlin. Can you tell if using this branch of merlin works? It merely uses use core::ops::{Deref, DerefMut}; and passes through the std feature.

  12. burdges commented on May 1, 2019

    @burdges
    Collaborator

    It appears my second commit there to Cargo.toml breaks their CI, but the first seemingly passed fine. It's maybe not required to pass through the std dependency unless you really use functionality only present with std I guess?

  13. brenzi commented on May 1, 2019

    @brenzi
    Author

    I just tried to use your merlin branch within sgx. no success. Same duplicate lang item error as above

    toml:

    merlin = { git ="https://github.com/burdges/merlin.git", branch="no_std", default-features = false}
    

    code:

    extern crate merlin;
    use merlin::{Transcript};
    
  14. 18 remaining items

  15. burdges commented on Aug 7, 2019

    @burdges
    Collaborator

    Any idea how rand::thread_rng exists when substrate compiles schnorrkel no_std? Or maybe we're not no_std? If not, then I'm not worried here.

  16. chesterliliang commented on Aug 7, 2019

    @chesterliliang

    Rand use prelude to re-export and I see some discussion about no_std and crate:prelude in std-using paths work just fine in 2018 edition #![no_std] and Add extern crate items to extern prelude. Not sure if it is related to our problem.

  17. burdges commented on Aug 7, 2019

    @burdges
    Collaborator

    We no longer depend upon rand, only rand_core, unless you pass the rand feature.

  18. burdges commented on Aug 7, 2019

    @burdges
    Collaborator

    I read https://github.com/rust-random/rand/blob/master/rand_os/Cargo.toml#L25 as activating std for rand_core whenever rand_os gets used, likely causing #31 (comment)

  19. burdges commented on Aug 11, 2019

    @burdges
    Collaborator

    I think rand_os now avoids activating std in rand_core as of rust-random/rand#859 so that's fixed.

    We cannot expect merlin to update to rand_core 0.6 anytime soon because adding RngCore::from_entropy proved to be the breaking change that broke the camels back, ala dalek-cryptography/curve25519-dalek#262 dalek-cryptography/curve25519-dalek#263 etc.

    As I said in dalek-cryptography/merlin#47 (comment) we can either

    Any thoughts on substrate's upgrade path for rand 0.7 @jacogr ? No rush right?

  20. burdges commented on Aug 11, 2019

    @burdges
    Collaborator

    I stopped the *_backend features from activating ed25519 in 33083a9 which likely contributed here, including to both @brenzi original problem and @chesterliliang more recent issue.

    I explained how a wrapper shim could support merlin in rust-random/rand#865 (comment) but it seemingly either needs an extra crate or else nightly for rust-lang/cargo#4953

  21. burdges commented on Aug 13, 2019

    @burdges
    Collaborator

    I've a "schizomerlin" branch https://github.com/w3f/schnorrkel/tree/schizomerlin that avoids invoking the std feature of rand_core, so test that one too.

    I doubt cargo feature work on stable, so if we need schizomerlin then we might factor RngCore5As4 out into a separate crate:
    8150ef6

  22. brenzi commented on Sep 21, 2019

    @brenzi
    Author

    Re-visiting this after a while of absence.

    Neither schizomerlin branch nor master work for me.

  23. brenzi commented on Sep 21, 2019

    @brenzi
    Author

    as schnorrkel depends on ed25519-dalek, the following might be an issue:
    dalek-cryptography/ed25519-dalek#95

    but strange enough, my minimal example actually works fine with no_std:
    https://github.com/scs/test-no-std/tree/schnorrkel
    If you build this with

    cargo build --no-default-features
    

    it actually works. But if I use schnorrkel within my real crate, it doesn't:

    Reproduce:

    1. get https://github.com/scs/substraTEE-worker/tree/schnorrkel-no-std-debug
    2. cd enclave
    3. cargo build --no-default-features
      should fail.

    but if you remove this use statement: https://github.com/scs/substraTEE-worker/blob/57b34bfda28dcac07749e86944d1ee17b639fbc8/primitives/src/sr25519.rs#L26

    it will build

  24. burdges commented on Sep 22, 2019

    @burdges
    Collaborator

    I think master should avoid ed25519-dalek thanks to 33083a9 although I never published that version. We should try both that and the schizomerlin commit 8150ef6 together.

    I've now forgotten if the schizomerlin commit should suffice. If rand_os activates std then we must avoid that too. You might avoid rand_os when you invoke without default features, but another options is upgrading merlin's rand_core version.

  25. brenzi commented on Sep 22, 2019

    @brenzi
    Author

    Schnorrkel actually works if patching ed25519-dalek to my fork:

    [patch.crates-io]
    ed25519-dalek = { git = "https://github.com/scs/ed25519-dalek.git", branch = "no_std_sgx"}
    
  26. burdges commented on Sep 22, 2019

    @burdges
    Collaborator

    I'd expect merlin might break rand_core without rand_core being upgrade, or at least using the schizomerlin commit 8150ef6, but maybe not.

  27. burdges commented on Nov 1, 2019

    @burdges
    Collaborator

    Is there no trouble from the failure crate here? We could drop support for it easily I think.

  28. burdges commented on Dec 17, 2019

    @burdges
    Collaborator

    Is this fixed? I'll publish some new version over Xmas hopefully, well more like January, so if not then maybe then.

  29. brenzi commented on Dec 18, 2019

    @brenzi
    Author

    We haven't tried with the unpatched ed25519-dalek. If it's fixed there, its fixed for us

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions