Skip to content

Commit 8b6d423

Browse files
authored
PSS: Improve interoperability with optional auto salt length detection during verification (#546)
This PR adds an optional method to automatically detect the salt length during RSA-PSS signature verification. Should Fix: #361 and similar situations. ### Problem The crate currently requires a fixed salt length for PSS verification. This prevents verification of signatures where the salt length is not known beforehand, a situation not uncommon in interoperability contexts. This capability was previously available but was removed in PR #294. ### Solution This change re-introduces salt length auto-detection as an explicit, opt-in feature. A new constructor, `VerifyingKey::new_with_auto_salt_len`, creates a verifier that performs this detection during verification. * The change is opt-in and does not affect default behavior. * It improves interoperability by handling signatures with variable / unknown salt lengths. * The salt detection logic is implemented in constant(-ish) time to resist timing attacks. ### Commit Structure This PR consists of three commits: 1. **test:** Adds a failing unit test to demonstrate the issue. 2. **feat:** Implements `new_with_auto_salt_len` and re-introduces the previous detection logic. 3. **fix:** Updates the detection logic to be less prone to timing attacks. Because salt length auto-detection is OpenSSL's default, it's likely that many systems were built without a mechanism to enforce or communicate a fixed salt length. Without this PR, it was impossible to reliably use this crate in my current project, and I suspect others face the same blocker. I've lost quite a bit of time dealing with seemingly random signature verification failures until I realized the crux of the problem.
1 parent 77b5bc5 commit 8b6d423

File tree

5 files changed

+113
-20
lines changed

5 files changed

+113
-20
lines changed

src/algorithms/pss.rs

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
1212
use alloc::vec::Vec;
1313
use digest::{Digest, DynDigest, FixedOutputReset};
14-
use subtle::{Choice, ConstantTimeEq};
14+
use subtle::{Choice, ConditionallySelectable, ConstantTimeEq};
1515

1616
use super::mgf::{mgf1_xor, mgf1_xor_digest};
1717
use crate::errors::{Error, Result};
@@ -170,7 +170,7 @@ fn emsa_pss_verify_pre<'a>(
170170
m_hash: &[u8],
171171
em: &'a mut [u8],
172172
em_bits: usize,
173-
s_len: usize,
173+
s_len: Option<usize>,
174174
h_len: usize,
175175
) -> Result<(&'a mut [u8], &'a mut [u8])> {
176176
// 1. If the length of M is greater than the input limitation for the
@@ -182,10 +182,12 @@ fn emsa_pss_verify_pre<'a>(
182182
return Err(Error::Verification);
183183
}
184184

185-
// 3. If emLen < hLen + sLen + 2, output "inconsistent" and stop.
186185
let em_len = em.len(); //(em_bits + 7) / 8;
187-
if em_len < h_len + s_len + 2 {
188-
return Err(Error::Verification);
186+
if let Some(s_len) = s_len {
187+
// 3. If emLen < hLen + sLen + 2, output "inconsistent" and stop.
188+
if em_len < h_len + s_len + 2 {
189+
return Err(Error::Verification);
190+
}
189191
}
190192

191193
// 4. If the rightmost octet of EM does not have hexadecimal value
@@ -227,10 +229,48 @@ fn emsa_pss_verify_salt(db: &[u8], em_len: usize, s_len: usize, h_len: usize) ->
227229
valid & rest[0].ct_eq(&0x01)
228230
}
229231

232+
/// Detect salt length by scanning DB for the 0x01 separator byte.
233+
/// Returns (s_len, valid) where s_len is 0 on failure.
234+
fn emsa_pss_get_salt_len(db: &[u8], em_len: usize, h_len: usize) -> (usize, Choice) {
235+
let em_len = em_len as u32;
236+
let h_len = h_len as u32;
237+
let max_scan_len = em_len - h_len - 2;
238+
239+
let mut separator_pos = 0u32;
240+
let mut found_separator = Choice::from(0u8);
241+
let mut padding_valid = Choice::from(1u8);
242+
243+
// Single forward scan to find separator and validate padding
244+
for i in 0..=max_scan_len {
245+
let byte_val = db[i as usize];
246+
let is_zero = byte_val.ct_eq(&0x00);
247+
let is_separator = byte_val.ct_eq(&0x01);
248+
let is_invalid = !(is_zero | is_separator);
249+
250+
// Update separator position if we found one and haven't found one before
251+
let should_update_pos = is_separator & !found_separator;
252+
separator_pos = u32::conditional_select(&separator_pos, &i, should_update_pos);
253+
found_separator =
254+
Choice::conditional_select(&found_separator, &Choice::from(1u8), should_update_pos);
255+
256+
// Padding is invalid if we see a non-zero, non-separator byte before finding separator
257+
let corrupts_padding = is_invalid & !found_separator;
258+
padding_valid &= !corrupts_padding;
259+
}
260+
261+
let salt_len = max_scan_len.wrapping_sub(separator_pos);
262+
let final_valid = found_separator & padding_valid;
263+
264+
// Return 0 length on failure
265+
let result_len = u32::conditional_select(&0u32, &salt_len, final_valid);
266+
267+
(result_len as usize, final_valid)
268+
}
269+
230270
pub(crate) fn emsa_pss_verify(
231271
m_hash: &[u8],
232272
em: &mut [u8],
233-
s_len: usize,
273+
s_len: Option<usize>,
234274
hash: &mut dyn DynDigest,
235275
key_bits: usize,
236276
) -> Result<()> {
@@ -252,7 +292,10 @@ pub(crate) fn emsa_pss_verify(
252292
// to zero.
253293
db[0] &= 0xFF >> /*uint*/(8 * em_len - em_bits);
254294

255-
let salt_valid = emsa_pss_verify_salt(db, em_len, s_len, h_len);
295+
let (s_len, salt_valid) = match s_len {
296+
Some(s_len) => (s_len, emsa_pss_verify_salt(db, em_len, s_len, h_len)),
297+
None => emsa_pss_get_salt_len(db, em_len, h_len),
298+
};
256299

257300
// 11. Let salt be the last s_len octets of DB.
258301
let salt = &db[db.len() - s_len..];
@@ -281,7 +324,7 @@ pub(crate) fn emsa_pss_verify(
281324
pub(crate) fn emsa_pss_verify_digest<D>(
282325
m_hash: &[u8],
283326
em: &mut [u8],
284-
s_len: usize,
327+
s_len: Option<usize>,
285328
key_bits: usize,
286329
) -> Result<()>
287330
where
@@ -307,7 +350,10 @@ where
307350
// to zero.
308351
db[0] &= 0xFF >> /*uint*/(8 * em_len - em_bits);
309352

310-
let salt_valid = emsa_pss_verify_salt(db, em_len, s_len, h_len);
353+
let (s_len, salt_valid) = match s_len {
354+
Some(s_len) => (s_len, emsa_pss_verify_salt(db, em_len, s_len, h_len)),
355+
None => emsa_pss_get_salt_len(db, em_len, h_len),
356+
};
311357

312358
// 11. Let salt be the last s_len octets of DB.
313359
let salt = &db[db.len() - s_len..];

src/pss.rs

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ pub struct Pss {
5151
pub digest: Box<dyn DynDigest + Send + Sync>,
5252

5353
/// Salt length.
54-
pub salt_len: usize,
54+
/// Required for signing, optional for verifying.
55+
pub salt_len: Option<usize>,
5556
}
5657

5758
impl Pss {
@@ -66,7 +67,7 @@ impl Pss {
6667
Self {
6768
blinded: false,
6869
digest: Box::new(T::new()),
69-
salt_len: len,
70+
salt_len: Some(len),
7071
}
7172
}
7273

@@ -84,7 +85,7 @@ impl Pss {
8485
Self {
8586
blinded: true,
8687
digest: Box::new(T::new()),
87-
salt_len: len,
88+
salt_len: Some(len),
8889
}
8990
}
9091
}
@@ -101,7 +102,7 @@ impl SignatureScheme for Pss {
101102
self.blinded,
102103
priv_key,
103104
hashed,
104-
self.salt_len,
105+
self.salt_len.expect("salt_len to be Some"),
105106
&mut *self.digest,
106107
)
107108
}
@@ -134,7 +135,7 @@ pub(crate) fn verify(
134135
sig: &BoxedUint,
135136
sig_len: usize,
136137
digest: &mut dyn DynDigest,
137-
salt_len: usize,
138+
salt_len: Option<usize>,
138139
) -> Result<()> {
139140
if sig_len != pub_key.size() {
140141
return Err(Error::Verification);
@@ -149,7 +150,7 @@ pub(crate) fn verify_digest<D>(
149150
pub_key: &RsaPublicKey,
150151
hashed: &[u8],
151152
sig: &BoxedUint,
152-
salt_len: usize,
153+
salt_len: Option<usize>,
153154
) -> Result<()>
154155
where
155156
D: Digest + FixedOutputReset,
@@ -261,6 +262,7 @@ mod test {
261262
use crate::pss::{BlindedSigningKey, Pss, Signature, SigningKey, VerifyingKey};
262263
use crate::{RsaPrivateKey, RsaPublicKey};
263264

265+
use crate::traits::PublicKeyParts;
264266
use hex_literal::hex;
265267
use pkcs1::DecodeRsaPrivateKey;
266268
use rand_chacha::{rand_core::SeedableRng, ChaCha8Rng};
@@ -622,4 +624,39 @@ tAboUGBxTDq3ZroNism3DaMIbKPyYrAqhKov1h5V
622624
.expect("failed to verify");
623625
}
624626
}
627+
628+
#[test]
629+
// Tests the case where the salt length used for signing differs from the default length
630+
// while the verifier uses auto-detection.
631+
fn test_sign_and_verify_pss_differing_salt_len() {
632+
let priv_key = get_private_key();
633+
634+
let tests = ["test\n"];
635+
let mut rng = ChaCha8Rng::from_seed([42; 32]);
636+
637+
// signing keys using different salt lengths
638+
let signing_keys = [
639+
// default salt length
640+
SigningKey::<Sha1>::new(priv_key.clone()),
641+
// maximum salt length
642+
SigningKey::<Sha1>::new_with_salt_len(
643+
priv_key.clone(),
644+
priv_key.size() - Sha1::output_size() - 2,
645+
),
646+
// unsalted
647+
SigningKey::<Sha1>::new_with_salt_len(priv_key.clone(), 0),
648+
];
649+
650+
// verifying key uses default salt length strategy
651+
let verifying_key = VerifyingKey::<Sha1>::new_with_auto_salt_len(priv_key.to_public_key());
652+
653+
for test in tests {
654+
for signing_key in &signing_keys {
655+
let sig = signing_key.sign_with_rng(&mut rng, test.as_bytes());
656+
verifying_key
657+
.verify(test.as_bytes(), &sig)
658+
.expect("verification to succeed");
659+
}
660+
}
661+
}
625662
}

src/pss/blinded_signing_key.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ where
219219
fn verifying_key(&self) -> Self::VerifyingKey {
220220
VerifyingKey {
221221
inner: self.inner.to_public_key(),
222-
salt_len: self.salt_len,
222+
salt_len: Some(self.salt_len),
223223
phantom: Default::default(),
224224
}
225225
}

src/pss/signing_key.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ where
251251
fn verifying_key(&self) -> Self::VerifyingKey {
252252
VerifyingKey {
253253
inner: self.inner.to_public_key(),
254-
salt_len: self.salt_len,
254+
salt_len: Some(self.salt_len),
255255
phantom: Default::default(),
256256
}
257257
}

src/pss/verifying_key.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ where
2727
D: Digest,
2828
{
2929
pub(super) inner: RsaPublicKey,
30-
pub(super) salt_len: usize,
30+
pub(super) salt_len: Option<usize>,
3131
pub(super) phantom: PhantomData<D>,
3232
}
3333

@@ -45,13 +45,23 @@ where
4545
pub fn new_with_salt_len(key: RsaPublicKey, salt_len: usize) -> Self {
4646
Self {
4747
inner: key,
48-
salt_len,
48+
salt_len: Some(salt_len),
49+
phantom: Default::default(),
50+
}
51+
}
52+
53+
/// Create a new RSASSA-PSS verifying key.
54+
/// Attempts to automatically detect the salt length.
55+
pub fn new_with_auto_salt_len(key: RsaPublicKey) -> Self {
56+
Self {
57+
inner: key,
58+
salt_len: None,
4959
phantom: Default::default(),
5060
}
5161
}
5262

5363
/// Return specified salt length for this key
54-
pub fn salt_len(&self) -> usize {
64+
pub fn salt_len(&self) -> Option<usize> {
5565
self.salt_len
5666
}
5767
}

0 commit comments

Comments
 (0)