From a264df22fa0e8f481c88667ca7712cec9820d8c2 Mon Sep 17 00:00:00 2001 From: Kornel Date: Tue, 26 Aug 2025 11:59:03 +0100 Subject: [PATCH 1/8] Clippy --- boring-sys/src/lib.rs | 1 + boring/src/stack.rs | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/boring-sys/src/lib.rs b/boring-sys/src/lib.rs index 8463fe1f..6f027919 100644 --- a/boring-sys/src/lib.rs +++ b/boring-sys/src/lib.rs @@ -20,6 +20,7 @@ use std::os::raw::{c_char, c_int, c_uint, c_ulong}; clippy::useless_transmute, clippy::derive_partial_eq_without_eq, clippy::ptr_offset_with_cast, + unpredictable_function_pointer_comparisons, // TODO: remove Eq/PartialEq in v5 dead_code )] mod generated { diff --git a/boring/src/stack.rs b/boring/src/stack.rs index 67958e15..5a95d98c 100644 --- a/boring/src/stack.rs +++ b/boring/src/stack.rs @@ -192,14 +192,14 @@ impl StackRef { } #[must_use] - pub fn iter(&self) -> Iter { + pub fn iter(&self) -> Iter<'_, T> { Iter { stack: self, idxs: 0..self.len(), } } - pub fn iter_mut(&mut self) -> IterMut { + pub fn iter_mut(&mut self) -> IterMut<'_, T> { IterMut { idxs: 0..self.len(), stack: self, From 404a753921d45b430759488c7efd287229ec3874 Mon Sep 17 00:00:00 2001 From: Kornel Date: Fri, 29 Aug 2025 19:31:36 +0100 Subject: [PATCH 2/8] Bump --- Cargo.toml | 8 ++++---- RELEASE_NOTES | 20 +++++++++++++++++++- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1ad33d8b..e45e9849 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,7 +8,7 @@ members = [ resolver = "2" [workspace.package] -version = "4.17.0" +version = "4.18.0" repository = "https://github.com/cloudflare/boring" edition = "2021" @@ -19,9 +19,9 @@ tag-prefix = "" publish = false [workspace.dependencies] -boring-sys = { version = "4.17.0", path = "./boring-sys" } -boring = { version = "4.17.0", path = "./boring" } -tokio-boring = { version = "4.17.0", path = "./tokio-boring" } +boring-sys = { version = "4.18.0", path = "./boring-sys" } +boring = { version = "4.18.0", path = "./boring" } +tokio-boring = { version = "4.18.0", path = "./tokio-boring" } bindgen = { version = "0.72.0", default-features = false, features = ["runtime"] } bytes = "1" diff --git a/RELEASE_NOTES b/RELEASE_NOTES index 17c57f82..fe369a6f 100644 --- a/RELEASE_NOTES +++ b/RELEASE_NOTES @@ -1,3 +1,21 @@ +4.18.0 +- 2025-05-29 Add set_verify_param +- 2025-05-28 Add support for X509_STORE_CTX_get0_untrusted +- 2025-06-02 Add X509VerifyParamRef::copy_from (#361) +- 2025-06-02 Fix X509VerifyContextRef::set_verify_param (#358) +- 2025-06-02 Ensure we call X509_STORE_CTX_cleanup on error path too (#360) +- 2025-06-02 Add mutable ex_data APIs for X509StoreContext +- 2025-06-02 Add X509StoreContextRef::init_without_cleanup +- 2025-06-04 Rename to reset_with_context_data +- 2025-06-05 Avoid panicking in error handling +- 2025-06-05 Don't unwrap when Result can be returned instead +- 2025-06-04 Make X509Store shareable between contexts +- 2025-06-05 Sprinkle #[must_use] (#368) +- 2025-06-05 Expose SSL_set1_groups to Efficiently Set Curves on SSL Session (#346) +- 2025-06-09 Upgrade bindgen to v0.72.0 +- 2025-06-13 Expose PKey::raw_{private,public}_key (#364) +- 2025-06-10 Don't link binaries on docs.rs +- 2025-06-11 Use cargo:warning for warnings 4.17.0 - 2025-05-27 Revert "feat(x509): Implement `Clone` for `X509Store` (#339)" (#353) @@ -545,7 +563,7 @@ - 2019-12-01 Change *const to *mut to try if it fixes tests - 2019-12-01 move EVP_PKCS82PKEY into evp module - 2019-12-01 Support for PKCS#8 unencrypted private key deserialization -- 2019-11-23 Update openssl/src/hash.rs +- 2019-11-23 Update openssl/src/hash.rs - 2019-11-22 Add EVP_md_null() and MessageDigest::md_null() - 2019-11-22 Fix up base64 docs - 2019-11-22 Cleanup From 3de138566027593b04030199d207a1ac58813859 Mon Sep 17 00:00:00 2001 From: Kornel Date: Wed, 3 Sep 2025 01:33:45 +0100 Subject: [PATCH 3/8] Fix doc links --- boring/src/derive.rs | 1 - boring/src/rsa.rs | 2 -- boring/src/ssl/async_callbacks.rs | 10 +++++----- boring/src/x509/mod.rs | 28 +++++++--------------------- 4 files changed, 12 insertions(+), 29 deletions(-) diff --git a/boring/src/derive.rs b/boring/src/derive.rs index 701d48a3..1952118d 100644 --- a/boring/src/derive.rs +++ b/boring/src/derive.rs @@ -51,7 +51,6 @@ impl<'a> Deriver<'a> { /// /// It can be used to size the buffer passed to [`Deriver::derive`]. #[corresponds(EVP_PKEY_derive)] - /// [`EVP_PKEY_derive`]: https://www.openssl.org/docs/man1.0.2/crypto/EVP_PKEY_derive_init.html pub fn len(&mut self) -> Result { unsafe { let mut len = 0; diff --git a/boring/src/rsa.rs b/boring/src/rsa.rs index 5a0ae24d..ff47e71b 100644 --- a/boring/src/rsa.rs +++ b/boring/src/rsa.rs @@ -413,7 +413,6 @@ impl Rsa { /// `n` is the modulus common to both public and private key. /// `e` is the public exponent. #[corresponds(RSA_new)] - /// [`RSA_set0_key`]: https://www.openssl.org/docs/man1.1.0/crypto/RSA_set0_key.html pub fn from_public_components(n: BigNum, e: BigNum) -> Result, ErrorStack> { unsafe { let rsa = cvt_p(ffi::RSA_new())?; @@ -472,7 +471,6 @@ impl RsaPrivateKeyBuilder { /// `n` is the modulus common to both public and private key. /// `e` is the public exponent and `d` is the private exponent. #[corresponds(RSA_new)] - /// [`RSA_set0_key`]: https://www.openssl.org/docs/man1.1.0/crypto/RSA_set0_key.html pub fn new(n: BigNum, e: BigNum, d: BigNum) -> Result { unsafe { let rsa = cvt_p(ffi::RSA_new())?; diff --git a/boring/src/ssl/async_callbacks.rs b/boring/src/ssl/async_callbacks.rs index 93226b48..e4bdf846 100644 --- a/boring/src/ssl/async_callbacks.rs +++ b/boring/src/ssl/async_callbacks.rs @@ -11,7 +11,7 @@ use std::pin::Pin; use std::sync::LazyLock; use std::task::{ready, Context, Poll, Waker}; -/// The type of futures to pass to [`SslContextBuilderExt::set_async_select_certificate_callback`]. +/// The type of futures to pass to [`SslContextBuilder::set_async_select_certificate_callback`]. pub type BoxSelectCertFuture = ExDataFuture>; /// The type of callbacks returned by [`BoxSelectCertFuture`] methods. @@ -25,19 +25,19 @@ pub type BoxPrivateKeyMethodFuture = pub type BoxPrivateKeyMethodFinish = Box Result>; -/// The type of futures to pass to [`SslContextBuilderExt::set_async_get_session_callback`]. +/// The type of futures to pass to [`SslContextBuilder::set_async_get_session_callback`]. pub type BoxGetSessionFuture = ExDataFuture>; /// The type of callbacks returned by [`BoxSelectCertFuture`] methods. pub type BoxGetSessionFinish = Box Option>; -/// The type of futures to pass to [`SslContextBuilderExt::set_async_custom_verify_callback`]. +/// The type of futures to pass to [`SslContextBuilder::set_async_custom_verify_callback`]. pub type BoxCustomVerifyFuture = ExDataFuture>; /// The type of callbacks returned by [`BoxCustomVerifyFuture`] methods. pub type BoxCustomVerifyFinish = Box Result<(), SslAlert>>; -/// Convenience alias for futures stored in [`Ssl`] ex data by [`SslContextBuilderExt`] methods. +/// Convenience alias for futures stored in [`Ssl`] ex data by [`SslContextBuilder`] methods. /// /// Public for documentation purposes. pub type ExDataFuture = Pin + Send>>; @@ -123,7 +123,7 @@ impl SslContextBuilder { /// /// # Safety /// - /// The returned [`SslSession`] must not be associated with a different [`SslContext`]. + /// The returned [`SslSession`] must not be associated with a different [`SslContextBuilder`]. pub unsafe fn set_async_get_session_callback(&mut self, callback: F) where F: Fn(&mut SslRef, &[u8]) -> Option + Send + Sync + 'static, diff --git a/boring/src/x509/mod.rs b/boring/src/x509/mod.rs index 5bac50c9..b9680e65 100644 --- a/boring/src/x509/mod.rs +++ b/boring/src/x509/mod.rs @@ -167,11 +167,10 @@ impl X509StoreContextRef { /// * `cert_chain` - The certificates chain. /// * `with_context` - The closure that is called with the initialized context. /// - /// This corresponds to [`X509_STORE_CTX_init`] before calling `with_context` and to - /// [`X509_STORE_CTX_cleanup`] after calling `with_context`. + /// Calls [`X509_STORE_CTX_cleanup`] after calling `with_context`. /// - /// [`X509_STORE_CTX_init`]: https://www.openssl.org/docs/man1.0.2/crypto/X509_STORE_CTX_init.html /// [`X509_STORE_CTX_cleanup`]: https://www.openssl.org/docs/man1.0.2/crypto/X509_STORE_CTX_cleanup.html + #[corresponds(X509_STORE_CTX_init)] pub fn init( &mut self, trust: &store::X509StoreRef, @@ -1287,10 +1286,7 @@ pub struct X509ReqBuilder(X509Req); impl X509ReqBuilder { /// Returns a builder for a certificate request. - /// - /// This corresponds to [`X509_REQ_new`]. - /// - ///[`X509_REQ_new`]: https://www.openssl.org/docs/man1.1.0/crypto/X509_REQ_new.html + #[corresponds(X509_REQ_new)] pub fn new() -> Result { unsafe { ffi::init(); @@ -1299,10 +1295,7 @@ impl X509ReqBuilder { } /// Set the numerical value of the version field. - /// - /// This corresponds to [`X509_REQ_set_version`]. - /// - ///[`X509_REQ_set_version`]: https://www.openssl.org/docs/man1.1.0/crypto/X509_REQ_set_version.html + #[corresponds(X509_REQ_set_version)] pub fn set_version(&mut self, version: i32) -> Result<(), ErrorStack> { unsafe { cvt(ffi::X509_REQ_set_version(self.0.as_ptr(), version.into())).map(|_| ()) } } @@ -1460,10 +1453,7 @@ impl X509ReqRef { } /// Returns the public key of the certificate request. - /// - /// This corresponds to [`X509_REQ_get_pubkey"] - /// - /// [`X509_REQ_get_pubkey`]: https://www.openssl.org/docs/man1.1.0/crypto/X509_REQ_get_pubkey.html + #[corresponds(X509_REQ_get_pubkey)] pub fn public_key(&self) -> Result, ErrorStack> { unsafe { let key = cvt_p(ffi::X509_REQ_get_pubkey(self.as_ptr()))?; @@ -1474,10 +1464,7 @@ impl X509ReqRef { /// Check if the certificate request is signed using the given public key. /// /// Returns `true` if verification succeeds. - /// - /// This corresponds to [`X509_REQ_verify"]. - /// - /// [`X509_REQ_verify`]: https://www.openssl.org/docs/man1.1.0/crypto/X509_REQ_verify.html + #[corresponds(X509_REQ_verify)] pub fn verify(&self, key: &PKeyRef) -> Result where T: HasPublic, @@ -1486,8 +1473,7 @@ impl X509ReqRef { } /// Returns the extensions of the certificate request. - /// - /// This corresponds to [`X509_REQ_get_extensions"] + #[corresponds(X509_REQ_get_extensions)] pub fn extensions(&self) -> Result, ErrorStack> { unsafe { let extensions = cvt_p(ffi::X509_REQ_get_extensions(self.as_ptr()))?; From 8966ca27b7cf21c75b3f095e66241ee45d707d11 Mon Sep 17 00:00:00 2001 From: Kornel Date: Wed, 3 Sep 2025 01:34:06 +0100 Subject: [PATCH 4/8] Test docs.rs docs --- .github/workflows/ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 56e636e5..083f6919 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -58,6 +58,10 @@ jobs: key: clippy-target-${{ runner.os }}-${{ steps.rust-version.outputs.version }}-${{ hashFiles('Cargo.lock') }} - name: Run clippy run: cargo clippy --all --all-targets + - name: Check docs + run: cargo doc --no-deps -p boring -p boring-sys --features rpk,pq-experimental,underscore-wildcards + env: + - DOCS_RS: 1 test: name: Test runs-on: ${{ matrix.os }} From c5045fb6b451cb85c51864518c89ae432226edff Mon Sep 17 00:00:00 2001 From: Kornel Date: Wed, 3 Sep 2025 16:58:07 +0100 Subject: [PATCH 5/8] Fix patched docs.rs builds --- .github/workflows/ci.yml | 2 +- boring-sys/build/main.rs | 38 +++++++++++++++++++------------------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 083f6919..46477de4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -61,7 +61,7 @@ jobs: - name: Check docs run: cargo doc --no-deps -p boring -p boring-sys --features rpk,pq-experimental,underscore-wildcards env: - - DOCS_RS: 1 + DOCS_RS: 1 test: name: Test runs-on: ${{ matrix.os }} diff --git a/boring-sys/build/main.rs b/boring-sys/build/main.rs index cb0e9fe3..1342e58e 100644 --- a/boring-sys/build/main.rs +++ b/boring-sys/build/main.rs @@ -468,6 +468,24 @@ fn get_extra_clang_args_for_bindgen(config: &Config) -> Vec { } fn ensure_patches_applied(config: &Config) -> io::Result<()> { + if config.env.assume_patched || config.env.path.is_some() { + println!( + "cargo:warning=skipping git patches application, provided\ + native BoringSSL is expected to have the patches included" + ); + return Ok(()); + } else if config.env.source_path.is_some() + && (config.features.rpk + || config.features.pq_experimental + || config.features.underscore_wildcards) + { + panic!( + "BORING_BSSL_ASSUME_PATCHED must be set when setting + BORING_BSSL_SOURCE_PATH and using any of the following + features: rpk, pq-experimental, underscore-wildcards" + ); + } + let mut lock_file = LockFile::open(&config.out_dir.join(".patch_lock"))?; let src_path = get_boringssl_source_path(config); let has_git = src_path.join(".git").exists(); @@ -552,25 +570,6 @@ fn built_boring_source_path(config: &Config) -> &PathBuf { static BUILD_SOURCE_PATH: OnceLock = OnceLock::new(); BUILD_SOURCE_PATH.get_or_init(|| { - if config.env.assume_patched { - println!( - "cargo:warning=skipping git patches application, provided\ - native BoringSSL is expected to have the patches included" - ); - } else if config.env.source_path.is_some() - && (config.features.rpk - || config.features.pq_experimental - || config.features.underscore_wildcards) - { - panic!( - "BORING_BSSL_ASSUME_PATCHED must be set when setting - BORING_BSSL_SOURCE_PATH and using any of the following - features: rpk, pq-experimental, underscore-wildcards" - ); - } else { - ensure_patches_applied(config).unwrap(); - } - let mut cfg = get_boringssl_cmake_config(config); let num_jobs = std::env::var("NUM_JOBS").ok().or_else(|| { @@ -661,6 +660,7 @@ fn get_cpp_runtime_lib(config: &Config) -> Option { fn main() { let config = Config::from_env(); + ensure_patches_applied(&config).unwrap(); if !config.env.docs_rs { emit_link_directives(&config); } From 8d77a5d40e4200df5550cba529e0350aa8f5d33e Mon Sep 17 00:00:00 2001 From: Kornel Date: Fri, 13 Jun 2025 12:42:33 +0100 Subject: [PATCH 6/8] Boring doesn't use function codes --- boring/src/error.rs | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/boring/src/error.rs b/boring/src/error.rs index c81978c5..0b01c978 100644 --- a/boring/src/error.rs +++ b/boring/src/error.rs @@ -208,20 +208,9 @@ impl Error { ffi::ERR_GET_LIB(self.code) } - /// Returns the name of the function reporting the error. - #[must_use] + /// Returns `None`. Boring doesn't use function codes. pub fn function(&self) -> Option<&'static str> { - if self.is_internal() { - return None; - } - unsafe { - let cstr = ffi::ERR_func_error_string(self.code); - if cstr.is_null() { - return None; - } - let bytes = CStr::from_ptr(cstr as *const _).to_bytes(); - str::from_utf8(bytes).ok() - } + None } /// Returns the reason for the error. @@ -256,6 +245,8 @@ impl Error { } /// Returns the line in the source file which encountered the error. + /// + /// 0 if unknown #[allow(clippy::unnecessary_cast)] #[must_use] pub fn line(&self) -> u32 { @@ -299,9 +290,6 @@ impl fmt::Debug for Error { builder.field("library", &library); } builder.field("library_code", &self.library_code()); - if let Some(function) = self.function() { - builder.field("function", &function); - } if let Some(reason) = self.reason() { builder.field("reason", &reason); } From a91bfdc67d50fba787cb810154fbee935fefdbce Mon Sep 17 00:00:00 2001 From: Kornel Date: Fri, 13 Jun 2025 12:21:41 +0100 Subject: [PATCH 7/8] 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) } } From 50fa2e672f78306d044359f0e962cee6f563d985 Mon Sep 17 00:00:00 2001 From: Kornel Date: Fri, 13 Jun 2025 13:05:24 +0100 Subject: [PATCH 8/8] Use ERR_clear_error --- boring/src/error.rs | 16 ++++++++++++++-- boring/src/sign.rs | 4 ++-- boring/src/x509/mod.rs | 2 +- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/boring/src/error.rs b/boring/src/error.rs index 08d8e82c..1d44357e 100644 --- a/boring/src/error.rs +++ b/boring/src/error.rs @@ -35,7 +35,8 @@ pub struct ErrorStack(Vec); impl ErrorStack { /// Pops the contents of the OpenSSL error stack, and returns it. - #[allow(clippy::must_use_candidate)] + #[corresponds(ERR_get_error_line_data)] + #[must_use = "Use ErrorStack::clear() to drop the error stack"] pub fn get() -> ErrorStack { let mut vec = vec![]; while let Some(err) = Error::get() { @@ -45,6 +46,7 @@ impl ErrorStack { } /// Pushes the errors back onto the OpenSSL error stack. + #[corresponds(ERR_put_error)] pub fn put(&self) { for error in self.errors() { error.put(); @@ -56,6 +58,14 @@ impl ErrorStack { pub(crate) fn internal_error(err: impl error::Error) -> Self { Self(vec![Error::new_internal(err.to_string())]) } + + /// Empties the current thread's error queue. + #[corresponds(ERR_clear_error)] + pub(crate) fn clear() { + unsafe { + ffi::ERR_clear_error(); + } + } } impl ErrorStack { @@ -120,7 +130,8 @@ static BORING_INTERNAL: &CStr = c"boring-rust"; impl Error { /// Pops the first error off the OpenSSL error stack. - #[allow(clippy::must_use_candidate)] + #[must_use = "Use ErrorStack::clear() to drop the error stack"] + #[corresponds(ERR_get_error_line_data)] pub fn get() -> Option { unsafe { ffi::init(); @@ -153,6 +164,7 @@ impl Error { } /// Pushes the error back onto the OpenSSL error stack. + #[corresponds(ERR_put_error)] pub fn put(&self) { unsafe { ffi::ERR_put_error( diff --git a/boring/src/sign.rs b/boring/src/sign.rs index 3a4a7e8f..b87e6010 100644 --- a/boring/src/sign.rs +++ b/boring/src/sign.rs @@ -478,7 +478,7 @@ impl<'a> Verifier<'a> { match r { 1 => Ok(true), 0 => { - ErrorStack::get(); // discard error stack + ErrorStack::clear(); // discard error stack Ok(false) } _ => Err(ErrorStack::get()), @@ -500,7 +500,7 @@ impl<'a> Verifier<'a> { match r { 1 => Ok(true), 0 => { - ErrorStack::get(); + ErrorStack::clear(); Ok(false) } _ => Err(ErrorStack::get()), diff --git a/boring/src/x509/mod.rs b/boring/src/x509/mod.rs index b9680e65..52f24fca 100644 --- a/boring/src/x509/mod.rs +++ b/boring/src/x509/mod.rs @@ -815,7 +815,7 @@ impl X509 { if ffi::ERR_GET_LIB(err) == ffi::ERR_LIB_PEM.0.try_into().unwrap() && ffi::ERR_GET_REASON(err) == ffi::PEM_R_NO_START_LINE { - ffi::ERR_clear_error(); + ErrorStack::clear(); break; }