From a91bfdc67d50fba787cb810154fbee935fefdbce Mon Sep 17 00:00:00 2001 From: Kornel Date: Fri, 13 Jun 2025 12:21:41 +0100 Subject: [PATCH] Error descriptions and docs --- boring/src/error.rs | 54 ++++++++++++++++++----------- boring/src/ssl/error.rs | 75 ++++++++++++++++++++++++++++++++--------- 2 files changed, 93 insertions(+), 36 deletions(-) diff --git a/boring/src/error.rs b/boring/src/error.rs index 0b01c978..08d8e82c 100644 --- a/boring/src/error.rs +++ b/boring/src/error.rs @@ -15,7 +15,8 @@ //! Err(e) => println!("Parsing Error: {:?}", e), //! } //! ``` -use libc::{c_char, c_uint}; +use libc::{c_char, c_int, c_uint}; +use openssl_macros::corresponds; use std::borrow::Cow; use std::error; use std::ffi::CStr; @@ -80,7 +81,9 @@ impl fmt::Display for ErrorStack { write!( fmt, "[{}]", - err.reason_internal().unwrap_or("unknown reason") + err.reason_internal() + .or_else(|| err.library()) + .unwrap_or("unknown reason") )?; } Ok(()) @@ -101,7 +104,7 @@ impl From for fmt::Error { } } -/// An error reported from OpenSSL. +/// A detailed error reported as part of an [`ErrorStack`]. #[derive(Clone)] pub struct Error { code: c_uint, @@ -179,7 +182,10 @@ impl Error { } } - /// Returns the raw OpenSSL error code for this error. + /// Returns a raw OpenSSL **packed** error code for this error, which **can't be reliably compared to any error constant**. + /// + /// Use [`Error::library_code()`] and [`Error::reason_code()`] instead. + /// Packed error codes are different than [SSL error codes](crate::ssl::ErrorCode). #[must_use] pub fn code(&self) -> c_uint { self.code @@ -201,10 +207,11 @@ impl Error { } } - /// Returns the raw OpenSSL error constant for the library reporting the - /// error. + /// Returns the raw OpenSSL error constant for the library reporting the error (`ERR_LIB_{name}`). + /// + /// Error [reason codes](Error::reason_code) are not globally unique, but scoped to each library. #[must_use] - pub fn library_code(&self) -> libc::c_int { + pub fn library_code(&self) -> c_int { ffi::ERR_GET_LIB(self.code) } @@ -226,9 +233,14 @@ impl Error { } } - /// Returns the raw OpenSSL error constant for the reason for the error. + /// Returns [library-specific](Error::library_code) reason code corresponding to some of the `{lib}_R_{reason}` constants. + /// + /// Reason codes are ambiguous, and different libraries reuse the same numeric values for different errors. + /// + /// For `ERR_LIB_SYS` the reason code is `errno`. `ERR_LIB_USER` can use any values. + /// Other libraries may use [`ERR_R_*`](ffi::ERR_R_FATAL) or their own codes. #[must_use] - pub fn reason_code(&self) -> libc::c_int { + pub fn reason_code(&self) -> c_int { ffi::ERR_GET_REASON(self.code) } @@ -285,17 +297,19 @@ impl Error { impl fmt::Debug for Error { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { let mut builder = fmt.debug_struct("Error"); - builder.field("code", &self.code()); - if let Some(library) = self.library() { - builder.field("library", &library); + builder.field("code", &self.code); + if !self.is_internal() { + if let Some(library) = self.library() { + builder.field("library", &library); + } + builder.field("library_code", &self.library_code()); + if let Some(reason) = self.reason() { + builder.field("reason", &reason); + } + builder.field("reason_code", &self.reason_code()); + builder.field("file", &self.file()); + builder.field("line", &self.line()); } - builder.field("library_code", &self.library_code()); - if let Some(reason) = self.reason() { - builder.field("reason", &reason); - } - builder.field("reason_code", &self.reason_code()); - builder.field("file", &self.file()); - builder.field("line", &self.line()); if let Some(data) = self.data() { builder.field("data", &data); } @@ -309,7 +323,7 @@ impl fmt::Display for Error { fmt, "{}\n\nCode: {:08X}\nLoc: {}:{}", self.reason_internal().unwrap_or("unknown TLS error"), - self.code(), + &self.code, self.file(), self.line() ) diff --git a/boring/src/ssl/error.rs b/boring/src/ssl/error.rs index f62e5f32..5acad820 100644 --- a/boring/src/ssl/error.rs +++ b/boring/src/ssl/error.rs @@ -1,16 +1,20 @@ use crate::ffi; use crate::x509::X509VerifyError; use libc::c_int; +use openssl_macros::corresponds; use std::error; use std::error::Error as StdError; +use std::ffi::CStr; use std::fmt; use std::io; use crate::error::ErrorStack; use crate::ssl::MidHandshakeSslStream; -/// An error code returned from SSL functions. -#[derive(Debug, Copy, Clone, PartialEq, Eq)] +/// `SSL_ERROR_*` error code returned from SSL functions. +/// +/// This is different than [packed error codes](crate::error::Error). +#[derive(Copy, Clone, PartialEq, Eq)] pub struct ErrorCode(c_int); impl ErrorCode { @@ -50,16 +54,52 @@ impl ErrorCode { /// An error occurred in the SSL library. pub const SSL: ErrorCode = ErrorCode(ffi::SSL_ERROR_SSL); + /// Wrap an `SSL_ERROR_*` error code. + /// + /// This is different than [packed error codes](crate::error::Error). #[must_use] + #[inline] + #[cfg_attr(debug_assertions, track_caller)] pub fn from_raw(raw: c_int) -> ErrorCode { - ErrorCode(raw) + let code = ErrorCode(raw); + debug_assert!( + raw < 64 || code.description().is_some(), + "{raw} is not an SSL_ERROR_* code" + ); + code } + /// An `SSL_ERROR_*` error code. + /// + /// This is different than [packed error codes](crate::error::Error). #[allow(clippy::trivially_copy_pass_by_ref)] #[must_use] pub fn as_raw(&self) -> c_int { self.0 } + + #[corresponds(SSL_error_description)] + pub fn description(self) -> Option<&'static str> { + unsafe { + let msg = ffi::SSL_error_description(self.0); + if msg.is_null() { + return None; + } + CStr::from_ptr(msg).to_str().ok() + } + } +} + +impl fmt::Display for ErrorCode { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{} ({})", self.description().unwrap_or("error"), self.0) + } +} + +impl fmt::Debug for ErrorCode { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(self, f) + } } #[derive(Debug)] @@ -68,7 +108,7 @@ pub(crate) enum InnerError { Ssl(ErrorStack), } -/// An SSL error. +/// A general SSL error, based on [`SSL_ERROR_*` error codes](ErrorCode). #[derive(Debug)] pub struct Error { pub(crate) code: ErrorCode, @@ -76,6 +116,7 @@ pub struct Error { } impl Error { + /// An `SSL_ERROR_*` error code. #[must_use] pub fn code(&self) -> ErrorCode { self.code @@ -96,6 +137,7 @@ impl Error { } } + /// Stack of [library-specific errors](crate::error::Error), if available. #[must_use] pub fn ssl_error(&self) -> Option<&ErrorStack> { match self.cause { @@ -131,26 +173,27 @@ impl From for Error { impl fmt::Display for Error { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - match self.code { - ErrorCode::ZERO_RETURN => fmt.write_str("the SSL session has been shut down"), + let msg = match self.code { + ErrorCode::ZERO_RETURN => "the SSL session has been shut down", ErrorCode::WANT_READ => match self.io_error() { - Some(_) => fmt.write_str("a nonblocking read call would have blocked"), - None => fmt.write_str("the operation should be retried"), + Some(_) => "a nonblocking read call would have blocked", + None => "the operation should be retried", }, ErrorCode::WANT_WRITE => match self.io_error() { - Some(_) => fmt.write_str("a nonblocking write call would have blocked"), - None => fmt.write_str("the operation should be retried"), + Some(_) => "a nonblocking write call would have blocked", + None => "the operation should be retried", }, ErrorCode::SYSCALL => match self.io_error() { - Some(err) => write!(fmt, "{err}"), - None => fmt.write_str("unexpected EOF"), + Some(err) => return err.fmt(fmt), + None => "unexpected EOF", }, ErrorCode::SSL => match self.ssl_error() { - Some(e) => write!(fmt, "{e}"), - None => fmt.write_str("unknown BoringSSL error"), + Some(err) => return err.fmt(fmt), + None => "unknown BoringSSL error", }, - ErrorCode(code) => write!(fmt, "unknown error code {code}"), - } + ErrorCode(code) => return code.fmt(fmt), + }; + fmt.write_str(msg) } }