Skip to content

Commit 6249132

Browse files
committed
Reject PSS salt lengths that do not fit in a u8
`get_pss_signature_algo_id` cast the salt length to `u8` with `as`, silently truncating any value >= 256 (e.g. 256 -> 0). The resulting RSASSA-PSS `AlgorithmIdentifier` then advertised the wrong salt length, so signatures produced with such a salt length failed verification. Take the salt length as `usize` in the helper and convert it with `u8::try_from`, returning a DER `Overflow` error when it does not fit, rather than mis-encoding it. Both `SigningKey` and `BlindedSigningKey` funnel through this helper, so both signing paths are covered. Fixes #703.
1 parent 4a6006f commit 6249132

3 files changed

Lines changed: 35 additions & 6 deletions

File tree

src/pss.rs

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,10 @@ use {
3939
crate::encoding::ID_RSASSA_PSS,
4040
const_oid::AssociatedOid,
4141
pkcs1::RsaPssParams,
42-
spki::{der::Any, AlgorithmIdentifierOwned},
42+
spki::{
43+
der::{Any, ErrorKind},
44+
AlgorithmIdentifierOwned,
45+
},
4346
};
4447

4548
/// Digital signatures using PSS padding.
@@ -272,15 +275,18 @@ pub fn get_default_pss_signature_algo_id<D>() -> spki::Result<AlgorithmIdentifie
272275
where
273276
D: Digest + AssociatedOid,
274277
{
275-
let salt_len: u8 = <D as Digest>::output_size() as u8;
276-
get_pss_signature_algo_id::<D>(salt_len)
278+
get_pss_signature_algo_id::<D>(<D as Digest>::output_size())
277279
}
278280

279281
#[cfg(feature = "encoding")]
280-
fn get_pss_signature_algo_id<D>(salt_len: u8) -> spki::Result<AlgorithmIdentifierOwned>
282+
fn get_pss_signature_algo_id<D>(salt_len: usize) -> spki::Result<AlgorithmIdentifierOwned>
281283
where
282284
D: Digest + AssociatedOid,
283285
{
286+
// The salt length is encoded as a single byte in `RsaPssParams`, so reject values that
287+
// would not round-trip rather than silently truncating them (which produced an
288+
// `AlgorithmIdentifier` advertising the wrong salt length and broke verification). See #703.
289+
let salt_len = u8::try_from(salt_len).map_err(|_| ErrorKind::Overflow.to_error())?;
284290
let pss_params = RsaPssParams::new::<D>(salt_len);
285291

286292
Ok(AlgorithmIdentifierOwned {
@@ -672,4 +678,27 @@ tAboUGBxTDq3ZroNism3DaMIbKPyYrAqhKov1h5V
672678
.expect("verification to succeed");
673679
}
674680
}
681+
682+
// Regression test for #703: a salt length that does not fit in a `u8` was silently truncated
683+
// when building the PSS `AlgorithmIdentifier` (e.g. 256 wrapped to 0), producing an identifier
684+
// that advertised the wrong salt length and broke signature verification. Such lengths must
685+
// now be rejected instead of mis-encoded.
686+
#[test]
687+
fn signature_algorithm_identifier_rejects_oversized_salt_len() {
688+
use spki::DynSignatureAlgorithmIdentifier;
689+
690+
let priv_key = get_private_key();
691+
692+
// 255 is the largest representable salt length and must still round-trip.
693+
let ok_key = SigningKey::<Sha1>::new_with_salt_len(priv_key.clone(), 255);
694+
assert!(ok_key.signature_algorithm_identifier().is_ok());
695+
696+
// 256 previously truncated to 0; it must now be rejected rather than silently mis-encoded.
697+
let bad_key = SigningKey::<Sha1>::new_with_salt_len(priv_key.clone(), 256);
698+
assert!(bad_key.signature_algorithm_identifier().is_err());
699+
700+
// The blinded signing key funnels through the same helper and must behave identically.
701+
let bad_blinded = BlindedSigningKey::<Sha1>::new_with_salt_len(priv_key, 256);
702+
assert!(bad_blinded.signature_algorithm_identifier().is_err());
703+
}
675704
}

src/pss/blinded_signing_key.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ where
184184
D: Digest + AssociatedOid,
185185
{
186186
fn signature_algorithm_identifier(&self) -> spki::Result<AlgorithmIdentifierOwned> {
187-
get_pss_signature_algo_id::<D>(self.salt_len as u8)
187+
get_pss_signature_algo_id::<D>(self.salt_len)
188188
}
189189
}
190190

src/pss/signing_key.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ where
221221
D: Digest + AssociatedOid,
222222
{
223223
fn signature_algorithm_identifier(&self) -> spki::Result<AlgorithmIdentifierOwned> {
224-
get_pss_signature_algo_id::<D>(self.salt_len as u8)
224+
get_pss_signature_algo_id::<D>(self.salt_len)
225225
}
226226
}
227227

0 commit comments

Comments
 (0)