Update from 0.8 to 0.9 broke verification after reading PEM and using Sha256 PSS

[poliorcetics]

[package]
name = "rsa-repro"
version = "0.1.0"
edition = "2021"

[dependencies]
hex = "0.4.3"
rand = "0.8.5"
rsa9 = { version = "0.9.2", features = ["sha2"], package = "rsa" }
rsa8 = { version = "0.8.2", features = ["sha2"], package = "rsa" }

fn main() {
 use rsa::pkcs8::EncodePublicKey;
 use rsa::sha2::Digest;
 use rsa::RsaPrivateKey;
 use rsa8 as rsa;

 let mut rng = rand::thread_rng();
 let bits = 2048;
 let data = vec![0_u8; 100];

 let private_key = RsaPrivateKey::new(&mut rng, bits).expect("failed to generate a key");
 let public_key = private_key.to_public_key();

 let mut h = rsa::sha2::Sha256::default();
 h.update(&data);
 let hash = h.finalize();

 let pss = rsa::Pss::new::<rsa::sha2::Sha256>();
 let sig = private_key.sign_with_rng(&mut rng, pss, &hash).unwrap();

 let pem = public_key
 .to_public_key_pem(rsa::pkcs8::LineEnding::LF)
 .unwrap();

 dbg!(&pem, hex::encode(hash), hex::encode(&sig));
 t_rsa8(&pem, &hash, &sig);
 t_rsa9(&pem, &hash, &sig);
}

fn t_rsa8(public_key_pem: &str, hashed_data: &[u8], sig: &[u8]) {
 use rsa8::pkcs8::DecodePublicKey;
 use rsa8::{PublicKey, RsaPublicKey};

 println!("START RSA-8");

 let pk = RsaPublicKey::from_public_key_pem(public_key_pem).expect("Failed to parse PEM");
 pk.verify(rsa8::Pss::new::<rsa8::sha2::Sha256>(), hashed_data, sig)
 .expect("Failed to verify data against signature");

 println!("END RSA-8");
}

fn t_rsa9(public_key_pem: &str, hashed_data: &[u8], sig: &[u8]) {
 use rsa9::pkcs8::DecodePublicKey;
 use rsa9::RsaPublicKey;

 println!("START RSA-9");

 let pk = RsaPublicKey::from_public_key_pem(public_key_pem).expect("Failed to parse PEM");
 pk.verify(rsa9::Pss::new::<rsa9::sha2::Sha256>(), hashed_data, sig)
 .expect("Failed to verify data against signature");

 println!("END RSA-9");
}

This breaks:

cargo run --release --quiet

[src/main.rs:25] &pem = "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA00i3sOjgQLWZqWJeO2DG\nzFp/fwH5tdlVaUA4lOBTGX0hb5+QQelaH/PxyaHuRQ7NSoiFo4VFvYPsMvtcleny\nWpfO0Iq7nVophNjo49KWrXJKkQs8wcNc0vG07PcVgUCvG7z9VX5BJxi1zxnXiUtC\nQ/KIuS93M9rzrRBP2sBwZaQWn2WJqPAj3dmt3JiPYaCdQzviSzMhzTRMEbtQQSfY\nMUhEq4xLKQXvatRRIgtDg/a15+YHGuxSGigYGGUaxn9jzqcsYKVmzAZVbhK0KrNs\n1vqXg0LXlVKvh5n49rDXRwGSXHuNgbX+nDUGqrYEgtKcioPd9ECZpebEPEXkjTBK\n0QIDAQAB\n-----END PUBLIC KEY-----\n"
[src/main.rs:25] hex::encode(hash) = "cd00e292c5970d3c5e2f0ffa5171e555bc46bfc4faddfb4a418b6840b86e79a3"
[src/main.rs:25] hex::encode(&sig) = "70d501c5426f0b197639cbc4c8ebe7f1ffc07baeb124940d43a36da97b89ccda2247b7c4ef1a6be01e3f696a049d2b5cd117cf6ab2f740223437943ba6fd361e5f7a67a5c7ac578318d1643f4719bc12d23c69be309116c1f72d58bfcde54d48f4d9914429d6c101153d43d287dd1f734c43db33c56e0bddbf648504ca59fbf2810a3fa8a1a34c4baf9d1bd54dddfb2c69ea122048772d902cd0b7939a89a0d9ea1a32d518e22e17a124ecb42d75c1ca1232e27c163b92f5321e22e83b4974cd85b494758030e16c35cf1717a9d7b02ccd471dfc82828c782999cd72a7879fdd516d38a67ba933f80757e2b9d293f67916cf29facd4613fc6bcb8e3ea9bd6a1c"
START RSA-8
END RSA-8
START RSA-9
thread 'main' panicked at 'Failed to verify data against signature: Verification', src/main.rs:51:10
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

This looks like a similar problem to #330 maybe ?

[tarcieri]

Not sure what's going on here.

It doesn't appear to be related to any of the hardening-related changes which happened in v0.9. The signatures all appear to be the correct length and it's getting past checks for that. And those were the only substantial changes in v0.9.

From what I observe, it gets all of the way through PSS verification, but the salt is invalid and there is a hash mismatch.

Note this only occurs when rsa v0.9 attempts to verify signatures produced with rsa v0.8. It can verify signatures produced by rsa v0.9 just fine.

I think the next steps for debugging are to check if a 3rd party implementation (e.g. ring) can verify signatures produced by both rsa v0.8 and v0.9.

[tarcieri]

I added a ring interop test to the following to the code example above (e.g. after the dbg!):

 let pkcs1 = public_key.to_pkcs1_der().unwrap();
 let public_key =
 ring::signature::UnparsedPublicKey::new(&ring::signature::RSA_PSS_2048_8192_SHA256, pkcs1);
 public_key.verify(&data, &sig).unwrap();

This test succeeds when using rsa v0.9 (i.e. changing use rsa8 as rsa to use rsa9 as rsa), but fails with rsa v0.8.

So it seems perhaps there was a bug in the PSS implementation in rsa v0.8, although offhand I'm not sure what specific change between the two versions would have caused that.

Regardless, rsa v0.9 seems to be producing signatures ring can verify, and matches ring's behavior rejecting the signature generated by rsa v0.8.

It would probably be good to add an integration test where ring verifies a signature, and it'd be interesting to bisect the changes to see when rsa starts producing signatures that ring can verify, but otherwise I'd call this all working as intended.

[tarcieri]

It looks like ring begins to successfully verify the signatures as of #294.

So it seems like the problem is rsa v0.8 was using "unsalted" PSS by default, which was removed in #294.

I'm going to go ahead and close this issue, but I think it would still be good to add interop tests with ring.

[vthib]

@tarcieri I have a more precise usecase that does not look related to either using a signature generated by rsa 0.8, or using unsalted PSS.

The reproduction can be done as such:

• generate a rsa key, no passphrase:

$ ssh-keygen -b 4096 -t rsa -N "" -m pkcs8 -f privkey
$ ssh-keygen -f privkey -e -m pkcs8 > pubkey

• use python cryptography lib to generate a signature

#!/usr/bin/env python
import os

from cryptography.hazmat.backends import default_backend
from cryptography.hazmat.primitives import hashes, serialization
from cryptography.hazmat.primitives.asymmetric import padding

with open("privkey", "rb") as f:
 priv = f.read()

private_key = serialization.load_pem_private_key(
 priv, password=None, backend=default_backend()
)

with open("data", "rb") as f:
 data = f.read()

signature = private_key.sign(
 data,
 padding.PSS(mgf=padding.MGF1(hashes.SHA256()), salt_length=padding.PSS.MAX_LENGTH),
 hashes.SHA256(),
)

with open("data.sig", "wb") as f:
 f.write(signature)

This is afaict the same code as in the documentation: https://cryptography.io/en/42.0.5/hazmat/primitives/asymmetric/rsa/#signing

And then use this code to verify it:

use rsa::pkcs8::DecodePublicKey;
use sha2::Digest;

fn main() {
 let mut args = std::env::args().skip(1);

 let pubkey = std::fs::read_to_string("pubkey").unwrap();
 let file = std::fs::read(args.next().unwrap()).unwrap();
 let sig = std::fs::read(args.next().unwrap()).unwrap();

 let hashed_data = sha2::Sha256::digest(file);

 let key = rsa::RsaPublicKey::from_public_key_pem(&pubkey).unwrap();
 match key.verify(
 rsa::Pss::new::<sha2::Sha256>(),
 &*hashed_data,
 &sig,
 ) {
 Ok(()) => println!("verification OK"),
 Err(err) => println!("verification KO: {:?}", err),
 }
}

And can be run like this: verify data data.sig.
With rsa 0.8, this prints "verification OK". With rsa 0.9, this prints KO.
As we can see in the configuration in the python code, a salted PSS is used afaict.

[tarcieri]

Can you try explicitly specifying the salt length in the PSS constructor? i.e.:

rsa::Pss::new_with_salt::<sha2::Sha256>(PSS_MAX_LENGTH)

Though I'm unclear why it would've verified with rsa v0.8.

It would also help a lot if you could test locally to see if #294 was the change that caused this as well.

[vthib]

I can confirm that setting the salt length works:

// Same algorithm as python's cryptography for padding.PSS.MAX_LENGTH salt size
let key_len = key.size() * 8;
let emlen = (key_len + 6) / 8;
let salt_length = emlen - <sha2::Sha256 as Digest>::output_size() - 2;

match key.verify(
 rsa::Pss::new_with_salt::<sha2::Sha256>(salt_length),
 ...

I can also confirm that #294 is the change that broke validation.
From my understanding, the previous version used to try to find the salt size, which is not really the same as not using any salt at all. so #294 did not just remove support for "unsalted" PSS, but also removed the auto detection of the salt size on verification.

The new version defaults to the digest size, which is padding.PSS.DIGEST_LENGTH in the python lib, and not padding.PSS.MAX_LENGTH, leading to the validation error.

[tarcieri]

Yes, the new version uses the digest length as the default, which as far as I can tell is the sanest choice of a default.