Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/tls12.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ const GCM_EXPLICIT_NONCE_LENGTH: usize = 8;
const GCM_IMPLICIT_NONCE_LENGTH: usize = 4;

static ECDSA_SCHEMES: &[SignatureScheme] = &[
SignatureScheme::ED25519,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙈

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not actually supported, and GPT claims that's fine from a FIPS perspective. This removal stops advertising it as supported.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, yeah, like this actually seems bad... like fundamentally not possible since the whole thing is that ED25519 is a different scheme from ECDSA.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[GPT 5.5] Evaluated against the current provider: Ed25519 is widely used, but this CNG-backed provider does not implement Ed25519 signing or verification (src/signer/ec.rs explicitly notes CNG does not support it here, and src/verify.rs has Ed25519 commented out). So I kept the removal from the TLS 1.2 ECDSA scheme list; advertising it would imply a capability the provider cannot satisfy.

SignatureScheme::ECDSA_NISTP521_SHA512,
SignatureScheme::ECDSA_NISTP384_SHA384,
SignatureScheme::ECDSA_NISTP256_SHA256,
Expand Down Expand Up @@ -339,3 +338,13 @@ impl MessageDecrypter for ChaCha20Poly1305Crypter {
Ok(msg.into_plain_message())
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn tls12_ecdsa_sign_schemes_do_not_advertise_ed25519() {
assert!(!ECDSA_SCHEMES.contains(&SignatureScheme::ED25519));
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is pointless, remove it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[GPT 5.5] Removed the test in 094eead.

}
141 changes: 141 additions & 0 deletions src/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,11 @@ pub static SUPPORTED_SIG_ALGS: WebPkiSupportedAlgorithms = WebPkiSupportedAlgori
RSA_PSS_SHA384,
RSA_PSS_SHA256,
RSA_PKCS1_SHA512,
RSA_PKCS1_SHA512_ABSENT_PARAMS,
RSA_PKCS1_SHA384,
RSA_PKCS1_SHA384_ABSENT_PARAMS,
RSA_PKCS1_SHA256,
RSA_PKCS1_SHA256_ABSENT_PARAMS,
],
mapping: &[
//Note: for TLS1.2 the curve is not fixed by SignatureScheme. For TLS1.3 it is.
Expand Down Expand Up @@ -69,6 +72,18 @@ pub(crate) static RSA_PKCS1_SHA256: &dyn SignatureVerificationAlgorithm = &Verif
params: Params::Rsa(RsaPadding::PKCS1),
};

/// RSA PKCS#1 1.5 signatures using SHA-256 with absent AlgorithmIdentifier parameters.
pub(crate) static RSA_PKCS1_SHA256_ABSENT_PARAMS: &dyn SignatureVerificationAlgorithm =
&VerificationAlgorithm {
display_name: "RSA_PKCS1_SHA256_ABSENT_PARAMS",
public_key_alg_id: alg_id::RSA_ENCRYPTION,
signature_alg_id: AlgorithmIdentifier::from_slice(&[
0x30, 0x0b, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0b,
]),
hash: SHA256,
params: Params::Rsa(RsaPadding::PKCS1),
};

/// RSA PKCS#1 1.5 signatures using SHA-384.
pub(crate) static RSA_PKCS1_SHA384: &dyn SignatureVerificationAlgorithm = &VerificationAlgorithm {
display_name: "RSA_PKCS1_SHA384",
Expand All @@ -78,6 +93,18 @@ pub(crate) static RSA_PKCS1_SHA384: &dyn SignatureVerificationAlgorithm = &Verif
params: Params::Rsa(RsaPadding::PKCS1),
};

/// RSA PKCS#1 1.5 signatures using SHA-384 with absent AlgorithmIdentifier parameters.
pub(crate) static RSA_PKCS1_SHA384_ABSENT_PARAMS: &dyn SignatureVerificationAlgorithm =
&VerificationAlgorithm {
display_name: "RSA_PKCS1_SHA384_ABSENT_PARAMS",
public_key_alg_id: alg_id::RSA_ENCRYPTION,
signature_alg_id: AlgorithmIdentifier::from_slice(&[
0x30, 0x0b, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0c,
]),
hash: SHA384,
params: Params::Rsa(RsaPadding::PKCS1),
};

/// RSA PKCS#1 1.5 signatures using SHA-512.
pub(crate) static RSA_PKCS1_SHA512: &dyn SignatureVerificationAlgorithm = &VerificationAlgorithm {
display_name: "RSA_PKCS1_SHA512",
Expand All @@ -87,6 +114,18 @@ pub(crate) static RSA_PKCS1_SHA512: &dyn SignatureVerificationAlgorithm = &Verif
params: Params::Rsa(RsaPadding::PKCS1),
};

/// RSA PKCS#1 1.5 signatures using SHA-512 with absent AlgorithmIdentifier parameters.
pub(crate) static RSA_PKCS1_SHA512_ABSENT_PARAMS: &dyn SignatureVerificationAlgorithm =
&VerificationAlgorithm {
display_name: "RSA_PKCS1_SHA512_ABSENT_PARAMS",
public_key_alg_id: alg_id::RSA_ENCRYPTION,
signature_alg_id: AlgorithmIdentifier::from_slice(&[
0x30, 0x0b, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0d,
]),
hash: SHA512,
params: Params::Rsa(RsaPadding::PKCS1),
};

/// RSA PSS signatures using SHA-256.
pub(crate) static RSA_PSS_SHA256: &dyn SignatureVerificationAlgorithm = &VerificationAlgorithm {
display_name: "RSA_PSS_SHA256",
Expand Down Expand Up @@ -210,6 +249,22 @@ enum Params {
unsafe impl Send for Params {}
unsafe impl Sync for Params {}

const RSA_MIN_MODULUS_BITS: usize = 2048;
const RSA_MAX_MODULUS_BITS: usize = 8192;
Comment on lines +259 to +260

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have the agent try and provide first-party documentation sources for stuff like this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[GPT 5.5] Added a source note in cddb6ca. The first-party source I used is rustls-webpki itself: its RSA verification algorithms are named/defined for 2048-8192-bit keys, e.g. RSA_PKCS1_2048_8192_SHA256 and the matching PSS variants in rustls-webpki-0.103.13/src/ring_algs.rs / aws_lc_rs_algs.rs. This provider now mirrors that policy before passing RSA public keys to CNG.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[GPT 5.5] Updated in d132442 to include a direct source link in the code comment. It now points at rustls-webpki 0.103.13’s first-party RSA verification algorithm definitions for 2048-8192-bit keys: https://docs.rs/rustls-webpki/0.103.13/src/rustls_webpki/aws_lc_rs_algs.rs.html#162-182

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It took three tries, but it got there.


fn rsa_public_key_allowed_by_webpki(key: &RsaPublicKey<'_>) -> bool {
(RSA_MIN_MODULUS_BITS..=RSA_MAX_MODULUS_BITS)
.contains(&rsa_modulus_bit_len(key.modulus.as_bytes()))
}

fn rsa_modulus_bit_len(modulus: &[u8]) -> usize {
let Some(first) = modulus.first() else {
return 0;
};

(modulus.len() - 1) * 8 + (u8::BITS as usize - first.leading_zeros() as usize)
Comment thread
datadog-prod-us1-5[bot] marked this conversation as resolved.
Outdated
}

#[derive(Debug)]
enum RsaPadding {
PKCS1,
Expand All @@ -236,6 +291,9 @@ impl<const HASH_SIZE: usize> SignatureVerificationAlgorithm for VerificationAlgo
match &self.params {
Params::Rsa(padding) => {
let key = RsaPublicKey::try_from(public_key).map_err(|_| InvalidSignature)?;
if !rsa_public_key_allowed_by_webpki(&key) {
return Err(InvalidSignature);
}
let handle = import_rsa_public_key(&key).map_err(|_| InvalidSignature)?;

match padding {
Expand Down Expand Up @@ -343,6 +401,89 @@ mod tests {
use super::*;
use wycheproof::TestResult;

const RSA_PKCS1_SHA256_ABSENT_PARAMS_DER: &[u8] = &[
0x30, 0x0b, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0b,
];
const RSA_PKCS1_SHA384_ABSENT_PARAMS_DER: &[u8] = &[
0x30, 0x0b, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0c,
];
const RSA_PKCS1_SHA512_ABSENT_PARAMS_DER: &[u8] = &[
0x30, 0x0b, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0d,
];

#[test]
fn supported_algorithms_include_rsa_pkcs1_absent_parameter_variants() {
for signature_alg_id in [
AlgorithmIdentifier::from_slice(RSA_PKCS1_SHA256_ABSENT_PARAMS_DER),
AlgorithmIdentifier::from_slice(RSA_PKCS1_SHA384_ABSENT_PARAMS_DER),
AlgorithmIdentifier::from_slice(RSA_PKCS1_SHA512_ABSENT_PARAMS_DER),
] {
assert!(SUPPORTED_SIG_ALGS
.all
.iter()
.any(|alg| alg.signature_alg_id() == signature_alg_id));
}
}

#[test]
fn rsa_public_key_policy_matches_webpki_2048_to_8192_bit_bounds() {
let key_2047 = rsa_public_key_with_modulus(&modulus_with_bit_len(2047));
let key_2048 = rsa_public_key_with_modulus(&modulus_with_bit_len(2048));
let key_8192 = rsa_public_key_with_modulus(&modulus_with_bit_len(8192));
let key_8193 = rsa_public_key_with_modulus(&modulus_with_bit_len(8193));

assert!(!rsa_public_key_allowed_by_webpki(&key_2047));
assert!(rsa_public_key_allowed_by_webpki(&key_2048));
assert!(rsa_public_key_allowed_by_webpki(&key_8192));
assert!(!rsa_public_key_allowed_by_webpki(&key_8193));
}

fn rsa_public_key_with_modulus(modulus: &[u8]) -> RsaPublicKey<'static> {
let mut der = Vec::new();
append_der_integer(&mut der, modulus);
append_der_integer(&mut der, &[0x01, 0x00, 0x01]);

let mut sequence = Vec::new();
sequence.push(0x30);
append_der_len(&mut sequence, der.len());
sequence.extend_from_slice(&der);

let sequence: &'static [u8] = Box::leak(sequence.into_boxed_slice());
RsaPublicKey::try_from(sequence).unwrap()
}

fn modulus_with_bit_len(bit_len: usize) -> Vec<u8> {
let len = bit_len.div_ceil(8);
let mut modulus = vec![0xff; len];
modulus[0] = 1 << ((bit_len - 1) % 8);
modulus
}

fn append_der_integer(der: &mut Vec<u8>, value: &[u8]) {
der.push(0x02);
let needs_leading_zero = value.first().is_some_and(|byte| byte & 0x80 != 0);
append_der_len(der, value.len() + usize::from(needs_leading_zero));
if needs_leading_zero {
der.push(0);
}
der.extend_from_slice(value);
}

fn append_der_len(der: &mut Vec<u8>, len: usize) {
if len < 128 {
der.push(len as u8);
Comment thread
datadog-prod-us1-5[bot] marked this conversation as resolved.
Outdated
return;
}

let len_bytes = len.to_be_bytes();
let first = len_bytes
.iter()
.position(|byte| *byte != 0)
.unwrap_or(len_bytes.len() - 1);
der.push(0x80 | (len_bytes.len() - first) as u8);
Comment thread
datadog-prod-us1-4[bot] marked this conversation as resolved.
Outdated
der.extend_from_slice(&len_bytes[first..]);
}

#[test]
fn test_open_ssl_algorithm_debug() {
assert_eq!(
Expand Down
Loading