Adopt RustCrypto new releases

[baloo]

This is probably a bit early, but this PR bumps the dependencies to use @RustCrypto next set of releases.

[dignifiedquire]

the overall changes are a lot simpler than I expected, which is nice

[baloo]

I need to track down the regression about compatibility. From my understanding the verifier is tolerant to LF/CR variations.

[hko-s]

I need to track down the regression about compatibility. From my understanding the verifier is tolerant to LF/CR variations.

That is a surprising regression.

LF/CR tolerance is a subtle and somewhat annoying aspect of OpenPGP - it interacts both with the type of the literal packet, and with the SignatureType. And cleartext signatures add a few more wrinkles.

However, I would not expect any of this OpenPGP-subtlety to interact with rust crypto upgrades O_o

[baloo]

Yeah, I definitely did not expect it either. But I haven't taken the time to root-cause it yet (not familiar enough with the code base, it's intimidating^^)

[baloo]

Turns out I messed up the Mpi::from(BoxedUint):

Details

diff --git a/src/types/mpi.rs b/src/types/mpi.rs
index 8a412b2d71..1f3e898125 100644
--- a/src/types/mpi.rs
+++ b/src/types/mpi.rs
@@ -118,27 +118,27 @@

 impl From<BoxedUint> for Mpi {
     fn from(other: BoxedUint) -> Self {
+        Self::from(&other)
+    }
+}
+
+impl From<&BoxedUint> for Mpi {
+    fn from(other: &BoxedUint) -> Self {
         // TODO(baloo): are Mpi used for private keys?
         //              This is only meant to be used for public keys
         Mpi(other.to_be_bytes_trimmed_vartime().into())
     }
 }

-impl From<&BoxedUint> for Mpi {
-    fn from(other: &BoxedUint) -> Self {
-        Mpi(other.to_be_bytes().into())
-    }
-}
-
 impl From<NonZero<BoxedUint>> for Mpi {
     fn from(other: NonZero<BoxedUint>) -> Self {
-        Mpi(other.to_be_bytes().into())
+        Self::from(other.get())
     }
 }

 impl From<&NonZero<BoxedUint>> for Mpi {
     fn from(other: &NonZero<BoxedUint>) -> Self {
-        Mpi(other.to_be_bytes().into())
+        Self::from(other.as_ref())
     }
 }

Some of the implementation didn't use the trimmed version which broke the fingerprinting and then broke the match.

[hko-s]

Awesome that you found it! (After 2h of debugging though? That sounds a bit brutal.)

The MPI format is a horrifying armada of footguns, but ... it is there, in the "old" formats.

(One of many nice things about the new OpenPGP RFC 9580 is that it uses plain fixed size encoding for modern types of key material and signatures.)

[baloo]

Awesome that you found it! (After 2h of debugging though? That sounds a bit brutal.)

nah, those 2 hours included giving dinner to the kids ^^

[overhacked]

@baloo, I see you've been pushing to this recently. I'm working on getting ssh-agent-lib ready for RustCrypto upgrades. The pgp example over there needs a lot of refactoring; before I start working on that in earnest, using this PR as the "new version", can I ask 1) are you already working on anything over at ssh-agent-lib, bringing it up to date for new RustCrypto, that isn't pushed to the public repo and 2) are you actively rebasing this PR so any work I did over at ssh-agent-lib that depends on this would be up-to-date when (🤞) this gets merged?

Also, I noticed when starting to play with getting ssh-agent-lib's PGP example up to date that the pgp::packet::PublicKey constructor introduced back in #142 seems to have been removed some time after pgp-0.15.0. Being able to construct a PublicKey from the underlying RSA/etc. components is the key part of pgp-wrapper.rs. If I start bringing the example up to date, what are your thoughts on me either opening a rpgp/rpgp PR that is stacked on this one or re-introducting the #142 constructor in this PR?

[hko-s]

Hey @overhacked! A quick reaction to one narrow point in your message:

Also, I noticed [..] that the pgp::packet::PublicKey constructor introduced back in #142 seems to have been removed [..]

That functionality should still be available, via:
https://docs.rs/pgp/latest/pgp/packet/struct.PublicKey.html#method.from_inner

The data structures have changed around a bit, but PubKeyInner should be usable just like PublicKey was, before. However, it seems that the type is not currently visible in the public API - at a quick glance, I don't understand why 🤔 .. either way, I think the way forward is to make it public.

[overhacked]

However, it seems that the type is not currently visible in the public API - at a quick glance, I don't understand why 🤔 .. either way, I think the way forward is to make it public.

IMO, having packet::PublicKey::from_inner exposed and needing to access PubKeyInner leads to a more difficult to understand API. I'd much rather PubKeyInner be invisible to the public API and a method on packet::PublicKey to construct it from key primitives.

[baloo]

@baloo, I see you've been pushing to this recently.

Yeah, fairly consistently over the last 6-8 months ^^

Mostly because I need it for work, but also rpgp is using pretty all of rustcrypto, and it's been extremely useful to find bugs in the upcoming release.

I'm working on getting ssh-agent-lib ready for RustCrypto upgrades. The pgp example over there needs a lot of refactoring; before I start working on that in earnest, using this PR as the "new version", can I ask 1) are you already working on anything over at ssh-agent-lib, bringing it up to date for new RustCrypto, that isn't pushed to the public repo

No, I'm not using ssh-agent-lib at the moment. Mostly as a lack of spare time, not lack of interest.

and 2) are you actively rebasing this PR so any work I did over at ssh-agent-lib that depends on this would be up-to-date when (🤞) this gets merged?

Yeah, I rebase this PR every week or so.

Also, I noticed when starting to play with getting ssh-agent-lib's PGP example up to date that the pgp::packet::PublicKey constructor introduced back in #142 seems to have been removed some time after pgp-0.15.0. Being able to construct a PublicKey from the underlying RSA/etc. components is the key part of pgp-wrapper.rs. If I start bringing the example up to date, what are your thoughts on me either opening a rpgp/rpgp PR that is stacked on this one or re-introducting the #142 constructor in this PR?

I'd rather not have to maintain your changes. What I would recommend would be to make those changes against main, then make a merge between those changes and this branch.

Those commits here are all made with jujutsu, and if you were to jj bookmark track baloo/rustcrypto-new-releases@baloo, your merge commit would get automatically rebased by jj, that way you can iterate with the authors of rpgp without having me in the loop.

[overhacked]

Thanks for the reply! This is an awesome reason for me to learn jj, which I'd been meaning to for a while. Also thanks for the clear answer about not wanting to maintain the additional patch in this PR; makes it easy to decide which way to move forward.

No, I'm not using ssh-agent-lib at the moment. Mostly as a lack of spare time, not lack of interest.

Since I've already got a working patch for upcoming RustCrypto 0.9+ for ssh-agent-lib except the PGP example, I'll go ahead and start a draft PR over there.

[baloo]

Thanks for the reply! This is an awesome reason for me to learn jj, which I'd been meaning to for a while. Also thanks for the clear answer about not wanting to maintain the additional patch in this PR; makes it easy to decide which way to move forward.

I mean, it's more that it's going to be a frustration for the both of us having to wait for each other to sync up, and I'd rather avoid that if possible.

No, I'm not using ssh-agent-lib at the moment. Mostly as a lack of spare time, not lack of interest.

Since I've already got a working patch for upcoming RustCrypto 0.9+ for ssh-agent-lib except the PGP example, I'll go ahead and start a draft PR over there.

Let's follow up there (I also have one, and rand_core changed its API, again)

[wiktor-k]

Happy to see more and more stuff getting out of rc status into stable! 🥹

Thanks for your hard work @baloo 🙇