wip: refactoring extension handling

src/crl.rs

@@ -4,13 +4,10 @@ use time::OffsetDateTime;
 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, KeyIdMethod, KeyUsagePurpose, RcgenError, SerialNumber, SignatureAlgorithm,
 };
@@ -247,37 +244,40 @@ impl CertificateRevocationListParams {
 			// 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_der(writer);
 			});
 
 			Ok(())
 		})
 	}
+	/// Returns the X.509 extensions that the [CertificateRevocationListParams] describe.
+	///
+	/// Additional extensions specific to the issuer [Certificate] will also 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(ext::authority_key_identifier(issuer))
+			.unwrap();
+
+		// Safety: there can be no duplicate CRL number ext OID.
+		exts.add_extension(ext::crl_number(&self.crl_number))
+			.unwrap();
+
+		if let Some(distribution_point) = &self.issuing_distribution_point {
+			// Safety: there can be no duplicate IDP ext OID.
+			exts.add_extension(ext::issuing_distribution_point(distribution_point))
+				.unwrap();
+		}
+
+		exts
+	}
 }
 
 /// A certificate revocation list (CRL) issuing distribution point, to be included in a CRL's
@@ -291,7 +291,7 @@ pub struct CrlIssuingDistributionPoint {
 }
 
 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
@@ -361,31 +361,27 @@ 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);
-			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);
-							},
-						)
-					});
-				});
+			let extensions = self.extensions();
+			if !extensions.is_empty() {
+				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(code) = self.reason_code {
+			// Safety: there can be no duplicate reason code ext OID.
+			exts.add_extension(ext::reason_code(code)).unwrap();
+		}
+
+		if let Some(invalidity_date) = self.invalidity_date {
+			// Safety: there can be no duplicate invalidity date ext OID.
+			exts.add_extension(ext::invalidity_date(invalidity_date))
+				.unwrap();
+		}
+
+		exts
+	}
 }

src/csr.rs

@@ -1,5 +1,5 @@
 #[cfg(feature = "x509-parser")]
-use crate::{DistinguishedName, SanType};
+use crate::{CustomExtension, DistinguishedName, SanType};
 #[cfg(feature = "pem")]
 use pem::Pem;
 use std::hash::Hash;
@@ -67,17 +67,48 @@ impl CertificateSigningRequest {
 		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 {
+		// 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<_>>();
+
+		if !requested_exts.is_empty() {
+			for ext in requested_exts {
+				let supported = match ext.parsed_extension() {
 					x509_parser::extensions::ParsedExtension::SubjectAlternativeName(san) => {
 						for name in &san.general_names {
 							params
 								.subject_alt_names
 								.push(SanType::try_from_general(name)?);
 						}
+						true
+					},
+					x509_parser::extensions::ParsedExtension::SubjectKeyIdentifier(ski) => {
+						params.key_identifier = ski.0.to_vec();
+						true
 					},
-					_ => return Err(RcgenError::UnsupportedExtension),
+					_ => false,
+				};
+				if !supported {
+					params.custom_extensions.push(CustomExtension {
+						oid: ext.oid.iter().unwrap().collect(),
+						critical: ext.critical,
+						content: ext.value.to_vec(),
+					})
 				}
 			}
 		}

src/error.rs

@@ -5,45 +5,47 @@ use std::fmt;
 #[non_exhaustive]
 /// The error type of the rcgen crate
 pub enum RcgenError {
+	/// 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,
-	#[cfg(feature = "x509-parser")]
-	/// Invalid subject alternative name type
-	InvalidNameType,
+	/// Duplicate extension OID
+	DuplicateExtension(String),
+	/// 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),
+	/// Invalid subject alternative name type
+	#[cfg(feature = "x509-parser")]
+	InvalidNameType,
+	/// 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(&'static str),
-	/// 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
+	#[cfg(feature = "pem")]
 	PemError(pem::PemError),
 	/// Error generated by a remote key operation
 	RemoteKeyError,
+	/// The `ring` library rejected the key upon loading
+	RingKeyRejected(&'static str),
+	/// Unspecified `ring` error
+	RingUnspecified,
+	/// Time conversion related errors
+	Time,
+	/// Unsupported extension requested in CSR
+	#[cfg(feature = "x509-parser")]
+	UnsupportedExtension,
 	/// 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 RcgenError {
@@ -92,6 +94,9 @@ impl fmt::Display for RcgenError {
 				f,
 				"CRL issuer must specify no key usage, or key usage including cRLSign"
 			)?,
+			DuplicateExtension(oid) => {
+				write!(f, "Extension with OID {oid} present multiple times")?
+			},
 		};
 		Ok(())
 	}