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

Arithmetic feature requires atomics #185

Closed
alistair23 opened this issue Sep 14, 2020 · 13 comments
Closed

Arithmetic feature requires atomics #185

alistair23 opened this issue Sep 14, 2020 · 13 comments

Comments

@alistair23
Copy link

Currently using the arithmetic feature requires atomics. Compilling on a platform without atomics (such as RV32IMC) results in this error:

error[E0432]: unresolved imports `core::sync::atomic::AtomicBool`, `core::sync::atomic::AtomicI16`, `core::sync::atomic::AtomicI32`, `core::sync::atomic::AtomicI8`, `core::sync::atomic::AtomicIsize`, `core::sync::atomic::AtomicPtr`, `core::sync::atomic::AtomicU16`, `core::sync::atomic::AtomicU32`, `core::sync::atomic::AtomicU8`, `core::sync::atomic::AtomicUsize`
  --> /home/alistair/.cargo/registry/src/github.com-1ecc6299db9ec823/radium-0.3.0/src/lib.rs:29:11
   |
29 |     self, AtomicBool, AtomicI16, AtomicI32, AtomicI8, AtomicIsize, AtomicPtr, AtomicU16, AtomicU32,
   |           ^^^^^^^^^^  ^^^^^^^^^  ^^^^^^^^^  ^^^^^^^^  ^^^^^^^^^^^  ^^^^^^^^^  ^^^^^^^^^  ^^^^^^^^^ no `AtomicU32` in `sync::atomic`
   |           |           |          |          |         |            |          |
   |           |           |          |          |         |            |          no `AtomicU16` in `sync::atomic`
   |           |           |          |          |         |            no `AtomicPtr` in `sync::atomic`
   |           |           |          |          |         no `AtomicIsize` in `sync::atomic`
   |           |           |          |          no `AtomicI8` in `sync::atomic`
   |           |           |          no `AtomicI32` in `sync::atomic`
   |           |           no `AtomicI16` in `sync::atomic`
   |           no `AtomicBool` in `sync::atomic`
30 |     AtomicU8, AtomicUsize, Ordering,
   |     ^^^^^^^^  ^^^^^^^^^^^ no `AtomicUsize` in `sync::atomic`
   |     |
   |     no `AtomicU8` in `sync::atomic`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0432`.
error: could not compile `radium`.

To learn more, run the command again with --verbose.
make: *** [Makefile:141: flash-opentitan] Error 101

cargo tree reports that Radium comes from:

├── ecdsa v0.8.0
│   ├── elliptic-curve v0.6.0
│   │   ├── bitvec v0.18.3
│   │   │   ├── funty v1.0.1
│   │   │   ├── radium v0.3.0
│   │   │   └── wyz v0.2.0

I have opened an issue for Radium: ferrilab/radium#3 but it would also be great if these libraries could stop depending on it.

@alistair23
Copy link
Author

alistair23 commented Sep 15, 2020

The current v0.4.1 release doesn't have the dependency, so it must be a change between v0.5.0 ad v0.6.0 of elliptic-curve.

This is probably the commit that causes the requirement on atomics: RustCrypto/traits@59e67e0

It would be great if signing and other operations could be supported without atomics via a feature flag.

@tarcieri
Copy link
Member

Yes, we're now pulling in bitvec by way of the ff crate.

This is actually a superfluous dependency (at least at present). I opened this issue about making bitvec an optional dependency of ff:

zkcrypto/ff#42

@alistair23
Copy link
Author

That would be great!

@alistair23
Copy link
Author

Otherwise I don't think signing is possible, I just keep getting this error when trying to call try_sign_recoverable_prehashed():

error[E0599]: no method named `try_sign_recoverable_prehashed` found for struct `ecdsa::signer::Signer<p256::NistP256>` in the current scope
   --> examples-features/ctap.rs:410:30
    |
410 |             let sig = signer.try_sign_recoverable_prehashed(data, None).unwrap();
    |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ method not found in `ecdsa::signer::Signer<p256::NistP256>`

error: aborting due to 2 previous errors; 6 warnings emitted

@tarcieri
Copy link
Member

@alistair23 that method doesn't exist in the p256 crate (it's a private, inherent method in the k256 crate alone).

Here's a usage example:

https://docs.rs/p256/0.5.0-rc/p256/ecdsa/index.html#signingverification-example

@alistair23
Copy link
Author

Thanks @tarcieri, unfortunately the new sign() function depends on arithmetic which requires atomics.

@tarcieri
Copy link
Member

Yeah, you need the ecdsa feature enabled in order to create signatures, which depends on arithmetic.

There's not that much that can be done about that immediately. I'd say stay on p256 0.4 until either ferrilab/radium#4 is merged and bitvec updated, and/or zkcrypto/ff#42 is merged and we can disable the bitvec dependency.

@alistair23
Copy link
Author

Hopefully one (or both) of those will be merged soon. Until then it looks like only sign_with_rng() is available, which doesn't appear to work without std.

@tarcieri
Copy link
Member

@alistair23 sign_with_rng doesn't need std. If you're seeing std linkage, it's probably because some other dependency of yours is activating the std feature of rand_core.

However, sign_with_rng also needs the ecdsa feature (and with it arithmetic) and is documented as such:

https://docs.rs/p256/0.5.0-rc/p256/ecdsa/type.SigningKey.html

This is supported on crate features ecdsa-core and ecdsa only.

@alistair23
Copy link
Author

Just an update, this seems to be fixed with a PR for Radium: ferrilab/radium#4

@alistair23
Copy link
Author

This has been fixed: ferrilab/radium#3

Do you mind updating the crate dependencies?

@tarcieri
Copy link
Member

Unfortunately it’s a deep transitive dependency. Looks like bitvec needs to update to radium v0.5 then ff needs to update to a new release of bitvec. After that group needs to upgrade ff, and then elliptic-curve can update ff and group.

@str4d
Copy link
Contributor

str4d commented Sep 20, 2020

The fix has been backported as bitvec 0.18.4, so you should now be able to cargo update to address this.

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

No branches or pull requests

3 participants