Skip to content

Commit

Permalink
tls: better error messages (istio#1337)
Browse files Browse the repository at this point in the history
The Debug form of these messages is much harder to read than the
Display, so do some tricks to get these to work
  • Loading branch information
howardjohn authored Oct 8, 2024
1 parent 439c87e commit 8080b04
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 5 deletions.
14 changes: 12 additions & 2 deletions src/tls/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,27 @@ pub enum TlsError {
#[error("signing error: {0}")]
SigningError(#[from] identity::Error),
#[error(
"identity verification error: peer did not present the expected SAN ({0:?}), got {1:?}"
"identity verification error: peer did not present the expected SAN ({}), got {}",
display_list(.0),
display_list(.1)
)]
SanError(Vec<Identity>, Vec<Identity>),
#[error(
"identity verification error: peer did not present the expected trustdomain ({0}), got {1:?}"
"identity verification error: peer did not present the expected trustdomain ({0}), got {}",
display_list(.1)
)]
SanTrustDomainError(String, Vec<Identity>),
#[error("ssl error: {0}")]
SslError(#[from] Error),
}

fn display_list<T: ToString>(i: &[T]) -> String {
i.iter()
.map(|id| id.to_string())
.collect::<Vec<String>>()
.join(",")
}

#[cfg(test)]
pub mod tests {
use std::time::Duration;
Expand Down
28 changes: 25 additions & 3 deletions src/tls/workload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
// limitations under the License.

use crate::identity::Identity;
use std::error::Error;
use std::fmt::{Debug, Display};

use crate::tls::lib::provider;
use crate::tls::{ServerCertProvider, TlsError};
Expand Down Expand Up @@ -205,14 +207,34 @@ impl IdentityVerifier {
}
debug!("identity mismatch {id:?} != {:?}", self.identity);
Err(rustls::Error::InvalidCertificate(
rustls::CertificateError::Other(rustls::OtherError(Arc::new(TlsError::SanError(
self.identity.clone(),
id,
rustls::CertificateError::Other(rustls::OtherError(Arc::new(DebugAsDisplay(
TlsError::SanError(self.identity.clone(), id),
)))),
))
}
}

/// DebugAsDisplay is a shim to make an object implement Debug with its Display format
/// This is to workaround rustls only using Debug which makes our errors worse.
struct DebugAsDisplay<T>(T);

impl<T: Display> Debug for DebugAsDisplay<T> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
Display::fmt(&self.0, f)
}
}
impl<T: Display> Display for DebugAsDisplay<T> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
Display::fmt(&self.0, f)
}
}

impl<T: Error + Display> Error for DebugAsDisplay<T> {
fn source(&self) -> Option<&(dyn Error + 'static)> {
self.0.source()
}
}

// Rustls doesn't natively validate URI SAN.
// Build our own verifier, inspired by https://github.com/rustls/rustls/blob/ccb79947a4811412ee7dcddcd0f51ea56bccf101/rustls/src/webpki/server_verifier.rs#L239.
impl ServerCertVerifier for IdentityVerifier {
Expand Down

0 comments on commit 8080b04

Please sign in to comment.