Skip to content

Commit

Permalink
allow null params in AlgorithmIdentifiers with SHA hash function OIDs (
Browse files Browse the repository at this point in the history
…#8974)

RFC 4055 section 2.1 states "All implementations MUST accept both
NULL and absent parameters as legal and equivalent encodings".

It also makes some somewhat conflicting statements after that, but
LibreSSL omits the null params for PSS, and OpenSSL parses this
without issue so tolerance it is.
  • Loading branch information
reaperhulk committed May 27, 2023
1 parent 288c302 commit 93c96b7
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 36 deletions.
3 changes: 3 additions & 0 deletions docs/development/test-vectors.rst
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,9 @@ Custom X.509 Vectors
OID to not match the outer signature algorithm OID.
* ``ms-certificate-template.pem`` - A certificate with a ``msCertificateTemplate``
extension.
* ``rsa_pss_sha256_no_null.pem`` - A certificate with an RSA PSS signature
with no encoded ``NULL`` for the PSS hash algorithm parameters. This certificate
was generated by LibreSSL.

Custom X.509 Request Vectors
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down
20 changes: 10 additions & 10 deletions src/rust/cryptography-x509/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,23 +22,23 @@ impl AlgorithmIdentifier<'_> {
#[derive(asn1::Asn1DefinedByRead, asn1::Asn1DefinedByWrite, PartialEq, Eq, Hash, Clone, Debug)]
pub enum AlgorithmParameters<'a> {
#[defined_by(oid::SHA1_OID)]
Sha1(asn1::Null),
Sha1(Option<asn1::Null>),
#[defined_by(oid::SHA224_OID)]
Sha224(asn1::Null),
Sha224(Option<asn1::Null>),
#[defined_by(oid::SHA256_OID)]
Sha256(asn1::Null),
Sha256(Option<asn1::Null>),
#[defined_by(oid::SHA384_OID)]
Sha384(asn1::Null),
Sha384(Option<asn1::Null>),
#[defined_by(oid::SHA512_OID)]
Sha512(asn1::Null),
Sha512(Option<asn1::Null>),
#[defined_by(oid::SHA3_224_OID)]
Sha3_224(asn1::Null),
Sha3_224(Option<asn1::Null>),
#[defined_by(oid::SHA3_256_OID)]
Sha3_256(asn1::Null),
Sha3_256(Option<asn1::Null>),
#[defined_by(oid::SHA3_384_OID)]
Sha3_384(asn1::Null),
Sha3_384(Option<asn1::Null>),
#[defined_by(oid::SHA3_512_OID)]
Sha3_512(asn1::Null),
Sha3_512(Option<asn1::Null>),

#[defined_by(oid::ED25519_OID)]
Ed25519,
Expand Down Expand Up @@ -227,7 +227,7 @@ pub struct DHParams<'a> {
// RSA-PSS ASN.1 default hash algorithm
pub const PSS_SHA1_HASH_ALG: AlgorithmIdentifier<'_> = AlgorithmIdentifier {
oid: asn1::DefinedByMarker::marker(),
params: AlgorithmParameters::Sha1(()),
params: AlgorithmParameters::Sha1(Some(())),
};

// This is defined as an AlgorithmIdentifier in RFC 4055,
Expand Down
20 changes: 10 additions & 10 deletions src/rust/src/x509/ocsp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ pub(crate) static ALGORITHM_PARAMETERS_TO_HASH: Lazy<
HashMap<common::AlgorithmParameters<'_>, &str>,
> = Lazy::new(|| {
let mut h = HashMap::new();
h.insert(common::AlgorithmParameters::Sha1(()), "SHA1");
h.insert(common::AlgorithmParameters::Sha224(()), "SHA224");
h.insert(common::AlgorithmParameters::Sha256(()), "SHA256");
h.insert(common::AlgorithmParameters::Sha384(()), "SHA384");
h.insert(common::AlgorithmParameters::Sha512(()), "SHA512");
h.insert(common::AlgorithmParameters::Sha1(Some(())), "SHA1");
h.insert(common::AlgorithmParameters::Sha224(Some(())), "SHA224");
h.insert(common::AlgorithmParameters::Sha256(Some(())), "SHA256");
h.insert(common::AlgorithmParameters::Sha384(Some(())), "SHA384");
h.insert(common::AlgorithmParameters::Sha512(Some(())), "SHA512");
h
});

Expand All @@ -30,35 +30,35 @@ pub(crate) static HASH_NAME_TO_ALGORITHM_IDENTIFIERS: Lazy<
"sha1",
common::AlgorithmIdentifier {
oid: asn1::DefinedByMarker::marker(),
params: common::AlgorithmParameters::Sha1(()),
params: common::AlgorithmParameters::Sha1(Some(())),
},
);
h.insert(
"sha224",
common::AlgorithmIdentifier {
oid: asn1::DefinedByMarker::marker(),
params: common::AlgorithmParameters::Sha224(()),
params: common::AlgorithmParameters::Sha224(Some(())),
},
);
h.insert(
"sha256",
common::AlgorithmIdentifier {
oid: asn1::DefinedByMarker::marker(),
params: common::AlgorithmParameters::Sha256(()),
params: common::AlgorithmParameters::Sha256(Some(())),
},
);
h.insert(
"sha384",
common::AlgorithmIdentifier {
oid: asn1::DefinedByMarker::marker(),
params: common::AlgorithmParameters::Sha384(()),
params: common::AlgorithmParameters::Sha384(Some(())),
},
);
h.insert(
"sha512",
common::AlgorithmIdentifier {
oid: asn1::DefinedByMarker::marker(),
params: common::AlgorithmParameters::Sha512(()),
params: common::AlgorithmParameters::Sha512(Some(())),
},
);
h
Expand Down
44 changes: 28 additions & 16 deletions src/rust/src/x509/sign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -507,14 +507,14 @@ fn identify_alg_params_for_hash_type(
hash_type: HashType,
) -> pyo3::PyResult<common::AlgorithmParameters<'static>> {
match hash_type {
HashType::Sha224 => Ok(common::AlgorithmParameters::Sha224(())),
HashType::Sha256 => Ok(common::AlgorithmParameters::Sha256(())),
HashType::Sha384 => Ok(common::AlgorithmParameters::Sha384(())),
HashType::Sha512 => Ok(common::AlgorithmParameters::Sha512(())),
HashType::Sha3_224 => Ok(common::AlgorithmParameters::Sha3_224(())),
HashType::Sha3_256 => Ok(common::AlgorithmParameters::Sha3_256(())),
HashType::Sha3_384 => Ok(common::AlgorithmParameters::Sha3_384(())),
HashType::Sha3_512 => Ok(common::AlgorithmParameters::Sha3_512(())),
HashType::Sha224 => Ok(common::AlgorithmParameters::Sha224(Some(()))),
HashType::Sha256 => Ok(common::AlgorithmParameters::Sha256(Some(()))),
HashType::Sha384 => Ok(common::AlgorithmParameters::Sha384(Some(()))),
HashType::Sha512 => Ok(common::AlgorithmParameters::Sha512(Some(()))),
HashType::Sha3_224 => Ok(common::AlgorithmParameters::Sha3_224(Some(()))),
HashType::Sha3_256 => Ok(common::AlgorithmParameters::Sha3_256(Some(()))),
HashType::Sha3_384 => Ok(common::AlgorithmParameters::Sha3_384(Some(()))),
HashType::Sha3_512 => Ok(common::AlgorithmParameters::Sha3_512(Some(()))),
HashType::None => Err(pyo3::exceptions::PyTypeError::new_err(
"Algorithm must be a registered hash algorithm, not None.",
)),
Expand Down Expand Up @@ -714,25 +714,37 @@ mod tests {
#[test]
fn test_identify_alg_params_for_hash_type() {
for (hash, params) in [
(HashType::Sha224, common::AlgorithmParameters::Sha224(())),
(HashType::Sha256, common::AlgorithmParameters::Sha256(())),
(HashType::Sha384, common::AlgorithmParameters::Sha384(())),
(HashType::Sha512, common::AlgorithmParameters::Sha512(())),
(
HashType::Sha224,
common::AlgorithmParameters::Sha224(Some(())),
),
(
HashType::Sha256,
common::AlgorithmParameters::Sha256(Some(())),
),
(
HashType::Sha384,
common::AlgorithmParameters::Sha384(Some(())),
),
(
HashType::Sha512,
common::AlgorithmParameters::Sha512(Some(())),
),
(
HashType::Sha3_224,
common::AlgorithmParameters::Sha3_224(()),
common::AlgorithmParameters::Sha3_224(Some(())),
),
(
HashType::Sha3_256,
common::AlgorithmParameters::Sha3_256(()),
common::AlgorithmParameters::Sha3_256(Some(())),
),
(
HashType::Sha3_384,
common::AlgorithmParameters::Sha3_384(()),
common::AlgorithmParameters::Sha3_384(Some(())),
),
(
HashType::Sha3_512,
common::AlgorithmParameters::Sha3_512(()),
common::AlgorithmParameters::Sha3_512(Some(())),
),
] {
assert_eq!(identify_alg_params_for_hash_type(hash).unwrap(), params);
Expand Down
17 changes: 17 additions & 0 deletions tests/x509/test_x509.py
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,23 @@ def test_load_cert_pub_key(self, backend):
cert.signature_hash_algorithm,
)

def test_load_pss_cert_no_null(self, backend):
"""
This test verifies that PSS certs where the hash algorithm
identifiers have no trailing null still load properly. LibreSSL
generates certs like this.
"""
cert = _load_cert(
os.path.join("x509", "custom", "rsa_pss_sha256_no_null.pem"),
x509.load_pem_x509_certificate,
)
assert isinstance(cert, x509.Certificate)
pss = cert.signature_algorithm_parameters
assert isinstance(pss, padding.PSS)
assert isinstance(pss._mgf, padding.MGF1)
assert isinstance(pss._mgf._algorithm, hashes.SHA256)
assert isinstance(cert.signature_hash_algorithm, hashes.SHA256)

def test_load_pss_sha1_mgf1_sha1(self, backend):
cert = _load_cert(
os.path.join("x509", "ee-pss-sha1-cert.pem"),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
-----BEGIN CERTIFICATE-----
MIIDADCCAbgCCQDEHaWKEwyb7zA9BgkqhkiG9w0BAQowMKANMAsGCWCGSAFlAwQC
AaEaMBgGCSqGSIb3DQEBCDALBglghkgBZQMEAgGiAwIBIDASMRAwDgYDVQQDDAd0
ZXN0aW5nMB4XDTIzMDUyNzA2NDExOVoXDTMzMDUyNDA2NDExOVowEjEQMA4GA1UE
AwwHdGVzdGluZzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAM2INK8b
04HqQ1ZYt94tDO0lFPOeCswGhKJQ9SRzTNpNB1XaJvrzz999gimmedUwgwVXHRdt
9WS/QXuyKzyeHcQFN8IPVylIMNGS9IEVa9NGNXzLVMIJYzDlwrEhQm6O4fUW8VtE
U85BXEw0yTEgeQxfuR688kjp/1bjkYsvLE/ID9EMgnXXmzunuqYxG+nmonfIYTgR
NpmXJJgp096sJHKaRkDaC7eApl6776kueFRRSiAIHY10wHqgOL0pBwIMSd/F/EKv
G0weUBLqjzus7G/+LdC6UoGWgV4EybvYlisH4SnLbNdvFilLWaNbgbD2R07hVaHs
8010rCq5RT766dcCAwEAATA9BgkqhkiG9w0BAQowMKANMAsGCWCGSAFlAwQCAaEa
MBgGCSqGSIb3DQEBCDALBglghkgBZQMEAgGiAwIBIAOCAQEAdKmJnR+UZaMi9RSI
ZBTN5SRv0nTJCwX/citYo8MMcsJ+DOLxR4tC9haYhRD9mIjks1NXcEKN+LqW9hDF
C5ptas03HeEY1NByS3wFSDRHggNFxpwmvX4hGp/8fjaf8EOb1rzh0TsJEgcv4h4Z
KeeSYvCtk5pMe+2lDgLfSegM22RFgXBj/wcI5JDxkGJ4M56++IM55HdXTY1cy7KY
woTtP8G6xzmKdVC+E8XGjBAbyzyommMpAI6aUnjW6oa4fD4ev1X17+/CQb1VyAYs
7nz4uBV1FTNAiUzjrf95KV5p2ir6YcOdspwuRbUJwGP+/1nXeN1pksnh56Fe3J5b
8Zw4cw==
-----END CERTIFICATE-----

0 comments on commit 93c96b7

Please sign in to comment.