diff --git a/rcgen/src/certificate.rs b/rcgen/src/certificate.rs index 47016720..6c518018 100644 --- a/rcgen/src/certificate.rs +++ b/rcgen/src/certificate.rs @@ -437,6 +437,17 @@ impl CertificateParams { pub_key: &K, issuer: &Issuer<'_, impl SigningKey>, ) -> Result, Error> { + // An empty distribution point would be encoded as an empty fullName, + // violating GeneralNames ::= SEQUENCE SIZE (1..MAX) OF GeneralName + // (RFC 5280 §4.2.1.13). + if self + .crl_distribution_points + .iter() + .any(|dp| dp.uris.is_empty()) + { + return Err(Error::EmptyCrlDistributionPointUris); + } + let der = sign_der(&issuer.signing_key, |writer| { let pub_key_spki = pub_key.subject_public_key_info(); // Write version @@ -482,8 +493,10 @@ impl CertificateParams { // write extensions let should_write_exts = self.use_authority_key_identifier_extension || !self.subject_alt_names.is_empty() + || !self.key_usages.is_empty() || !self.extended_key_usages.is_empty() || self.name_constraints.iter().any(|c| !c.is_empty()) + || !self.crl_distribution_points.is_empty() || matches!(self.is_ca, IsCa::ExplicitNoCa) || matches!(self.is_ca, IsCa::Ca(_)) || !self.custom_extensions.is_empty(); @@ -1190,6 +1203,66 @@ mod tests { assert!(found); } + #[cfg(feature = "crypto")] + #[test] + fn test_empty_crl_distribution_point_uris_rejected() { + let params = CertificateParams { + crl_distribution_points: vec![CrlDistributionPoint { uris: Vec::new() }], + ..CertificateParams::default() + }; + + // A distribution point with no URIs would be encoded as an empty + // fullName, violating GeneralNames ::= SEQUENCE SIZE (1..MAX) OF + // GeneralName (RFC 5280 §4.2.1.13), so it must be rejected. + let key_pair = KeyPair::generate().unwrap(); + assert_eq!( + params.self_signed(&key_pair).unwrap_err(), + Error::EmptyCrlDistributionPointUris + ); + } + + #[cfg(feature = "crypto")] + #[test] + fn test_with_key_usages_only() { + // The KeyUsage extension must be present even when it is the only + // extension requested by the params. + let params = CertificateParams { + key_usages: vec![ + KeyUsagePurpose::DigitalSignature, + KeyUsagePurpose::KeyEncipherment, + ], + ..CertificateParams::default() + }; + + let key_pair = KeyPair::generate().unwrap(); + let cert = params.self_signed(&key_pair).unwrap(); + + let (_rem, cert) = x509_parser::parse_x509_certificate(cert.der()).unwrap(); + assert!(cert.key_usage().unwrap().is_some()); + } + + #[cfg(feature = "crypto")] + #[test] + fn test_with_crl_distribution_points_only() { + // The CRL distribution points extension must be present even when it + // is the only extension requested by the params. + let params = CertificateParams { + crl_distribution_points: vec![CrlDistributionPoint { + uris: vec!["http://crl.example.com".to_string()], + }], + ..CertificateParams::default() + }; + + let key_pair = KeyPair::generate().unwrap(); + let cert = params.self_signed(&key_pair).unwrap(); + + let (_rem, cert) = x509_parser::parse_x509_certificate(cert.der()).unwrap(); + assert!(cert.iter_extensions().any(|ext| matches!( + ext.parsed_extension(), + x509_parser::extensions::ParsedExtension::CRLDistributionPoints(_) + ))); + } + #[cfg(feature = "crypto")] #[test] fn test_with_key_usages_decipheronly_only() { diff --git a/rcgen/src/crl.rs b/rcgen/src/crl.rs index 379970a5..3addf637 100644 --- a/rcgen/src/crl.rs +++ b/rcgen/src/crl.rs @@ -8,7 +8,7 @@ use crate::key_pair::sign_der; #[cfg(feature = "pem")] use crate::ENCODE_CONFIG; use crate::{ - oid, write_distinguished_name, write_dt_utc_or_generalized, + dt_to_generalized, oid, write_distinguished_name, write_dt_utc_or_generalized, write_x509_authority_key_identifier, write_x509_extension, Error, Issuer, KeyIdMethod, KeyUsagePurpose, SerialNumber, SigningKey, }; @@ -196,6 +196,17 @@ impl CertificateRevocationListParams { return Err(Error::IssuerNotCrlSigner); } + // An empty distribution point would be encoded as an empty fullName, + // violating GeneralNames ::= SEQUENCE SIZE (1..MAX) OF GeneralName + // (RFC 5280 §4.2.1.13). + if self + .issuing_distribution_point + .as_ref() + .is_some_and(|idp| idp.distribution_point.uris.is_empty()) + { + return Err(Error::EmptyCrlDistributionPointUris); + } + Ok(CertificateRevocationList { der: self.serialize_der(issuer)?.into(), }) @@ -375,26 +386,33 @@ impl RevokedCertParams { // optional for conforming CRL issuers and applications. However, CRL // issuers SHOULD include reason codes (Section 5.3.1) and invalidity // dates (Section 5.3.2) whenever this information is available. - let has_reason_code = - matches!(self.reason_code, Some(reason) if reason != RevocationReason::Unspecified); + // RFC 5280 §5.3.1: "The reason code CRL entry extension SHOULD be + // absent instead of using the unspecified (0) reasonCode value." + let reason_code = self + .reason_code + .filter(|reason| *reason != RevocationReason::Unspecified); let has_invalidity_date = self.invalidity_date.is_some(); - if has_reason_code || has_invalidity_date { + if reason_code.is_some() || has_invalidity_date { writer.next().write_sequence(|writer| { // Write reason code if present. - if let Some(reason_code) = self.reason_code { + if let Some(reason_code) = reason_code { write_x509_extension(writer.next(), oid::CRL_REASONS, false, |writer| { writer.write_enum(reason_code as i64); }); } // Write invalidity date if present. + // RFC 5280 §5.3.2: InvalidityDate ::= GeneralizedTime. + // Unlike the Time CHOICE used elsewhere, dates in the + // UTCTime range (1950-2049) must still be encoded as + // GeneralizedTime. if let Some(invalidity_date) = self.invalidity_date { write_x509_extension( writer.next(), oid::CRL_INVALIDITY_DATE, false, |writer| { - write_dt_utc_or_generalized(writer, invalidity_date); + writer.write_generalized_time(&dt_to_generalized(invalidity_date)); }, ) } @@ -403,3 +421,104 @@ impl RevokedCertParams { }) } } + +#[cfg(all(test, feature = "crypto"))] +mod tests { + use x509_parser::num_bigint::BigUint; + use x509_parser::{oid_registry, parse_x509_crl}; + + use super::*; + use crate::{date_time_ymd, BasicConstraints, CertificateParams, IsCa, KeyPair}; + + #[test] + fn test_empty_issuing_distribution_point_uris_rejected() { + let crl = CertificateRevocationListParams { + this_update: date_time_ymd(2025, 5, 1), + next_update: date_time_ymd(2026, 5, 1), + crl_number: SerialNumber::from(1234u64), + issuing_distribution_point: Some(CrlIssuingDistributionPoint { + distribution_point: CrlDistributionPoint { uris: Vec::new() }, + scope: None, + }), + revoked_certs: Vec::new(), + key_identifier_method: KeyIdMethod::Sha256, + }; + + // A distribution point with no URIs would be encoded as an empty + // fullName, violating GeneralNames ::= SEQUENCE SIZE (1..MAX) OF + // GeneralName (RFC 5280 §4.2.1.13), so it must be rejected. + assert_eq!( + crl.signed_by(&test_issuer()).unwrap_err(), + Error::EmptyCrlDistributionPointUris + ); + } + + #[test] + fn test_unspecified_reason_code_not_written() { + let crl = test_crl(RevokedCertParams { + serial_number: SerialNumber::from(9999u64), + revocation_time: date_time_ymd(2025, 5, 1), + reason_code: Some(RevocationReason::Unspecified), + invalidity_date: Some(date_time_ymd(2025, 4, 1)), + }); + + let (_rem, parsed) = parse_x509_crl(crl.der()).unwrap(); + let revoked = parsed.iter_revoked_certificates().next().unwrap(); + // RFC 5280 §5.3.1: "The reason code CRL entry extension SHOULD be + // absent instead of using the unspecified (0) reasonCode value." + assert!(revoked + .extensions() + .iter() + .all(|ext| ext.oid != oid_registry::OID_X509_EXT_REASON_CODE)); + } + + #[test] + fn test_invalidity_date_generalized_time() { + let crl = test_crl(RevokedCertParams { + serial_number: SerialNumber::from(9999u64), + revocation_time: date_time_ymd(2025, 5, 1), + reason_code: None, + invalidity_date: Some(date_time_ymd(2025, 4, 1)), + }); + + let (_rem, parsed) = parse_x509_crl(crl.der()).unwrap(); + let revoked = parsed.iter_revoked_certificates().next().unwrap(); + assert_eq!(revoked.user_certificate, BigUint::from(9999u64)); + + let invalidity_date = revoked + .extensions() + .iter() + .find(|ext| ext.oid == oid_registry::OID_X509_EXT_INVALIDITY_DATE) + .unwrap(); + // RFC 5280 §5.3.2: InvalidityDate ::= GeneralizedTime. Unlike the Time + // CHOICE used elsewhere, dates in the UTCTime range (1950-2049) must + // still be encoded as GeneralizedTime. + assert_eq!(invalidity_date.value, b"\x18\x0f20250401000000Z"); + } + + fn test_crl(revoked_cert: RevokedCertParams) -> CertificateRevocationList { + CertificateRevocationListParams { + this_update: date_time_ymd(2025, 5, 1), + next_update: date_time_ymd(2026, 5, 1), + crl_number: SerialNumber::from(1234u64), + issuing_distribution_point: None, + revoked_certs: vec![revoked_cert], + key_identifier_method: KeyIdMethod::Sha256, + } + .signed_by(&test_issuer()) + .unwrap() + } + + fn test_issuer() -> Issuer<'static, KeyPair> { + let mut issuer_params = + CertificateParams::new(vec!["crl.issuer.example.com".to_string()]).unwrap(); + issuer_params.serial_number = Some(SerialNumber::from(9999u64)); + issuer_params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + issuer_params.key_usages = vec![ + KeyUsagePurpose::KeyCertSign, + KeyUsagePurpose::DigitalSignature, + KeyUsagePurpose::CrlSign, + ]; + Issuer::new(issuer_params, KeyPair::generate().unwrap()) + } +} diff --git a/rcgen/src/error.rs b/rcgen/src/error.rs index e6ae3961..9ba0b30e 100644 --- a/rcgen/src/error.rs +++ b/rcgen/src/error.rs @@ -45,6 +45,8 @@ pub enum Error { InvalidCrlNextUpdate, /// CRL issuer specifies Key Usages that don't include cRLSign. IssuerNotCrlSigner, + /// A CRL distribution point was specified without any URIs. + EmptyCrlDistributionPointUris, #[cfg(not(feature = "crypto"))] /// Missing serial number MissingSerialNumber, @@ -97,6 +99,9 @@ impl fmt::Display for Error { f, "CRL issuer must specify no key usage, or key usage including cRLSign" )?, + EmptyCrlDistributionPointUris => { + write!(f, "CRL distribution points must include at least one URI")? + }, #[cfg(not(feature = "crypto"))] MissingSerialNumber => write!(f, "A serial number must be specified")?, #[cfg(feature = "x509-parser")] diff --git a/rcgen/src/key_pair.rs b/rcgen/src/key_pair.rs index 102cd4df..a421b0d8 100644 --- a/rcgen/src/key_pair.rs +++ b/rcgen/src/key_pair.rs @@ -259,9 +259,6 @@ impl KeyPair { } else if alg == &PKCS_RSA_SHA512 { let rsakp = RsaKeyPair::from_pkcs8(&serialized_der)._err()?; KeyPairKind::Rsa(rsakp, &signature::RSA_PKCS1_SHA512) - } else if alg == &PKCS_RSA_PSS_SHA256 { - let rsakp = RsaKeyPair::from_pkcs8(&serialized_der)._err()?; - KeyPairKind::Rsa(rsakp, &signature::RSA_PSS_SHA256) } else { #[cfg(feature = "aws_lc_rs")] if alg == &PKCS_ECDSA_P521_SHA256 { @@ -386,9 +383,6 @@ impl KeyPair { } else if alg == &PKCS_RSA_SHA512 { let rsakp = rsa_key_pair_from(&serialized_der)._err()?; KeyPairKind::Rsa(rsakp, &signature::RSA_PKCS1_SHA512) - } else if alg == &PKCS_RSA_PSS_SHA256 { - let rsakp = rsa_key_pair_from(&serialized_der)._err()?; - KeyPairKind::Rsa(rsakp, &signature::RSA_PSS_SHA256) } else { panic!("Unknown SignatureAlgorithm specified!"); }; diff --git a/rcgen/src/oid.rs b/rcgen/src/oid.rs index 3b1c0eb9..e537b741 100644 --- a/rcgen/src/oid.rs +++ b/rcgen/src/oid.rs @@ -35,9 +35,6 @@ pub(crate) const ML_DSA_87: &[u64] = &[2, 16, 840, 1, 101, 3, 4, 3, 19]; /// rsaEncryption in [RFC 4055](https://www.rfc-editor.org/rfc/rfc4055#section-6) pub(crate) const RSA_ENCRYPTION: &[u64] = &[1, 2, 840, 113549, 1, 1, 1]; -/// id-RSASSA-PSS in [RFC 4055](https://www.rfc-editor.org/rfc/rfc4055#section-6) -pub(crate) const RSASSA_PSS: &[u64] = &[1, 2, 840, 113549, 1, 1, 10]; - /// id-ce-keyUsage in [RFC 5280](https://tools.ietf.org/html/rfc5280#appendix-A.2) pub(crate) const KEY_USAGE: &[u64] = &[2, 5, 29, 15]; diff --git a/rcgen/src/sign_algo.rs b/rcgen/src/sign_algo.rs index 9e6916cb..2d2fb71e 100644 --- a/rcgen/src/sign_algo.rs +++ b/rcgen/src/sign_algo.rs @@ -6,7 +6,7 @@ use aws_lc_rs::unstable::signature::{ PqdsaSigningAlgorithm, ML_DSA_44_SIGNING, ML_DSA_65_SIGNING, ML_DSA_87_SIGNING, }; use yasna::models::ObjectIdentifier; -use yasna::{DERWriter, Tag}; +use yasna::DERWriter; #[cfg(feature = "crypto")] use crate::ring_like::signature::{self, EcdsaSigningAlgorithm, EdDSAParameters, RsaEncoding}; @@ -28,11 +28,6 @@ pub(crate) enum SignatureAlgorithmParams { None, /// Write null parameters Null, - /// RSASSA-PSS-params as per RFC 4055 - RsaPss { - hash_algorithm: &'static [u64], - salt_length: u64, - }, } /// Signature algorithm type @@ -54,8 +49,6 @@ impl fmt::Debug for SignatureAlgorithm { write!(f, "PKCS_RSA_SHA384") } else if self == &PKCS_RSA_SHA512 { write!(f, "PKCS_RSA_SHA512") - } else if self == &PKCS_RSA_PSS_SHA256 { - write!(f, "PKCS_RSA_PSS_SHA256") } else if self == &PKCS_ECDSA_P256_SHA256 { write!(f, "PKCS_ECDSA_P256_SHA256") } else if self == &PKCS_ECDSA_P384_SHA384 { @@ -103,7 +96,6 @@ impl SignatureAlgorithm { &PKCS_RSA_SHA256, &PKCS_RSA_SHA384, &PKCS_RSA_SHA512, - //&PKCS_RSA_PSS_SHA256, &PKCS_ECDSA_P256_SHA256, &PKCS_ECDSA_P384_SHA384, #[cfg(feature = "aws_lc_rs")] @@ -163,27 +155,6 @@ pub(crate) mod algo { params: SignatureAlgorithmParams::Null, }; - // TODO: not really sure whether the certs we generate actually work. - // Both openssl and webpki reject them. It *might* be possible that openssl - // accepts the certificate if the key is a proper RSA-PSS key, but ring doesn't - // support those: https://github.com/briansmith/ring/issues/1353 - // - /// RSA signing with PKCS#1 2.1 RSASSA-PSS padding and SHA-256 hashing as per [RFC 4055](https://tools.ietf.org/html/rfc4055) - pub(crate) static PKCS_RSA_PSS_SHA256: SignatureAlgorithm = SignatureAlgorithm { - // We could also use RSA_ENCRYPTION here, but it's recommended - // to use ID-RSASSA-PSS if possible. - oids_sign_alg: &[RSASSA_PSS], - #[cfg(feature = "crypto")] - sign_alg: SignAlgo::Rsa(&signature::RSA_PSS_SHA256), - oid_components: RSASSA_PSS, //&[1, 2, 840, 113549, 1, 1, 13], - // rSASSA-PSS-SHA256-Params in RFC 4055 - params: SignatureAlgorithmParams::RsaPss { - // id-sha256 in https://datatracker.ietf.org/doc/html/rfc4055#section-2.1 - hash_algorithm: &[2, 16, 840, 1, 101, 3, 4, 2, 1], - salt_length: 20, - }, - }; - /// ECDSA signing using the P-256 curves and SHA-256 hashing as per [RFC 5758](https://tools.ietf.org/html/rfc5758#section-3.2) pub static PKCS_ECDSA_P256_SHA256: SignatureAlgorithm = SignatureAlgorithm { oids_sign_alg: &[EC_PUBLIC_KEY, EC_SECP_256_R1], @@ -299,41 +270,6 @@ impl SignatureAlgorithm { SignatureAlgorithmParams::Null => { writer.next().write_null(); }, - SignatureAlgorithmParams::RsaPss { - hash_algorithm, - salt_length, - } => { - writer.next().write_sequence(|writer| { - // https://datatracker.ietf.org/doc/html/rfc4055#section-3.1 - - let oid = ObjectIdentifier::from_slice(hash_algorithm); - // hashAlgorithm - writer.next().write_tagged(Tag::context(0), |writer| { - writer.write_sequence(|writer| { - writer.next().write_oid(&oid); - }); - }); - // maskGenAlgorithm - writer.next().write_tagged(Tag::context(1), |writer| { - writer.write_sequence(|writer| { - // id-mgf1 in RFC 4055 - const ID_MGF1: &[u64] = &[1, 2, 840, 113549, 1, 1, 8]; - let oid = ObjectIdentifier::from_slice(ID_MGF1); - writer.next().write_oid(&oid); - writer.next().write_sequence(|writer| { - let oid = ObjectIdentifier::from_slice(hash_algorithm); - writer.next().write_oid(&oid); - writer.next().write_null(); - }); - }); - }); - // saltLength - writer.next().write_tagged(Tag::context(2), |writer| { - writer.write_u64(salt_length); - }); - // We *must* omit the trailerField element as per RFC 4055 section 3.1 - }) - }, } } /// Writes the algorithm identifier as it appears inside a signature diff --git a/verify-tests/tests/openssl.rs b/verify-tests/tests/openssl.rs index bf736794..c19d22f2 100644 --- a/verify-tests/tests/openssl.rs +++ b/verify-tests/tests/openssl.rs @@ -284,7 +284,6 @@ fn test_openssl_rsa_combinations_given() { &rcgen::PKCS_RSA_SHA256, &rcgen::PKCS_RSA_SHA384, &rcgen::PKCS_RSA_SHA512, - //&rcgen::PKCS_RSA_PSS_SHA256, ]; for (i, alg) in alg_list.iter().enumerate() { let (params, _) = util::default_params(); diff --git a/verify-tests/tests/webpki.rs b/verify-tests/tests/webpki.rs index 75ad79c8..2138f788 100644 --- a/verify-tests/tests/webpki.rs +++ b/verify-tests/tests/webpki.rs @@ -291,7 +291,6 @@ fn test_webpki_rsa_combinations_given() { webpki::ring::RSA_PKCS1_2048_8192_SHA512, &signature::RSA_PKCS1_SHA512, ), - //(&rcgen::PKCS_RSA_PSS_SHA256, &webpki::RSA_PSS_2048_8192_SHA256_LEGACY_KEY, &signature::RSA_PSS_SHA256), ]; for c in configs { let (params, _) = util::default_params();