NFC: Encapsulate `Budget` internals.

Make it clear that nothing is reaching into the internals of `Budget`.
In particular, clarify that the tests are not messing around with the
defaults.
This commit is contained in:
Brian Smith 2023-09-29 18:47:10 -07:00
parent 93aca11db8
commit 7793842de1
4 changed files with 102 additions and 81 deletions

57
src/budget.rs Normal file
View File

@ -0,0 +1,57 @@
// Copyright 2015 Brian Smith.
// Portions Copyright 2033 Daniel McCarney.
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
// copyright notice and this permission notice appear in all copies.
//
// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHORS DISCLAIM ALL WARRANTIES
// WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
// MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR
// ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
// WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
// 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;
pub(super) struct Budget {
signatures: usize,
build_chain_calls: usize,
}
impl Budget {
#[inline]
pub fn consume_signature(&mut self) -> Result<(), InternalError> {
self.signatures = self
.signatures
.checked_sub(1)
.ok_or(InternalError::MaximumSignatureChecksExceeded)?;
Ok(())
}
#[inline]
pub fn consume_build_chain_call(&mut self) -> Result<(), InternalError> {
self.build_chain_calls = self
.build_chain_calls
.checked_sub(1)
.ok_or(InternalError::MaximumPathBuildCallsExceeded)?;
Ok(())
}
}
impl Default for Budget {
fn default() -> Self {
Self {
// This limit is taken from the remediation for golang CVE-2018-16875. However,
// note that golang subsequently implemented AKID matching due to this limit
// being hit in real applications (see <https://github.com/spiffe/spire/issues/1004>).
// So this may actually be too aggressive.
signatures: 100,
// This limit is taken from NSS libmozpkix, see:
// <https://github.com/nss-dev/nss/blob/bb4a1d38dd9e92923525ac6b5ed0288479f3f3fc/lib/mozpkix/lib/pkixbuild.cpp#L381-L393>
build_chain_calls: 200_000,
}
}
}

View File

@ -106,3 +106,41 @@ impl fmt::Display for Error {
/// Requires the `std` feature.
#[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 {
MaximumSignatureChecksExceeded,
/// The maximum number of internal path building calls has been reached. Path complexity is too great.
MaximumPathBuildCallsExceeded,
}
pub(crate) enum ErrorOrInternalError {
Error(Error),
InternalError(InternalError),
}
impl ErrorOrInternalError {
pub fn is_fatal(&self) -> bool {
match self {
ErrorOrInternalError::Error(_) => false,
ErrorOrInternalError::InternalError(InternalError::MaximumSignatureChecksExceeded)
| ErrorOrInternalError::InternalError(InternalError::MaximumPathBuildCallsExceeded) => {
true
}
}
}
}
impl From<InternalError> for ErrorOrInternalError {
fn from(value: InternalError) -> Self {
Self::InternalError(value)
}
}
impl From<Error> for ErrorOrInternalError {
fn from(error: Error) -> Self {
Self::Error(error)
}
}

View File

@ -41,6 +41,8 @@
#[cfg_attr(test, macro_use)]
extern crate alloc;
mod budget;
#[macro_use]
mod der;

View File

@ -15,8 +15,11 @@
use core::default::Default;
use crate::{
budget::Budget,
cert::{self, Cert, EndEntityOrCa},
der, name, signed_data, time, Error, SignatureAlgorithm, TrustAnchor,
der,
error::ErrorOrInternalError,
name, signed_data, time, Error, SignatureAlgorithm, TrustAnchor,
};
pub fn build_chain(
@ -46,44 +49,6 @@ pub fn build_chain(
})
}
/// Errors that we cannot report externally since `Error` wasn't declared
/// non-exhaustive, but which we need to differentiate internally (at least
/// for testing).
enum InternalError {
MaximumSignatureChecksExceeded,
/// The maximum number of internal path building calls has been reached. Path complexity is too great.
MaximumPathBuildCallsExceeded,
}
enum ErrorOrInternalError {
Error(Error),
InternalError(InternalError),
}
impl ErrorOrInternalError {
fn is_fatal(&self) -> bool {
match self {
ErrorOrInternalError::Error(_) => false,
ErrorOrInternalError::InternalError(InternalError::MaximumSignatureChecksExceeded)
| ErrorOrInternalError::InternalError(InternalError::MaximumPathBuildCallsExceeded) => {
true
}
}
}
}
impl From<InternalError> for ErrorOrInternalError {
fn from(value: InternalError) -> Self {
Self::InternalError(value)
}
}
impl From<Error> for ErrorOrInternalError {
fn from(error: Error) -> Self {
Self::Error(error)
}
}
#[allow(clippy::too_many_arguments)]
fn build_chain_inner(
required_eku_if_present: KeyPurposeId,
@ -248,47 +213,6 @@ fn check_signed_chain_name_constraints(
Ok(())
}
struct Budget {
signatures: usize,
build_chain_calls: usize,
}
impl Budget {
#[inline]
fn consume_signature(&mut self) -> Result<(), InternalError> {
self.signatures = self
.signatures
.checked_sub(1)
.ok_or(InternalError::MaximumSignatureChecksExceeded)?;
Ok(())
}
#[inline]
fn consume_build_chain_call(&mut self) -> Result<(), InternalError> {
self.build_chain_calls = self
.build_chain_calls
.checked_sub(1)
.ok_or(InternalError::MaximumPathBuildCallsExceeded)?;
Ok(())
}
}
impl Default for Budget {
fn default() -> Self {
Self {
// This limit is taken from the remediation for golang CVE-2018-16875. However,
// note that golang subsequently implemented AKID matching due to this limit
// being hit in real applications (see <https://github.com/spiffe/spire/issues/1004>).
// So this may actually be too aggressive.
signatures: 100,
// This limit is taken from NSS libmozpkix, see:
// <https://github.com/nss-dev/nss/blob/bb4a1d38dd9e92923525ac6b5ed0288479f3f3fc/lib/mozpkix/lib/pkixbuild.cpp#L381-L393>
build_chain_calls: 200_000,
}
}
}
fn check_issuer_independent_properties(
cert: &Cert,
time: time::Time,
@ -497,7 +421,7 @@ mod tests {
use core::convert::TryFrom;
use super::*;
use crate::verify_cert::{Budget, InternalError};
use crate::error::InternalError;
enum ChainTrustAnchor {
InChain,