From 2bc82e8d1c7eb0b90e7c07ad53124866b6d521f5 Mon Sep 17 00:00:00 2001 From: James Larisch Date: Wed, 28 May 2025 15:40:15 -0400 Subject: [PATCH 01/13] Add support for X509_STORE_CTX_get0_untrusted --- boring/src/ssl/test/cert_verify.rs | 33 ++++++++++++++++++++++++------ boring/src/x509/mod.rs | 15 ++++++++++++++ 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/boring/src/ssl/test/cert_verify.rs b/boring/src/ssl/test/cert_verify.rs index e8e54061..b55cb26a 100644 --- a/boring/src/ssl/test/cert_verify.rs +++ b/boring/src/ssl/test/cert_verify.rs @@ -72,13 +72,34 @@ fn callback_receives_correct_certificate() { assert!(x509.cert().is_some()); assert!(x509.verify_result().is_err()); - let root = x509.current_cert().unwrap(); - let digest = root.digest(MessageDigest::sha1()).unwrap(); - assert_eq!(hex::encode(digest), root_sha1); + let root = x509 + .current_cert() + .unwrap() + .digest(MessageDigest::sha1()) + .unwrap(); + assert_eq!(hex::encode(root), root_sha1); - let leaf = x509.cert().unwrap(); - let digest = leaf.digest(MessageDigest::sha1()).unwrap(); - assert_eq!(hex::encode(digest), leaf_sha1); + let leaf = x509.cert().unwrap().digest(MessageDigest::sha1()).unwrap(); + assert_eq!(hex::encode(leaf), leaf_sha1); + + // Test that `untrusted` is set to the original chain. + assert_eq!(x509.untrusted().unwrap().len(), 2); + let leaf = x509 + .untrusted() + .unwrap() + .get(0) + .unwrap() + .digest(MessageDigest::sha1()) + .unwrap(); + assert_eq!(hex::encode(leaf), leaf_sha1); + let root = x509 + .untrusted() + .unwrap() + .get(1) + .unwrap() + .digest(MessageDigest::sha1()) + .unwrap(); + assert_eq!(hex::encode(root), root_sha1); true }); diff --git a/boring/src/x509/mod.rs b/boring/src/x509/mod.rs index bfd7fdd5..94871b93 100644 --- a/boring/src/x509/mod.rs +++ b/boring/src/x509/mod.rs @@ -216,6 +216,21 @@ impl X509StoreContextRef { } } + /// Returns a reference to the `X509` certificates used to initialize the + /// [`X509StoreContextRef`]. + #[corresponds(X509_STORE_CTX_get0_untrusted)] + pub fn untrusted(&self) -> Option<&StackRef> { + unsafe { + let certs = ffi::X509_STORE_CTX_get0_untrusted(self.as_ptr()); + + if certs.is_null() { + None + } else { + Some(StackRef::from_ptr(certs)) + } + } + } + /// Returns a reference to the certificate being verified. /// May return None if a raw public key is being verified. #[corresponds(X509_STORE_CTX_get0_cert)] From 7a52fbbe99b6377ca664692462808c84ba81c8a4 Mon Sep 17 00:00:00 2001 From: Anthony Ramine <123095+nox@users.noreply.github.com> Date: Mon, 2 Jun 2025 16:39:11 +0200 Subject: [PATCH 02/13] Add X509VerifyParamRef::copy_from (#361) --- boring/src/x509/verify.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/boring/src/x509/verify.rs b/boring/src/x509/verify.rs index 7546442d..8b63b7bc 100644 --- a/boring/src/x509/verify.rs +++ b/boring/src/x509/verify.rs @@ -182,4 +182,12 @@ impl X509VerifyParamRef { pub fn set_depth(&mut self, depth: c_int) { unsafe { ffi::X509_VERIFY_PARAM_set_depth(self.as_ptr(), depth) } } + + /// Copies parameters from `src`. + /// + /// If a parameter is unset in `src`, the existing value in `self`` is preserved. + #[corresponds(X509_VERIFY_PARAM_set1)] + pub fn copy_from(&mut self, src: &Self) -> Result<(), ErrorStack> { + unsafe { cvt(ffi::X509_VERIFY_PARAM_set1(self.as_ptr(), src.as_ptr())).map(|_| ()) } + } } From 6789a72fc0bdfc30448d1dc5d804ea150d809271 Mon Sep 17 00:00:00 2001 From: Anthony Ramine <123095+nox@users.noreply.github.com> Date: Mon, 2 Jun 2025 16:39:25 +0200 Subject: [PATCH 03/13] Fix X509VerifyContextRef::set_verify_param (#358) This method takes ownership of the given verify param. --- boring/src/x509/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/boring/src/x509/mod.rs b/boring/src/x509/mod.rs index 94871b93..57a0083f 100644 --- a/boring/src/x509/mod.rs +++ b/boring/src/x509/mod.rs @@ -38,7 +38,7 @@ use crate::ssl::SslRef; use crate::stack::{Stack, StackRef, Stackable}; use crate::string::OpensslString; use crate::util::ForeignTypeRefExt; -use crate::x509::verify::X509VerifyParamRef; +use crate::x509::verify::{X509VerifyParam, X509VerifyParamRef}; use crate::{cvt, cvt_n, cvt_p}; pub mod extension; @@ -148,9 +148,9 @@ impl X509StoreContextRef { unsafe { X509VerifyParamRef::from_ptr_mut(ffi::X509_STORE_CTX_get0_param(self.as_ptr())) } } - /// Sets the X509 verifification configuration on the X509_STORE_CTX. + /// Sets the X509 verification configuration. #[corresponds(X509_STORE_CTX_set0_param)] - pub fn set_verify_param(&mut self, param: &mut X509VerifyParamRef) { + pub fn set_verify_param(&mut self, param: X509VerifyParam) { unsafe { ffi::X509_STORE_CTX_set0_param(self.as_ptr(), param.as_ptr()) } } From 15975ddde44c6dfab03bb6e514db8a97f5fb3a6a Mon Sep 17 00:00:00 2001 From: Anthony Ramine <123095+nox@users.noreply.github.com> Date: Mon, 2 Jun 2025 16:40:44 +0200 Subject: [PATCH 04/13] Ensure we call X509_STORE_CTX_cleanup on error path too (#360) As X509_STORE_CTX_init may fail after setting some values that should outlive the store context, we must ensure we clean things up on its error path too. We also know it's always ok to call X509_STORE_CTX_cleanupas X509_STORE_CTX_init starts with a call to it. --- boring/src/x509/mod.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/boring/src/x509/mod.rs b/boring/src/x509/mod.rs index 57a0083f..308dc85d 100644 --- a/boring/src/x509/mod.rs +++ b/boring/src/x509/mod.rs @@ -130,14 +130,15 @@ impl X509StoreContextRef { } unsafe { + let cleanup = Cleanup(self); + cvt(ffi::X509_STORE_CTX_init( - self.as_ptr(), + cleanup.0.as_ptr(), trust.as_ptr(), cert.as_ptr(), cert_chain.as_ptr(), ))?; - let cleanup = Cleanup(self); with_context(cleanup.0) } } From 45f8589d489e6177359727b5d59a6223da13aa75 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 2 Jun 2025 08:56:54 +0200 Subject: [PATCH 05/13] Add mutable ex_data APIs for X509StoreContext --- boring/src/lib.rs | 15 ++++++++++ boring/src/ssl/mod.rs | 17 ++--------- boring/src/x509/mod.rs | 67 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 83 insertions(+), 16 deletions(-) diff --git a/boring/src/lib.rs b/boring/src/lib.rs index aaa268b6..4b84b7c5 100644 --- a/boring/src/lib.rs +++ b/boring/src/lib.rs @@ -108,6 +108,8 @@ extern crate libc; #[cfg(test)] extern crate hex; +use std::ffi::{c_long, c_void}; + #[doc(inline)] pub use crate::ffi::init; @@ -193,3 +195,16 @@ fn cvt_n(r: c_int) -> Result { Ok(r) } } + +unsafe extern "C" fn free_data_box( + _parent: *mut c_void, + ptr: *mut c_void, + _ad: *mut ffi::CRYPTO_EX_DATA, + _idx: c_int, + _argl: c_long, + _argp: *mut c_void, +) { + if !ptr.is_null() { + drop(Box::::from_raw(ptr as *mut T)); + } +} diff --git a/boring/src/ssl/mod.rs b/boring/src/ssl/mod.rs index c6dd92e7..a03cdf6e 100644 --- a/boring/src/ssl/mod.rs +++ b/boring/src/ssl/mod.rs @@ -58,7 +58,7 @@ //! } //! ``` use foreign_types::{ForeignType, ForeignTypeRef, Opaque}; -use libc::{c_char, c_int, c_long, c_uchar, c_uint, c_void}; +use libc::{c_char, c_int, c_uchar, c_uint, c_void}; use openssl_macros::corresponds; use std::any::TypeId; use std::collections::HashMap; @@ -81,7 +81,6 @@ use crate::dh::DhRef; use crate::ec::EcKeyRef; use crate::error::ErrorStack; use crate::ex_data::Index; -use crate::ffi; use crate::nid::Nid; use crate::pkey::{HasPrivate, PKeyRef, Params, Private}; use crate::srtp::{SrtpProtectionProfile, SrtpProtectionProfileRef}; @@ -95,6 +94,7 @@ use crate::x509::{ X509Name, X509Ref, X509StoreContextRef, X509VerifyError, X509VerifyResult, X509, }; use crate::{cvt, cvt_0i, cvt_n, cvt_p, init}; +use crate::{ffi, free_data_box}; pub use self::async_callbacks::{ AsyncPrivateKeyMethod, AsyncPrivateKeyMethodError, AsyncSelectCertError, BoxCustomVerifyFinish, @@ -439,19 +439,6 @@ static SESSION_CTX_INDEX: LazyLock> = static RPK_FLAG_INDEX: LazyLock> = LazyLock::new(|| SslContext::new_ex_index().unwrap()); -unsafe extern "C" fn free_data_box( - _parent: *mut c_void, - ptr: *mut c_void, - _ad: *mut ffi::CRYPTO_EX_DATA, - _idx: c_int, - _argl: c_long, - _argp: *mut c_void, -) { - if !ptr.is_null() { - drop(Box::::from_raw(ptr as *mut T)); - } -} - /// An error returned from the SNI callback. #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub struct SniError(c_int); diff --git a/boring/src/x509/mod.rs b/boring/src/x509/mod.rs index 308dc85d..8ffa732d 100644 --- a/boring/src/x509/mod.rs +++ b/boring/src/x509/mod.rs @@ -30,7 +30,6 @@ use crate::bio::{MemBio, MemBioSlice}; use crate::conf::ConfRef; use crate::error::ErrorStack; use crate::ex_data::Index; -use crate::ffi; use crate::hash::{DigestBytes, MessageDigest}; use crate::nid::Nid; use crate::pkey::{HasPrivate, HasPublic, PKey, PKeyRef, Public}; @@ -40,6 +39,7 @@ use crate::string::OpensslString; use crate::util::ForeignTypeRefExt; use crate::x509::verify::{X509VerifyParam, X509VerifyParamRef}; use crate::{cvt, cvt_n, cvt_p}; +use crate::{ffi, free_data_box}; pub mod extension; pub mod store; @@ -72,6 +72,22 @@ impl X509StoreContext { cvt_p(ffi::X509_STORE_CTX_new()).map(|p| X509StoreContext::from_ptr(p)) } } + + /// Returns a new extra data index. + /// + /// Each invocation of this function is guaranteed to return a distinct index. These can be used + /// to store data in the context that can be retrieved later by callbacks, for example. + #[corresponds(SSL_CTX_get_ex_new_index)] + pub fn new_ex_index() -> Result, ErrorStack> + where + T: 'static + Sync + Send, + { + unsafe { + ffi::init(); + let idx = cvt_n(get_new_x509_store_ctx_idx(Some(free_data_box::)))?; + Ok(Index::from_raw(idx)) + } + } } impl X509StoreContextRef { @@ -88,6 +104,44 @@ impl X509StoreContextRef { } } + /// Returns a mutable reference to the extra data at the specified index. + #[corresponds(X509_STORE_CTX_get_ex_data)] + pub fn ex_data_mut(&mut self, index: Index) -> Option<&mut T> { + unsafe { + let data = ffi::X509_STORE_CTX_get_ex_data(self.as_ptr(), index.as_raw()); + if data.is_null() { + None + } else { + Some(&mut *(data as *mut T)) + } + } + } + + /// Sets or overwrites the extra data at the specified index. + /// + /// This can be used to provide data to callbacks registered with the context. Use the + /// `Ssl::new_ex_index` method to create an `Index`. + /// + /// The previous value, if any, will be returned. + #[corresponds(X509_STORE_CTX_set_ex_data)] + pub fn set_ex_data(&mut self, index: Index, data: T) { + if let Some(old) = self.ex_data_mut(index) { + *old = data; + + return; + } + + unsafe { + let data = Box::new(data); + + ffi::X509_STORE_CTX_set_ex_data( + self.as_ptr(), + index.as_raw(), + Box::into_raw(data) as *mut c_void, + ); + } + } + /// Returns the verify result of the context. #[corresponds(X509_STORE_CTX_get_error)] pub fn verify_result(&self) -> X509VerifyResult { @@ -1688,3 +1742,14 @@ unsafe fn X509_OBJECT_free(x: *mut ffi::X509_OBJECT) { ffi::X509_OBJECT_free_contents(x); ffi::OPENSSL_free(x as *mut libc::c_void); } + +unsafe fn get_new_x509_store_ctx_idx(f: ffi::CRYPTO_EX_free) -> c_int { + // hack around https://rt.openssl.org/Ticket/Display.html?id=3710&user=guest&pass=guest + static ONCE: Once = Once::new(); + + ONCE.call_once(|| { + ffi::X509_STORE_CTX_get_ex_new_index(0, ptr::null_mut(), ptr::null_mut(), None, None); + }); + + ffi::X509_STORE_CTX_get_ex_new_index(0, ptr::null_mut(), ptr::null_mut(), None, f) +} From 56e9fef055b2f7b62fd36fdbe31ddcdbce983c46 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 2 Jun 2025 09:00:09 +0200 Subject: [PATCH 06/13] Add X509StoreContextRef::init_without_cleanup As X509_STORE_CTX_init requires its arguments to outlive the store context, we take ownership of all of them and put them in the store context's ex data, ensuring the soundness of the operation without the mandatory call to X509_STORE_CTX_cleanup after a closure is run. --- boring/src/x509/mod.rs | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/boring/src/x509/mod.rs b/boring/src/x509/mod.rs index 8ffa732d..0ebb83aa 100644 --- a/boring/src/x509/mod.rs +++ b/boring/src/x509/mod.rs @@ -21,6 +21,7 @@ use std::path::Path; use std::ptr; use std::slice; use std::str; +use std::sync::{LazyLock, Once}; use crate::asn1::{ Asn1BitStringRef, Asn1IntegerRef, Asn1Object, Asn1ObjectRef, Asn1StringRef, Asn1TimeRef, @@ -48,6 +49,15 @@ pub mod verify; #[cfg(test)] mod tests; +static STORE_INDEX: LazyLock> = + LazyLock::new(|| X509StoreContext::new_ex_index().unwrap()); + +static CERT_INDEX: LazyLock> = + LazyLock::new(|| X509StoreContext::new_ex_index().unwrap()); + +static CERT_CHAIN_INDEX: LazyLock>> = + LazyLock::new(|| X509StoreContext::new_ex_index().unwrap()); + foreign_type_and_impl_send_sync! { type CType = ffi::X509_STORE_CTX; fn drop = ffi::X509_STORE_CTX_free; @@ -197,6 +207,38 @@ impl X509StoreContextRef { } } + pub fn init_without_cleanup( + &mut self, + trust: store::X509Store, + cert: X509, + cert_chain: Stack, + ) -> Result<(), ErrorStack> { + unsafe { + if let Err(e) = cvt(ffi::X509_STORE_CTX_init( + self.as_ptr(), + trust.as_ptr(), + cert.as_ptr(), + cert_chain.as_ptr(), + )) { + ffi::X509_STORE_CTX_cleanup(self.as_ptr()); + + return Err(e); + } + } + + self.set_ex_data(*STORE_INDEX, trust); + self.set_ex_data(*CERT_INDEX, cert); + self.set_ex_data(*CERT_CHAIN_INDEX, cert_chain); + + Ok(()) + } + + /// Returns a reference to the X509 verification configuration. + #[corresponds(X509_STORE_CTX_get0_param)] + pub fn verify_param(&mut self) -> &X509VerifyParamRef { + unsafe { X509VerifyParamRef::from_ptr(ffi::X509_STORE_CTX_get0_param(self.as_ptr())) } + } + /// Returns a mutable reference to the X509 verification configuration. #[corresponds(X509_STORE_CTX_get0_param)] pub fn verify_param_mut(&mut self) -> &mut X509VerifyParamRef { From 05f798adc485b7a37028f49a5828fa5bf07aadf3 Mon Sep 17 00:00:00 2001 From: Kornel Date: Wed, 4 Jun 2025 23:17:12 +0100 Subject: [PATCH 07/13] Rename to reset_with_context_data --- boring/src/x509/mod.rs | 11 ++++++++--- boring/src/x509/tests/mod.rs | 12 ++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/boring/src/x509/mod.rs b/boring/src/x509/mod.rs index 0ebb83aa..e9b2937c 100644 --- a/boring/src/x509/mod.rs +++ b/boring/src/x509/mod.rs @@ -131,8 +131,6 @@ impl X509StoreContextRef { /// /// This can be used to provide data to callbacks registered with the context. Use the /// `Ssl::new_ex_index` method to create an `Index`. - /// - /// The previous value, if any, will be returned. #[corresponds(X509_STORE_CTX_set_ex_data)] pub fn set_ex_data(&mut self, index: Index, data: T) { if let Some(old) = self.ex_data_mut(index) { @@ -207,7 +205,14 @@ impl X509StoreContextRef { } } - pub fn init_without_cleanup( + /// Initializes this context with the given certificate, certificates chain and certificate + /// store. + /// + /// * `trust` - The certificate store with the trusted certificates. + /// * `cert` - The certificate that should be verified. + /// * `cert_chain` - The certificates chain. + #[corresponds(X509_STORE_CTX_init)] + pub fn reset_with_context_data( &mut self, trust: store::X509Store, cert: X509, diff --git a/boring/src/x509/tests/mod.rs b/boring/src/x509/tests/mod.rs index b3867ce2..f02c3fe3 100644 --- a/boring/src/x509/tests/mod.rs +++ b/boring/src/x509/tests/mod.rs @@ -451,14 +451,26 @@ fn test_verify_cert() { let mut store_bldr = X509StoreBuilder::new().unwrap(); store_bldr.add_cert(ca).unwrap(); let store = store_bldr.build(); + let empty_store = X509StoreBuilder::new().unwrap().build(); let mut context = X509StoreContext::new().unwrap(); assert!(context .init(&store, &cert, &chain, |c| c.verify_cert()) .unwrap()); + assert!(!context + .init(&empty_store, &cert, &chain, |c| c.verify_cert()) + .unwrap()); assert!(context .init(&store, &cert, &chain, |c| c.verify_cert()) .unwrap()); + + context + .reset_with_context_data(empty_store, cert.clone(), Stack::new().unwrap()) + .unwrap(); + assert!(!context.verify_cert().unwrap()); + + context.reset_with_context_data(store, cert, chain).unwrap(); + assert!(context.verify_cert().unwrap()); } #[test] From 29c05d41cd9f8efa1c0493dc6a69eac817f9e4e8 Mon Sep 17 00:00:00 2001 From: Kornel Date: Thu, 5 Jun 2025 01:44:25 +0100 Subject: [PATCH 08/13] Avoid panicking in error handling --- boring/src/error.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/boring/src/error.rs b/boring/src/error.rs index 1f951341..dd39e94e 100644 --- a/boring/src/error.rs +++ b/boring/src/error.rs @@ -118,9 +118,8 @@ impl Error { // in the error stack, so we'll need to copy it off if it's dynamic let data = if flags & ffi::ERR_FLAG_STRING != 0 { let bytes = CStr::from_ptr(data as *const _).to_bytes(); - let data = str::from_utf8(bytes).unwrap(); - let data = Cow::Owned(data.to_string()); - Some(data) + let data = String::from_utf8_lossy(bytes).into_owned(); + Some(data.into()) } else { None }; @@ -178,7 +177,7 @@ impl Error { return None; } let bytes = CStr::from_ptr(cstr as *const _).to_bytes(); - Some(str::from_utf8(bytes).unwrap()) + str::from_utf8(bytes).ok() } } @@ -196,7 +195,7 @@ impl Error { return None; } let bytes = CStr::from_ptr(cstr as *const _).to_bytes(); - Some(str::from_utf8(bytes).unwrap()) + str::from_utf8(bytes).ok() } } @@ -208,7 +207,7 @@ impl Error { return None; } let bytes = CStr::from_ptr(cstr as *const _).to_bytes(); - Some(str::from_utf8(bytes).unwrap()) + str::from_utf8(bytes).ok() } } @@ -220,9 +219,11 @@ impl Error { /// Returns the name of the source file which encountered the error. pub fn file(&self) -> &'static str { unsafe { - assert!(!self.file.is_null()); + if self.file.is_null() { + return ""; + } let bytes = CStr::from_ptr(self.file as *const _).to_bytes(); - str::from_utf8(bytes).unwrap() + str::from_utf8(bytes).unwrap_or_default() } } @@ -233,9 +234,8 @@ impl Error { } /// Returns additional data describing the error. - #[allow(clippy::option_as_ref_deref)] pub fn data(&self) -> Option<&str> { - self.data.as_ref().map(|s| &**s) + self.data.as_deref() } } From bcec9462aff4ba7fd11d508ee8469bf66abf5168 Mon Sep 17 00:00:00 2001 From: Kornel Date: Thu, 5 Jun 2025 02:15:42 +0100 Subject: [PATCH 09/13] Don't unwrap when Result can be returned instead --- boring/src/asn1.rs | 4 +-- boring/src/bn.rs | 4 +-- boring/src/error.rs | 55 ++++++++++++++++++++++++++++++++++++++++-- boring/src/nid.rs | 8 +++--- boring/src/pkcs12.rs | 6 ++--- boring/src/pkey.rs | 2 +- boring/src/ssl/mod.rs | 24 ++++++++++-------- boring/src/x509/mod.rs | 13 +++++----- 8 files changed, 86 insertions(+), 30 deletions(-) diff --git a/boring/src/asn1.rs b/boring/src/asn1.rs index 9fb3fcaf..ceb76c0c 100644 --- a/boring/src/asn1.rs +++ b/boring/src/asn1.rs @@ -323,7 +323,7 @@ impl Asn1Time { #[allow(clippy::should_implement_trait)] pub fn from_str(s: &str) -> Result { unsafe { - let s = CString::new(s).unwrap(); + let s = CString::new(s).map_err(ErrorStack::internal_error)?; let time = Asn1Time::new()?; cvt(ffi::ASN1_TIME_set_string(time.as_ptr(), s.as_ptr()))?; @@ -560,7 +560,7 @@ impl Asn1Object { pub fn from_str(txt: &str) -> Result { unsafe { ffi::init(); - let txt = CString::new(txt).unwrap(); + let txt = CString::new(txt).map_err(ErrorStack::internal_error)?; let obj: *mut ffi::ASN1_OBJECT = cvt_p(ffi::OBJ_txt2obj(txt.as_ptr() as *const _, 0))?; Ok(Asn1Object::from_ptr(obj)) } diff --git a/boring/src/bn.rs b/boring/src/bn.rs index 58edbabb..f6773a95 100644 --- a/boring/src/bn.rs +++ b/boring/src/bn.rs @@ -838,7 +838,7 @@ impl BigNum { pub fn from_dec_str(s: &str) -> Result { unsafe { ffi::init(); - let c_str = CString::new(s.as_bytes()).unwrap(); + let c_str = CString::new(s.as_bytes()).map_err(ErrorStack::internal_error)?; let mut bn = ptr::null_mut(); cvt(ffi::BN_dec2bn(&mut bn, c_str.as_ptr() as *const _))?; Ok(BigNum::from_ptr(bn)) @@ -850,7 +850,7 @@ impl BigNum { pub fn from_hex_str(s: &str) -> Result { unsafe { ffi::init(); - let c_str = CString::new(s.as_bytes()).unwrap(); + let c_str = CString::new(s.as_bytes()).map_err(ErrorStack::internal_error)?; let mut bn = ptr::null_mut(); cvt(ffi::BN_hex2bn(&mut bn, c_str.as_ptr() as *const _))?; Ok(BigNum::from_ptr(bn)) diff --git a/boring/src/error.rs b/boring/src/error.rs index dd39e94e..5d3e61fe 100644 --- a/boring/src/error.rs +++ b/boring/src/error.rs @@ -48,6 +48,12 @@ impl ErrorStack { error.put(); } } + + /// Used to report errors from the Rust crate + #[cold] + pub(crate) fn internal_error(err: impl error::Error) -> Self { + Self(vec![Error::new_internal(err.to_string())]) + } } impl ErrorStack { @@ -69,7 +75,11 @@ impl fmt::Display for ErrorStack { fmt.write_str(" ")?; } first = false; - write!(fmt, "[{}]", err.reason().unwrap_or("unknown reason"))?; + write!( + fmt, + "[{}]", + err.reason_internal().unwrap_or("unknown reason") + )?; } Ok(()) } @@ -101,6 +111,8 @@ pub struct Error { unsafe impl Sync for Error {} unsafe impl Send for Error {} +static BORING_INTERNAL: &CStr = c"boring-rust"; + impl Error { /// Returns the first error on the OpenSSL error stack. pub fn get() -> Option { @@ -171,6 +183,9 @@ impl Error { /// Returns the name of the library reporting the error, if available. pub fn library(&self) -> Option<&'static str> { + if self.is_internal() { + return None; + } unsafe { let cstr = ffi::ERR_lib_error_string(self.code); if cstr.is_null() { @@ -189,6 +204,9 @@ impl Error { /// Returns the name of the function reporting the error. 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() { @@ -237,6 +255,28 @@ impl Error { pub fn data(&self) -> Option<&str> { self.data.as_deref() } + + fn new_internal(msg: String) -> Self { + Self { + code: ffi::ERR_PACK(ffi::ERR_LIB_NONE.0 as _, 0, 0) as _, + file: BORING_INTERNAL.as_ptr(), + line: 0, + data: Some(msg.into()), + } + } + + fn is_internal(&self) -> bool { + std::ptr::eq(self.file, BORING_INTERNAL.as_ptr()) + } + + // reason() needs 'static + fn reason_internal(&self) -> Option<&str> { + if self.is_internal() { + self.data() + } else { + self.reason() + } + } } impl fmt::Debug for Error { @@ -268,7 +308,7 @@ impl fmt::Display for Error { write!( fmt, "{}\n\nCode: {:08X}\nLoc: {}:{}", - self.reason().unwrap_or("unknown TLS error"), + self.reason_internal().unwrap_or("unknown TLS error"), self.code(), self.file(), self.line() @@ -277,3 +317,14 @@ impl fmt::Display for Error { } impl error::Error for Error {} + +#[test] +fn internal_err() { + let e = ErrorStack::internal_error(io::Error::other("hello, boring")); + assert_eq!(1, e.errors().len()); + assert!(e.to_string().contains("hello, boring"), "{e} {e:?}"); + + e.put(); + let e = ErrorStack::get(); + assert!(e.to_string().contains("hello, boring"), "{e} {e:?}"); +} diff --git a/boring/src/nid.rs b/boring/src/nid.rs index 11607626..f2243723 100644 --- a/boring/src/nid.rs +++ b/boring/src/nid.rs @@ -84,8 +84,8 @@ impl Nid { #[allow(clippy::trivially_copy_pass_by_ref)] pub fn long_name(&self) -> Result<&'static str, ErrorStack> { unsafe { - cvt_p(ffi::OBJ_nid2ln(self.0) as *mut c_char) - .map(|nameptr| str::from_utf8(CStr::from_ptr(nameptr).to_bytes()).unwrap()) + let nameptr = cvt_p(ffi::OBJ_nid2ln(self.0) as *mut c_char)?; + str::from_utf8(CStr::from_ptr(nameptr).to_bytes()).map_err(ErrorStack::internal_error) } } @@ -94,8 +94,8 @@ impl Nid { #[allow(clippy::trivially_copy_pass_by_ref)] pub fn short_name(&self) -> Result<&'static str, ErrorStack> { unsafe { - cvt_p(ffi::OBJ_nid2sn(self.0) as *mut c_char) - .map(|nameptr| str::from_utf8(CStr::from_ptr(nameptr).to_bytes()).unwrap()) + let nameptr = cvt_p(ffi::OBJ_nid2sn(self.0) as *mut c_char)?; + str::from_utf8(CStr::from_ptr(nameptr).to_bytes()).map_err(ErrorStack::internal_error) } } diff --git a/boring/src/pkcs12.rs b/boring/src/pkcs12.rs index 8604f6d1..72b40906 100644 --- a/boring/src/pkcs12.rs +++ b/boring/src/pkcs12.rs @@ -34,7 +34,7 @@ impl Pkcs12Ref { /// Extracts the contents of the `Pkcs12`. pub fn parse(&self, pass: &str) -> Result { unsafe { - let pass = CString::new(pass.as_bytes()).unwrap(); + let pass = CString::new(pass.as_bytes()).map_err(ErrorStack::internal_error)?; let mut pkey = ptr::null_mut(); let mut cert = ptr::null_mut(); @@ -161,8 +161,8 @@ impl Pkcs12Builder { T: HasPrivate, { unsafe { - let pass = CString::new(password).unwrap(); - let friendly_name = CString::new(friendly_name).unwrap(); + let pass = CString::new(password).map_err(ErrorStack::internal_error)?; + let friendly_name = CString::new(friendly_name).map_err(ErrorStack::internal_error)?; let pkey = pkey.as_ptr(); let cert = cert.as_ptr(); let ca = self diff --git a/boring/src/pkey.rs b/boring/src/pkey.rs index 1c4012ca..fc9a78e2 100644 --- a/boring/src/pkey.rs +++ b/boring/src/pkey.rs @@ -408,7 +408,7 @@ impl PKey { unsafe { ffi::init(); let bio = MemBioSlice::new(der)?; - let passphrase = CString::new(passphrase).unwrap(); + let passphrase = CString::new(passphrase).map_err(ErrorStack::internal_error)?; cvt_p(ffi::d2i_PKCS8PrivateKey_bio( bio.as_ptr(), ptr::null_mut(), diff --git a/boring/src/ssl/mod.rs b/boring/src/ssl/mod.rs index a03cdf6e..c293ed00 100644 --- a/boring/src/ssl/mod.rs +++ b/boring/src/ssl/mod.rs @@ -1269,7 +1269,8 @@ impl SslContextBuilder { #[cfg(feature = "rpk")] assert!(!self.is_rpk, "This API is not supported for RPK"); - let file = CString::new(file.as_ref().as_os_str().to_str().unwrap()).unwrap(); + let file = CString::new(file.as_ref().as_os_str().as_encoded_bytes()) + .map_err(ErrorStack::internal_error)?; unsafe { cvt(ffi::SSL_CTX_load_verify_locations( self.as_ptr(), @@ -1340,7 +1341,8 @@ impl SslContextBuilder { #[cfg(feature = "rpk")] assert!(!self.is_rpk, "This API is not supported for RPK"); - let file = CString::new(file.as_ref().as_os_str().to_str().unwrap()).unwrap(); + let file = CString::new(file.as_ref().as_os_str().as_encoded_bytes()) + .map_err(ErrorStack::internal_error)?; unsafe { cvt(ffi::SSL_CTX_use_certificate_file( self.as_ptr(), @@ -1361,7 +1363,8 @@ impl SslContextBuilder { &mut self, file: P, ) -> Result<(), ErrorStack> { - let file = CString::new(file.as_ref().as_os_str().to_str().unwrap()).unwrap(); + let file = CString::new(file.as_ref().as_os_str().as_encoded_bytes()) + .map_err(ErrorStack::internal_error)?; unsafe { cvt(ffi::SSL_CTX_use_certificate_chain_file( self.as_ptr(), @@ -1401,7 +1404,8 @@ impl SslContextBuilder { file: P, file_type: SslFiletype, ) -> Result<(), ErrorStack> { - let file = CString::new(file.as_ref().as_os_str().to_str().unwrap()).unwrap(); + let file = CString::new(file.as_ref().as_os_str().as_encoded_bytes()) + .map_err(ErrorStack::internal_error)?; unsafe { cvt(ffi::SSL_CTX_use_PrivateKey_file( self.as_ptr(), @@ -1564,7 +1568,7 @@ impl SslContextBuilder { #[corresponds(SSL_CTX_set_tlsext_use_srtp)] pub fn set_tlsext_use_srtp(&mut self, protocols: &str) -> Result<(), ErrorStack> { unsafe { - let cstr = CString::new(protocols).unwrap(); + let cstr = CString::new(protocols).map_err(ErrorStack::internal_error)?; let r = ffi::SSL_CTX_set_tlsext_use_srtp(self.as_ptr(), cstr.as_ptr()); // fun fact, set_tlsext_use_srtp has a reversed return code D: @@ -1908,7 +1912,7 @@ impl SslContextBuilder { /// Sets the context's supported signature algorithms. #[corresponds(SSL_CTX_set1_sigalgs_list)] pub fn set_sigalgs_list(&mut self, sigalgs: &str) -> Result<(), ErrorStack> { - let sigalgs = CString::new(sigalgs).unwrap(); + let sigalgs = CString::new(sigalgs).map_err(ErrorStack::internal_error)?; unsafe { cvt(ffi::SSL_CTX_set1_sigalgs_list(self.as_ptr(), sigalgs.as_ptr()) as c_int) .map(|_| ()) @@ -1968,7 +1972,7 @@ impl SslContextBuilder { #[cfg(not(feature = "kx-safe-default"))] #[corresponds(SSL_CTX_set1_curves_list)] pub fn set_curves_list(&mut self, curves: &str) -> Result<(), ErrorStack> { - let curves = CString::new(curves).unwrap(); + let curves = CString::new(curves).map_err(ErrorStack::internal_error)?; unsafe { cvt_0i(ffi::SSL_CTX_set1_curves_list( self.as_ptr(), @@ -2802,7 +2806,7 @@ impl SslRef { #[corresponds(SSL_set1_curves_list)] pub fn set_curves_list(&mut self, curves: &str) -> Result<(), ErrorStack> { - let curves = CString::new(curves).unwrap(); + let curves = CString::new(curves).map_err(ErrorStack::internal_error)?; unsafe { cvt_0i(ffi::SSL_set1_curves_list( self.as_ptr(), @@ -3086,7 +3090,7 @@ impl SslRef { /// It has no effect for a server-side connection. #[corresponds(SSL_set_tlsext_host_name)] pub fn set_hostname(&mut self, hostname: &str) -> Result<(), ErrorStack> { - let cstr = CString::new(hostname).unwrap(); + let cstr = CString::new(hostname).map_err(ErrorStack::internal_error)?; unsafe { cvt(ffi::SSL_set_tlsext_host_name(self.as_ptr(), cstr.as_ptr() as *mut _) as c_int) .map(|_| ()) @@ -3273,7 +3277,7 @@ impl SslRef { #[corresponds(SSL_set_tlsext_use_srtp)] pub fn set_tlsext_use_srtp(&mut self, protocols: &str) -> Result<(), ErrorStack> { unsafe { - let cstr = CString::new(protocols).unwrap(); + let cstr = CString::new(protocols).map_err(ErrorStack::internal_error)?; let r = ffi::SSL_set_tlsext_use_srtp(self.as_ptr(), cstr.as_ptr()); // fun fact, set_tlsext_use_srtp has a reversed return code D: diff --git a/boring/src/x509/mod.rs b/boring/src/x509/mod.rs index e9b2937c..2e17dbb3 100644 --- a/boring/src/x509/mod.rs +++ b/boring/src/x509/mod.rs @@ -894,8 +894,8 @@ impl X509Extension { name: &str, value: &str, ) -> Result { - let name = CString::new(name).unwrap(); - let value = CString::new(value).unwrap(); + let name = CString::new(name).map_err(ErrorStack::internal_error)?; + let value = CString::new(value).map_err(ErrorStack::internal_error)?; let mut ctx; unsafe { ffi::init(); @@ -940,7 +940,7 @@ impl X509Extension { name: Nid, value: &str, ) -> Result { - let value = CString::new(value).unwrap(); + let value = CString::new(value).map_err(ErrorStack::internal_error)?; let mut ctx; unsafe { ffi::init(); @@ -1004,7 +1004,7 @@ impl X509NameBuilder { #[corresponds(X509_NAME_add_entry_by_txt)] pub fn append_entry_by_text(&mut self, field: &str, value: &str) -> Result<(), ErrorStack> { unsafe { - let field = CString::new(field).unwrap(); + let field = CString::new(field).map_err(ErrorStack::internal_error)?; assert!(value.len() <= ValueLen::MAX as usize); cvt(ffi::X509_NAME_add_entry_by_txt( self.0.as_ptr(), @@ -1028,7 +1028,7 @@ impl X509NameBuilder { ty: Asn1Type, ) -> Result<(), ErrorStack> { unsafe { - let field = CString::new(field).unwrap(); + let field = CString::new(field).map_err(ErrorStack::internal_error)?; assert!(value.len() <= ValueLen::MAX as usize); cvt(ffi::X509_NAME_add_entry_by_txt( self.0.as_ptr(), @@ -1116,7 +1116,8 @@ impl X509Name { /// /// This is commonly used in conjunction with `SslContextBuilder::set_client_ca_list`. pub fn load_client_ca_file>(file: P) -> Result, ErrorStack> { - let file = CString::new(file.as_ref().as_os_str().to_str().unwrap()).unwrap(); + let file = CString::new(file.as_ref().as_os_str().as_encoded_bytes()) + .map_err(ErrorStack::internal_error)?; unsafe { cvt_p(ffi::SSL_load_client_CA_file(file.as_ptr())).map(|p| Stack::from_ptr(p)) } } From 4d178a7f9f29a637c0857afdb0485c64a31c4620 Mon Sep 17 00:00:00 2001 From: Kornel Date: Tue, 20 May 2025 15:46:11 +0100 Subject: [PATCH 10/13] Clippy --- boring-sys/build/config.rs | 6 +++--- boring-sys/build/main.rs | 29 +++++++++++++---------------- boring/examples/mk_certs.rs | 4 ++-- boring/src/asn1.rs | 11 +++++++++-- boring/src/base64.rs | 2 +- boring/src/bn.rs | 24 ++++++++++++------------ boring/src/hash.rs | 16 ++++++++-------- boring/src/rsa.rs | 8 ++++---- boring/src/ssl/bio.rs | 5 +---- boring/src/ssl/callbacks.rs | 6 +++--- boring/src/ssl/ech.rs | 2 +- boring/src/ssl/error.rs | 12 ++++++------ boring/src/ssl/mod.rs | 6 +++--- boring/src/symm.rs | 4 ++-- boring/src/x509/extension.rs | 2 +- boring/src/x509/mod.rs | 2 +- boring/src/x509/tests/mod.rs | 4 ++-- hyper-boring/src/v0.rs | 8 ++++---- hyper-boring/tests/v0.rs | 6 +++--- tokio-boring/src/lib.rs | 2 +- 20 files changed, 80 insertions(+), 79 deletions(-) diff --git a/boring-sys/build/config.rs b/boring-sys/build/config.rs index 353f4d20..7f63b2fa 100644 --- a/boring-sys/build/config.rs +++ b/boring-sys/build/config.rs @@ -152,9 +152,9 @@ impl Env { let kind = if host == target { "HOST" } else { "TARGET" }; // TODO(rmehra): look for just `name` first, as most people just set that - var(&format!("{}_{}", name, target)) - .or_else(|| var(&format!("{}_{}", name, target_with_underscores))) - .or_else(|| var(&format!("{}_{}", kind, name))) + var(&format!("{name}_{target}")) + .or_else(|| var(&format!("{name}_{target_with_underscores}"))) + .or_else(|| var(&format!("{kind}_{name}"))) .or_else(|| var(name)) }; diff --git a/boring-sys/build/main.rs b/boring-sys/build/main.rs index 6cdcad66..129ad484 100644 --- a/boring-sys/build/main.rs +++ b/boring-sys/build/main.rs @@ -165,7 +165,7 @@ fn get_boringssl_platform_output_path(config: &Config) -> String { let deb_info = match debug_env_var.to_str() { Some("false") => false, Some("true") => true, - _ => panic!("Unknown DEBUG={:?} env var.", debug_env_var), + _ => panic!("Unknown DEBUG={debug_env_var:?} env var."), }; let opt_env_var = config @@ -184,12 +184,12 @@ fn get_boringssl_platform_output_path(config: &Config) -> String { } } Some("s" | "z") => "MinSizeRel", - _ => panic!("Unknown OPT_LEVEL={:?} env var.", opt_env_var), + _ => panic!("Unknown OPT_LEVEL={opt_env_var:?} env var."), }; subdir.to_string() } else { - "".to_string() + String::new() } } @@ -242,7 +242,7 @@ fn get_boringssl_cmake_config(config: &Config) -> cmake::Config { } let toolchain_file = android_ndk_home.join("build/cmake/android.toolchain.cmake"); let toolchain_file = toolchain_file.to_str().unwrap(); - eprintln!("android toolchain={}", toolchain_file); + eprintln!("android toolchain={toolchain_file}"); boringssl_cmake.define("CMAKE_TOOLCHAIN_FILE", toolchain_file); // 21 is the minimum level tested. You can give higher value. @@ -273,7 +273,7 @@ fn get_boringssl_cmake_config(config: &Config) -> cmake::Config { "" }; - let cflag = format!("{} {}", bitcode_cflag, target_cflag); + let cflag = format!("{bitcode_cflag} {target_cflag}"); boringssl_cmake.define("CMAKE_ASM_FLAGS", &cflag); boringssl_cmake.cflag(&cflag); } @@ -339,7 +339,7 @@ fn verify_fips_clang_version() -> (&'static str, &'static str) { let output = match Command::new(tool).arg("--version").output() { Ok(o) => o, Err(e) => { - eprintln!("warning: missing {}, trying other compilers: {}", tool, e); + eprintln!("warning: missing {tool}, trying other compilers: {e}"); // NOTE: hard-codes that the loop below checks the version return None; } @@ -369,13 +369,11 @@ fn verify_fips_clang_version() -> (&'static str, &'static str) { return (cc, cxx); } else if cc == "cc" { panic!( - "unsupported clang version \"{}\": FIPS requires clang {}", - cc_version, REQUIRED_CLANG_VERSION + "unsupported clang version \"{cc_version}\": FIPS requires clang {REQUIRED_CLANG_VERSION}" ); } else if !cc_version.is_empty() { eprintln!( - "warning: FIPS requires clang version {}, skipping incompatible version \"{}\"", - REQUIRED_CLANG_VERSION, cc_version + "warning: FIPS requires clang version {REQUIRED_CLANG_VERSION}, skipping incompatible version \"{cc_version}\"" ); } } @@ -425,7 +423,7 @@ fn get_extra_clang_args_for_bindgen(config: &Config) -> Vec { .unwrap(); if !output.status.success() { if let Some(exit_code) = output.status.code() { - eprintln!("xcrun failed: exit code {}", exit_code); + eprintln!("xcrun failed: exit code {exit_code}"); } else { eprintln!("xcrun failed: killed"); } @@ -452,8 +450,7 @@ fn get_extra_clang_args_for_bindgen(config: &Config) -> Vec { Ok(toolchain) => toolchain, Err(e) => { eprintln!( - "warning: failed to find prebuilt Android NDK toolchain for bindgen: {}", - e + "warning: failed to find prebuilt Android NDK toolchain for bindgen: {e}" ); // Uh... let's try anyway, I guess? return params; @@ -537,8 +534,8 @@ fn run_command(command: &mut Command) -> io::Result { if !out.status.success() { let err = match out.status.code() { - Some(code) => format!("{:?} exited with status: {}", command, code), - None => format!("{:?} was terminated by signal", command), + Some(code) => format!("{command:?} exited with status: {code}"), + None => format!("{command:?} was terminated by signal"), }; return Err(io::Error::other(err)); @@ -696,7 +693,7 @@ fn main() { } if let Some(cpp_lib) = get_cpp_runtime_lib(&config) { - println!("cargo:rustc-link-lib={}", cpp_lib); + println!("cargo:rustc-link-lib={cpp_lib}"); } println!("cargo:rustc-link-lib=static=crypto"); println!("cargo:rustc-link-lib=static=ssl"); diff --git a/boring/examples/mk_certs.rs b/boring/examples/mk_certs.rs index ce0c8492..a130dee6 100644 --- a/boring/examples/mk_certs.rs +++ b/boring/examples/mk_certs.rs @@ -146,7 +146,7 @@ fn real_main() -> Result<(), ErrorStack> { // Verify that this cert was issued by this ca match ca_cert.issued(&cert) { Ok(()) => println!("Certificate verified!"), - Err(ver_err) => println!("Failed to verify certificate: {}", ver_err), + Err(ver_err) => println!("Failed to verify certificate: {ver_err}"), }; Ok(()) @@ -155,6 +155,6 @@ fn real_main() -> Result<(), ErrorStack> { fn main() { match real_main() { Ok(()) => println!("Finished."), - Err(e) => println!("Error: {}", e), + Err(e) => println!("Error: {e}"), }; } diff --git a/boring/src/asn1.rs b/boring/src/asn1.rs index ceb76c0c..c5c2f196 100644 --- a/boring/src/asn1.rs +++ b/boring/src/asn1.rs @@ -304,7 +304,8 @@ impl Asn1Time { /// Creates a new time on specified interval in days from now pub fn days_from_now(days: u32) -> Result { - Asn1Time::from_period(days as c_long * 60 * 60 * 24) + // the type varies between platforms, so both into() and try_into() trigger Clippy lints + Self::from_period((days * 60 * 60 * 24) as _) } /// Creates a new time from the specified `time_t` value @@ -494,7 +495,13 @@ impl Asn1IntegerRef { /// [`bn`]: ../bn/struct.BigNumRef.html#method.to_asn1_integer #[corresponds(ASN1_INTEGER_set)] pub fn set(&mut self, value: i32) -> Result<(), ErrorStack> { - unsafe { cvt(crate::ffi::ASN1_INTEGER_set(self.as_ptr(), value as c_long)).map(|_| ()) } + unsafe { + cvt(crate::ffi::ASN1_INTEGER_set( + self.as_ptr(), + c_long::from(value), + )) + .map(|_| ()) + } } } diff --git a/boring/src/base64.rs b/boring/src/base64.rs index 75cc9cc8..98be9d0b 100644 --- a/boring/src/base64.rs +++ b/boring/src/base64.rs @@ -101,7 +101,7 @@ mod tests { #[test] fn test_encode_block() { - assert_eq!("".to_string(), encode_block(b"")); + assert_eq!(String::new(), encode_block(b"")); assert_eq!("Zg==".to_string(), encode_block(b"f")); assert_eq!("Zm8=".to_string(), encode_block(b"fo")); assert_eq!("Zm9v".to_string(), encode_block(b"foo")); diff --git a/boring/src/bn.rs b/boring/src/bn.rs index f6773a95..c60685bd 100644 --- a/boring/src/bn.rs +++ b/boring/src/bn.rs @@ -121,19 +121,19 @@ impl BigNumRef { /// Adds a `u32` to `self`. #[corresponds(BN_add_word)] pub fn add_word(&mut self, w: u32) -> Result<(), ErrorStack> { - unsafe { cvt(ffi::BN_add_word(self.as_ptr(), w as ffi::BN_ULONG)).map(|_| ()) } + unsafe { cvt(ffi::BN_add_word(self.as_ptr(), ffi::BN_ULONG::from(w))).map(|_| ()) } } /// Subtracts a `u32` from `self`. #[corresponds(BN_sub_word)] pub fn sub_word(&mut self, w: u32) -> Result<(), ErrorStack> { - unsafe { cvt(ffi::BN_sub_word(self.as_ptr(), w as ffi::BN_ULONG)).map(|_| ()) } + unsafe { cvt(ffi::BN_sub_word(self.as_ptr(), ffi::BN_ULONG::from(w))).map(|_| ()) } } /// Multiplies a `u32` by `self`. #[corresponds(BN_mul_word)] pub fn mul_word(&mut self, w: u32) -> Result<(), ErrorStack> { - unsafe { cvt(ffi::BN_mul_word(self.as_ptr(), w as ffi::BN_ULONG)).map(|_| ()) } + unsafe { cvt(ffi::BN_mul_word(self.as_ptr(), ffi::BN_ULONG::from(w))).map(|_| ()) } } /// Divides `self` by a `u32`, returning the remainder. @@ -263,7 +263,7 @@ impl BigNumRef { /// `self` positive. #[corresponds(BN_set_negative)] pub fn set_negative(&mut self, negative: bool) { - unsafe { ffi::BN_set_negative(self.as_ptr(), negative as c_int) } + unsafe { ffi::BN_set_negative(self.as_ptr(), c_int::from(negative)) } } /// Compare the absolute values of `self` and `oth`. @@ -332,7 +332,7 @@ impl BigNumRef { self.as_ptr(), bits.into(), msb.0, - odd as c_int, + c_int::from(odd), )) .map(|_| ()) } @@ -347,7 +347,7 @@ impl BigNumRef { self.as_ptr(), bits.into(), msb.0, - odd as c_int, + c_int::from(odd), )) .map(|_| ()) } @@ -388,7 +388,7 @@ impl BigNumRef { cvt(ffi::BN_generate_prime_ex( self.as_ptr(), bits as c_int, - safe as c_int, + c_int::from(safe), add.map(|n| n.as_ptr()).unwrap_or(ptr::null_mut()), rem.map(|n| n.as_ptr()).unwrap_or(ptr::null_mut()), ptr::null_mut(), @@ -712,7 +712,7 @@ impl BigNumRef { self.as_ptr(), checks.into(), ctx.as_ptr(), - do_trial_division as c_int, + c_int::from(do_trial_division), ptr::null_mut(), )) .map(|r| r != 0) @@ -829,7 +829,7 @@ impl BigNum { #[corresponds(BN_set_word)] pub fn from_u32(n: u32) -> Result { BigNum::new().and_then(|v| unsafe { - cvt(ffi::BN_set_word(v.as_ptr(), n as ffi::BN_ULONG)).map(|_| v) + cvt(ffi::BN_set_word(v.as_ptr(), ffi::BN_ULONG::from(n))).map(|_| v) }) } @@ -928,7 +928,7 @@ impl PartialEq for BigNumRef { impl PartialEq for BigNumRef { fn eq(&self, oth: &BigNum) -> bool { - self.eq(oth.deref()) + self.eq(&**oth) } } @@ -956,7 +956,7 @@ impl PartialOrd for BigNumRef { impl PartialOrd for BigNumRef { fn partial_cmp(&self, oth: &BigNum) -> Option { - Some(self.cmp(oth.deref())) + Some(self.cmp(&**oth)) } } @@ -980,7 +980,7 @@ impl PartialOrd for BigNum { impl Ord for BigNum { fn cmp(&self, oth: &BigNum) -> Ordering { - self.deref().cmp(oth.deref()) + self.deref().cmp(&**oth) } } diff --git a/boring/src/hash.rs b/boring/src/hash.rs index ba5d7bab..3e7f6eb5 100644 --- a/boring/src/hash.rs +++ b/boring/src/hash.rs @@ -308,7 +308,7 @@ impl DerefMut for DigestBytes { impl AsRef<[u8]> for DigestBytes { #[inline] fn as_ref(&self) -> &[u8] { - self.deref() + self } } @@ -455,7 +455,7 @@ mod tests { #[test] fn test_md5() { - for test in MD5_TESTS.iter() { + for test in &MD5_TESTS { hash_test(MessageDigest::md5(), test); } } @@ -463,7 +463,7 @@ mod tests { #[test] fn test_md5_recycle() { let mut h = Hasher::new(MessageDigest::md5()).unwrap(); - for test in MD5_TESTS.iter() { + for test in &MD5_TESTS { hash_recycle_test(&mut h, test); } } @@ -514,7 +514,7 @@ mod tests { fn test_sha1() { let tests = [("616263", "a9993e364706816aba3e25717850c26c9cd0d89d")]; - for test in tests.iter() { + for test in &tests { hash_test(MessageDigest::sha1(), test); } } @@ -526,7 +526,7 @@ mod tests { "23097d223405d8228642a477bda255b32aadbce4bda0b3f7e36c9da7", )]; - for test in tests.iter() { + for test in &tests { hash_test(MessageDigest::sha224(), test); } } @@ -538,7 +538,7 @@ mod tests { "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad", )]; - for test in tests.iter() { + for test in &tests { hash_test(MessageDigest::sha256(), test); } } @@ -551,7 +551,7 @@ mod tests { 192992a274fc1a836ba3c23a3feebbd454d4423643ce80e2a9ac94fa54ca49f", )]; - for test in tests.iter() { + for test in &tests { hash_test(MessageDigest::sha512(), test); } } @@ -563,7 +563,7 @@ mod tests { "53048e2681941ef99b2e29b76b4c7dabe4c2d0c634fc6d46e0e2f13107e7af23", )]; - for test in tests.iter() { + for test in &tests { hash_test(MessageDigest::sha512_256(), test); } } diff --git a/boring/src/rsa.rs b/boring/src/rsa.rs index 7bb641fb..bef3b276 100644 --- a/boring/src/rsa.rs +++ b/boring/src/rsa.rs @@ -143,7 +143,7 @@ where to: &mut [u8], padding: Padding, ) -> Result { - assert!(from.len() <= i32::MAX as usize); + assert!(i32::try_from(from.len()).is_ok()); assert!(to.len() >= self.size() as usize); unsafe { @@ -170,7 +170,7 @@ where to: &mut [u8], padding: Padding, ) -> Result { - assert!(from.len() <= i32::MAX as usize); + assert!(i32::try_from(from.len()).is_ok()); assert!(to.len() >= self.size() as usize); unsafe { @@ -334,7 +334,7 @@ where to: &mut [u8], padding: Padding, ) -> Result { - assert!(from.len() <= i32::MAX as usize); + assert!(i32::try_from(from.len()).is_ok()); assert!(to.len() >= self.size() as usize); unsafe { @@ -360,7 +360,7 @@ where to: &mut [u8], padding: Padding, ) -> Result { - assert!(from.len() <= i32::MAX as usize); + assert!(i32::try_from(from.len()).is_ok()); assert!(to.len() >= self.size() as usize); unsafe { diff --git a/boring/src/ssl/bio.rs b/boring/src/ssl/bio.rs index f3b83672..e700dbe3 100644 --- a/boring/src/ssl/bio.rs +++ b/boring/src/ssl/bio.rs @@ -85,10 +85,7 @@ pub unsafe extern "C" fn take_stream(bio: *mut BIO) -> S { pub unsafe fn set_dtls_mtu_size(bio: *mut BIO, mtu_size: usize) { if mtu_size as u64 > c_long::MAX as u64 { - panic!( - "Given MTU size {} can't be represented in a positive `c_long` range", - mtu_size - ) + panic!("Given MTU size {mtu_size} can't be represented in a positive `c_long` range") } state::(bio).dtls_mtu_size = mtu_size as c_long; } diff --git a/boring/src/ssl/callbacks.rs b/boring/src/ssl/callbacks.rs index 8ab17b98..8ad4ba55 100644 --- a/boring/src/ssl/callbacks.rs +++ b/boring/src/ssl/callbacks.rs @@ -40,7 +40,7 @@ where // because there is no `X509StoreContextRef::ssl_mut(&mut self)` method. let verify = unsafe { &*(verify as *const F) }; - verify(preverify_ok != 0, ctx) as c_int + c_int::from(verify(preverify_ok != 0, ctx)) } pub(super) unsafe extern "C" fn raw_custom_verify( @@ -89,7 +89,7 @@ where // so the callback can't replace itself. let verify = unsafe { &*(verify as *const F) }; - verify(ctx) as c_int + c_int::from(verify(ctx)) } pub(super) unsafe extern "C" fn ssl_raw_custom_verify( @@ -235,7 +235,7 @@ where .expect("BUG: ssl verify callback missing") .clone(); - callback(preverify_ok != 0, ctx) as c_int + c_int::from(callback(preverify_ok != 0, ctx)) } pub(super) unsafe extern "C" fn raw_sni( diff --git a/boring/src/ssl/ech.rs b/boring/src/ssl/ech.rs index 27ccc5b5..cdc1f53b 100644 --- a/boring/src/ssl/ech.rs +++ b/boring/src/ssl/ech.rs @@ -35,7 +35,7 @@ impl SslEchKeysBuilder { unsafe { cvt_0i(ffi::SSL_ECH_KEYS_add( self.keys.as_ptr(), - is_retry_config as c_int, + c_int::from(is_retry_config), ech_config.as_ptr(), ech_config.len(), key.as_ptr(), diff --git a/boring/src/ssl/error.rs b/boring/src/ssl/error.rs index a17243df..a83a084e 100644 --- a/boring/src/ssl/error.rs +++ b/boring/src/ssl/error.rs @@ -136,14 +136,14 @@ impl fmt::Display for Error { None => fmt.write_str("the operation should be retried"), }, ErrorCode::SYSCALL => match self.io_error() { - Some(err) => write!(fmt, "{}", err), + Some(err) => write!(fmt, "{err}"), None => fmt.write_str("unexpected EOF"), }, ErrorCode::SSL => match self.ssl_error() { - Some(e) => write!(fmt, "{}", e), + Some(e) => write!(fmt, "{e}"), None => fmt.write_str("unknown BoringSSL error"), }, - ErrorCode(code) => write!(fmt, "unknown error code {}", code), + ErrorCode(code) => write!(fmt, "unknown error code {code}"), } } } @@ -185,7 +185,7 @@ impl fmt::Display for HandshakeError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match *self { HandshakeError::SetupFailure(ref e) => { - write!(f, "TLS stream setup failed {}", e) + write!(f, "TLS stream setup failed {e}") } HandshakeError::Failure(ref s) => fmt_mid_handshake_error(s, f, "TLS handshake failed"), HandshakeError::WouldBlock(ref s) => { @@ -209,8 +209,8 @@ fn fmt_mid_handshake_error( match s.ssl().verify_result() { // INVALID_CALL is returned if no verification took place, // such as before a cert is sent. - Ok(()) | Err(X509VerifyError::INVALID_CALL) => write!(f, "{}", prefix)?, - Err(verify) => write!(f, "{}: cert verification failed - {}", prefix, verify)?, + Ok(()) | Err(X509VerifyError::INVALID_CALL) => write!(f, "{prefix}")?, + Err(verify) => write!(f, "{prefix}: cert verification failed - {verify}")?, } write!(f, " {}", s.error()) diff --git a/boring/src/ssl/mod.rs b/boring/src/ssl/mod.rs index c293ed00..980026a1 100644 --- a/boring/src/ssl/mod.rs +++ b/boring/src/ssl/mod.rs @@ -593,7 +593,7 @@ impl TryFrom for SslVersion { type Error = &'static str; fn try_from(value: u16) -> Result { - match value as i32 { + match i32::from(value) { ffi::SSL3_VERSION | ffi::TLS1_VERSION | ffi::TLS1_1_VERSION @@ -908,7 +908,7 @@ impl SslInfoCallbackAlert { /// The value of the SSL alert. pub fn alert(&self) -> SslAlert { - let value = self.0 & (u8::MAX as i32); + let value = self.0 & i32::from(u8::MAX); SslAlert(value) } } @@ -1226,7 +1226,7 @@ impl SslContextBuilder { #[corresponds(SSL_CTX_set_read_ahead)] pub fn set_read_ahead(&mut self, read_ahead: bool) { unsafe { - ffi::SSL_CTX_set_read_ahead(self.as_ptr(), read_ahead as c_int); + ffi::SSL_CTX_set_read_ahead(self.as_ptr(), c_int::from(read_ahead)); } } diff --git a/boring/src/symm.rs b/boring/src/symm.rs index 1df9a77c..4f5527b2 100644 --- a/boring/src/symm.rs +++ b/boring/src/symm.rs @@ -341,7 +341,7 @@ impl Crypter { } iv.as_ptr() as *mut _ } - (Some(_), None) | (None, None) => ptr::null_mut(), + (Some(_) | None, None) => ptr::null_mut(), (None, Some(_)) => panic!("an IV is required for this cipher"), }; cvt(ffi::EVP_CipherInit_ex( @@ -363,7 +363,7 @@ impl Crypter { /// be a multiple of the cipher's block size. pub fn pad(&mut self, padding: bool) { unsafe { - ffi::EVP_CIPHER_CTX_set_padding(self.ctx, padding as c_int); + ffi::EVP_CIPHER_CTX_set_padding(self.ctx, c_int::from(padding)); } } diff --git a/boring/src/x509/extension.rs b/boring/src/x509/extension.rs index 639d28bd..e8797327 100644 --- a/boring/src/x509/extension.rs +++ b/boring/src/x509/extension.rs @@ -78,7 +78,7 @@ impl BasicConstraints { value.push_str("FALSE"); } if let Some(pathlen) = self.pathlen { - write!(value, ",pathlen:{}", pathlen).unwrap(); + write!(value, ",pathlen:{pathlen}").unwrap(); } X509Extension::new_nid(None, None, Nid::BASIC_CONSTRAINTS, &value) } diff --git a/boring/src/x509/mod.rs b/boring/src/x509/mod.rs index 2e17dbb3..2640f784 100644 --- a/boring/src/x509/mod.rs +++ b/boring/src/x509/mod.rs @@ -1516,7 +1516,7 @@ impl X509VerifyError { ffi::init(); unsafe { - let s = ffi::X509_verify_cert_error_string(self.0 as c_long); + let s = ffi::X509_verify_cert_error_string(c_long::from(self.0)); str::from_utf8(CStr::from_ptr(s).to_bytes()).unwrap() } } diff --git a/boring/src/x509/tests/mod.rs b/boring/src/x509/tests/mod.rs index f02c3fe3..c7919466 100644 --- a/boring/src/x509/tests/mod.rs +++ b/boring/src/x509/tests/mod.rs @@ -37,7 +37,7 @@ fn test_cert_loading() { fn test_debug() { let cert = include_bytes!("../../../test/cert.pem"); let cert = X509::from_pem(cert).unwrap(); - let debugged = format!("{:#?}", cert); + let debugged = format!("{cert:#?}"); assert!(debugged.contains(r#"serial_number: "8771f7bdee982fa5""#)); assert!(debugged.contains(r#"signature_algorithm: sha256WithRSAEncryption"#)); @@ -497,7 +497,7 @@ fn test_save_subject_der() { let cert = X509::from_pem(cert).unwrap(); let der = cert.subject_name().to_der().unwrap(); - println!("der: {:?}", der); + println!("der: {der:?}"); assert!(!der.is_empty()); } diff --git a/hyper-boring/src/v0.rs b/hyper-boring/src/v0.rs index 172d1640..07597498 100644 --- a/hyper-boring/src/v0.rs +++ b/hyper-boring/src/v0.rs @@ -259,10 +259,10 @@ where let last = host.len() - 1; let mut chars = host.chars(); - if let (Some('['), Some(']')) = (chars.next(), chars.last()) { - if host[1..last].parse::().is_ok() { - host = &host[1..last]; - } + if (chars.next(), chars.last()) == (Some('['), Some(']')) + && host[1..last].parse::().is_ok() + { + host = &host[1..last]; } } diff --git a/hyper-boring/tests/v0.rs b/hyper-boring/tests/v0.rs index 08cfce12..f52e1851 100644 --- a/hyper-boring/tests/v0.rs +++ b/hyper-boring/tests/v0.rs @@ -76,7 +76,7 @@ async fn localhost() { let file = File::create("../target/keyfile.log").unwrap(); ssl.set_keylog_callback(move |_, line| { - let _ = writeln!(&file, "{}", line); + let _ = writeln!(&file, "{line}"); }); let ssl = HttpsConnector::with_connector(connector, ssl).unwrap(); @@ -84,7 +84,7 @@ async fn localhost() { for _ in 0..3 { let resp = client - .get(format!("https://foobar.com:{}", port).parse().unwrap()) + .get(format!("https://foobar.com:{port}").parse().unwrap()) .await .unwrap(); assert!(resp.status().is_success(), "{}", resp.status()); @@ -147,7 +147,7 @@ async fn alpn_h2() { let client = Client::builder().build::<_, Body>(ssl); let resp = client - .get(format!("https://foobar.com:{}", port).parse().unwrap()) + .get(format!("https://foobar.com:{port}").parse().unwrap()) .await .unwrap(); assert!(resp.status().is_success(), "{}", resp.status()); diff --git a/tokio-boring/src/lib.rs b/tokio-boring/src/lib.rs index 89d10127..0dae747a 100644 --- a/tokio-boring/src/lib.rs +++ b/tokio-boring/src/lib.rs @@ -234,7 +234,7 @@ where fn poll_shutdown(mut self: Pin<&mut Self>, ctx: &mut Context) -> Poll> { match self.run_in_context(ctx, |s| s.shutdown()) { - Ok(ShutdownResult::Sent) | Ok(ShutdownResult::Received) => {} + Ok(ShutdownResult::Sent | ShutdownResult::Received) => {} Err(ref e) if e.code() == ErrorCode::ZERO_RETURN => {} Err(ref e) if e.code() == ErrorCode::WANT_READ || e.code() == ErrorCode::WANT_WRITE => { return Poll::Pending; From 5d57b3a05701f091d39ce8d1474ea3c5878a867f Mon Sep 17 00:00:00 2001 From: Kornel Date: Wed, 4 Jun 2025 20:20:53 +0100 Subject: [PATCH 11/13] Make X509Store shareable between contexts #362 --- boring/src/ssl/mod.rs | 56 ++++++++++++++++++++++++++++++++++++-- boring/src/ssl/test/mod.rs | 49 ++++++++++++++++++++++++++++++++- boring/src/x509/store.rs | 8 ++++++ 3 files changed, 110 insertions(+), 3 deletions(-) diff --git a/boring/src/ssl/mod.rs b/boring/src/ssl/mod.rs index 980026a1..85028833 100644 --- a/boring/src/ssl/mod.rs +++ b/boring/src/ssl/mod.rs @@ -88,7 +88,7 @@ use crate::ssl::bio::BioMethod; use crate::ssl::callbacks::*; use crate::ssl::error::InnerError; use crate::stack::{Stack, StackRef, Stackable}; -use crate::x509::store::{X509Store, X509StoreBuilderRef, X509StoreRef}; +use crate::x509::store::{X509Store, X509StoreBuilder, X509StoreBuilderRef, X509StoreRef}; use crate::x509::verify::X509VerifyParamRef; use crate::x509::{ X509Name, X509Ref, X509StoreContextRef, X509VerifyError, X509VerifyResult, X509, @@ -933,6 +933,8 @@ extern "C" fn rpk_verify_failure_callback( /// A builder for `SslContext`s. pub struct SslContextBuilder { ctx: SslContext, + /// If it's not shared, it can be exposed as mutable + has_shared_cert_store: bool, #[cfg(feature = "rpk")] is_rpk: bool, } @@ -1006,7 +1008,11 @@ impl SslContextBuilder { #[cfg(feature = "rpk")] pub unsafe fn from_ptr(ctx: *mut ffi::SSL_CTX, is_rpk: bool) -> SslContextBuilder { let ctx = SslContext::from_ptr(ctx); - let mut builder = SslContextBuilder { ctx, is_rpk }; + let mut builder = SslContextBuilder { + ctx, + is_rpk, + has_shared_cert_store: false, + }; builder.set_ex_data(*RPK_FLAG_INDEX, is_rpk); @@ -1022,6 +1028,7 @@ impl SslContextBuilder { pub unsafe fn from_ptr(ctx: *mut ffi::SSL_CTX) -> SslContextBuilder { SslContextBuilder { ctx: SslContext::from_ptr(ctx), + has_shared_cert_store: false, } } @@ -1206,17 +1213,48 @@ impl SslContextBuilder { } } + /// Use [`set_cert_store_builder`] or [`set_cert_store_ref`] instead. + /// /// Replaces the context's certificate store. #[corresponds(SSL_CTX_set_cert_store)] + #[deprecated(note = "Use set_cert_store_builder or set_cert_store_ref instead")] pub fn set_cert_store(&mut self, cert_store: X509Store) { #[cfg(feature = "rpk")] assert!(!self.is_rpk, "This API is not supported for RPK"); + self.has_shared_cert_store = false; unsafe { ffi::SSL_CTX_set_cert_store(self.as_ptr(), cert_store.into_ptr()); } } + /// Replaces the context's certificate store, and allows mutating the store afterwards. + #[corresponds(SSL_CTX_set_cert_store)] + pub fn set_cert_store_builder(&mut self, cert_store: X509StoreBuilder) { + #[cfg(feature = "rpk")] + assert!(!self.is_rpk, "This API is not supported for RPK"); + + self.has_shared_cert_store = false; + unsafe { + ffi::SSL_CTX_set_cert_store(self.as_ptr(), cert_store.into_ptr()); + } + } + + /// Replaces the context's certificate store, and keeps it immutable. + /// + /// This method allows sharing the `X509Store`, but calls to `cert_store_mut` will panic. + #[corresponds(SSL_CTX_set_cert_store)] + pub fn set_cert_store_ref(&mut self, cert_store: &X509Store) { + #[cfg(feature = "rpk")] + assert!(!self.is_rpk, "This API is not supported for RPK"); + + self.has_shared_cert_store = true; + unsafe { + ffi::X509_STORE_up_ref(cert_store.as_ptr()); + ffi::SSL_CTX_set_cert_store(self.as_ptr(), cert_store.as_ptr()); + } + } + /// Controls read ahead behavior. /// /// If enabled, OpenSSL will read as much data as is available from the underlying stream, @@ -1701,11 +1739,25 @@ impl SslContextBuilder { } /// Returns a mutable reference to the context's certificate store. + /// + /// Newly-created `SslContextBuilder` will have its own default mutable store. + /// + /// ## Panics + /// + /// * If a shared store has been set via [`set_cert_store_ref`] + /// * If context has been created for Raw Public Key verification (requires `rpk` Cargo feature) + /// #[corresponds(SSL_CTX_get_cert_store)] pub fn cert_store_mut(&mut self) -> &mut X509StoreBuilderRef { #[cfg(feature = "rpk")] assert!(!self.is_rpk, "This API is not supported for RPK"); + assert!( + !self.has_shared_cert_store, + "Shared X509Store can't be mutated. Make a new store" + ); + // OTOH, it's not safe to return a shared &X509Store when the builder owns it exclusively + unsafe { X509StoreBuilderRef::from_ptr_mut(ffi::SSL_CTX_get_cert_store(self.as_ptr())) } } diff --git a/boring/src/ssl/test/mod.rs b/boring/src/ssl/test/mod.rs index fdcb7096..40806b5f 100644 --- a/boring/src/ssl/test/mod.rs +++ b/boring/src/ssl/test/mod.rs @@ -1,4 +1,4 @@ -use hex; +use foreign_types::{ForeignType, ForeignTypeRef}; use std::io; use std::io::prelude::*; use std::mem; @@ -18,6 +18,7 @@ use crate::ssl::{ ExtensionType, ShutdownResult, ShutdownState, Ssl, SslAcceptor, SslAcceptorBuilder, SslConnector, SslContext, SslFiletype, SslMethod, SslOptions, SslStream, SslVerifyMode, }; +use crate::x509::store::X509StoreBuilder; use crate::x509::verify::X509CheckFlags; use crate::x509::{X509Name, X509}; @@ -308,6 +309,52 @@ fn test_select_cert_ok() { client.connect(); } +#[test] +fn test_mutable_store() { + #![allow(deprecated)] + + let cert = include_bytes!("../../../test/cert.pem"); + let cert = X509::from_pem(cert).unwrap(); + let cert2 = include_bytes!("../../../test/root-ca.pem"); + let cert2 = X509::from_pem(cert2).unwrap(); + + let mut ctx = SslContext::builder(SslMethod::tls()).unwrap(); + ctx.cert_store_mut().add_cert(cert.clone()).unwrap(); + assert_eq!(1, ctx.cert_store().objects_len()); + + ctx.set_cert_store_builder(X509StoreBuilder::new().unwrap()); + assert_eq!(0, ctx.cert_store().objects_len()); + + ctx.cert_store_mut().add_cert(cert.clone()).unwrap(); + assert_eq!(1, ctx.cert_store().objects_len()); + + let mut new_store = X509StoreBuilder::new().unwrap(); + new_store.add_cert(cert).unwrap(); + new_store.add_cert(cert2).unwrap(); + let new_store = new_store.build(); + assert_eq!(2, new_store.objects_len()); + + ctx.set_cert_store_ref(&new_store); + assert_eq!(2, ctx.cert_store().objects_len()); + assert!(std::ptr::eq(new_store.as_ptr(), ctx.cert_store().as_ptr())); + + let ctx = ctx.build(); + assert!(std::ptr::eq(new_store.as_ptr(), ctx.cert_store().as_ptr())); + + drop(new_store); + assert_eq!(2, ctx.cert_store().objects_len()); +} + +#[test] +#[should_panic(expected = "mutated")] +fn shared_store_must_not_be_mutated() { + let mut ctx = SslContext::builder(SslMethod::tls()).unwrap(); + + let shared = X509StoreBuilder::new().unwrap().build(); + ctx.set_cert_store_ref(&shared); + ctx.cert_store_mut(); +} + #[test] fn test_select_cert_error() { let mut server = Server::builder(); diff --git a/boring/src/x509/store.rs b/boring/src/x509/store.rs index d5669920..d4bcd045 100644 --- a/boring/src/x509/store.rs +++ b/boring/src/x509/store.rs @@ -115,6 +115,14 @@ impl X509StoreBuilderRef { pub fn set_param(&mut self, param: &X509VerifyParamRef) -> Result<(), ErrorStack> { unsafe { cvt(ffi::X509_STORE_set1_param(self.as_ptr(), param.as_ptr())).map(|_| ()) } } + + /// For testing only + #[cfg(test)] + pub fn objects_len(&self) -> usize { + unsafe { + StackRef::::from_ptr(ffi::X509_STORE_get0_objects(self.as_ptr())).len() + } + } } foreign_type_and_impl_send_sync! { From 5fa9c81c88817cf7d8a4c84b8bf03f08456e7158 Mon Sep 17 00:00:00 2001 From: Kornel Date: Thu, 5 Jun 2025 20:40:35 +0100 Subject: [PATCH 12/13] Sprinkle #[must_use] (#368) --- boring-sys/src/lib.rs | 4 ++ boring/src/asn1.rs | 10 ++++ boring/src/base64.rs | 1 + boring/src/bn.rs | 6 +++ boring/src/conf.rs | 1 + boring/src/dsa.rs | 6 +++ boring/src/ec.rs | 7 +++ boring/src/ecdsa.rs | 2 + boring/src/error.rs | 16 +++++- boring/src/ex_data.rs | 2 + boring/src/fips.rs | 1 + boring/src/hash.rs | 12 +++++ boring/src/memcmp.rs | 3 +- boring/src/nid.rs | 3 ++ boring/src/pkcs12.rs | 1 + boring/src/pkey.rs | 6 +++ boring/src/rsa.rs | 12 +++++ boring/src/sha.rs | 18 +++++++ boring/src/sign.rs | 1 + boring/src/srtp.rs | 5 ++ boring/src/ssl/connector.rs | 8 +++ boring/src/ssl/error.rs | 6 +++ boring/src/ssl/mod.rs | 96 ++++++++++++++++++++++++++++++++++++ boring/src/ssl/test/mod.rs | 2 +- boring/src/stack.rs | 4 ++ boring/src/symm.rs | 26 ++++++++++ boring/src/version.rs | 6 +++ boring/src/x509/extension.rs | 6 +++ boring/src/x509/mod.rs | 39 +++++++++++++++ boring/src/x509/store.rs | 3 ++ boring/src/x509/verify.rs | 1 + hyper-boring/src/lib.rs | 2 + tokio-boring/src/lib.rs | 8 +++ 33 files changed, 320 insertions(+), 4 deletions(-) diff --git a/boring-sys/src/lib.rs b/boring-sys/src/lib.rs index 404a9008..8463fe1f 100644 --- a/boring-sys/src/lib.rs +++ b/boring-sys/src/lib.rs @@ -32,18 +32,22 @@ pub type BN_ULONG = u64; #[cfg(target_pointer_width = "32")] pub type BN_ULONG = u32; +#[must_use] pub const fn ERR_PACK(l: c_int, f: c_int, r: c_int) -> c_ulong { ((l as c_ulong & 0x0FF) << 24) | ((f as c_ulong & 0xFFF) << 12) | (r as c_ulong & 0xFFF) } +#[must_use] pub const fn ERR_GET_LIB(l: c_uint) -> c_int { ((l >> 24) & 0x0FF) as c_int } +#[must_use] pub const fn ERR_GET_FUNC(l: c_uint) -> c_int { ((l >> 12) & 0xFFF) as c_int } +#[must_use] pub const fn ERR_GET_REASON(l: c_uint) -> c_int { (l & 0xFFF) as c_int } diff --git a/boring/src/asn1.rs b/boring/src/asn1.rs index c5c2f196..891ec9f5 100644 --- a/boring/src/asn1.rs +++ b/boring/src/asn1.rs @@ -143,11 +143,13 @@ impl Asn1Type { pub const BMPSTRING: Asn1Type = Asn1Type(ffi::V_ASN1_BMPSTRING); /// Constructs an `Asn1Type` from a raw OpenSSL value. + #[must_use] pub fn from_raw(value: c_int) -> Self { Asn1Type(value) } /// Returns the raw OpenSSL value represented by this type. + #[must_use] pub fn as_raw(&self) -> c_int { self.0 } @@ -415,17 +417,20 @@ impl Asn1StringRef { /// /// [`as_utf8`]: struct.Asn1String.html#method.as_utf8 #[corresponds(ASN1_STRING_get0_data)] + #[must_use] pub fn as_slice(&self) -> &[u8] { unsafe { slice::from_raw_parts(ASN1_STRING_get0_data(self.as_ptr()), self.len()) } } /// Returns the number of bytes in the string. #[corresponds(ASN1_STRING_length)] + #[must_use] pub fn len(&self) -> usize { unsafe { ffi::ASN1_STRING_length(self.as_ptr()) as usize } } /// Determines if the string is empty. + #[must_use] pub fn is_empty(&self) -> bool { self.len() == 0 } @@ -473,6 +478,7 @@ impl Asn1IntegerRef { #[allow(clippy::unnecessary_cast)] #[allow(missing_docs)] #[deprecated(since = "0.10.6", note = "use to_bn instead")] + #[must_use] pub fn get(&self) -> i64 { unsafe { crate::ffi::ASN1_INTEGER_get(self.as_ptr()) as i64 } } @@ -520,17 +526,20 @@ foreign_type_and_impl_send_sync! { impl Asn1BitStringRef { /// Returns the Asn1BitString as a slice. #[corresponds(ASN1_STRING_get0_data)] + #[must_use] pub fn as_slice(&self) -> &[u8] { unsafe { slice::from_raw_parts(ASN1_STRING_get0_data(self.as_ptr() as *mut _), self.len()) } } /// Returns the number of bytes in the string. #[corresponds(ASN1_STRING_length)] + #[must_use] pub fn len(&self) -> usize { unsafe { ffi::ASN1_STRING_length(self.as_ptr() as *const _) as usize } } /// Determines if the string is empty. + #[must_use] pub fn is_empty(&self) -> bool { self.len() == 0 } @@ -576,6 +585,7 @@ impl Asn1Object { impl Asn1ObjectRef { /// Returns the NID associated with this OID. + #[must_use] pub fn nid(&self) -> Nid { unsafe { Nid::from_raw(ffi::OBJ_obj2nid(self.as_ptr())) } } diff --git a/boring/src/base64.rs b/boring/src/base64.rs index 98be9d0b..0c6fd06e 100644 --- a/boring/src/base64.rs +++ b/boring/src/base64.rs @@ -11,6 +11,7 @@ use openssl_macros::corresponds; /// /// Panics if the input length or computed output length overflow a signed C integer. #[corresponds(EVP_EncodeBlock)] +#[must_use] pub fn encode_block(src: &[u8]) -> String { assert!(src.len() <= c_int::MAX as usize); let src_len = src.len(); diff --git a/boring/src/bn.rs b/boring/src/bn.rs index c60685bd..bf4ca1c7 100644 --- a/boring/src/bn.rs +++ b/boring/src/bn.rs @@ -198,6 +198,7 @@ impl BigNumRef { /// Returns `true` if the `n`th bit of `self` is set to 1, `false` otherwise. #[corresponds(BN_is_bit_set)] #[allow(clippy::useless_conversion)] + #[must_use] pub fn is_bit_set(&self, n: i32) -> bool { unsafe { ffi::BN_is_bit_set(self.as_ptr(), n.into()) == 1 } } @@ -279,23 +280,27 @@ impl BigNumRef { /// assert_eq!(s.ucmp(&o), Ordering::Equal); /// ``` #[corresponds(BN_ucmp)] + #[must_use] pub fn ucmp(&self, oth: &BigNumRef) -> Ordering { unsafe { ffi::BN_ucmp(self.as_ptr(), oth.as_ptr()).cmp(&0) } } /// Returns `true` if `self` is negative. #[corresponds(BN_is_negative)] + #[must_use] pub fn is_negative(&self) -> bool { unsafe { BN_is_negative(self.as_ptr()) == 1 } } /// Returns the number of significant bits in `self`. #[corresponds(BN_num_bits)] + #[must_use] pub fn num_bits(&self) -> i32 { unsafe { ffi::BN_num_bits(self.as_ptr()) as i32 } } /// Returns the size of `self` in bytes. Implemented natively. + #[must_use] pub fn num_bytes(&self) -> i32 { (self.num_bits() + 7) / 8 } @@ -732,6 +737,7 @@ impl BigNumRef { /// assert_eq!(BigNum::from_slice(&s_vec).unwrap(), r); /// ``` #[corresponds(BN_bn2bin)] + #[must_use] pub fn to_vec(&self) -> Vec { let size = self.num_bytes() as usize; let mut v = Vec::with_capacity(size); diff --git a/boring/src/conf.rs b/boring/src/conf.rs index c94dde4a..01907a08 100644 --- a/boring/src/conf.rs +++ b/boring/src/conf.rs @@ -19,6 +19,7 @@ impl ConfMethod { } /// Convert to raw pointer. + #[must_use] pub fn as_ptr(&self) -> *mut c_void { self.0 } diff --git a/boring/src/dsa.rs b/boring/src/dsa.rs index 72d6947e..be13da9f 100644 --- a/boring/src/dsa.rs +++ b/boring/src/dsa.rs @@ -98,6 +98,7 @@ where } /// Returns a reference to the public key component of `self`. + #[must_use] pub fn pub_key(&self) -> &BigNumRef { unsafe { let mut pub_key = ptr::null(); @@ -126,6 +127,7 @@ where } /// Returns a reference to the private key component of `self`. + #[must_use] pub fn priv_key(&self) -> &BigNumRef { unsafe { let mut priv_key = ptr::null(); @@ -141,11 +143,13 @@ where { /// Returns the maximum size of the signature output by `self` in bytes. #[corresponds(DSA_size)] + #[must_use] pub fn size(&self) -> u32 { unsafe { ffi::DSA_size(self.as_ptr()) as u32 } } /// Returns the DSA prime parameter of `self`. + #[must_use] pub fn p(&self) -> &BigNumRef { unsafe { let mut p = ptr::null(); @@ -155,6 +159,7 @@ where } /// Returns the DSA sub-prime parameter of `self`. + #[must_use] pub fn q(&self) -> &BigNumRef { unsafe { let mut q = ptr::null(); @@ -164,6 +169,7 @@ where } /// Returns the DSA base parameter of `self`. + #[must_use] pub fn g(&self) -> &BigNumRef { unsafe { let mut g = ptr::null(); diff --git a/boring/src/ec.rs b/boring/src/ec.rs index 8008927a..588408aa 100644 --- a/boring/src/ec.rs +++ b/boring/src/ec.rs @@ -167,18 +167,21 @@ impl EcGroupRef { /// Returns the degree of the curve. #[corresponds(EC_GROUP_get_degree)] #[allow(clippy::unnecessary_cast)] + #[must_use] pub fn degree(&self) -> u32 { unsafe { ffi::EC_GROUP_get_degree(self.as_ptr()) as u32 } } /// Returns the number of bits in the group order. #[corresponds(EC_GROUP_order_bits)] + #[must_use] pub fn order_bits(&self) -> u32 { unsafe { ffi::EC_GROUP_order_bits(self.as_ptr()) as u32 } } /// Returns the generator for the given curve as a [`EcPoint`]. #[corresponds(EC_GROUP_get0_generator)] + #[must_use] pub fn generator(&self) -> &EcPointRef { unsafe { let ptr = ffi::EC_GROUP_get0_generator(self.as_ptr()); @@ -216,6 +219,7 @@ impl EcGroupRef { /// Returns the name of the curve, if a name is associated. #[corresponds(EC_GROUP_get_curve_name)] + #[must_use] pub fn curve_name(&self) -> Option { let nid = unsafe { ffi::EC_GROUP_get_curve_name(self.as_ptr()) }; if nid > 0 { @@ -498,6 +502,7 @@ where /// Return [`EcPoint`] associated with the private key #[corresponds(EC_KEY_get0_private_key)] + #[must_use] pub fn private_key(&self) -> &BigNumRef { unsafe { let ptr = ffi::EC_KEY_get0_private_key(self.as_ptr()); @@ -512,6 +517,7 @@ where { /// Returns the public key. #[corresponds(EC_KEY_get0_public_key)] + #[must_use] pub fn public_key(&self) -> &EcPointRef { unsafe { let ptr = ffi::EC_KEY_get0_public_key(self.as_ptr()); @@ -542,6 +548,7 @@ where { /// Return [`EcGroup`] of the `EcKey` #[corresponds(EC_KEY_get0_group)] + #[must_use] pub fn group(&self) -> &EcGroupRef { unsafe { let ptr = ffi::EC_KEY_get0_group(self.as_ptr()); diff --git a/boring/src/ecdsa.rs b/boring/src/ecdsa.rs index a56f7b68..4d5bb097 100644 --- a/boring/src/ecdsa.rs +++ b/boring/src/ecdsa.rs @@ -93,6 +93,7 @@ impl EcdsaSigRef { /// Returns internal component: `r` of an `EcdsaSig`. (See X9.62 or FIPS 186-2) #[corresponds(ECDSA_SIG_get0)] + #[must_use] pub fn r(&self) -> &BigNumRef { unsafe { let mut r = ptr::null(); @@ -103,6 +104,7 @@ impl EcdsaSigRef { /// Returns internal components: `s` of an `EcdsaSig`. (See X9.62 or FIPS 186-2) #[corresponds(ECDSA_SIG_get0)] + #[must_use] pub fn s(&self) -> &BigNumRef { unsafe { let mut s = ptr::null(); diff --git a/boring/src/error.rs b/boring/src/error.rs index 5d3e61fe..c81978c5 100644 --- a/boring/src/error.rs +++ b/boring/src/error.rs @@ -33,7 +33,8 @@ use crate::ffi; pub struct ErrorStack(Vec); impl ErrorStack { - /// Returns the contents of the OpenSSL error stack. + /// Pops the contents of the OpenSSL error stack, and returns it. + #[allow(clippy::must_use_candidate)] pub fn get() -> ErrorStack { let mut vec = vec![]; while let Some(err) = Error::get() { @@ -58,6 +59,7 @@ impl ErrorStack { impl ErrorStack { /// Returns the errors in the stack. + #[must_use] pub fn errors(&self) -> &[Error] { &self.0 } @@ -114,7 +116,8 @@ unsafe impl Send for Error {} static BORING_INTERNAL: &CStr = c"boring-rust"; impl Error { - /// Returns the first error on the OpenSSL error stack. + /// Pops the first error off the OpenSSL error stack. + #[allow(clippy::must_use_candidate)] pub fn get() -> Option { unsafe { ffi::init(); @@ -177,11 +180,13 @@ impl Error { } /// Returns the raw OpenSSL error code for this error. + #[must_use] pub fn code(&self) -> c_uint { self.code } /// Returns the name of the library reporting the error, if available. + #[must_use] pub fn library(&self) -> Option<&'static str> { if self.is_internal() { return None; @@ -198,11 +203,13 @@ impl Error { /// Returns the raw OpenSSL error constant for the library reporting the /// error. + #[must_use] pub fn library_code(&self) -> libc::c_int { ffi::ERR_GET_LIB(self.code) } /// Returns the name of the function reporting the error. + #[must_use] pub fn function(&self) -> Option<&'static str> { if self.is_internal() { return None; @@ -218,6 +225,7 @@ impl Error { } /// Returns the reason for the error. + #[must_use] pub fn reason(&self) -> Option<&'static str> { unsafe { let cstr = ffi::ERR_reason_error_string(self.code); @@ -230,11 +238,13 @@ impl Error { } /// Returns the raw OpenSSL error constant for the reason for the error. + #[must_use] pub fn reason_code(&self) -> libc::c_int { ffi::ERR_GET_REASON(self.code) } /// Returns the name of the source file which encountered the error. + #[must_use] pub fn file(&self) -> &'static str { unsafe { if self.file.is_null() { @@ -247,11 +257,13 @@ impl Error { /// Returns the line in the source file which encountered the error. #[allow(clippy::unnecessary_cast)] + #[must_use] pub fn line(&self) -> u32 { self.line as u32 } /// Returns additional data describing the error. + #[must_use] pub fn data(&self) -> Option<&str> { self.data.as_deref() } diff --git a/boring/src/ex_data.rs b/boring/src/ex_data.rs index d4f00212..e141f5c4 100644 --- a/boring/src/ex_data.rs +++ b/boring/src/ex_data.rs @@ -21,11 +21,13 @@ impl Index { /// # Safety /// /// The caller must ensure that the index correctly maps to a `U` value stored in a `T`. + #[must_use] pub unsafe fn from_raw(idx: c_int) -> Index { Index(idx, PhantomData) } #[allow(clippy::trivially_copy_pass_by_ref)] + #[must_use] pub fn as_raw(&self) -> c_int { self.0 } diff --git a/boring/src/fips.rs b/boring/src/fips.rs index 23fbefdf..8e451226 100644 --- a/boring/src/fips.rs +++ b/boring/src/fips.rs @@ -8,6 +8,7 @@ use openssl_macros::corresponds; /// Determines if the library is running in the FIPS 140-2 mode of operation. #[corresponds(FIPS_mode)] +#[must_use] pub fn enabled() -> bool { unsafe { ffi::FIPS_mode() != 0 } } diff --git a/boring/src/hash.rs b/boring/src/hash.rs index 3e7f6eb5..5deadade 100644 --- a/boring/src/hash.rs +++ b/boring/src/hash.rs @@ -22,12 +22,14 @@ impl MessageDigest { /// # Safety /// /// The caller must ensure the pointer is valid. + #[must_use] pub unsafe fn from_ptr(x: *const ffi::EVP_MD) -> Self { MessageDigest(x) } /// Returns the `MessageDigest` corresponding to an `Nid`. #[corresponds(EVP_get_digestbynid)] + #[must_use] pub fn from_nid(type_: Nid) -> Option { unsafe { let ptr = ffi::EVP_get_digestbynid(type_.as_raw()); @@ -39,47 +41,57 @@ impl MessageDigest { } } + #[must_use] pub fn md5() -> MessageDigest { unsafe { MessageDigest(ffi::EVP_md5()) } } + #[must_use] pub fn sha1() -> MessageDigest { unsafe { MessageDigest(ffi::EVP_sha1()) } } + #[must_use] pub fn sha224() -> MessageDigest { unsafe { MessageDigest(ffi::EVP_sha224()) } } + #[must_use] pub fn sha256() -> MessageDigest { unsafe { MessageDigest(ffi::EVP_sha256()) } } + #[must_use] pub fn sha384() -> MessageDigest { unsafe { MessageDigest(ffi::EVP_sha384()) } } + #[must_use] pub fn sha512() -> MessageDigest { unsafe { MessageDigest(ffi::EVP_sha512()) } } + #[must_use] pub fn sha512_256() -> MessageDigest { unsafe { MessageDigest(ffi::EVP_sha512_256()) } } #[allow(clippy::trivially_copy_pass_by_ref)] + #[must_use] pub fn as_ptr(&self) -> *const ffi::EVP_MD { self.0 } /// The size of the digest in bytes. #[allow(clippy::trivially_copy_pass_by_ref)] + #[must_use] pub fn size(&self) -> usize { unsafe { ffi::EVP_MD_size(self.0) } } /// The name of the digest. #[allow(clippy::trivially_copy_pass_by_ref)] + #[must_use] pub fn type_(&self) -> Nid { Nid::from_raw(unsafe { ffi::EVP_MD_type(self.0) }) } diff --git a/boring/src/memcmp.rs b/boring/src/memcmp.rs index a475a4f6..99cefdbe 100644 --- a/boring/src/memcmp.rs +++ b/boring/src/memcmp.rs @@ -61,6 +61,7 @@ use libc::size_t; /// assert!(!eq(&a, &b)); /// assert!(!eq(&a, &c)); /// ``` +#[must_use] pub fn eq(a: &[u8], b: &[u8]) -> bool { assert!(a.len() == b.len()); let ret = unsafe { @@ -87,6 +88,6 @@ mod tests { #[test] #[should_panic] fn test_diff_lens() { - eq(&[], &[1]); + let _ = eq(&[], &[1]); } } diff --git a/boring/src/nid.rs b/boring/src/nid.rs index f2243723..43fc19f4 100644 --- a/boring/src/nid.rs +++ b/boring/src/nid.rs @@ -51,12 +51,14 @@ pub struct Nid(c_int); #[allow(non_snake_case)] impl Nid { /// Create a `Nid` from an integer representation. + #[must_use] pub fn from_raw(raw: c_int) -> Nid { Nid(raw) } /// Return the integer representation of a `Nid`. #[allow(clippy::trivially_copy_pass_by_ref)] + #[must_use] pub fn as_raw(&self) -> c_int { self.0 } @@ -64,6 +66,7 @@ impl Nid { /// Returns the `Nid`s of the digest and public key algorithms associated with a signature ID. #[corresponds(OBJ_find_sigid_algs)] #[allow(clippy::trivially_copy_pass_by_ref)] + #[must_use] pub fn signature_algorithms(&self) -> Option { unsafe { let mut digest = 0; diff --git a/boring/src/pkcs12.rs b/boring/src/pkcs12.rs index 72b40906..dd255e3a 100644 --- a/boring/src/pkcs12.rs +++ b/boring/src/pkcs12.rs @@ -80,6 +80,7 @@ impl Pkcs12 { /// * `nid_cert` - `nid::PBE_WITHSHA1AND40BITRC2_CBC` /// * `iter` - `2048` /// * `mac_iter` - `2048` + #[must_use] pub fn builder() -> Pkcs12Builder { ffi::init(); diff --git a/boring/src/pkey.rs b/boring/src/pkey.rs index fc9a78e2..07d3b548 100644 --- a/boring/src/pkey.rs +++ b/boring/src/pkey.rs @@ -83,12 +83,14 @@ impl Id { pub const X448: Id = Id(ffi::EVP_PKEY_X448); /// Creates a `Id` from an integer representation. + #[must_use] pub fn from_raw(value: c_int) -> Id { Id(value) } /// Returns the integer representation of the `Id`. #[allow(clippy::trivially_copy_pass_by_ref)] + #[must_use] pub fn as_raw(&self) -> c_int { self.0 } @@ -176,12 +178,14 @@ impl PKeyRef { /// Returns the `Id` that represents the type of this key. #[corresponds(EVP_PKEY_id)] + #[must_use] pub fn id(&self) -> Id { unsafe { Id::from_raw(ffi::EVP_PKEY_id(self.as_ptr())) } } /// Returns the maximum size of a signature in bytes. #[corresponds(EVP_PKEY_size)] + #[must_use] pub fn size(&self) -> usize { unsafe { ffi::EVP_PKEY_size(self.as_ptr()) as usize } } @@ -211,11 +215,13 @@ where /// /// This corresponds to the bit length of the modulus of an RSA key, and the bit length of the /// group order for an elliptic curve key, for example. + #[must_use] pub fn bits(&self) -> u32 { unsafe { ffi::EVP_PKEY_bits(self.as_ptr()) as u32 } } /// Compares the public component of this key with another. + #[must_use] pub fn public_eq(&self, other: &PKeyRef) -> bool where U: HasPublic, diff --git a/boring/src/rsa.rs b/boring/src/rsa.rs index bef3b276..5a0ae24d 100644 --- a/boring/src/rsa.rs +++ b/boring/src/rsa.rs @@ -67,12 +67,14 @@ impl Padding { pub const PKCS1_PSS: Padding = Padding(ffi::RSA_PKCS1_PSS_PADDING); /// Creates a `Padding` from an integer representation. + #[must_use] pub fn from_raw(value: c_int) -> Padding { Padding(value) } /// Returns the integer representation of `Padding`. #[allow(clippy::trivially_copy_pass_by_ref)] + #[must_use] pub fn as_raw(&self) -> c_int { self.0 } @@ -187,6 +189,7 @@ where /// Returns a reference to the private exponent of the key. #[corresponds(RSA_get0_key)] + #[must_use] pub fn d(&self) -> &BigNumRef { unsafe { let mut d = ptr::null(); @@ -197,6 +200,7 @@ where /// Returns a reference to the first factor of the exponent of the key. #[corresponds(RSA_get0_factors)] + #[must_use] pub fn p(&self) -> Option<&BigNumRef> { unsafe { let mut p = ptr::null(); @@ -211,6 +215,7 @@ where /// Returns a reference to the second factor of the exponent of the key. #[corresponds(RSA_get0_factors)] + #[must_use] pub fn q(&self) -> Option<&BigNumRef> { unsafe { let mut q = ptr::null(); @@ -225,6 +230,7 @@ where /// Returns a reference to the first exponent used for CRT calculations. #[corresponds(RSA_get0_crt_params)] + #[must_use] pub fn dmp1(&self) -> Option<&BigNumRef> { unsafe { let mut dp = ptr::null(); @@ -239,6 +245,7 @@ where /// Returns a reference to the second exponent used for CRT calculations. #[corresponds(RSA_get0_crt_params)] + #[must_use] pub fn dmq1(&self) -> Option<&BigNumRef> { unsafe { let mut dq = ptr::null(); @@ -253,6 +260,7 @@ where /// Returns a reference to the coefficient used for CRT calculations. #[corresponds(RSA_get0_crt_params)] + #[must_use] pub fn iqmp(&self) -> Option<&BigNumRef> { unsafe { let mut qi = ptr::null(); @@ -319,6 +327,7 @@ where /// Returns the size of the modulus in bytes. #[corresponds(RSA_size)] #[allow(clippy::unnecessary_cast)] + #[must_use] pub fn size(&self) -> u32 { unsafe { ffi::RSA_size(self.as_ptr()) as u32 } } @@ -377,6 +386,7 @@ where /// Returns a reference to the modulus of the key. #[corresponds(RSA_get0_key)] + #[must_use] pub fn n(&self) -> &BigNumRef { unsafe { let mut n = ptr::null(); @@ -387,6 +397,7 @@ where /// Returns a reference to the public exponent of the key. #[corresponds(RSA_get0_key)] + #[must_use] pub fn e(&self) -> &BigNumRef { unsafe { let mut e = ptr::null(); @@ -513,6 +524,7 @@ impl RsaPrivateKeyBuilder { } /// Returns the Rsa key. + #[must_use] pub fn build(self) -> Rsa { self.rsa } diff --git a/boring/src/sha.rs b/boring/src/sha.rs index 98aa26ba..f0f9b15c 100644 --- a/boring/src/sha.rs +++ b/boring/src/sha.rs @@ -55,6 +55,7 @@ use std::mem::MaybeUninit; /// compatibility with existing systems. #[inline] #[allow(deprecated)] // https://github.com/rust-lang/rust/issues/63566 +#[must_use] pub fn sha1(data: &[u8]) -> [u8; 20] { unsafe { let mut hash: MaybeUninit<[u8; 20]> = MaybeUninit::uninit(); @@ -66,6 +67,7 @@ pub fn sha1(data: &[u8]) -> [u8; 20] { /// Computes the SHA224 hash of some data. #[inline] #[allow(deprecated)] // https://github.com/rust-lang/rust/issues/63566 +#[must_use] pub fn sha224(data: &[u8]) -> [u8; 28] { unsafe { let mut hash: MaybeUninit<[u8; 28]> = MaybeUninit::uninit(); @@ -77,6 +79,7 @@ pub fn sha224(data: &[u8]) -> [u8; 28] { /// Computes the SHA256 hash of some data. #[inline] #[allow(deprecated)] // https://github.com/rust-lang/rust/issues/63566 +#[must_use] pub fn sha256(data: &[u8]) -> [u8; 32] { unsafe { let mut hash: MaybeUninit<[u8; 32]> = MaybeUninit::uninit(); @@ -88,6 +91,7 @@ pub fn sha256(data: &[u8]) -> [u8; 32] { /// Computes the SHA384 hash of some data. #[inline] #[allow(deprecated)] // https://github.com/rust-lang/rust/issues/63566 +#[must_use] pub fn sha384(data: &[u8]) -> [u8; 48] { unsafe { let mut hash: MaybeUninit<[u8; 48]> = MaybeUninit::uninit(); @@ -99,6 +103,7 @@ pub fn sha384(data: &[u8]) -> [u8; 48] { /// Computes the SHA512 hash of some data. #[inline] #[allow(deprecated)] // https://github.com/rust-lang/rust/issues/63566 +#[must_use] pub fn sha512(data: &[u8]) -> [u8; 64] { unsafe { let mut hash: MaybeUninit<[u8; 64]> = MaybeUninit::uninit(); @@ -110,6 +115,7 @@ pub fn sha512(data: &[u8]) -> [u8; 64] { /// Computes the SHA512-256 hash of some data. #[inline] #[allow(deprecated)] // https://github.com/rust-lang/rust/issues/63566 +#[must_use] pub fn sha512_256(data: &[u8]) -> [u8; 32] { unsafe { let mut hash: MaybeUninit<[u8; 32]> = MaybeUninit::uninit(); @@ -138,6 +144,7 @@ impl Sha1 { /// Creates a new hasher. #[inline] #[allow(deprecated)] // https://github.com/rust-lang/rust/issues/63566 + #[must_use] pub fn new() -> Sha1 { unsafe { let mut ctx = MaybeUninit::uninit(); @@ -159,6 +166,7 @@ impl Sha1 { /// Returns the hash of the data. #[inline] #[allow(deprecated)] // https://github.com/rust-lang/rust/issues/63566 + #[must_use] pub fn finish(mut self) -> [u8; 20] { unsafe { let mut hash: MaybeUninit<[u8; 20]> = MaybeUninit::uninit(); @@ -183,6 +191,7 @@ impl Sha224 { /// Creates a new hasher. #[inline] #[allow(deprecated)] // https://github.com/rust-lang/rust/issues/63566 + #[must_use] pub fn new() -> Sha224 { unsafe { let mut ctx = MaybeUninit::uninit(); @@ -204,6 +213,7 @@ impl Sha224 { /// Returns the hash of the data. #[inline] #[allow(deprecated)] // https://github.com/rust-lang/rust/issues/63566 + #[must_use] pub fn finish(mut self) -> [u8; 28] { unsafe { let mut hash: MaybeUninit<[u8; 28]> = MaybeUninit::uninit(); @@ -228,6 +238,7 @@ impl Sha256 { /// Creates a new hasher. #[inline] #[allow(deprecated)] // https://github.com/rust-lang/rust/issues/63566 + #[must_use] pub fn new() -> Sha256 { unsafe { let mut ctx = MaybeUninit::uninit(); @@ -249,6 +260,7 @@ impl Sha256 { /// Returns the hash of the data. #[inline] #[allow(deprecated)] // https://github.com/rust-lang/rust/issues/63566 + #[must_use] pub fn finish(mut self) -> [u8; 32] { unsafe { let mut hash: MaybeUninit<[u8; 32]> = MaybeUninit::uninit(); @@ -273,6 +285,7 @@ impl Sha384 { /// Creates a new hasher. #[inline] #[allow(deprecated)] // https://github.com/rust-lang/rust/issues/63566 + #[must_use] pub fn new() -> Sha384 { unsafe { let mut ctx = MaybeUninit::uninit(); @@ -294,6 +307,7 @@ impl Sha384 { /// Returns the hash of the data. #[inline] #[allow(deprecated)] // https://github.com/rust-lang/rust/issues/63566 + #[must_use] pub fn finish(mut self) -> [u8; 48] { unsafe { let mut hash: MaybeUninit<[u8; 48]> = MaybeUninit::uninit(); @@ -318,6 +332,7 @@ impl Sha512 { /// Creates a new hasher. #[inline] #[allow(deprecated)] // https://github.com/rust-lang/rust/issues/63566 + #[must_use] pub fn new() -> Sha512 { unsafe { let mut ctx = MaybeUninit::uninit(); @@ -339,6 +354,7 @@ impl Sha512 { /// Returns the hash of the data. #[inline] #[allow(deprecated)] // https://github.com/rust-lang/rust/issues/63566 + #[must_use] pub fn finish(mut self) -> [u8; 64] { unsafe { let mut hash: MaybeUninit<[u8; 64]> = MaybeUninit::uninit(); @@ -363,6 +379,7 @@ impl Sha512_256 { /// Creates a new hasher. #[inline] #[allow(deprecated)] // https://github.com/rust-lang/rust/issues/63566 + #[must_use] pub fn new() -> Sha512_256 { unsafe { let mut ctx = MaybeUninit::uninit(); @@ -384,6 +401,7 @@ impl Sha512_256 { /// Returns the hash of the data. #[inline] #[allow(deprecated)] // https://github.com/rust-lang/rust/issues/63566 + #[must_use] pub fn finish(mut self) -> [u8; 32] { unsafe { let mut hash: MaybeUninit<[u8; 32]> = MaybeUninit::uninit(); diff --git a/boring/src/sign.rs b/boring/src/sign.rs index 89e7ba1c..3a4a7e8f 100644 --- a/boring/src/sign.rs +++ b/boring/src/sign.rs @@ -60,6 +60,7 @@ impl RsaPssSaltlen { } /// Sets the salt length to the given value. + #[must_use] pub fn custom(val: c_int) -> RsaPssSaltlen { RsaPssSaltlen(val) } diff --git a/boring/src/srtp.rs b/boring/src/srtp.rs index 84deca9e..8d23b722 100644 --- a/boring/src/srtp.rs +++ b/boring/src/srtp.rs @@ -20,9 +20,12 @@ impl Stackable for SrtpProtectionProfile { } impl SrtpProtectionProfileRef { + #[must_use] pub fn id(&self) -> SrtpProfileId { SrtpProfileId::from_raw(unsafe { (*self.as_ptr()).id }) } + + #[must_use] pub fn name(&self) -> &'static str { unsafe { CStr::from_ptr((*self.as_ptr()).name as *const _) } .to_str() @@ -47,12 +50,14 @@ impl SrtpProfileId { pub const SRTP_NULL_SHA1_32: SrtpProfileId = SrtpProfileId(ffi::SRTP_NULL_SHA1_32 as _); /// Creates a `SrtpProfileId` from an integer representation. + #[must_use] pub fn from_raw(value: c_ulong) -> SrtpProfileId { SrtpProfileId(value) } /// Returns the integer representation of `SrtpProfileId`. #[allow(clippy::trivially_copy_pass_by_ref)] + #[must_use] pub fn as_raw(&self) -> c_ulong { self.0 } diff --git a/boring/src/ssl/connector.rs b/boring/src/ssl/connector.rs index e910a324..111b45c2 100644 --- a/boring/src/ssl/connector.rs +++ b/boring/src/ssl/connector.rs @@ -137,11 +137,13 @@ impl SslConnector { } /// Consumes the `SslConnector`, returning the inner raw `SslContext`. + #[must_use] pub fn into_context(self) -> SslContext { self.0 } /// Returns a shared reference to the inner raw `SslContext`. + #[must_use] pub fn context(&self) -> &SslContextRef { &self.0 } @@ -152,6 +154,7 @@ pub struct SslConnectorBuilder(SslContextBuilder); impl SslConnectorBuilder { /// Consumes the builder, returning an `SslConnector`. + #[must_use] pub fn build(self) -> SslConnector { SslConnector(self.0.build()) } @@ -180,6 +183,7 @@ pub struct ConnectConfiguration { impl ConnectConfiguration { /// A builder-style version of `set_use_server_name_indication`. + #[must_use] pub fn use_server_name_indication(mut self, use_sni: bool) -> ConnectConfiguration { self.set_use_server_name_indication(use_sni); self @@ -193,6 +197,7 @@ impl ConnectConfiguration { } /// A builder-style version of `set_verify_hostname`. + #[must_use] pub fn verify_hostname(mut self, verify_hostname: bool) -> ConnectConfiguration { self.set_verify_hostname(verify_hostname); self @@ -396,11 +401,13 @@ impl SslAcceptor { } /// Consumes the `SslAcceptor`, returning the inner raw `SslContext`. + #[must_use] pub fn into_context(self) -> SslContext { self.0 } /// Returns a shared reference to the inner raw `SslContext`. + #[must_use] pub fn context(&self) -> &SslContextRef { &self.0 } @@ -411,6 +418,7 @@ pub struct SslAcceptorBuilder(SslContextBuilder); impl SslAcceptorBuilder { /// Consumes the builder, returning a `SslAcceptor`. + #[must_use] pub fn build(self) -> SslAcceptor { SslAcceptor(self.0.build()) } diff --git a/boring/src/ssl/error.rs b/boring/src/ssl/error.rs index a83a084e..f62e5f32 100644 --- a/boring/src/ssl/error.rs +++ b/boring/src/ssl/error.rs @@ -50,11 +50,13 @@ impl ErrorCode { /// An error occurred in the SSL library. pub const SSL: ErrorCode = ErrorCode(ffi::SSL_ERROR_SSL); + #[must_use] pub fn from_raw(raw: c_int) -> ErrorCode { ErrorCode(raw) } #[allow(clippy::trivially_copy_pass_by_ref)] + #[must_use] pub fn as_raw(&self) -> c_int { self.0 } @@ -74,10 +76,12 @@ pub struct Error { } impl Error { + #[must_use] pub fn code(&self) -> ErrorCode { self.code } + #[must_use] pub fn io_error(&self) -> Option<&io::Error> { match self.cause { Some(InnerError::Io(ref e)) => Some(e), @@ -92,6 +96,7 @@ impl Error { } } + #[must_use] pub fn ssl_error(&self) -> Option<&ErrorStack> { match self.cause { Some(InnerError::Ssl(ref e)) => Some(e), @@ -99,6 +104,7 @@ impl Error { } } + #[must_use] pub fn would_block(&self) -> bool { matches!( self.code, diff --git a/boring/src/ssl/mod.rs b/boring/src/ssl/mod.rs index 85028833..5d3ee1db 100644 --- a/boring/src/ssl/mod.rs +++ b/boring/src/ssl/mod.rs @@ -248,6 +248,7 @@ pub struct SslMethod(*const ffi::SSL_METHOD); impl SslMethod { /// Support all versions of the TLS protocol. #[corresponds(TLS_method)] + #[must_use] pub fn tls() -> SslMethod { unsafe { SslMethod(TLS_method()) } } @@ -260,18 +261,21 @@ impl SslMethod { /// Support all versions of the DTLS protocol. #[corresponds(DTLS_method)] + #[must_use] pub fn dtls() -> SslMethod { unsafe { SslMethod(DTLS_method()) } } /// Support all versions of the TLS protocol, explicitly as a client. #[corresponds(TLS_client_method)] + #[must_use] pub fn tls_client() -> SslMethod { unsafe { SslMethod(TLS_client_method()) } } /// Support all versions of the TLS protocol, explicitly as a server. #[corresponds(TLS_server_method)] + #[must_use] pub fn tls_server() -> SslMethod { unsafe { SslMethod(TLS_server_method()) } } @@ -282,12 +286,14 @@ impl SslMethod { /// /// The caller must ensure the pointer is valid. #[corresponds(TLS_server_method)] + #[must_use] pub unsafe fn from_ptr(ptr: *const ffi::SSL_METHOD) -> SslMethod { SslMethod(ptr) } /// Returns a pointer to the underlying OpenSSL value. #[allow(clippy::trivially_copy_pass_by_ref)] + #[must_use] pub fn as_ptr(&self) -> *const ffi::SSL_METHOD { self.0 } @@ -378,12 +384,14 @@ impl SslFiletype { pub const ASN1: SslFiletype = SslFiletype(ffi::SSL_FILETYPE_ASN1); /// Constructs an `SslFiletype` from a raw OpenSSL value. + #[must_use] pub fn from_raw(raw: c_int) -> SslFiletype { SslFiletype(raw) } /// Returns the raw OpenSSL value represented by this type. #[allow(clippy::trivially_copy_pass_by_ref)] + #[must_use] pub fn as_raw(&self) -> c_int { self.0 } @@ -398,12 +406,14 @@ impl StatusType { pub const OCSP: StatusType = StatusType(ffi::TLSEXT_STATUSTYPE_ocsp); /// Constructs a `StatusType` from a raw OpenSSL value. + #[must_use] pub fn from_raw(raw: c_int) -> StatusType { StatusType(raw) } /// Returns the raw OpenSSL value represented by this type. #[allow(clippy::trivially_copy_pass_by_ref)] + #[must_use] pub fn as_raw(&self) -> c_int { self.0 } @@ -418,12 +428,14 @@ impl NameType { pub const HOST_NAME: NameType = NameType(ffi::TLSEXT_NAMETYPE_host_name); /// Constructs a `StatusType` from a raw OpenSSL value. + #[must_use] pub fn from_raw(raw: c_int) -> StatusType { StatusType(raw) } /// Returns the raw OpenSSL value represented by this type. #[allow(clippy::trivially_copy_pass_by_ref)] + #[must_use] pub fn as_raw(&self) -> c_int { self.0 } @@ -733,6 +745,7 @@ impl SslCurve { /// Returns the curve name #[corresponds(SSL_get_curve_name)] + #[must_use] pub fn name(&self) -> Option<&'static str> { unsafe { let ptr = ffi::SSL_get_curve_name(self.0 as u16); @@ -830,6 +843,7 @@ impl CertificateCompressionAlgorithm { /// /// [`SslContextBuilder::set_alpn_protos`]: struct.SslContextBuilder.html#method.set_alpn_protos #[corresponds(SSL_select_next_proto)] +#[must_use] pub fn select_next_proto<'a>(server: &'a [u8], client: &'a [u8]) -> Option<&'a [u8]> { if server.is_empty() || client.is_empty() { return None; @@ -901,12 +915,14 @@ pub struct SslInfoCallbackAlert(c_int); impl SslInfoCallbackAlert { /// The level of the SSL alert. + #[must_use] pub fn alert_level(&self) -> Ssl3AlertLevel { let value = self.0 >> 8; Ssl3AlertLevel(value) } /// The value of the SSL alert. + #[must_use] pub fn alert(&self) -> SslAlert { let value = self.0 & i32::from(u8::MAX); SslAlert(value) @@ -1033,6 +1049,7 @@ impl SslContextBuilder { } /// Returns a pointer to the raw OpenSSL value. + #[must_use] pub fn as_ptr(&self) -> *mut ffi::SSL_CTX { self.ctx.as_ptr() } @@ -1490,6 +1507,7 @@ impl SslContextBuilder { /// /// [`ciphers`]: https://www.openssl.org/docs/manmaster/man1/ciphers.html #[corresponds(SSL_CTX_get_ciphers)] + #[must_use] pub fn ciphers(&self) -> Option<&StackRef> { self.ctx.ciphers() } @@ -1508,6 +1526,7 @@ impl SslContextBuilder { /// Returns the options used by the context. #[corresponds(SSL_CTX_get_options)] + #[must_use] pub fn options(&self) -> SslOptions { let bits = unsafe { ffi::SSL_CTX_get_options(self.as_ptr()) }; SslOptions::from_bits_retain(bits) @@ -1731,6 +1750,7 @@ impl SslContextBuilder { /// Returns a shared reference to the context's certificate store. #[corresponds(SSL_CTX_get_cert_store)] + #[must_use] pub fn cert_store(&self) -> &X509StoreBuilderRef { #[cfg(feature = "rpk")] assert!(!self.is_rpk, "This API is not supported for RPK"); @@ -2086,6 +2106,7 @@ impl SslContextBuilder { } /// Consumes the builder, returning a new `SslContext`. + #[must_use] pub fn build(self) -> SslContext { self.ctx } @@ -2169,6 +2190,7 @@ impl SslContext { /// /// [`ciphers`]: https://www.openssl.org/docs/manmaster/man1/ciphers.html #[corresponds(SSL_CTX_get_ciphers)] + #[must_use] pub fn ciphers(&self) -> Option<&StackRef> { unsafe { let ciphers = ffi::SSL_CTX_get_ciphers(self.as_ptr()); @@ -2184,6 +2206,7 @@ impl SslContext { impl SslContextRef { /// Returns the certificate associated with this `SslContext`, if present. #[corresponds(SSL_CTX_get0_certificate)] + #[must_use] pub fn certificate(&self) -> Option<&X509Ref> { #[cfg(feature = "rpk")] assert!(!self.is_rpk(), "This API is not supported for RPK"); @@ -2200,6 +2223,7 @@ impl SslContextRef { /// Returns the private key associated with this `SslContext`, if present. #[corresponds(SSL_CTX_get0_privatekey)] + #[must_use] pub fn private_key(&self) -> Option<&PKeyRef> { unsafe { let ptr = ffi::SSL_CTX_get0_privatekey(self.as_ptr()); @@ -2213,6 +2237,7 @@ impl SslContextRef { /// Returns a shared reference to the certificate store used for verification. #[corresponds(SSL_CTX_get_cert_store)] + #[must_use] pub fn cert_store(&self) -> &X509StoreRef { #[cfg(feature = "rpk")] assert!(!self.is_rpk(), "This API is not supported for RPK"); @@ -2222,6 +2247,7 @@ impl SslContextRef { /// Returns a shared reference to the stack of certificates making up the chain from the leaf. #[corresponds(SSL_CTX_get_extra_chain_certs)] + #[must_use] pub fn extra_chain_certs(&self) -> &StackRef { unsafe { let mut chain = ptr::null_mut(); @@ -2233,6 +2259,7 @@ impl SslContextRef { /// Returns a reference to the extra data at the specified index. #[corresponds(SSL_CTX_get_ex_data)] + #[must_use] pub fn ex_data(&self, index: Index) -> Option<&T> { unsafe { let data = ffi::SSL_CTX_get_ex_data(self.as_ptr(), index.as_raw()); @@ -2288,6 +2315,7 @@ impl SslContextRef { /// The caller of this method is responsible for ensuring that the session has never been used with another /// `SslContext` than this one. #[corresponds(SSL_CTX_add_session)] + #[must_use] pub unsafe fn add_session(&self, session: &SslSessionRef) -> bool { ffi::SSL_CTX_add_session(self.as_ptr(), session.as_ptr()) != 0 } @@ -2301,6 +2329,7 @@ impl SslContextRef { /// The caller of this method is responsible for ensuring that the session has never been used with another /// `SslContext` than this one. #[corresponds(SSL_CTX_remove_session)] + #[must_use] pub unsafe fn remove_session(&self, session: &SslSessionRef) -> bool { ffi::SSL_CTX_remove_session(self.as_ptr(), session.as_ptr()) != 0 } @@ -2310,6 +2339,7 @@ impl SslContextRef { /// A value of 0 means that the cache size is unbounded. #[corresponds(SSL_CTX_sess_get_cache_size)] #[allow(clippy::useless_conversion)] + #[must_use] pub fn session_cache_size(&self) -> u64 { unsafe { ffi::SSL_CTX_sess_get_cache_size(self.as_ptr()).into() } } @@ -2318,6 +2348,7 @@ impl SslContextRef { /// /// [`SslContextBuilder::set_verify`]: struct.SslContextBuilder.html#method.set_verify #[corresponds(SSL_CTX_get_verify_mode)] + #[must_use] pub fn verify_mode(&self) -> SslVerifyMode { #[cfg(feature = "rpk")] assert!(!self.is_rpk(), "This API is not supported for RPK"); @@ -2370,6 +2401,7 @@ pub struct ClientHello<'ssl>(&'ssl ffi::SSL_CLIENT_HELLO); impl ClientHello<'_> { /// Returns the data of a given extension, if present. #[corresponds(SSL_early_callback_ctx_extension_get)] + #[must_use] pub fn get_extension(&self, ext_type: ExtensionType) -> Option<&[u8]> { unsafe { let mut ptr = ptr::null(); @@ -2387,36 +2419,43 @@ impl ClientHello<'_> { unsafe { SslRef::from_ptr_mut(self.0.ssl) } } + #[must_use] pub fn ssl(&self) -> &SslRef { unsafe { SslRef::from_ptr(self.0.ssl) } } /// Returns the servername sent by the client via Server Name Indication (SNI). + #[must_use] pub fn servername(&self, type_: NameType) -> Option<&str> { self.ssl().servername(type_) } /// Returns the version sent by the client in its Client Hello record. + #[must_use] pub fn client_version(&self) -> SslVersion { SslVersion(self.0.version) } /// Returns a string describing the protocol version of the connection. + #[must_use] pub fn version_str(&self) -> &'static str { self.ssl().version_str() } /// Returns the raw data of the client hello message + #[must_use] pub fn as_bytes(&self) -> &[u8] { unsafe { slice::from_raw_parts(self.0.client_hello, self.0.client_hello_len) } } /// Returns the client random data + #[must_use] pub fn random(&self) -> &[u8] { unsafe { slice::from_raw_parts(self.0.random, self.0.random_len) } } /// Returns the raw list of ciphers supported by the client in its Client Hello record. + #[must_use] pub fn ciphers(&self) -> &[u8] { unsafe { slice::from_raw_parts(self.0.cipher_suites, self.0.cipher_suites_len) } } @@ -2427,6 +2466,7 @@ pub struct SslCipher(*mut ffi::SSL_CIPHER); impl SslCipher { #[corresponds(SSL_get_cipher_by_value)] + #[must_use] pub fn from_value(value: u16) -> Option { unsafe { let ptr = ffi::SSL_get_cipher_by_value(value); @@ -2484,6 +2524,7 @@ unsafe impl ForeignTypeRef for SslCipherRef { impl SslCipherRef { /// Returns the name of the cipher. #[corresponds(SSL_CIPHER_get_name)] + #[must_use] pub fn name(&self) -> &'static str { unsafe { let ptr = ffi::SSL_CIPHER_get_name(self.as_ptr()); @@ -2493,6 +2534,7 @@ impl SslCipherRef { /// Returns the RFC-standard name of the cipher, if one exists. #[corresponds(SSL_CIPHER_standard_name)] + #[must_use] pub fn standard_name(&self) -> Option<&'static str> { unsafe { let ptr = ffi::SSL_CIPHER_standard_name(self.as_ptr()); @@ -2506,6 +2548,7 @@ impl SslCipherRef { /// Returns the SSL/TLS protocol version that first defined the cipher. #[corresponds(SSL_CIPHER_get_version)] + #[must_use] pub fn version(&self) -> &'static str { let version = unsafe { let ptr = ffi::SSL_CIPHER_get_version(self.as_ptr()); @@ -2518,6 +2561,7 @@ impl SslCipherRef { /// Returns the number of bits used for the cipher. #[corresponds(SSL_CIPHER_get_bits)] #[allow(clippy::useless_conversion)] + #[must_use] pub fn bits(&self) -> CipherBits { unsafe { let mut algo_bits = 0; @@ -2531,6 +2575,7 @@ impl SslCipherRef { /// Returns a textual description of the cipher. #[corresponds(SSL_CIPHER_description)] + #[must_use] pub fn description(&self) -> String { unsafe { // SSL_CIPHER_description requires a buffer of at least 128 bytes. @@ -2542,12 +2587,14 @@ impl SslCipherRef { /// Returns one if the cipher uses an AEAD cipher. #[corresponds(SSL_CIPHER_is_aead)] + #[must_use] pub fn cipher_is_aead(&self) -> bool { unsafe { ffi::SSL_CIPHER_is_aead(self.as_ptr()) != 0 } } /// Returns the NID corresponding to the cipher's authentication type. #[corresponds(SSL_CIPHER_get_auth_nid)] + #[must_use] pub fn cipher_auth_nid(&self) -> Option { let n = unsafe { ffi::SSL_CIPHER_get_auth_nid(self.as_ptr()) }; if n == 0 { @@ -2559,6 +2606,7 @@ impl SslCipherRef { /// Returns the NID corresponding to the cipher. #[corresponds(SSL_CIPHER_get_cipher_nid)] + #[must_use] pub fn cipher_nid(&self) -> Option { let n = unsafe { ffi::SSL_CIPHER_get_cipher_nid(self.as_ptr()) }; if n == 0 { @@ -2610,6 +2658,7 @@ impl ToOwned for SslSessionRef { impl SslSessionRef { /// Returns the SSL session ID. #[corresponds(SSL_SESSION_get_id)] + #[must_use] pub fn id(&self) -> &[u8] { unsafe { let mut len = 0; @@ -2620,6 +2669,7 @@ impl SslSessionRef { /// Returns the length of the master key. #[corresponds(SSL_SESSION_get_master_key)] + #[must_use] pub fn master_key_len(&self) -> usize { unsafe { SSL_SESSION_get_master_key(self.as_ptr(), ptr::null_mut(), 0) } } @@ -2635,6 +2685,7 @@ impl SslSessionRef { /// Returns the time at which the session was established, in seconds since the Unix epoch. #[corresponds(SSL_SESSION_get_time)] #[allow(clippy::useless_conversion)] + #[must_use] pub fn time(&self) -> u64 { unsafe { ffi::SSL_SESSION_get_time(self.as_ptr()) } } @@ -2644,12 +2695,14 @@ impl SslSessionRef { /// A session older than this time should not be used for session resumption. #[corresponds(SSL_SESSION_get_timeout)] #[allow(clippy::useless_conversion)] + #[must_use] pub fn timeout(&self) -> u32 { unsafe { ffi::SSL_SESSION_get_timeout(self.as_ptr()) } } /// Returns the session's TLS protocol version. #[corresponds(SSL_SESSION_get_protocol_version)] + #[must_use] pub fn protocol_version(&self) -> SslVersion { unsafe { let version = ffi::SSL_SESSION_get_protocol_version(self.as_ptr()); @@ -2904,6 +2957,7 @@ impl SslRef { /// Returns the [`SslCurve`] used for this `SslRef`. #[corresponds(SSL_get_curve_id)] + #[must_use] pub fn curve(&self) -> Option { let curve_id = unsafe { ffi::SSL_get_curve_id(self.as_ptr()) }; if curve_id == 0 { @@ -2914,6 +2968,7 @@ impl SslRef { /// Returns an `ErrorCode` value for the most recent operation on this `SslRef`. #[corresponds(SSL_get_error)] + #[must_use] pub fn error_code(&self, ret: c_int) -> ErrorCode { unsafe { ErrorCode::from_raw(ffi::SSL_get_error(self.as_ptr(), ret)) } } @@ -2950,6 +3005,7 @@ impl SslRef { /// Returns the verify mode that was set using `set_verify`. #[corresponds(SSL_get_verify_mode)] + #[must_use] pub fn verify_mode(&self) -> SslVerifyMode { #[cfg(feature = "rpk")] assert!( @@ -3094,6 +3150,7 @@ impl SslRef { /// Returns the stack of available SslCiphers for `SSL`, sorted by preference. #[corresponds(SSL_get_ciphers)] + #[must_use] pub fn ciphers(&self) -> &StackRef { unsafe { let cipher_list = ffi::SSL_get_ciphers(self.as_ptr()); @@ -3103,6 +3160,7 @@ impl SslRef { /// Returns the current cipher if the session is active. #[corresponds(SSL_get_current_cipher)] + #[must_use] pub fn current_cipher(&self) -> Option<&SslCipherRef> { unsafe { let ptr = ffi::SSL_get_current_cipher(self.as_ptr()); @@ -3117,6 +3175,7 @@ impl SslRef { /// Returns a short string describing the state of the session. #[corresponds(SSL_state_string)] + #[must_use] pub fn state_string(&self) -> &'static str { let state = unsafe { let ptr = ffi::SSL_state_string(self.as_ptr()); @@ -3128,6 +3187,7 @@ impl SslRef { /// Returns a longer string describing the state of the session. #[corresponds(SSL_state_string_long)] + #[must_use] pub fn state_string_long(&self) -> &'static str { let state = unsafe { let ptr = ffi::SSL_state_string_long(self.as_ptr()); @@ -3151,6 +3211,7 @@ impl SslRef { /// Returns the peer's certificate, if present. #[corresponds(SSL_get_peer_certificate)] + #[must_use] pub fn peer_certificate(&self) -> Option { #[cfg(feature = "rpk")] assert!( @@ -3173,6 +3234,7 @@ impl SslRef { /// On the client side, the chain includes the leaf certificate, but on the server side it does /// not. Fun! #[corresponds(SSL_get_peer_certificate)] + #[must_use] pub fn peer_cert_chain(&self) -> Option<&StackRef> { #[cfg(feature = "rpk")] assert!( @@ -3192,6 +3254,7 @@ impl SslRef { /// Like [`SslContext::certificate`]. #[corresponds(SSL_get_certificate)] + #[must_use] pub fn certificate(&self) -> Option<&X509Ref> { #[cfg(feature = "rpk")] assert!( @@ -3211,6 +3274,7 @@ impl SslRef { /// Like [`SslContext::private_key`]. #[corresponds(SSL_get_privatekey)] + #[must_use] pub fn private_key(&self) -> Option<&PKeyRef> { unsafe { let ptr = ffi::SSL_get_privatekey(self.as_ptr()); @@ -3223,6 +3287,7 @@ impl SslRef { } #[deprecated(since = "0.10.5", note = "renamed to `version_str`")] + #[must_use] pub fn version(&self) -> &str { self.version_str() } @@ -3242,6 +3307,7 @@ impl SslRef { /// Returns a string describing the protocol version of the session. #[corresponds(SSL_get_version)] + #[must_use] pub fn version_str(&self) -> &'static str { let version = unsafe { let ptr = ffi::SSL_get_version(self.as_ptr()); @@ -3295,6 +3361,7 @@ impl SslRef { /// Gets the maximum supported protocol version. #[corresponds(SSL_get_max_proto_version)] + #[must_use] pub fn max_proto_version(&self) -> Option { let r = unsafe { ffi::SSL_get_max_proto_version(self.as_ptr()) }; if r == 0 { @@ -3309,6 +3376,7 @@ impl SslRef { /// The protocol's name is returned is an opaque sequence of bytes. It is up to the client /// to interpret it. #[corresponds(SSL_get0_alpn_selected)] + #[must_use] pub fn selected_alpn_protocol(&self) -> Option<&[u8]> { unsafe { let mut data: *const c_uchar = ptr::null(); @@ -3345,6 +3413,7 @@ impl SslRef { /// /// DTLS extension "use_srtp" as defined in RFC5764 has to be enabled. #[corresponds(SSL_get_strp_profiles)] + #[must_use] pub fn srtp_profiles(&self) -> Option<&StackRef> { unsafe { let chain = ffi::SSL_get_srtp_profiles(self.as_ptr()); @@ -3361,6 +3430,7 @@ impl SslRef { /// /// DTLS extension "use_srtp" as defined in RFC5764 has to be enabled. #[corresponds(SSL_get_selected_srtp_profile)] + #[must_use] pub fn selected_srtp_profile(&self) -> Option<&SrtpProtectionProfileRef> { unsafe { let profile = ffi::SSL_get_selected_srtp_profile(self.as_ptr()); @@ -3378,6 +3448,7 @@ impl SslRef { /// If this is greater than 0, the next call to `read` will not call down to the underlying /// stream. #[corresponds(SSL_pending)] + #[must_use] pub fn pending(&self) -> usize { unsafe { ffi::SSL_pending(self.as_ptr()) as usize } } @@ -3395,6 +3466,7 @@ impl SslRef { /// // FIXME maybe rethink in 0.11? #[corresponds(SSL_get_servername)] + #[must_use] pub fn servername(&self, type_: NameType) -> Option<&str> { self.servername_raw(type_) .and_then(|b| str::from_utf8(b).ok()) @@ -3408,6 +3480,7 @@ impl SslRef { /// /// Unlike `servername`, this method does not require the name be valid UTF-8. #[corresponds(SSL_get_servername)] + #[must_use] pub fn servername_raw(&self, type_: NameType) -> Option<&[u8]> { unsafe { let name = ffi::SSL_get_servername(self.as_ptr(), type_.0); @@ -3429,6 +3502,7 @@ impl SslRef { /// Returns the context corresponding to the current connection. #[corresponds(SSL_get_SSL_CTX)] + #[must_use] pub fn ssl_context(&self) -> &SslContextRef { unsafe { let ssl_ctx = ffi::SSL_get_SSL_CTX(self.as_ptr()); @@ -3467,6 +3541,7 @@ impl SslRef { /// Returns a shared reference to the SSL session. #[corresponds(SSL_get_session)] + #[must_use] pub fn session(&self) -> Option<&SslSessionRef> { unsafe { let p = ffi::SSL_get_session(self.as_ptr()); @@ -3544,6 +3619,7 @@ impl SslRef { /// Determines if the session provided to `set_session` was successfully reused. #[corresponds(SSL_session_reused)] + #[must_use] pub fn session_reused(&self) -> bool { unsafe { ffi::SSL_session_reused(self.as_ptr()) != 0 } } @@ -3558,6 +3634,7 @@ impl SslRef { /// Returns the server's OCSP response, if present. #[corresponds(SSL_get_tlsext_status_ocsp_resp)] + #[must_use] pub fn ocsp_status(&self) -> Option<&[u8]> { unsafe { let mut p = ptr::null(); @@ -3589,6 +3666,7 @@ impl SslRef { /// Determines if this `Ssl` is configured for server-side or client-side use. #[corresponds(SSL_is_server)] + #[must_use] pub fn is_server(&self) -> bool { unsafe { SSL_is_server(self.as_ptr()) != 0 } } @@ -3637,6 +3715,7 @@ impl SslRef { /// Returns a reference to the extra data at the specified index. #[corresponds(SSL_get_ex_data)] + #[must_use] pub fn ex_data(&self, index: Index) -> Option<&T> { unsafe { let data = ffi::SSL_get_ex_data(self.as_ptr(), index.as_raw()); @@ -3684,6 +3763,7 @@ impl SslRef { /// Determines if the initial handshake has been completed. #[corresponds(SSL_is_init_finished)] + #[must_use] pub fn is_init_finished(&self) -> bool { unsafe { ffi::SSL_is_init_finished(self.as_ptr()) != 0 } } @@ -3779,6 +3859,7 @@ impl SslRef { /// connection using the returned `ECHConfigList`. #[cfg(not(feature = "fips"))] #[corresponds(SSL_get0_ech_retry_configs)] + #[must_use] pub fn get_ech_retry_configs(&self) -> Option<&[u8]> { unsafe { let mut data = ptr::null(); @@ -3801,6 +3882,7 @@ impl SslRef { /// authenticate retry configs. #[cfg(not(feature = "fips"))] #[corresponds(SSL_get0_ech_name_override)] + #[must_use] pub fn get_ech_name_override(&self) -> Option<&[u8]> { unsafe { let mut data: *const c_char = ptr::null(); @@ -3818,6 +3900,7 @@ impl SslRef { // Whether or not `SSL` negotiated ECH. #[cfg(not(feature = "fips"))] #[corresponds(SSL_ech_accepted)] + #[must_use] pub fn ech_accepted(&self) -> bool { unsafe { ffi::SSL_ech_accepted(self.as_ptr()) != 0 } } @@ -3850,6 +3933,7 @@ pub struct MidHandshakeSslStream { impl MidHandshakeSslStream { /// Returns a shared reference to the inner stream. + #[must_use] pub fn get_ref(&self) -> &S { self.stream.get_ref() } @@ -3860,6 +3944,7 @@ impl MidHandshakeSslStream { } /// Returns a shared reference to the `Ssl` of the stream. + #[must_use] pub fn ssl(&self) -> &SslRef { self.stream.ssl() } @@ -3870,21 +3955,25 @@ impl MidHandshakeSslStream { } /// Returns the underlying error which interrupted this handshake. + #[must_use] pub fn error(&self) -> &Error { &self.error } /// Consumes `self`, returning its error. + #[must_use] pub fn into_error(self) -> Error { self.error } /// Returns the source data stream. + #[must_use] pub fn into_source_stream(self) -> S { self.stream.into_inner() } /// Returns both the error and the source data stream, consuming `self`. + #[must_use] pub fn into_parts(self) -> (Error, S) { (self.error, self.stream.into_inner()) } @@ -4152,11 +4241,13 @@ impl SslStream { } /// Converts the SslStream to the underlying data stream. + #[must_use] pub fn into_inner(self) -> S { unsafe { bio::take_stream::(self.ssl.get_raw_rbio()) } } /// Returns a shared reference to the underlying stream. + #[must_use] pub fn get_ref(&self) -> &S { unsafe { let bio = self.ssl.get_raw_rbio(); @@ -4178,6 +4269,7 @@ impl SslStream { } /// Returns a shared reference to the `Ssl` object associated with this stream. + #[must_use] pub fn ssl(&self) -> &SslRef { &self.ssl } @@ -4251,6 +4343,7 @@ where /// This method calls [`Self::set_connect_state`] and returns without actually /// initiating the handshake. The caller is then free to call /// [`MidHandshakeSslStream`] and loop on [`HandshakeError::WouldBlock`]. + #[must_use] pub fn setup_connect(mut self) -> MidHandshakeSslStream { self.set_connect_state(); @@ -4282,6 +4375,7 @@ where /// This method calls [`Self::set_accept_state`] and returns without actually /// initiating the handshake. The caller is then free to call /// [`MidHandshakeSslStream`] and loop on [`HandshakeError::WouldBlock`]. + #[must_use] pub fn setup_accept(mut self) -> MidHandshakeSslStream { self.set_accept_state(); @@ -4335,6 +4429,7 @@ where impl SslStreamBuilder { /// Returns a shared reference to the underlying stream. + #[must_use] pub fn get_ref(&self) -> &S { unsafe { let bio = self.inner.ssl.get_raw_rbio(); @@ -4356,6 +4451,7 @@ impl SslStreamBuilder { } /// Returns a shared reference to the `Ssl` object associated with this builder. + #[must_use] pub fn ssl(&self) -> &SslRef { &self.inner.ssl } diff --git a/boring/src/ssl/test/mod.rs b/boring/src/ssl/test/mod.rs index 40806b5f..9b7b024c 100644 --- a/boring/src/ssl/test/mod.rs +++ b/boring/src/ssl/test/mod.rs @@ -42,7 +42,7 @@ static KEY: &[u8] = include_bytes!("../../../test/key.pem"); #[test] fn get_ctx_options() { let ctx = SslContext::builder(SslMethod::tls()).unwrap(); - ctx.options(); + let _ = ctx.options(); } #[test] diff --git a/boring/src/stack.rs b/boring/src/stack.rs index 78b2773b..67958e15 100644 --- a/boring/src/stack.rs +++ b/boring/src/stack.rs @@ -180,15 +180,18 @@ impl StackRef { } /// Returns the number of items in the stack. + #[must_use] pub fn len(&self) -> usize { unsafe { OPENSSL_sk_num(self.as_stack()) } } /// Determines if the stack is empty. + #[must_use] pub fn is_empty(&self) -> bool { self.len() == 0 } + #[must_use] pub fn iter(&self) -> Iter { Iter { stack: self, @@ -205,6 +208,7 @@ impl StackRef { /// Returns a reference to the element at the given index in the /// stack or `None` if the index is out of bounds + #[must_use] pub fn get(&self, idx: usize) -> Option<&T::Ref> { unsafe { if idx >= self.len() { diff --git a/boring/src/symm.rs b/boring/src/symm.rs index 4f5527b2..fff8a4a1 100644 --- a/boring/src/symm.rs +++ b/boring/src/symm.rs @@ -79,6 +79,7 @@ pub struct Cipher(*const ffi::EVP_CIPHER); impl Cipher { /// Looks up the cipher for a certain nid. #[corresponds(EVP_get_cipherbynid)] + #[must_use] pub fn from_nid(nid: Nid) -> Option { let ptr = unsafe { ffi::EVP_get_cipherbyname(ffi::OBJ_nid2sn(nid.as_raw())) }; if ptr.is_null() { @@ -88,82 +89,102 @@ impl Cipher { } } + #[must_use] pub fn aes_128_ecb() -> Cipher { unsafe { Cipher(ffi::EVP_aes_128_ecb()) } } + #[must_use] pub fn aes_128_cbc() -> Cipher { unsafe { Cipher(ffi::EVP_aes_128_cbc()) } } + #[must_use] pub fn aes_128_ctr() -> Cipher { unsafe { Cipher(ffi::EVP_aes_128_ctr()) } } + #[must_use] pub fn aes_128_gcm() -> Cipher { unsafe { Cipher(ffi::EVP_aes_128_gcm()) } } + #[must_use] pub fn aes_128_ofb() -> Cipher { unsafe { Cipher(ffi::EVP_aes_128_ofb()) } } + #[must_use] pub fn aes_192_ecb() -> Cipher { unsafe { Cipher(ffi::EVP_aes_192_ecb()) } } + #[must_use] pub fn aes_192_cbc() -> Cipher { unsafe { Cipher(ffi::EVP_aes_192_cbc()) } } + #[must_use] pub fn aes_192_ctr() -> Cipher { unsafe { Cipher(ffi::EVP_aes_192_ctr()) } } + #[must_use] pub fn aes_192_gcm() -> Cipher { unsafe { Cipher(ffi::EVP_aes_192_gcm()) } } + #[must_use] pub fn aes_192_ofb() -> Cipher { unsafe { Cipher(ffi::EVP_aes_192_ofb()) } } + #[must_use] pub fn aes_256_ecb() -> Cipher { unsafe { Cipher(ffi::EVP_aes_256_ecb()) } } + #[must_use] pub fn aes_256_cbc() -> Cipher { unsafe { Cipher(ffi::EVP_aes_256_cbc()) } } + #[must_use] pub fn aes_256_ctr() -> Cipher { unsafe { Cipher(ffi::EVP_aes_256_ctr()) } } + #[must_use] pub fn aes_256_gcm() -> Cipher { unsafe { Cipher(ffi::EVP_aes_256_gcm()) } } + #[must_use] pub fn aes_256_ofb() -> Cipher { unsafe { Cipher(ffi::EVP_aes_256_ofb()) } } + #[must_use] pub fn des_cbc() -> Cipher { unsafe { Cipher(ffi::EVP_des_cbc()) } } + #[must_use] pub fn des_ecb() -> Cipher { unsafe { Cipher(ffi::EVP_des_ecb()) } } + #[must_use] pub fn des_ede3() -> Cipher { unsafe { Cipher(ffi::EVP_des_ede3()) } } + #[must_use] pub fn des_ede3_cbc() -> Cipher { unsafe { Cipher(ffi::EVP_des_ede3_cbc()) } } + #[must_use] pub fn rc4() -> Cipher { unsafe { Cipher(ffi::EVP_rc4()) } } @@ -173,17 +194,20 @@ impl Cipher { /// # Safety /// /// The caller must ensure the pointer is valid for the `'static` lifetime. + #[must_use] pub unsafe fn from_ptr(ptr: *const ffi::EVP_CIPHER) -> Cipher { Cipher(ptr) } #[allow(clippy::trivially_copy_pass_by_ref)] + #[must_use] pub fn as_ptr(&self) -> *const ffi::EVP_CIPHER { self.0 } /// Returns the length of keys used with this cipher. #[allow(clippy::trivially_copy_pass_by_ref)] + #[must_use] pub fn key_len(&self) -> usize { unsafe { EVP_CIPHER_key_length(self.0) as usize } } @@ -191,6 +215,7 @@ impl Cipher { /// Returns the length of the IV used with this cipher, or `None` if the /// cipher does not use an IV. #[allow(clippy::trivially_copy_pass_by_ref)] + #[must_use] pub fn iv_len(&self) -> Option { unsafe { let len = EVP_CIPHER_iv_length(self.0) as usize; @@ -208,6 +233,7 @@ impl Cipher { /// /// Stream ciphers such as RC4 have a block size of 1. #[allow(clippy::trivially_copy_pass_by_ref)] + #[must_use] pub fn block_size(&self) -> usize { unsafe { EVP_CIPHER_block_size(self.0) as usize } } diff --git a/boring/src/version.rs b/boring/src/version.rs index 821441bd..744241fd 100644 --- a/boring/src/version.rs +++ b/boring/src/version.rs @@ -42,11 +42,13 @@ use crate::ffi::{ /// Version 0.9.5a had an interim interpretation that is like the current one, except the patch level got the highest bit set, to keep continuity. The number was therefore 0x0090581f /// /// The return value of this function can be compared to the macro to make sure that the correct version of the library has been loaded, especially when using DLLs on Windows systems. +#[must_use] pub fn number() -> i64 { unsafe { OpenSSL_version_num() as i64 } } /// The text variant of the version number and the release date. For example, "OpenSSL 0.9.5a 1 Apr 2000". +#[must_use] pub fn version() -> &'static str { unsafe { CStr::from_ptr(OpenSSL_version(OPENSSL_VERSION)) @@ -57,6 +59,7 @@ pub fn version() -> &'static str { /// The compiler flags set for the compilation process in the form "compiler: ..." if available or /// "compiler: information not available" otherwise. +#[must_use] pub fn c_flags() -> &'static str { unsafe { CStr::from_ptr(OpenSSL_version(OPENSSL_CFLAGS)) @@ -66,6 +69,7 @@ pub fn c_flags() -> &'static str { } /// The date of the build process in the form "built on: ..." if available or "built on: date not available" otherwise. +#[must_use] pub fn built_on() -> &'static str { unsafe { CStr::from_ptr(OpenSSL_version(OPENSSL_BUILT_ON)) @@ -75,6 +79,7 @@ pub fn built_on() -> &'static str { } /// The "Configure" target of the library build in the form "platform: ..." if available or "platform: information not available" otherwise. +#[must_use] pub fn platform() -> &'static str { unsafe { CStr::from_ptr(OpenSSL_version(OPENSSL_PLATFORM)) @@ -84,6 +89,7 @@ pub fn platform() -> &'static str { } /// The "OPENSSLDIR" setting of the library build in the form "OPENSSLDIR: "..."" if available or "OPENSSLDIR: N/A" otherwise. +#[must_use] pub fn dir() -> &'static str { unsafe { CStr::from_ptr(OpenSSL_version(OPENSSL_DIR)) diff --git a/boring/src/x509/extension.rs b/boring/src/x509/extension.rs index e8797327..1ed6f4f5 100644 --- a/boring/src/x509/extension.rs +++ b/boring/src/x509/extension.rs @@ -38,6 +38,7 @@ impl Default for BasicConstraints { impl BasicConstraints { /// Construct a new `BasicConstraints` extension. + #[must_use] pub fn new() -> BasicConstraints { BasicConstraints { critical: false, @@ -106,6 +107,7 @@ impl Default for KeyUsage { impl KeyUsage { /// Construct a new `KeyUsage` extension. + #[must_use] pub fn new() -> KeyUsage { KeyUsage { critical: false, @@ -234,6 +236,7 @@ impl Default for ExtendedKeyUsage { impl ExtendedKeyUsage { /// Construct a new `ExtendedKeyUsage` extension. + #[must_use] pub fn new() -> ExtendedKeyUsage { ExtendedKeyUsage { critical: false, @@ -329,6 +332,7 @@ impl Default for SubjectKeyIdentifier { impl SubjectKeyIdentifier { /// Construct a new `SubjectKeyIdentifier` extension. + #[must_use] pub fn new() -> SubjectKeyIdentifier { SubjectKeyIdentifier { critical: false } } @@ -365,6 +369,7 @@ impl Default for AuthorityKeyIdentifier { impl AuthorityKeyIdentifier { /// Construct a new `AuthorityKeyIdentifier` extension. + #[must_use] pub fn new() -> AuthorityKeyIdentifier { AuthorityKeyIdentifier { critical: false, @@ -433,6 +438,7 @@ impl Default for SubjectAlternativeName { impl SubjectAlternativeName { /// Construct a new `SubjectAlternativeName` extension. + #[must_use] pub fn new() -> SubjectAlternativeName { SubjectAlternativeName { critical: false, diff --git a/boring/src/x509/mod.rs b/boring/src/x509/mod.rs index 2640f784..5bac50c9 100644 --- a/boring/src/x509/mod.rs +++ b/boring/src/x509/mod.rs @@ -103,6 +103,7 @@ impl X509StoreContext { impl X509StoreContextRef { /// Returns application data pertaining to an `X509` store context. #[corresponds(X509_STORE_CTX_get_ex_data)] + #[must_use] pub fn ex_data(&self, index: Index) -> Option<&T> { unsafe { let data = ffi::X509_STORE_CTX_get_ex_data(self.as_ptr(), index.as_raw()); @@ -284,6 +285,7 @@ impl X509StoreContextRef { /// Returns a reference to the certificate which caused the error or None if /// no certificate is relevant to the error. #[corresponds(X509_STORE_CTX_get_current_cert)] + #[must_use] pub fn current_cert(&self) -> Option<&X509Ref> { unsafe { let ptr = ffi::X509_STORE_CTX_get_current_cert(self.as_ptr()); @@ -300,12 +302,14 @@ impl X509StoreContextRef { /// entity certificate, one if it is the certificate which signed the end /// entity certificate and so on. #[corresponds(X509_STORE_CTX_get_error_depth)] + #[must_use] pub fn error_depth(&self) -> u32 { unsafe { ffi::X509_STORE_CTX_get_error_depth(self.as_ptr()) as u32 } } /// Returns a reference to a complete valid `X509` certificate chain. #[corresponds(X509_STORE_CTX_get0_chain)] + #[must_use] pub fn chain(&self) -> Option<&StackRef> { unsafe { let chain = X509_STORE_CTX_get0_chain(self.as_ptr()); @@ -321,6 +325,7 @@ impl X509StoreContextRef { /// Returns a reference to the `X509` certificates used to initialize the /// [`X509StoreContextRef`]. #[corresponds(X509_STORE_CTX_get0_untrusted)] + #[must_use] pub fn untrusted(&self) -> Option<&StackRef> { unsafe { let certs = ffi::X509_STORE_CTX_get0_untrusted(self.as_ptr()); @@ -336,6 +341,7 @@ impl X509StoreContextRef { /// Returns a reference to the certificate being verified. /// May return None if a raw public key is being verified. #[corresponds(X509_STORE_CTX_get0_cert)] + #[must_use] pub fn cert(&self) -> Option<&X509Ref> { unsafe { let ptr = ffi::X509_STORE_CTX_get0_cert(self.as_ptr()); @@ -448,6 +454,7 @@ impl X509Builder { /// /// Set `issuer` to `None` if the certificate will be self-signed. #[corresponds(X509V3_set_ctx)] + #[must_use] pub fn x509v3_context<'a>( &'a self, issuer: Option<&'a X509Ref>, @@ -505,6 +512,7 @@ impl X509Builder { } /// Consumes the builder, returning the certificate. + #[must_use] pub fn build(self) -> X509 { self.0 } @@ -521,6 +529,7 @@ foreign_type_and_impl_send_sync! { impl X509Ref { /// Returns this certificate's subject name. #[corresponds(X509_get_subject_name)] + #[must_use] pub fn subject_name(&self) -> &X509NameRef { unsafe { let name = ffi::X509_get_subject_name(self.as_ptr()); @@ -530,12 +539,14 @@ impl X509Ref { /// Returns the hash of the certificates subject #[corresponds(X509_subject_name_hash)] + #[must_use] pub fn subject_name_hash(&self) -> u32 { unsafe { ffi::X509_subject_name_hash(self.as_ptr()) as u32 } } /// Returns this certificate's subject alternative name entries, if they exist. #[corresponds(X509_get_ext_d2i)] + #[must_use] pub fn subject_alt_names(&self) -> Option> { unsafe { let stack = ffi::X509_get_ext_d2i( @@ -554,6 +565,7 @@ impl X509Ref { /// Returns this certificate's issuer name. #[corresponds(X509_get_issuer_name)] + #[must_use] pub fn issuer_name(&self) -> &X509NameRef { unsafe { let name = ffi::X509_get_issuer_name(self.as_ptr()); @@ -563,6 +575,7 @@ impl X509Ref { /// Returns this certificate's issuer alternative name entries, if they exist. #[corresponds(X509_get_ext_d2i)] + #[must_use] pub fn issuer_alt_names(&self) -> Option> { unsafe { let stack = ffi::X509_get_ext_d2i( @@ -581,6 +594,7 @@ impl X509Ref { /// Returns this certificate's subject key id, if it exists. #[corresponds(X509_get0_subject_key_id)] + #[must_use] pub fn subject_key_id(&self) -> Option<&Asn1StringRef> { unsafe { let data = ffi::X509_get0_subject_key_id(self.as_ptr()); @@ -590,6 +604,7 @@ impl X509Ref { /// Returns this certificate's authority key id, if it exists. #[corresponds(X509_get0_authority_key_id)] + #[must_use] pub fn authority_key_id(&self) -> Option<&Asn1StringRef> { unsafe { let data = ffi::X509_get0_authority_key_id(self.as_ptr()); @@ -633,6 +648,7 @@ impl X509Ref { /// Returns the certificate's Not After validity period. #[corresponds(X509_getm_notAfter)] + #[must_use] pub fn not_after(&self) -> &Asn1TimeRef { unsafe { let date = X509_getm_notAfter(self.as_ptr()); @@ -643,6 +659,7 @@ impl X509Ref { /// Returns the certificate's Not Before validity period. #[corresponds(X509_getm_notBefore)] + #[must_use] pub fn not_before(&self) -> &Asn1TimeRef { unsafe { let date = X509_getm_notBefore(self.as_ptr()); @@ -653,6 +670,7 @@ impl X509Ref { /// Returns the certificate's signature #[corresponds(X509_get0_signature)] + #[must_use] pub fn signature(&self) -> &Asn1BitStringRef { unsafe { let mut signature = ptr::null(); @@ -664,6 +682,7 @@ impl X509Ref { /// Returns the certificate's signature algorithm. #[corresponds(X509_get0_signature)] + #[must_use] pub fn signature_algorithm(&self) -> &X509AlgorithmRef { unsafe { let mut algor = ptr::null(); @@ -705,6 +724,7 @@ impl X509Ref { /// Returns this certificate's serial number. #[corresponds(X509_get_serialNumber)] + #[must_use] pub fn serial_number(&self) -> &Asn1IntegerRef { unsafe { let r = ffi::X509_get_serialNumber(self.as_ptr()); @@ -860,6 +880,7 @@ impl Stackable for X509 { pub struct X509v3Context<'a>(ffi::X509V3_CTX, PhantomData<(&'a X509Ref, &'a ConfRef)>); impl X509v3Context<'_> { + #[must_use] pub fn as_ptr(&self) -> *mut ffi::X509V3_CTX { &self.0 as *const _ as *mut _ } @@ -1085,6 +1106,7 @@ impl X509NameBuilder { } /// Return an `X509Name`. + #[must_use] pub fn build(self) -> X509Name { // Round-trip through bytes because OpenSSL is not const correct and // names in a "modified" state compute various things lazily. This can @@ -1137,6 +1159,7 @@ impl Stackable for X509Name { impl X509NameRef { /// Returns the name entries by the nid. + #[must_use] pub fn entries_by_nid(&self, nid: Nid) -> X509NameEntries<'_> { X509NameEntries { name: self, @@ -1146,6 +1169,7 @@ impl X509NameRef { } /// Returns an iterator over all `X509NameEntry` values + #[must_use] pub fn entries(&self) -> X509NameEntries<'_> { X509NameEntries { name: self, @@ -1158,6 +1182,7 @@ impl X509NameRef { /// /// This function will return `None` if the underlying string contains invalid utf-8. #[corresponds(X509_NAME_print_ex)] + #[must_use] pub fn print_ex(&self, flags: i32) -> Option { unsafe { let bio = MemBio::new().ok()?; @@ -1231,6 +1256,7 @@ foreign_type_and_impl_send_sync! { impl X509NameEntryRef { /// Returns the field value of an `X509NameEntry`. #[corresponds(X509_NAME_ENTRY_get_data)] + #[must_use] pub fn data(&self) -> &Asn1StringRef { unsafe { let data = ffi::X509_NAME_ENTRY_get_data(self.as_ptr()); @@ -1241,6 +1267,7 @@ impl X509NameEntryRef { /// Returns the `Asn1Object` value of an `X509NameEntry`. /// This is useful for finding out about the actual `Nid` when iterating over all `X509NameEntries`. #[corresponds(X509_NAME_ENTRY_get_object)] + #[must_use] pub fn object(&self) -> &Asn1ObjectRef { unsafe { let object = ffi::X509_NAME_ENTRY_get_object(self.as_ptr()); @@ -1303,6 +1330,7 @@ impl X509ReqBuilder { /// Return an `X509v3Context`. This context object can be used to construct /// certain `X509` extensions. + #[must_use] pub fn x509v3_context<'a>(&'a self, conf: Option<&'a ConfRef>) -> X509v3Context<'a> { unsafe { let mut ctx = mem::zeroed(); @@ -1356,6 +1384,7 @@ impl X509ReqBuilder { } /// Returns the `X509Req`. + #[must_use] pub fn build(self) -> X509Req { self.0 } @@ -1414,12 +1443,14 @@ impl X509ReqRef { /// Returns the numerical value of the version field of the certificate request. #[corresponds(X509_REQ_get_version)] + #[must_use] pub fn version(&self) -> i32 { unsafe { X509_REQ_get_version(self.as_ptr()) as i32 } } /// Returns the subject name of the certificate request. #[corresponds(X509_REQ_get_subject_name)] + #[must_use] pub fn subject_name(&self) -> &X509NameRef { unsafe { let name = X509_REQ_get_subject_name(self.as_ptr()); @@ -1505,6 +1536,7 @@ impl X509VerifyError { /// Return the integer representation of an [`X509VerifyError`]. #[allow(clippy::trivially_copy_pass_by_ref)] + #[must_use] pub fn as_raw(&self) -> c_int { self.0 } @@ -1512,6 +1544,7 @@ impl X509VerifyError { /// Return a human readable error string from the verification error. #[corresponds(X509_verify_cert_error_string)] #[allow(clippy::trivially_copy_pass_by_ref)] + #[must_use] pub fn error_string(&self) -> &'static str { ffi::init(); @@ -1681,21 +1714,25 @@ impl GeneralNameRef { } /// Returns the contents of this `GeneralName` if it is an `rfc822Name`. + #[must_use] pub fn email(&self) -> Option<&str> { self.ia5_string(ffi::GEN_EMAIL) } /// Returns the contents of this `GeneralName` if it is a `dNSName`. + #[must_use] pub fn dnsname(&self) -> Option<&str> { self.ia5_string(ffi::GEN_DNS) } /// Returns the contents of this `GeneralName` if it is an `uniformResourceIdentifier`. + #[must_use] pub fn uri(&self) -> Option<&str> { self.ia5_string(ffi::GEN_URI) } /// Returns the contents of this `GeneralName` if it is an `iPAddress`. + #[must_use] pub fn ipaddress(&self) -> Option<&[u8]> { unsafe { if (*self.as_ptr()).type_ != ffi::GEN_IPADD { @@ -1741,6 +1778,7 @@ foreign_type_and_impl_send_sync! { impl X509AlgorithmRef { /// Returns the ASN.1 OID of this algorithm. + #[must_use] pub fn object(&self) -> &Asn1ObjectRef { unsafe { let mut oid = ptr::null(); @@ -1760,6 +1798,7 @@ foreign_type_and_impl_send_sync! { } impl X509ObjectRef { + #[must_use] pub fn x509(&self) -> Option<&X509Ref> { unsafe { let ptr = X509_OBJECT_get0_X509(self.as_ptr()); diff --git a/boring/src/x509/store.rs b/boring/src/x509/store.rs index d4bcd045..1f6d839b 100644 --- a/boring/src/x509/store.rs +++ b/boring/src/x509/store.rs @@ -71,6 +71,7 @@ impl X509StoreBuilder { } /// Constructs the `X509Store`. + #[must_use] pub fn build(self) -> X509Store { let store = X509Store(self.0); mem::forget(self); @@ -144,6 +145,7 @@ impl X509StoreRef { note = "This method is unsound https://github.com/sfackler/rust-openssl/issues/2096" )] #[corresponds(X509_STORE_get0_objects)] + #[must_use] pub fn objects(&self) -> &StackRef { unsafe { StackRef::from_ptr(ffi::X509_STORE_get0_objects(self.as_ptr())) } } @@ -151,6 +153,7 @@ impl X509StoreRef { /// For testing only, where it doesn't have to expose an unsafe pointer #[cfg(test)] #[allow(deprecated)] + #[must_use] pub fn objects_len(&self) -> usize { self.objects().len() } diff --git a/boring/src/x509/verify.rs b/boring/src/x509/verify.rs index 8b63b7bc..d89c67c8 100644 --- a/boring/src/x509/verify.rs +++ b/boring/src/x509/verify.rs @@ -112,6 +112,7 @@ impl X509VerifyParamRef { /// Gets verification flags. #[corresponds(X509_VERIFY_PARAM_get_flags)] + #[must_use] pub fn flags(&self) -> X509VerifyFlags { let bits = unsafe { ffi::X509_VERIFY_PARAM_get_flags(self.as_ptr()) }; X509VerifyFlags::from_bits_retain(bits) diff --git a/hyper-boring/src/lib.rs b/hyper-boring/src/lib.rs index 0daa3f6a..d206eb6b 100644 --- a/hyper-boring/src/lib.rs +++ b/hyper-boring/src/lib.rs @@ -30,6 +30,7 @@ pub struct HttpsLayerSettings { impl HttpsLayerSettings { /// Constructs an [`HttpsLayerSettingsBuilder`] for configuring settings + #[must_use] pub fn builder() -> HttpsLayerSettingsBuilder { HttpsLayerSettingsBuilder(HttpsLayerSettings::default()) } @@ -54,6 +55,7 @@ impl HttpsLayerSettingsBuilder { } /// Consumes the builder, returning a new [`HttpsLayerSettings`] + #[must_use] pub fn build(self) -> HttpsLayerSettings { self.0 } diff --git a/tokio-boring/src/lib.rs b/tokio-boring/src/lib.rs index 0dae747a..374a0bde 100644 --- a/tokio-boring/src/lib.rs +++ b/tokio-boring/src/lib.rs @@ -113,6 +113,7 @@ where impl SslStreamBuilder { /// Returns a shared reference to the `Ssl` object associated with this builder. + #[must_use] pub fn ssl(&self) -> &SslRef { self.inner.ssl() } @@ -135,6 +136,7 @@ pub struct SslStream(ssl::SslStream>); impl SslStream { /// Returns a shared reference to the `Ssl` object associated with this stream. + #[must_use] pub fn ssl(&self) -> &SslRef { self.0.ssl() } @@ -145,6 +147,7 @@ impl SslStream { } /// Returns a shared reference to the underlying stream. + #[must_use] pub fn get_ref(&self) -> &S { &self.0.get_ref().stream } @@ -253,6 +256,7 @@ pub struct HandshakeError(ssl::HandshakeError>); impl HandshakeError { /// Returns a shared reference to the `Ssl` object associated with this error. + #[must_use] pub fn ssl(&self) -> Option<&SslRef> { match &self.0 { ssl::HandshakeError::Failure(s) => Some(s.ssl()), @@ -261,6 +265,7 @@ impl HandshakeError { } /// Converts error to the source data stream that was used for the handshake. + #[must_use] pub fn into_source_stream(self) -> Option { match self.0 { ssl::HandshakeError::Failure(s) => Some(s.into_source_stream().stream), @@ -269,6 +274,7 @@ impl HandshakeError { } /// Returns a reference to the source data stream. + #[must_use] pub fn as_source_stream(&self) -> Option<&S> { match &self.0 { ssl::HandshakeError::Failure(s) => Some(&s.get_ref().stream), @@ -277,6 +283,7 @@ impl HandshakeError { } /// Returns the error code, if any. + #[must_use] pub fn code(&self) -> Option { match &self.0 { ssl::HandshakeError::Failure(s) => Some(s.error().code()), @@ -285,6 +292,7 @@ impl HandshakeError { } /// Returns a reference to the inner I/O error, if any. + #[must_use] pub fn as_io_error(&self) -> Option<&io::Error> { match &self.0 { ssl::HandshakeError::Failure(s) => s.error().io_error(), From 17d137e33bf3126daf7d095f84695cdf5e03ff56 Mon Sep 17 00:00:00 2001 From: Justin-Kwan <37853180+Justin-Kwan@users.noreply.github.com> Date: Thu, 5 Jun 2025 18:25:28 -0700 Subject: [PATCH 13/13] Expose SSL_set1_groups to Efficiently Set Curves on SSL Session (#346) --- boring/src/ssl/mod.rs | 46 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/boring/src/ssl/mod.rs b/boring/src/ssl/mod.rs index 5d3ee1db..90934d06 100644 --- a/boring/src/ssl/mod.rs +++ b/boring/src/ssl/mod.rs @@ -697,6 +697,11 @@ impl From for SslSignatureAlgorithm { } } +/// Numeric identifier of a TLS curve. +#[repr(transparent)] +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub struct SslCurveNid(c_int); + /// A TLS Curve. #[repr(transparent)] #[derive(Debug, Copy, Clone, PartialEq, Eq)] @@ -767,7 +772,7 @@ impl SslCurve { // underlying boringssl version is upgraded, this should be removed in favor of the new // SSL_CTX_set1_group_ids API. #[allow(dead_code)] - fn nid(&self) -> Option { + pub fn nid(&self) -> Option { match self.0 { ffi::SSL_CURVE_SECP224R1 => Some(ffi::NID_secp224r1), ffi::SSL_CURVE_SECP256R1 => Some(ffi::NID_X9_62_prime256v1), @@ -798,6 +803,7 @@ impl SslCurve { ffi::SSL_CURVE_X25519_MLKEM768 => Some(ffi::NID_X25519MLKEM768), _ => None, } + .map(SslCurveNid) } } @@ -2062,7 +2068,10 @@ impl SslContextBuilder { #[corresponds(SSL_CTX_set1_curves)] #[cfg(not(feature = "kx-safe-default"))] pub fn set_curves(&mut self, curves: &[SslCurve]) -> Result<(), ErrorStack> { - let curves: Vec = curves.iter().filter_map(|curve| curve.nid()).collect(); + let curves: Vec = curves + .iter() + .filter_map(|curve| curve.nid().map(|nid| nid.0)) + .collect(); unsafe { cvt_0i(ffi::SSL_CTX_set1_curves( @@ -2909,6 +2918,25 @@ impl SslRef { unsafe { ffi::SSL_get_rbio(self.as_ptr()) } } + /// Sets the options used by the ongoing session, returning the old set. + /// + /// # Note + /// + /// This *enables* the specified options, but does not disable unspecified options. Use + /// `clear_options` for that. + #[corresponds(SSL_set_options)] + pub fn set_options(&mut self, option: SslOptions) -> SslOptions { + let bits = unsafe { ffi::SSL_set_options(self.as_ptr(), option.bits()) }; + SslOptions::from_bits_retain(bits) + } + + /// Clears the options used by the ongoing session, returning the old set. + #[corresponds(SSL_clear_options)] + pub fn clear_options(&mut self, option: SslOptions) -> SslOptions { + let bits = unsafe { ffi::SSL_clear_options(self.as_ptr(), option.bits()) }; + SslOptions::from_bits_retain(bits) + } + #[corresponds(SSL_set1_curves_list)] pub fn set_curves_list(&mut self, curves: &str) -> Result<(), ErrorStack> { let curves = CString::new(curves).map_err(ErrorStack::internal_error)?; @@ -2921,6 +2949,20 @@ impl SslRef { } } + /// Sets the ongoing session's supported groups by their named identifiers + /// (formerly referred to as curves). + #[corresponds(SSL_set1_groups)] + pub fn set_group_nids(&mut self, group_nids: &[SslCurveNid]) -> Result<(), ErrorStack> { + unsafe { + cvt_0i(ffi::SSL_set1_curves( + self.as_ptr(), + group_nids.as_ptr() as *const _, + group_nids.len(), + )) + .map(|_| ()) + } + } + #[cfg(feature = "kx-safe-default")] fn client_set_default_curves_list(&mut self) { let curves = if cfg!(feature = "kx-client-pq-preferred") {