wip: refactoring extension handling

rcgen/src/crl.rs

@@ -4,13 +4,10 @@
 use yasna::DERWriter;
 use yasna::Tag;
 
-use crate::oid::*;
+use crate::ext::Extensions;
 #[cfg(feature = "pem")]
 use crate::ENCODE_CONFIG;
-use crate::{
-	write_distinguished_name, write_dt_utc_or_generalized, write_x509_authority_key_identifier,
-	write_x509_extension,
-};
+use crate::{ext, write_distinguished_name, write_dt_utc_or_generalized};
 use crate::{Certificate, Error, KeyIdMethod, KeyUsagePurpose, SerialNumber, SignatureAlgorithm};
 
 /// A certificate revocation list (CRL)
@@ -245,41 +242,42 @@
 			// RFC 5280 §5.1.2.7:
 			//   This field may only appear if the version is 2 (Section 5.1.2.1).  If
 			//   present, this field is a sequence of one or more CRL extensions.
-			// RFC 5280 §5.2:
-			//   Conforming CRL issuers are REQUIRED to include the authority key
-			//   identifier (Section 5.2.1) and the CRL number (Section 5.2.3)
-			//   extensions in all CRLs issued.
-			writer.next().write_tagged(Tag::context(0), |writer| {
-				writer.write_sequence(|writer| {
-					// Write authority key identifier.
-					write_x509_authority_key_identifier(writer.next(), ca);
-
-					// Write CRL number.
-					write_x509_extension(writer.next(), OID_CRL_NUMBER, false, |writer| {
-						writer.write_bigint_bytes(self.crl_number.as_ref(), true);
-					});
-
-					// Write issuing distribution point (if present).
-					if let Some(issuing_distribution_point) = &self.issuing_distribution_point {
-						write_x509_extension(
-							writer.next(),
-							OID_CRL_ISSUING_DISTRIBUTION_POINT,
-							true,
-							|writer| {
-								issuing_distribution_point.write_der(writer);
-							},
-						);
-					}
-				});
-			});
+			self.extensions(ca).write_crl_der(writer.next());
 
 			Ok(())
 		})
 	}
+	/// Returns the X.509 extensions that the [CertificateRevocationListParams] describe.
+	///
+	/// If an issuer [Certificate] is provided, additional extensions specific to the issuer will
+	/// be included (e.g. the authority key identifier).
+	fn extensions(&self, issuer: &Certificate) -> Extensions {
+		let mut exts = Extensions::default();
+
+		// RFC 5280 §5.2:
+		//   Conforming CRL issuers are REQUIRED to include the authority key
+		//   identifier (Section 5.2.1) and the CRL number (Section 5.2.3)
+		//   extensions in all CRLs issued.
+		// Safety: `exts` is empty at this point - there can be no duplicate AKI ext OID.
+		exts.add_extension(Box::new(ext::AuthorityKeyIdentifier::from(issuer)))
+			.unwrap();
+
+		// Safety: there can be no duplicate CRL number ext OID by this point.
+		exts.add_extension(Box::new(ext::CrlNumber::from_params(&self)))
+			.unwrap();
+
+		if let Some(idp_ext) = ext::IssuingDistributionPoint::from_params(&self) {
+			// Safety: there can be no duplicate IDP ext OID by this point.
+			exts.add_extension(Box::new(idp_ext)).unwrap();
+		}
+
+		exts
+	}
 }
 
 /// A certificate revocation list (CRL) issuing distribution point, to be included in a CRL's
 /// [issuing distribution point extension](https://datatracker.ietf.org/doc/html/rfc5280#section-5.2.5).
+#[derive(Debug, Clone, Eq, PartialEq)]
 pub struct CrlIssuingDistributionPoint {
 	/// The CRL's distribution point, containing a sequence of URIs the CRL can be retrieved from.
 	pub distribution_point: CrlDistributionPoint,
@@ -289,7 +287,7 @@
 }
 
 impl CrlIssuingDistributionPoint {
-	fn write_der(&self, writer: DERWriter) {
+	pub(crate) fn write_der(&self, writer: DERWriter) {
 		// IssuingDistributionPoint SEQUENCE
 		writer.write_sequence(|writer| {
 			// distributionPoint [0] DistributionPointName OPTIONAL
@@ -359,31 +357,23 @@
 			//   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);
-			let has_invalidity_date = self.invalidity_date.is_some();
-			if has_reason_code || has_invalidity_date {
-				writer.next().write_sequence(|writer| {
-					// Write reason code if present.
-					self.reason_code.map(|reason_code| {
-						write_x509_extension(writer.next(), OID_CRL_REASONS, false, |writer| {
-							writer.write_enum(reason_code as i64);
-						});
-					});
-
-					// Write invalidity date if present.
-					self.invalidity_date.map(|invalidity_date| {
-						write_x509_extension(
-							writer.next(),
-							OID_CRL_INVALIDITY_DATE,
-							false,
-							|writer| {
-								write_dt_utc_or_generalized(writer, invalidity_date);
-							},
-						)
-					});
-				});
-			}
+			self.extensions().write_der(writer.next());
 		})
 	}
+	/// Returns the X.509 extensions that the [RevokedCertParams] describe.
+	fn extensions(&self) -> Extensions {
+		let mut exts = Extensions::default();
+
+		if let Some(reason_code_ext) = ext::ReasonCode::from_params(&self) {
+			// Safety: there can be no duplicate reason code ext OID by this point.
+			exts.add_extension(Box::new(reason_code_ext)).unwrap();
+		}
+
+		if let Some(invalidity_date_ext) = ext::InvalidityDate::from_params(&self) {
+			// Safety: there can be no duplicate invalidity date ext OID by this point.
+			exts.add_extension(Box::new(invalidity_date_ext)).unwrap();
+		}
+
+		exts
+	}
 }

rcgen/src/csr.rs

@@ -1,5 +1,5 @@
 #[cfg(feature = "x509-parser")]
-use crate::{DistinguishedName, SanType};
+use crate::{ext, DistinguishedName};
 #[cfg(feature = "pem")]
 use pem::Pem;
 use std::hash::Hash;
@@ -45,6 +45,7 @@
 	///
 	/// Currently, this only supports the `Subject Alternative Name` extension.
 	/// On encountering other extensions, this function will return an error.
+	// TODO(@cpu): update this doc comment.
 	#[cfg(feature = "x509-parser")]
 	pub fn from_der(csr: &[u8]) -> Result<Self, Error> {
 		use x509_parser::prelude::FromDer;
@@ -66,27 +67,60 @@
 		params.distinguished_name = DistinguishedName::from_name(&info.subject)?;
 		let raw = info.subject_pki.subject_public_key.data.to_vec();
 
-		if let Some(extensions) = csr.requested_extensions() {
-			for ext in extensions {
-				match ext {
-					x509_parser::extensions::ParsedExtension::SubjectAlternativeName(san) => {
-						for name in &san.general_names {
-							params
-								.subject_alt_names
-								.push(SanType::try_from_general(name)?);
-						}
-					},
-					_ => return Err(Error::UnsupportedExtension),
+		// Pull out the extension requests attributes from the CSR.
+		// Note: we avoid using csr.requested_extensions() here because it maps to the parsed
+		// extension value and we want the raw extension value to handle unknown extensions
+		// ourselves.
+		let requested_exts = csr
+			.certification_request_info
+			.iter_attributes()
+			.filter_map(|attr| {
+				if let x509_parser::prelude::ParsedCriAttribute::ExtensionRequest(requested) =
+					&attr.parsed_attribute()
+				{
+					Some(requested.extensions.iter().collect::<Vec<_>>())
+				} else {
+					None
 				}
+			})
+			.flatten()
+			.collect::<Vec<_>>();
+
+		for ext in requested_exts {
+			use x509_parser::extensions::ParsedExtension;
+
+			let supported = match ext.parsed_extension() {
+				ext @ ParsedExtension::SubjectAlternativeName(_) => {
+					ext::SubjectAlternativeName::from_parsed(&mut params, ext)?
+				},
+				ext @ ParsedExtension::KeyUsage(_) => ext::KeyUsage::from_parsed(&mut params, ext)?,
+				ext @ ParsedExtension::ExtendedKeyUsage(_) => {
+					ext::ExtendedKeyUsage::from_parsed(&mut params, ext)?
+				},
+				ext @ ParsedExtension::NameConstraints(_) => {
+					ext::NameConstraints::from_parsed(&mut params, ext)?
+				},
+				ext @ ParsedExtension::CRLDistributionPoints(_) => {
+					ext::CrlDistributionPoints::from_parsed(&mut params, ext)?
+				},
+				ext @ ParsedExtension::SubjectKeyIdentifier(_) => {
+					ext::SubjectKeyIdentifier::from_parsed(&mut params, ext)?
+				},
+				ext @ ParsedExtension::BasicConstraints(_) => {
+					ext::BasicConstraints::from_parsed(&mut params, ext)?
+				},
+				ParsedExtension::AuthorityKeyIdentifier(_) => {
+					true // We always handle emitting this ourselves - don't copy it as a custom extension.
+				},
+				_ => false,
+			};
+			if !supported {
+				params
+					.custom_extensions
+					.push(ext::CustomExtension::from_parsed(ext)?);
 			}
 		}
 
-		// Not yet handled:
-		// * is_ca
-		// * extended_key_usages
-		// * name_constraints
-		// and any other extensions.
-
 		Ok(Self {
 			params,
 			public_key: PublicKey { alg, raw },

rcgen/src/error.rs

@@ -4,45 +4,58 @@
 #[non_exhaustive]
 /// The error type of the rcgen crate
 pub enum Error {
+	/// The provided certificate's signature algorithm
+	/// is incompatible with the given key pair
+	CertificateKeyPairMismatch,
 	/// The given certificate couldn't be parsed
 	CouldNotParseCertificate,
 	/// The given certificate signing request couldn't be parsed
 	CouldNotParseCertificationRequest,
 	/// The given key pair couldn't be parsed
 	CouldNotParseKeyPair,
+	/// Duplicate extension OID
+	DuplicateExtension(String),
+	/// Invalid ACME identifier extension digest length.
+	InvalidAcmeIdentifierLength,
+	/// Invalid certificate revocation list (CRL) next update.
+	InvalidCrlNextUpdate,
+	/// An IP address was provided as a byte array, but the byte array was an invalid length.
+	InvalidIpAddressOctetLength(usize),
 	#[cfg(feature = "x509-parser")]
 	/// Invalid subject alternative name type
 	InvalidNameType,
-	/// An IP address was provided as a byte array, but the byte array was an invalid length.
-	InvalidIpAddressOctetLength(usize),
+	/// CRL issuer specifies Key Usages that don't include cRLSign.
+	IssuerNotCrlSigner,
 	/// There is no support for generating
 	/// keys for the given algorithm
 	KeyGenerationUnavailable,
-	#[cfg(feature = "x509-parser")]
-	/// Unsupported extension requested in CSR
-	UnsupportedExtension,
-	/// The requested signature algorithm is not supported
-	UnsupportedSignatureAlgorithm,
-	/// Unspecified `ring` error
-	RingUnspecified,
-	/// The `ring` library rejected the key upon loading
-	RingKeyRejected(String),
-	/// The provided certificate's signature algorithm
-	/// is incompatible with the given key pair
-	CertificateKeyPairMismatch,
-	/// Time conversion related errors
-	Time,
 	#[cfg(feature = "pem")]
 	/// Error from the pem crate
 	PemError(String),
 	/// Error generated by a remote key operation
 	RemoteKeyError,
+	/// The `ring` library rejected the key upon loading
+	RingKeyRejected(String),
+	/// Unspecified `ring` error
+	RingUnspecified,
+	/// Time conversion related errors
+	Time,
+	/// Unsupported basic constraints extension path length in CSR
+	#[cfg(feature = "x509-parser")]
+	UnsupportedBasicConstraintsPathLen,
+	/// Unsupported CRL distribution point extension in CSR
+	#[cfg(feature = "x509-parser")]
+	UnsupportedCrlDistributionPoint,
+	#[cfg(feature = "x509-parser")]
+	/// Unsupported extension requested in CSR
+	UnsupportedExtension,
+	/// Unsupported general name type in CSR
+	#[cfg(feature = "x509-parser")]
+	UnsupportedGeneralName,
 	/// Unsupported field when generating a CSR
 	UnsupportedInCsr,
-	/// Invalid certificate revocation list (CRL) next update.
-	InvalidCrlNextUpdate,
-	/// CRL issuer specifies Key Usages that don't include cRLSign.
-	IssuerNotCrlSigner,
+	/// The requested signature algorithm is not supported
+	UnsupportedSignatureAlgorithm,
 }
 
 impl fmt::Display for Error {
@@ -91,6 +104,22 @@
 				f,
 				"CRL issuer must specify no key usage, or key usage including cRLSign"
 			)?,
+			DuplicateExtension(oid) => {
+				write!(f, "Extension with OID {oid} present multiple times")?
+			},
+			#[cfg(feature = "x509-parser")]
+			UnsupportedGeneralName => write!(f, "Unsupported general name in CSR",)?,
+			#[cfg(feature = "x509-parser")]
+			UnsupportedCrlDistributionPoint => write!(f, "Unsupported CRL distribution point in CSR",)?,
+			#[cfg(feature = "x509-parser")]
+			UnsupportedBasicConstraintsPathLen => write!(
+				f,
+				"Unsupported basic constraints extension path length constraint in CSR"
+			)?,
+			InvalidAcmeIdentifierLength => write!(
+				f,
+				"Invalid ACME identifier extension digest length. Must be 32 bytes.",
+			)?,
 		};
 		Ok(())
 	}