PAKEs icon indicating copy to clipboard operation
PAKEs copied to clipboard

deps: update to newer rand/hkdf crates

Open warner opened this issue 5 years ago • 8 comments

I don't know what the Rust convention is, but when I see cargo outdated telling me that there are newer versions of dependencies that we might use, I'm tempted to upgrade. SPAKE2 is currently out-of-date on HKDF and the rand crate.

I've got a PR for spake2's use of HKDF that I'll submit in a minute, but we can't update to rand-0.7 until curve25519-dalek does the same, because the random-element selection API cites a rand_core::CryptoRng trait that must be the same on both sides of the interface.

I haven't looked closely at SRP, but it's behind on both rand (which should be easy) and generic-array (about which I have no idea).

warner avatar Aug 07 '19 00:08 warner

Also note that bumping rand to v0.7 will raise MSRV to 1.32. Personally I prefer a conservative view point stating that MSRV bump should be considered a breaking change. Also it will make sense to migrate to 2018 edition as well.

I have some ideas for SRP API changes, so I wanted to bump dependencies together with them.

newpavlov avatar Aug 07 '19 08:08 newpavlov

Sounds good to me. I'll keep an eye on curve25519-dalek and will file a PR as soon as they've updated too (I have no idea when that might be).

Incidentally, how does one learn what the MSRV is for any particular crate? Do you just keep trying to compile it with older and older ones until something fails? Or is there some Cargo.toml notation, or tool that tells you what features you're using, or something like that?

warner avatar Aug 07 '19 17:08 warner

AFAIK there is no such tool (you can't even declare crate MSRV right now...). You only can check MSRV as part of CI tests and automatically re-run them periodically to detect potential regressions in your dependency tree.

newpavlov avatar Aug 07 '19 17:08 newpavlov

Got it, thanks.

I ran rustup run 1.31.1 cargo test on the PAKEs repo, and learned that curve25519-dalek v1.2.2 doesn't work, so maybe I accidentally brought us to MSRV=1.32 when I did the 2018-edition update about 8 months ago? (commit bd19c40). Or maybe they incremented their MSRV without a major-version change? I'll experiment to see how e.g. v1.0.x/v1.1.x/v1.2.x differ, maybe we should have used a tighter constraint.

   Compiling curve25519-dalek v1.2.2                                                                               
error[E0658]: `Self` struct constructors are unstable (see issue #51994)                                           
   --> /home/warner/.cargo/registry/src/github.com-1ecc6299db9ec823/curve25519-dalek-1.2.2/src/edwards.rs:707:9    
    |                                                                                                              
707 |         Self(scalar_mul::precomputed_straus::VartimePrecomputedStraus::new(static_points))                   
    |         ^^^^                                                                                                 
                                                                                                                   
error[E0658]: `Self` struct constructors are unstable (see issue #51994)                                           
   --> /home/warner/.cargo/registry/src/github.com-1ecc6299db9ec823/curve25519-dalek-1.2.2/src/ristretto.rs:925:9  
    |                                                                                                              
925 |         Self(                                                                                                
    |         ^^^^                                                                                                 
                                                                                                                   
error: aborting due to 2 previous errors                                                                           
                                                                                                                   
For more information about this error, try `rustc --explain E0658`.                                                
error: Could not compile `curve25519-dalek`.                                                                       

I see that our .travis.yml is pinning 1.31.0 for cargo fmt, but otherwise isn't pinning any particular version (it uses stable and beta and nightly). Should we add a 1.31.1 to that list to test our purported MSRV?

warner avatar Aug 07 '19 17:08 warner

rand v0.7 has MSRV equal to 1.32 and I plan to use the same version for future updates of RustCrypto crates, so I think it makes sense to go with it instead of 1.31. And yes, I think it's worth to add MSRV test to CI.

newpavlov avatar Aug 07 '19 17:08 newpavlov

It looks like curve25519-dalek 1.0.0-1.0.3 worked with rustc-1.31.1, but their 1.1.0 release requires 1.32.0 or newer. So a constraint of 1.0 (instead of just 1) would have avoided the accidental MSRV bump.

I'll file a PR to add CI for 1.32.0, and to tighten the dependency. I'll also increase the RUSTFMT pin to 1.32 since that's what we need now (and it looks like a few cosmetic changes will result).

warner avatar Aug 07 '19 17:08 warner

Ok, the travis PR is filed, and as expected it fails against 1.31.1 (I put it in a allow_failures section).

The cargo fmt from 1.32.0 and 1.33.0 doesn't like the missing semicolons in spake2/src/lib.rs (line 735/743/751/761), but the one from 1.31.1 and 1.34.2 is happy with it.. maybe they had a regression for a few versions. Would you want to run the RUSTFMT build against 1.34.2 (or maybe something newer) even if our MSRV is 1.32.0?

warner avatar Aug 07 '19 18:08 warner

I always prefer to keep rustfmt in allowed failures. I think it's less annoyance for contributors, and IIRC there were cases when rustfmt developers have changed defaults, which in turn resulted in CI failures across projects. So I think it will be enough to test formatting only on the latest stable.

newpavlov avatar Aug 07 '19 18:08 newpavlov