search

RustCrypto / traits

Collection of cryptography-related traits
582 stars 190 forks source link

cipher v0.3.0 release #392

Closed tarcieri closed 3 years ago

tarcieri commented 3 years ago

This is a tracking issue for a final v0.3.0 release of the cipher crate, containing such changes as refactored block cipher traits (#352), unified error types (#373), and lots of cleanups of things that were missed with the "initial" release of the cipher crate (v0.2, bumped from the v0.1 releases before we owned it).

From my perspective it's ready to release, however there are at least a few things it might be worth considering getting in there before we do:

@newpavlov any others?

newpavlov commented 3 years ago

I have a draft with changes, which I would like to add to the v0.3 release. They include some renamings (e.g. SyncStreamCipher -> StreamCipher, StreamCipher -> AsyncStreamCipher), code reorganization, addition of a modified block mode trait. I got stuck on designing a trait for exposing block nature of stream ciphers (the block-buffer PR is part of this work), but I probably will postpone it for a later patch release. I also consider adding a trait for AE, but I haven't found a good API for it yet which would fit nicely with the block mode API.

tarcieri commented 3 years ago

The renamed traits sound great!

tarcieri commented 3 years ago

@newpavlov I'm happy to punt on #444 until after cipher v0.3.0

How do you feel about #510? What else is remaining? A block-modes integration?

tarcieri commented 3 years ago

How about an end-of-February (i.e. ~March 1st) deadline for a v0.3.0 release?

newpavlov commented 3 years ago

Yes, sounds good! I plan to return to cipher after finishing with RustCrypto/hashes#217.

tarcieri commented 3 years ago

Checking in on this: how are we doing?

@newpavlov looks like you've merged quite a bit lately. I'm not sure what's remaining for a release. Is there anything I can help with?

tarcieri commented 3 years ago

@newpavlov what specifically remains to be done here?

tarcieri commented 3 years ago

I'm thinking if we can't get another cipher release out e.g. this week that it might make sense to cut another cipher prerelease along with prereleases of the most used AEAD crates to unblock people who are stuck on the rand_core upgrade.

newpavlov commented 3 years ago

Sorry for the delay! I plan to update the cipher PR today with some significant changes.

tarcieri commented 3 years ago

@newpavlov was https://github.com/RustCrypto/traits/pull/567 the PR you're talking about above, or per the notes in that PR are you planning a follow-up?

newpavlov commented 3 years ago

@tarcieri Yes, I am currently working on it. Unfortunately the previous direction was quite an over-engineering after I've looked on it with fresh eyes, so I try to simplify it now. The main idea is to inverse control, instead of looping in higher-level code I plan to transfer control over it to block ciphers implementations. Since it's be done via callbacks (one for filling slice of blocks and second for processing encrypted/decrypted results) and since they are effectively inlined, my hope is that it will result in near optimal performance. But there are some complications, e.g. some block modes require pre-encrypted blocks, another one is that it's unclear how to properly combine encryptor/decryptors with MACs. They may have different parallel block sizes, so in theory we should find least common multiple, but we can not do it with the callback approach.

tarcieri commented 3 years ago

Checking in once again on how we're doing here.

At this point things are really starting to get problematic for me as this is holding up the upgrade to rand v0.8 / rand_core v0.6. Pretty much the rest of the ecosystem has updated, including most recently the rsa crate. I'm getting pretty frequent questions from people who are confused why there are type system errors trying to use newer versions of rand with the current crates.

I can get immediately unblocked on things by cutting releases of the ecdsa crate using the current release of hmac along with releases of the https://github.com/rustcrypto/elliptic-curves crates, which would unblock me immediately on the rand updates. I was holding out hoping we could do a synchronized update there, but if it's going to take awhile longer I'd really not continue to delay.

In hindsight I kind of wish we just released cipher v0.3 unchanged from the v0.3.0-pre.4 release I cut December 2020. It's also holding up things like releasing the first versions of crates with runtime autodetection of SIMD features, along with the new stream module in the aead crate.

tarcieri commented 3 years ago

I have a potential proposal to unblock all of the above which I think would be relatively simple to execute given the current state of the repos. First, the crates in https://github.com/RustCrypto/universal-hashes/ are ready to release, so everything else aside I can go ahead and cut new releases of those.

Beyond that:

The block-ciphers repo is currently using the 0.3.0-pre.4 release of cipher as published to https://crates.io

I propose making a final v0.3.0 release of cipher which is otherwise identical to 0.3.0-pre.4, followed by a v0.11.0 release of crypto-mac which includes the cipher updates.

Aside from bumping crypto-mac, everything else is ready for another release all the way up through aead, which is updated to aead v0.4 and is only (aside from universal hashes) blocked on bumping block ciphers/stream ciphers.

@newpavlov WDYT about that? I think it would give you more time to work on additional changes to cipher. If you think you'll be done with what you're working on soon, great, but otherwise in the past few weeks the v0.3.0 cipher release has become the one thing blocking everything I outlined in my previous comment.

newpavlov commented 3 years ago

@tarcieri Sorry again! I got distracted by other stuff and was not able to work much on it lately. Give me several days and I will publish relevant PRs, after that, depending on your opinion of the proposed changes, we can either finalize them or publish the current state of repos as v0.3 and publish the changes as v0.4.

tarcieri commented 3 years ago

Sure, sounds good

tarcieri commented 3 years ago

I've done an initial review of #589 and I think the direction is good. However,...

The general direction seems like experimental changes that should probably take some time to bake. I think we already have enough of these sort of "risky" changes (namely CPU feature autodetection) that were ready to ship half a year ago at this point which I would like to see people actually use before we introduce another round of such potentially risky changes.

I'd really, really like to ship the stuff that's ready at this point. It's a blocker for a lot of stuff I'm working on, and keeps coming up with people who are confused about which version of rand_core to use.

At this point I'd very strongly suggest we ship what I proposed above. If nothing else, at this point the unshipped work is very long winded and it will be hard enough to even write changelogs.

I think it'd be good to give you @newpavlov more time to experiment with your changes, and also decouple such changes from things like CPU feature autodetection. I appreciate the work you're doing but the backlog of unshipped changes we have at this point is reaching a threshold I consider "time sensitive" and I really think we're long overdue for another round of releases.

kevinhoffman commented 3 years ago

@newpavlov @tarcieri -- we're definitely very excited to use the CPU autodetection feature (we use aes-gcm-siv crate and downstream crates), so anything that could be done to get a new version shipped with this included in it would be much appreciated! There doesn't seem to be a downside to @tarcieri's proposed plan above to release the latest as v0.3.0 and then incorporate all these enhancements into a v0.4.0 as they're ready?

Thanks for all of the incredible work on these crates, they have really good performance and the quality of both the crypto and the dev interfaces are very high!

newpavlov commented 3 years ago

@tarcieri I think it's worth to merge some of the less radical changes from the experimental PRs, i.e. keep the ParBlocks-based API and leave the callback-based one with custom fat pointers for future versions. I will try to prepare PR in the following days. If I will not do it before the next Tuesday-Wednesday, feel free to proceed with releases as you see fit.

Side note: It's a bit annoying that our trait crates become somewhat coupled, which pushes for synchronized development and releases.

decouple such changes from things like CPU feature autodetection

IIRC autodetection is compatible with the currently released trait crates, so it's should be possible to backport autodetection and release it in aes v0.6.1. But for the sake of simplicity of the block ciphers repository history it's probably worth to release it in aes v0.7.0.

tarcieri commented 3 years ago

I will try to prepare PR in the following days. If I will not do it before the next Tuesday-Wednesday, feel free to proceed with releases as you see fit.

Sounds good!

Side note: It's a bit annoying that our trait crates become somewhat coupled, which pushes for synchronized development and releases.

I think it's pretty inevitable in that higher-level constructions need to combine primitives that work together.

It's annoying, for example, that ECDSA depends on the cipher crate, but that's by way of HMAC and the hmac crate's dependence on crypto-mac which is in turn dependent on cipher.

If we could make crypto-mac somehow independent of cipher, that would help a lot I think, but also introduces additional problems, so whatever way you go it feels tricky.

newpavlov commented 3 years ago

It's annoying, for example, that ECDSA depends on the cipher crate, but that's by way of HMAC and the hmac crate's dependence on crypto-mac which is in turn dependent on cipher.

This is one of the reasons why I think it's worth to merge some changes from the experimental PR instead of using the current state of the master branch. In it FromBlockCipher and FromBlockCipherNonce are replaced with InnerInit and InnerIvInit respectively, which are defined in the crypto-common crate. In other words, hmac no longer depends on cipher, though at the cost of a bit less clear trait names.

tarcieri commented 3 years ago

Indeed that much seems worth porting over if it can decouple cipher and crypto-mac.

tarcieri commented 3 years ago

If I will not do it before the next Tuesday-Wednesday, feel free to proceed with releases as you see fit.

I think it's worth attempting cutting new prereleases of crypto-common, cipher, and crypto-mac based on the current master of this repo and getting all of the existing code updated to use those.

After we've done that, perhaps we can circle back on any remaining last minute changes and then cut a final release.

Edit: I did that with the following releases:

tarcieri commented 3 years ago

I looked at trying to do an update to the current cipher changes on the master branch, cutting a cipher v0.3.0-pre.5 release, but I had trouble integrating it into stream-ciphers: https://github.com/RustCrypto/stream-ciphers/pull/225

I'm in favor of just cutting a cipher v0.3.0 release based on v0.3.0-pre.4 simply because all of the other repos are otherwise up-to-date end-to-end at this point, and to see any of the new changes through at this point we really need to update all of the other repos and make sure those changes actually make sense.

tarcieri commented 3 years ago

Went ahead and cut a cipher v0.3.0 release based on cipher v0.3.0-pre.4 (not pre.5 due to upgrade issues) as well as a crypto-mac v0.11.0.

I upgraded block-ciphers/stream-ciphers to use this release successfully.

I haven't cut any additional releases, so there is a small "abort" window here where I could back out these releases by yanking them.

Otherwise, if this seems good, I'd like to move forward with updating everything else and then cutting releases of the dependent crates.

Edit: I opened a PR to update RustCrypto/MACs.

tarcieri commented 3 years ago

Getting dependabot alerts for cipher v0.3.0 and crypto-mac v0.11.0 now, so I'd suggest we go forward with releases.

tarcieri commented 3 years ago

I've gone ahead and cut releases of pretty much everything impacted. The only thing remaining are a few of the AEAD crates, and I'm doing some final cleanups on those before releasing each one.

cipher v0.3.0 is shipped! :shipit: