Add `verify_is_valid_tls_server_cert_ext`.

Add a public API that allows the caller to distinguish the new DoS
errors.

src/budget.rs

@@ -13,7 +13,7 @@
 // ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
-use crate::error::InternalError;
+use crate::ErrorExt;
 
 pub(super) struct Budget {
     signatures: usize,
@@ -22,23 +22,23 @@ pub(super) struct Budget {
 
 impl Budget {
     #[inline]
-    pub fn consume_signature(&mut self) -> Result<(), InternalError> {
+    pub fn consume_signature(&mut self) -> Result<(), ErrorExt> {
         checked_sub(
             &mut self.signatures,
-            InternalError::MaximumSignatureChecksExceeded,
+            ErrorExt::MaximumSignatureChecksExceeded,
         )
     }
 
     #[inline]
-    pub fn consume_build_chain_call(&mut self) -> Result<(), InternalError> {
+    pub fn consume_build_chain_call(&mut self) -> Result<(), ErrorExt> {
         checked_sub(
             &mut self.build_chain_calls,
-            InternalError::MaximumPathBuildCallsExceeded,
+            ErrorExt::MaximumPathBuildCallsExceeded,
         )
     }
 }
 
-fn checked_sub(value: &mut usize, underflow_error: InternalError) -> Result<(), InternalError> {
+fn checked_sub(value: &mut usize, underflow_error: ErrorExt) -> Result<(), ErrorExt> {
     *value = value.checked_sub(1).ok_or(underflow_error)?;
     Ok(())
 }

src/end_entity.rs

@@ -13,7 +13,7 @@
 // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
 use crate::{
-    cert, name, signed_data, verify_cert, DnsNameRef, Error, SignatureAlgorithm, Time,
+    cert, name, signed_data, verify_cert, DnsNameRef, Error, ErrorExt, SignatureAlgorithm, Time,
     TlsClientTrustAnchors, TlsServerTrustAnchors,
 };
 
@@ -79,6 +79,25 @@ impl<'a> EndEntityCert<'a> {
         &self.inner
     }
 
+    /// Backward-SemVer-compatible wrapper around `verify_is_valid_tls_server_cert_ext`.
+    ///
+    /// Errors that aren't representable as an `Error` are mapped to `Error::UnknownIssuer`.
+    pub fn verify_is_valid_tls_server_cert(
+        &self,
+        supported_sig_algs: &[&SignatureAlgorithm],
+        trust_anchors: &TlsServerTrustAnchors,
+        intermediate_certs: &[&[u8]],
+        time: Time,
+    ) -> Result<(), Error> {
+        self.verify_is_valid_tls_server_cert_ext(
+            supported_sig_algs,
+            trust_anchors,
+            intermediate_certs,
+            time,
+        )
+        .map_err(ErrorExt::into_error_lossy)
+    }
+
     /// Verifies that the end-entity certificate is valid for use by a TLS
     /// server.
     ///
@@ -89,13 +108,13 @@ impl<'a> EndEntityCert<'a> {
     /// intermediate certificates that the server sent in the TLS handshake.
     /// `time` is the time for which the validation is effective (usually the
     /// current time).
-    pub fn verify_is_valid_tls_server_cert(
+    pub fn verify_is_valid_tls_server_cert_ext(
         &self,
         supported_sig_algs: &[&SignatureAlgorithm],
         &TlsServerTrustAnchors(trust_anchors): &TlsServerTrustAnchors,
         intermediate_certs: &[&[u8]],
         time: Time,
-    ) -> Result<(), Error> {
+    ) -> Result<(), ErrorExt> {
         verify_cert::build_chain(
             verify_cert::EKU_SERVER_AUTH,
             supported_sig_algs,
@@ -120,7 +139,7 @@ impl<'a> EndEntityCert<'a> {
     /// `cert` is the purported end-entity certificate of the client. `time` is
     /// the time for which the validation is effective (usually the current
     /// time).
-    pub fn verify_is_valid_tls_client_cert(
+    pub fn verify_is_valid_tls_client_cert_ext(
         &self,
         supported_sig_algs: &[&SignatureAlgorithm],
         &TlsClientTrustAnchors(trust_anchors): &TlsClientTrustAnchors,
@@ -135,6 +154,7 @@ impl<'a> EndEntityCert<'a> {
             &self.inner,
             time,
         )
+        .map_err(ErrorExt::into_error_lossy)
     }
 
     /// Verifies that the certificate is valid for the given DNS host name.

src/error.rs

@@ -107,45 +107,37 @@ impl fmt::Display for Error {
 #[cfg(feature = "std")]
 impl ::std::error::Error for Error {}
 
-/// Errors that we cannot report externally since `Error` wasn't declared
-/// non-exhaustive, but which we need to differentiate internally (at least
-/// for testing).
-pub(crate) enum InternalError {
+/// An error that occurs during certificate validation or name validation.
+///
+/// `ErrorExt` effectively extends `Error` to support reporting new errors. Because `Error` is not
+/// declared `#[non_exhaustive]` it could not be directly extended in a backward-compatible way.
+#[non_exhaustive]
+pub enum ErrorExt {
+    Error(Error),
     MaximumSignatureChecksExceeded,
     /// The maximum number of internal path building calls has been reached. Path complexity is too great.
     MaximumPathBuildCallsExceeded,
 }
 
-impl InternalError {
-    fn is_fatal(&self) -> bool {
-        matches!(
-            self,
-            Self::MaximumSignatureChecksExceeded | Self::MaximumPathBuildCallsExceeded
-        )
-    }
-}
-
-pub(crate) enum ErrorOrInternalError {
-    Error(Error),
-    InternalError(InternalError),
-}
-
-impl ErrorOrInternalError {
-    pub fn is_fatal(&self) -> bool {
+impl ErrorExt {
+    pub(crate) fn is_fatal(&self) -> bool {
         match self {
-            ErrorOrInternalError::Error(_) => false,
-            ErrorOrInternalError::InternalError(e) => e.is_fatal(),
+            Self::Error(_) => false,
+            Self::MaximumSignatureChecksExceeded | Self::MaximumPathBuildCallsExceeded => true,
+        }
+    }
+
+    pub(crate) fn into_error_lossy(self) -> Error {
+        match self {
+            Self::Error(e) => e,
+            Self::MaximumSignatureChecksExceeded | Self::MaximumPathBuildCallsExceeded => {
+                Error::UnknownIssuer
+            }
         }
     }
 }
 
-impl From<InternalError> for ErrorOrInternalError {
-    fn from(value: InternalError) -> Self {
-        Self::InternalError(value)
-    }
-}
-
-impl From<Error> for ErrorOrInternalError {
+impl From<Error> for ErrorExt {
     fn from(error: Error) -> Self {
         Self::Error(error)
     }

src/lib.rs

@@ -59,7 +59,7 @@ mod verify_cert;
 
 pub use {
     end_entity::EndEntityCert,
-    error::Error,
+    error::{Error, ErrorExt},
     name::{DnsNameRef, InvalidDnsNameError},
     signed_data::{
         SignatureAlgorithm, ECDSA_P256_SHA256, ECDSA_P256_SHA384, ECDSA_P384_SHA256,

src/verify_cert.rs

@@ -18,7 +18,7 @@ use crate::{
     budget::Budget,
     cert::{self, Cert, EndEntityOrCa},
     der, equal,
-    error::ErrorOrInternalError,
+    error::ErrorExt,
     name, signed_data, time, Error, SignatureAlgorithm, TrustAnchor,
 };
 
@@ -29,8 +29,8 @@ pub fn build_chain(
     intermediate_certs: &[&[u8]],
     cert: &Cert,
     time: time::Time,
-) -> Result<(), Error> {
-    let result = build_chain_inner(
+) -> Result<(), ErrorExt> {
+    build_chain_inner(
         required_eku_if_present,
         supported_sig_algs,
         trust_anchors,
@@ -39,14 +39,7 @@ pub fn build_chain(
         time,
         0,
         &mut Budget::default(),
-    );
-    result.map_err(|error| {
-        match error {
-            ErrorOrInternalError::Error(e) => e,
-            // Eat internal errors,
-            ErrorOrInternalError::InternalError(_) => Error::UnknownIssuer,
-        }
-    })
+    )
 }
 
 #[allow(clippy::too_many_arguments)]
@@ -59,7 +52,7 @@ fn build_chain_inner(
     time: time::Time,
     sub_ca_count: usize,
     budget: &mut Budget,
-) -> Result<(), ErrorOrInternalError> {
+) -> Result<(), ErrorExt> {
     let used_as_ca = used_as_ca(&cert.ee_or_ca);
 
     check_issuer_independent_properties(
@@ -164,7 +157,7 @@ fn check_signatures(
     cert_chain: &Cert,
     trust_anchor_key: untrusted::Input,
     budget: &mut Budget,
-) -> Result<(), ErrorOrInternalError> {
+) -> Result<(), ErrorExt> {
     let mut spki_value = trust_anchor_key;
     let mut cert = cert_chain;
     loop {
@@ -395,8 +388,8 @@ fn check_eku(
 
 fn loop_while_non_fatal_error<V>(
     values: V,
-    mut f: impl FnMut(V::Item) -> Result<(), ErrorOrInternalError>,
-) -> Result<(), ErrorOrInternalError>
+    mut f: impl FnMut(V::Item) -> Result<(), ErrorExt>,
+) -> Result<(), ErrorExt>
 where
     V: IntoIterator,
 {
@@ -424,7 +417,6 @@ mod tests {
     use core::convert::TryFrom;
 
     use super::*;
-    use crate::error::InternalError;
 
     enum ChainTrustAnchor {
         InChain,
@@ -434,7 +426,7 @@ mod tests {
     fn build_degenerate_chain(
         intermediate_count: usize,
         trust_anchor: ChainTrustAnchor,
-    ) -> ErrorOrInternalError {
+    ) -> ErrorExt {
         use crate::ECDSA_P256_SHA256;
         use crate::{EndEntityCert, Time};
 
@@ -504,16 +496,13 @@ mod tests {
     fn test_too_many_signatures() {
         assert!(matches!(
             build_degenerate_chain(5, ChainTrustAnchor::NotInChain),
-            ErrorOrInternalError::InternalError(InternalError::MaximumSignatureChecksExceeded),
+            ErrorExt::MaximumSignatureChecksExceeded,
         ));
     }
 
     #[test]
     fn test_too_many_path_calls() {
         let result = build_degenerate_chain(10, ChainTrustAnchor::InChain);
-        assert!(matches!(
-            result,
-            ErrorOrInternalError::InternalError(InternalError::MaximumPathBuildCallsExceeded),
-        ));
+        assert!(matches!(result, ErrorExt::MaximumPathBuildCallsExceeded,));
     }
 }