refactor(x509-cert): decompose AsExtension into Criticality + AsExtension

The existing `AsExtension` trait has two design problems:

1. **Criticality is coupled to the type.** The `critical()` method attempts
   to determine criticality from `subject` and `extensions`, but these are
   not the only factors. Policy decisions, certificate profiles, or issuer
   preferences may also influence criticality. Some extensions (like
   `BasicConstraints`) "MAY appear as critical or non-critical" per RFC 5280.

2. **Requires `der::Encode`.** The trait bounds force all extensions to be
   DER-encodable, but an extension's `extnValue` is just an `OCTET STRING` -
   the encoding could theoretically be anything.

This refactor extracts a new `Criticality` trait and simplifies `AsExtension`:

```rust
pub trait Criticality {
    fn criticality(&self, subject: &Name, extensions: &[Extension]) -> bool;
}

pub trait AsExtension {
    type Error;
    fn to_extension(&self, subject: &Name, extensions: &[Extension])
        -> Result<Extension, Self::Error>;
}
```

A blanket impl provides `AsExtension` for `T: Criticality + AssociatedOid + der::Encode`,
so most existing extension types just need to rename their impl.

**New capabilities:**

1. Override criticality at the call site using a tuple:
   ```rust
   // Use default criticality.
   builder.add_extension(&SubjectAltName(names)))?;

   // Override default criticality.
   builder.add_extension(&(true, SubjectAltName(names)))?;
   ```

2. Implement `AsExtension` directly for non-DER-encoded extensions:
   ```rust
   impl AsExtension for MyCustomExtension {
       type Error = MyError;
       fn to_extension(&self, ..) -> Result<Extension, MyError> {
           // Custom encoding logic
       }
   }
   ```

3. Builders return `E::Error` directly, letting callers decide error handling:
   ```rust
   builder.add_extension(&ext)?;           // propagate E::Error
   builder.add_extension(&ext).map_err(…)? // convert to another type
   ```

**Migration:** Replace `impl AsExtension` with `impl Criticality` and rename
`critical()` to `criticality()`. The blanket impl provides `AsExtension`
automatically.

Author: npmccallum

x509-cert/src/builder.rs

@@ -219,10 +219,9 @@ where
     /// Extensions need to implement [`AsExtension`], examples may be found in
     /// in [`AsExtension` documentation](../ext/trait.AsExtension.html#examples) or
     /// [the implementors](../ext/trait.AsExtension.html#implementors).
-    pub fn add_extension<E: AsExtension>(&mut self, extension: &E) -> Result<()> {
+    pub fn add_extension<E: AsExtension>(&mut self, extension: &E) -> core::result::Result<(), E::Error> {
         let ext = extension.to_extension(&self.tbs.subject, &self.extensions)?;
         self.extensions.push(ext);
-
         Ok(())
     }
 }

x509-cert/src/ext.rs

@@ -44,6 +44,26 @@ pub struct Extension {
 /// [RFC 5280 Section 4.1.2.9]: https://datatracker.ietf.org/doc/html/rfc5280#section-4.1.2.9
 pub type Extensions = alloc::vec::Vec<Extension>;
 
+/// Trait for types that define their default criticality as an extension.
+///
+/// This is used for most der::Encode types that are used as extensions.
+pub trait Criticality {
+    /// Should the extension be marked critical
+    ///
+    /// This affects the behavior of a validator when using the generated certificate.
+    /// See [RFC 5280 Section 4.2]:
+    /// ```text
+    /// A certificate-using system MUST reject the certificate if it encounters
+    /// a critical extension it does not recognize or a critical extension
+    /// that contains information that it cannot process.  A non-critical
+    /// extension MAY be ignored if it is not recognized, but MUST be
+    /// processed if it is recognized.
+    /// ```
+    ///
+    /// [RFC 5280 Section 4.2]: https://www.rfc-editor.org/rfc/rfc5280#section-4.2
+    fn criticality(&self, subject: &crate::name::Name, extensions: &[Extension]) -> bool;
+}
+
 /// Trait to be implemented by extensions to allow them to be formatted as x509 v3 extensions by
 /// builder.
 ///
@@ -65,40 +85,50 @@ pub type Extensions = alloc::vec::Vec<Extension>;
 ///     const OID: ObjectIdentifier = ObjectIdentifier::new_unwrap("1.3.6.1.4.1.32473.1");
 /// }
 ///
-/// impl ext::AsExtension for CaptainAge {
-///     fn critical(&self, _subject: &name::Name, _extensions: &[ext::Extension]) -> bool {
+/// impl ext::Criticality for CaptainAge {
+///     fn criticality(&self, _subject: &name::Name, _extensions: &[ext::Extension]) -> bool {
 ///         false
 ///     }
 /// }
 /// ```
-pub trait AsExtension: AssociatedOid + der::Encode {
-    /// Should the extension be marked critical
-    ///
-    /// This affects the behavior of a validator when using the generated certificate.
-    /// See [RFC 5280 Section 4.2]:
-    /// ```text
-    /// A certificate-using system MUST reject the certificate if it encounters
-    /// a critical extension it does not recognize or a critical extension
-    /// that contains information that it cannot process.  A non-critical
-    /// extension MAY be ignored if it is not recognized, but MUST be
-    /// processed if it is recognized.
-    /// ```
-    ///
-    /// [RFC 5280 Section 4.2]: https://www.rfc-editor.org/rfc/rfc5280#section-4.2
-    fn critical(&self, subject: &crate::name::Name, extensions: &[Extension]) -> bool;
+pub trait AsExtension {
+    /// The error type returned when encoding the extension.
+    type Error;
 
     /// Returns the Extension with the content encoded.
     fn to_extension(
         &self,
         subject: &crate::name::Name,
         extensions: &[Extension],
-    ) -> Result<Extension, der::Error> {
-        let content = OctetString::new(<Self as der::Encode>::to_der(self)?)?;
+    ) -> Result<Extension, Self::Error>;
+}
+
+impl<T: Criticality + AssociatedOid + der::Encode> AsExtension for T {
+    type Error = der::Error;
 
+    fn to_extension(
+        &self,
+        subject: &crate::name::Name,
+        extensions: &[Extension],
+    ) -> Result<Extension, Self::Error> {
         Ok(Extension {
             extn_id: <Self as AssociatedOid>::OID,
-            critical: self.critical(subject, extensions),
-            extn_value: content,
+            critical: self.criticality(subject, extensions),
+            extn_value: OctetString::new(self.to_der()?)?,
         })
     }
 }
+
+impl<T: AsExtension> AsExtension for (bool, T) {
+    type Error = T::Error;
+
+    fn to_extension(
+        &self,
+        subject: &crate::name::Name,
+        extensions: &[Extension],
+    ) -> Result<Extension, Self::Error> {
+        let mut extension = self.1.to_extension(subject, extensions)?;
+        extension.critical = self.0;
+        Ok(extension)
+    }
+}

x509-cert/src/ext/pkix.rs

@@ -78,8 +78,8 @@ impl AssociatedOid for SubjectAltName {
 
 impl_newtype!(SubjectAltName, name::GeneralNames);
 
-impl crate::ext::AsExtension for SubjectAltName {
-    fn critical(&self, subject: &crate::name::Name, _extensions: &[super::Extension]) -> bool {
+impl crate::ext::Criticality for SubjectAltName {
+    fn criticality(&self, subject: &crate::name::Name, _extensions: &[super::Extension]) -> bool {
         // https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.6
         //   Further, if the only subject identity included in the certificate is
         //   an alternative name form (e.g., an electronic mail address), then the

x509-cert/src/ext/pkix/constraints/basic.rs

@@ -23,8 +23,8 @@ impl AssociatedOid for BasicConstraints {
     const OID: ObjectIdentifier = ID_CE_BASIC_CONSTRAINTS;
 }
 
-impl crate::ext::AsExtension for BasicConstraints {
-    fn critical(
+impl crate::ext::Criticality for BasicConstraints {
+    fn criticality(
         &self,
         _subject: &crate::name::Name,
         _extensions: &[crate::ext::Extension],

x509-cert/src/macros.rs

@@ -81,14 +81,14 @@ macro_rules! impl_newtype {
     };
 }
 
-/// Implements the AsExtension traits for every defined Extension paylooad
+/// Implements the Criticality trait for every defined Extension paylooad
 macro_rules! impl_extension {
     ($newtype:ty) => {
         impl_extension!($newtype, critical = false);
     };
     ($newtype:ty, critical = $critical:expr) => {
-        impl crate::ext::AsExtension for $newtype {
-            fn critical(
+        impl crate::ext::Criticality for $newtype {
+            fn criticality(
                 &self,
                 _subject: &crate::name::Name,
                 _extensions: &[crate::ext::Extension],

x509-cert/src/request/builder.rs

@@ -80,7 +80,7 @@ impl RequestBuilder {
     /// Extensions need to implement [`AsExtension`], examples may be found in
     /// in [`AsExtension` documentation](../ext/trait.AsExtension.html#examples) or
     /// [the implementors](../ext/trait.AsExtension.html#implementors).
-    pub fn add_extension<E: AsExtension>(&mut self, extension: &E) -> Result<()> {
+    pub fn add_extension<E: AsExtension>(&mut self, extension: &E) -> core::result::Result<(), E::Error> {
         let ext = extension.to_extension(&self.info.subject, &self.extension_req.0)?;
 
         self.extension_req.0.push(ext);

x509-ocsp/src/basic.rs

@@ -171,7 +171,7 @@ mod builder {
         /// extension encoding fails.
         ///
         /// [RFC 6960 Section 4.4]: https://datatracker.ietf.org/doc/html/rfc6960#section-4.4
-        pub fn with_extension(mut self, ext: impl AsExtension) -> Result<Self, Error> {
+        pub fn with_extension<E: AsExtension>(mut self, ext: E) -> Result<Self, E::Error> {
             let ext = ext.to_extension(&Name::default(), &[])?;
             match self.single_extensions {
                 Some(ref mut exts) => exts.push(ext),

x509-ocsp/src/builder/request.rs

@@ -96,7 +96,7 @@ impl OcspRequestBuilder {
     /// extension encoding fails.
     ///
     /// [RFC 6960 Section 4.4]: https://datatracker.ietf.org/doc/html/rfc6960#section-4.4
-    pub fn with_extension(mut self, ext: impl AsExtension) -> Result<Self, Error> {
+    pub fn with_extension<E: AsExtension>(mut self, ext: E) -> Result<Self, E::Error> {
         let ext = ext.to_extension(&Name::default(), &[])?;
         match self.tbs.request_extensions {
             Some(ref mut exts) => exts.push(ext),

x509-ocsp/src/builder/response.rs

@@ -101,7 +101,7 @@ impl OcspResponseBuilder {
     /// extension encoding fails.
     ///
     /// [RFC 6960 Section 4.4]: https://datatracker.ietf.org/doc/html/rfc6960#section-4.4
-    pub fn with_extension(mut self, ext: impl AsExtension) -> Result<Self, Error> {
+    pub fn with_extension<E: AsExtension>(mut self, ext: E) -> Result<Self, E::Error> {
         let ext = ext.to_extension(&Name::default(), &[])?;
         match self.response_extensions {
             Some(ref mut exts) => exts.push(ext),

x509-ocsp/src/ext.rs

@@ -15,7 +15,7 @@ use der::{
 };
 use spki::AlgorithmIdentifierOwned;
 use x509_cert::{
-    ext::{AsExtension, Extension, pkix::AuthorityInfoAccessSyntax},
+    ext::{Criticality, Extension, pkix::AuthorityInfoAccessSyntax},
     impl_newtype,
     name::Name,
 };
@@ -26,8 +26,8 @@ use rand_core::CryptoRng;
 // x509-cert's is not exported
 macro_rules! impl_extension {
     ($newtype:ty, critical = $critical:expr) => {
-        impl AsExtension for $newtype {
-            fn critical(&self, _subject: &Name, _extensions: &[Extension]) -> bool {
+        impl Criticality for $newtype {
+            fn criticality(&self, _subject: &Name, _extensions: &[Extension]) -> bool {
                 $critical
             }
         }