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

Fix minimal version of ecdsa dependency #933

Closed
wants to merge 1 commit into from
Closed

Fix minimal version of ecdsa dependency #933

wants to merge 1 commit into from

Conversation

DariuszDepta
Copy link

In k256 crate, in [dependencies], the minimal version of ecdsa crate is 0.16.8. But in [dev-dependencies] in the same crate the minimal version of ecdsa is 0.16.0. This leads to compiling different versions of ecdsa crate in complex dependency tree, especially when using -Zminimal-versions option. Additionally there are differences in crate ecdsa between version 0.16.0 and 0.16.8, such that prevents us to compile a project when 0.16.0 version is used by the Rust compiler.

This PR assigns the same minimal version of ecdsa in all crates of this project, exactly as it was originally updated in k256 crate.

@DariuszDepta DariuszDepta changed the title Set minimal version of ecdsa dependency Fix minimal version of ecdsa dependency Oct 2, 2023
@tarcieri
Copy link
Member

tarcieri commented Oct 2, 2023

In k256 crate, in [dependencies], the minimal version of ecdsa crate is 0.16.8

How did you come to this conclusion?

I was able to compile a project containing the following in Cargo.toml:

[dependencies]
k256 = { version = "0.13.1", features = ["ecdsa"] }
ecdsa = "=0.16.0"
$ cargo build --release
   ...
   Compiling ecdsa v0.16.0
   Compiling k256 v0.13.1
   ...
    Finished release [optimized] target(s) in 3.23s

@DariuszDepta
Copy link
Author

Sure, that's clear.

The case is, that our project: https://github.com/CosmWasm/cw-multi-test does not compile using command:
cargo build -Zminimal-versions
when we do not fix the ecdsa version to 0.16.8 like this:

[dev-dependencies]
# We don't use these dependencies directly,
# we tighten versions that builds with `-Zminimal-versions` work.
ecdsa = "0.16.8"

@tarcieri
Copy link
Member

tarcieri commented Oct 2, 2023

That sounds like an issue with your project.

To show that the dependencies need to be adjusted here, you need a minimal repro which involves only crates in this repo and their dependencies.

@DariuszDepta
Copy link
Author

We have the similar problem in our project, that you have in your project:

git clone https://github.com/RustCrypto/elliptic-curves.git
cd elliptic-curves/
rm Cargo.lock
cargo +nightly build -Zminimal-versions

    Updating crates.io index
   Compiling zeroize v1.5.3
   Compiling typenum v1.14.0
   Compiling version_check v0.9.0
   Compiling const-oid v0.9.2
   Compiling subtle v2.4.0
   Compiling libc v0.2.133
   Compiling getrandom v0.2.0
   Compiling cfg-if v0.1.2
   Compiling base64ct v1.4.1
   Compiling base16ct v0.2.0
   Compiling cpufeatures v0.2.5
   Compiling cfg-if v1.0.0
   Compiling once_cell v1.18.0
   Compiling generic-array v0.14.7
   Compiling pem-rfc7468 v0.7.0
   Compiling der v0.7.2
   Compiling rand_core v0.6.4
   Compiling ff v0.13.0
   Compiling group v0.13.0
   Compiling spki v0.7.2
   Compiling pkcs8 v0.10.2
   Compiling crypto-common v0.1.3
   Compiling block-buffer v0.10.1
   Compiling crypto-bigint v0.5.0
   Compiling sec1 v0.7.3
   Compiling digest v0.10.7
   Compiling hmac v0.12.0
   Compiling signature v2.0.0
   Compiling sha2 v0.10.1
   Compiling sm3 v0.4.0
   Compiling hkdf v0.12.0
   Compiling rfc6979 v0.4.0
   Compiling elliptic-curve v0.13.5
error[E0432]: unresolved import `hkdf::hmac`
  --> /home/confio/.cargo/registry/src/index.crates.io-6f17d22bba15001f/elliptic-curve-0.13.5/src/ecdh.rs:36:12
   |
36 | use hkdf::{hmac::SimpleHmac, Hkdf};
   |            ^^^^ could not find `hmac` in `hkdf`

error[E0107]: struct takes 1 generic argument but 2 generic arguments were supplied
   --> /home/confio/.cargo/registry/src/index.crates.io-6f17d22bba15001f/elliptic-curve-0.13.5/src/ecdh.rs:192:54
    |
192 |     pub fn extract<D>(&self, salt: Option<&[u8]>) -> Hkdf<D, SimpleHmac<D>>
    |                                                      ^^^^    ------------- help: remove this generic argument
    |                                                      |
    |                                                      expected 1 generic argument
    |
note: struct defined here, with 1 generic parameter: `D`
   --> /home/confio/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hkdf-0.12.0/src/lib.rs:199:12
    |
199 | pub struct Hkdf<D>
    |            ^^^^ -

Some errors have detailed explanations: E0107, E0432.
For more information about an error, try `rustc --explain E0107`.
error: could not compile `elliptic-curve` (lib) due to 2 previous errors
warning: build failed, waiting for other jobs to finish...

For completeness, there is Rust environment used to build your project:

Default host: x86_64-unknown-linux-gnu

installed toolchains
--------------------

stable-x86_64-unknown-linux-gnu (default)
nightly-x86_64-unknown-linux-gnu

installed targets for active toolchain
--------------------------------------

wasm32-unknown-unknown
x86_64-unknown-linux-gnu

active toolchain
----------------

stable-x86_64-unknown-linux-gnu (default)
rustc 1.72.1 (d5c2e9c34 2023-09-13)

I hope this helps to better understand our concerns as users of your project ;-)

@tarcieri
Copy link
Member

tarcieri commented Oct 2, 2023

Okay, that does appear to be a minimal-versions problem, but one which seems completely unrelated to the ecdsa crate and has to do with the elliptic-curve and hkdf crates

@DariuszDepta
Copy link
Author

Yes, because this is in your project. ecdsa problem is in our project. Hopefully we can overcome our problem by fixing minimal ecdsa version in your project ;-).

@tarcieri
Copy link
Member

tarcieri commented Oct 2, 2023

If your project requires ecdsa v0.16.8, you need to specify that in your dependencies.

k256 itself builds fine with ecdsa v0.16.0.

@DariuszDepta
Copy link
Author

DariuszDepta commented Oct 2, 2023

Let's take a look at k256:

[dependencies]
[...]
ecdsa-core = { version = "0.16.8", package = "ecdsa", optional = true, default-features = false, features = ["der"] }
[...]

[dev-dependencies]
[...]
ecdsa-core = { version = "0.16", package = "ecdsa", default-features = false, features = ["dev"] }
[...]

The minimal version is 0.16.8 for [dependencies] and 0.16.0 for [dev-dependencies]. We have dependency to k256 in both of them in our project. With minimal-versions compiler chooses 0.16.0, and then functions defined in newer versions than 0.16.0 are not recognized and our code does not compile.

Our code depends only on k256, not ecdsa directly. That's why it is little bit cumbersome to have to specify dev-dependency to ecdsa to make the code compile.

@tarcieri
Copy link
Member

tarcieri commented Oct 2, 2023

Please provide a minimal reproduction that demonstrates the problem and causes k256 not to compile using only a Cargo.toml file.

FWIW, I've already provided a counterexample here.

@tarcieri
Copy link
Member

tarcieri commented Oct 2, 2023

Here's a fix for the hkdf requirement in elliptic-curve: RustCrypto/traits#1353

@DariuszDepta
Copy link
Author

Wonderful! You have just fixed the minimal version of hkdf to 0.12.1. We need to fix ecdsa at 0.16.8. You have this version already used in k256 [dependency]. Why not to fix it in other places too?

@tarcieri
Copy link
Member

tarcieri commented Oct 2, 2023

Because we try to correctly specify the minimal supported version.

In the case of elliptic-curve, it doesn't compile against hkdf v0.12.0. It does, however, compile against hkdf v0.12.1, so the minimal supported version is v0.12.1.

In the case of k256, it compiles against ecdsa v0.16.0 as I demonstrated here, so the minimal supported version is v0.16.0.

Your project apparently requires ecdsa v0.16.8 for whatever reason, so your minimal supported version of ecdsa is v0.16.8. So, you need to add an explicit requirement as such if you expect your code to be minimal-versions clean.

I'm going to go ahead and close this. If you still think there is an issue in k256, at the very absolute minimum you need to provide an example of a compile error occurring internal to k256 due to it using functionality which requires a newer version of ecdsa than what is specified.

@tarcieri tarcieri closed this Oct 2, 2023
@DariuszDepta
Copy link
Author

DariuszDepta commented Oct 3, 2023

Just for the record (and other users), below is the simplest example that shows how clients of k256 crate may encounter compilation problems when building their own applications with -Zminimal-versions option.

Create a new Rust project

$ cargo init example
$ cd example

Add k256 dependency to Cargo.toml

[package]
name = "example"
version = "0.1.0"
edition = "2021"

[dependencies]
k256 = { version = "0.13.1", features = ["ecdsa"] }

Use some code from re-exported ecdsa crate

use k256::ecdsa::Signature;

fn main() {
    let data: [u8;64] = [0;64];
    let _ = Signature::from_bytes(&data.into());
}

Compile (should work)

cargo clean && rm Cargo.lock && cargo +stable build
    Updating crates.io index
   Compiling zeroize v1.6.0
   Compiling typenum v1.17.0
   Compiling version_check v0.9.4
   Compiling const-oid v0.9.5
   Compiling subtle v2.5.0
   Compiling libc v0.2.148
   Compiling cfg-if v1.0.0
   Compiling base16ct v0.2.0
   Compiling cpufeatures v0.2.9
   Compiling once_cell v1.18.0
   Compiling der v0.7.8
   Compiling generic-array v0.14.7
   Compiling getrandom v0.2.10
   Compiling rand_core v0.6.4
   Compiling spki v0.7.2
   Compiling ff v0.13.0
   Compiling pkcs8 v0.10.2
   Compiling group v0.13.0
   Compiling block-buffer v0.10.4
   Compiling crypto-common v0.1.6
   Compiling crypto-bigint v0.5.3
   Compiling sec1 v0.7.3
   Compiling digest v0.10.7
   Compiling hmac v0.12.1
   Compiling signature v2.1.0
   Compiling sha2 v0.10.8
   Compiling rfc6979 v0.4.0
   Compiling elliptic-curve v0.13.6
   Compiling ecdsa v0.16.8
   Compiling k256 v0.13.1
   Compiling example v0.1.0 (/home/confio/Work/diagnoses/example)
    Finished dev [unoptimized + debuginfo] target(s) in 4.65s

Compile with -Zminimal-versions option (will fail)

cargo clean && rm Cargo.lock && cargo +nightly build -Zminimal-versions
    Updating crates.io index
   Compiling zeroize v1.5.3
   Compiling typenum v1.14.0
   Compiling version_check v0.9.0
   Compiling subtle v2.4.0
   Compiling libc v0.2.95
   Compiling getrandom v0.2.0
   Compiling cfg-if v0.1.2
   Compiling const-oid v0.9.2
   Compiling base16ct v0.2.0
   Compiling cpufeatures v0.2.5
   Compiling cfg-if v1.0.0
   Compiling once_cell v1.17.0
   Compiling der v0.7.1
   Compiling generic-array v0.14.6
   Compiling rand_core v0.6.4
   Compiling ff v0.13.0
   Compiling group v0.13.0
   Compiling spki v0.7.0
   Compiling pkcs8 v0.10.1
   Compiling crypto-common v0.1.3
   Compiling block-buffer v0.10.1
   Compiling crypto-bigint v0.5.0
   Compiling sec1 v0.7.1
   Compiling digest v0.10.6
   Compiling hmac v0.12.0
   Compiling signature v2.0.0
   Compiling sha2 v0.10.1
   Compiling rfc6979 v0.4.0
   Compiling elliptic-curve v0.13.0
   Compiling ecdsa v0.16.0
   Compiling k256 v0.13.1
   Compiling example v0.1.0 (/home/confio/Work/diagnoses/example)
error[E0599]: no function or associated item named `from_bytes` found for struct `ecdsa::Signature` in the current scope
 --> src/main.rs:5:24
  |
5 |     let _ = Signature::from_bytes(&data.into());
  |                        ^^^^^^^^^^
  |                        |
  |                        function or associated item not found in `Signature<Secp256k1>`
  |                        help: there is a method with a similar name: `to_bytes`

For more information about this error, try `rustc --explain E0599`.
error: could not compile `example` (bin "example") due to previous error

And why?

Because ecdsa version 0.16.8 is not backward compatible with version 0.16.0, see: https://semver.org/#spec-item-6 for explanation.

How to fix it?

This PR was intended to fix this issue.

@tarcieri
Copy link
Member

tarcieri commented Oct 3, 2023

The issue in your example is in your main.rs, not k256. Note the filename where the error is occurring is src/main.rs:5 and not any code in k256 itself.

As I requested earlier:

#933 (comment)

Please provide a minimal reproduction that demonstrates the problem and causes k256 not to compile using only a Cargo.toml file.

However, again, I've already demonstrated k256 will compile with ecdsa v0.16.0.

Putting some code in the test library which requires newer versions of ecdsa doesn't count, because what you're trying to demonstrate is an incompatibility between versions of k256 and ecdsa, not user-facing code.

You just need to add ecdsa = "0.16.8" to your Cargo.toml, because your code only supports ecdsa v0.16.8 at a minimum. That's it.

Adding it to k256 is the wrong place, because k256 builds fine with ecdsa v0.16.0.

@tarcieri
Copy link
Member

tarcieri commented Oct 3, 2023

Note: as of #936, cargo +nightly build -Zminimal-versions completes successfully

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