From 1f71900319ea4dc54be29866997ac5b40827318f Mon Sep 17 00:00:00 2001 From: Kornel Date: Wed, 4 Feb 2026 01:14:04 +0000 Subject: [PATCH] FIXMEs --- README.md | 1 + boring/src/ec.rs | 10 ++++------ boring/src/rsa.rs | 14 +++++--------- boring/src/ssl/test/cert_compressor.rs | 6 +++--- boring/src/ssl/test/mod.rs | 8 ++++---- boring/src/ssl/test/verify.rs | 2 +- boring/src/x509/store.rs | 7 +++---- boring/src/x509/tests/mod.rs | 4 ++-- boring/src/x509/tests/trusted_first.rs | 2 +- 9 files changed, 24 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 42611afdc..dde5796ea 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,7 @@ and [hyper](https://github.com/hyperium/hyper) built on top of it. * `Ssl::new_from_ref` -> `Ssl::new()`. * `X509Builder::append_extension2` -> `X509Builder::append_extension`. * `X509Store` is now cheaply cloneable, but immutable. `SslContextBuilder.cert_store_mut()` can't be used after `.set_cert_store()`. Use `.set_cert_store_builder()` if you need `.cert_store_mut()`. + * `X509StoreBuilder::add_cert` takes a reference. * `hyper` 0.x support has been removed. Use `hyper` 1.x. ## Contribution diff --git a/boring/src/ec.rs b/boring/src/ec.rs index 745b81fcb..9eea6cdc9 100644 --- a/boring/src/ec.rs +++ b/boring/src/ec.rs @@ -267,8 +267,7 @@ impl EcPointRef { group: &EcGroupRef, q: &EcPointRef, m: &BigNumRef, - // FIXME should be &mut - ctx: &BigNumContextRef, + ctx: &mut BigNumContextRef, ) -> Result<(), ErrorStack> { unsafe { cvt(ffi::EC_POINT_mul( @@ -287,8 +286,7 @@ impl EcPointRef { &mut self, group: &EcGroupRef, n: &BigNumRef, - // FIXME should be &mut - ctx: &BigNumContextRef, + ctx: &mut BigNumContextRef, ) -> Result<(), ErrorStack> { unsafe { cvt(ffi::EC_POINT_mul( @@ -838,7 +836,7 @@ mod test { let mut ctx = BigNumContext::new().unwrap(); let mut public_key = EcPoint::new(&group).unwrap(); public_key - .mul_generator(&group, key.private_key(), &ctx) + .mul_generator(&group, key.private_key(), &mut ctx) .unwrap(); assert!(public_key.eq(&group, key.public_key(), &mut ctx).unwrap()); } @@ -850,7 +848,7 @@ mod test { let one = BigNum::from_u32(1).unwrap(); let mut ctx = BigNumContext::new().unwrap(); let mut ecp = EcPoint::new(&group).unwrap(); - ecp.mul_generator(&group, &one, &ctx).unwrap(); + ecp.mul_generator(&group, &one, &mut ctx).unwrap(); assert!(ecp.eq(&group, gen, &mut ctx).unwrap()); } diff --git a/boring/src/rsa.rs b/boring/src/rsa.rs index 79dfa6df9..a762a95fe 100644 --- a/boring/src/rsa.rs +++ b/boring/src/rsa.rs @@ -416,7 +416,7 @@ impl Rsa { pub fn from_public_components(n: BigNum, e: BigNum) -> Result, ErrorStack> { unsafe { let rsa = cvt_p(ffi::RSA_new())?; - RSA_set0_key(rsa, n.as_ptr(), e.as_ptr(), ptr::null_mut()); + cvt(RSA_set0_key(rsa, n.as_ptr(), e.as_ptr(), ptr::null_mut()))?; mem::forget((n, e)); Ok(Rsa::from_ptr(rsa)) } @@ -474,7 +474,7 @@ impl RsaPrivateKeyBuilder { pub fn new(n: BigNum, e: BigNum, d: BigNum) -> Result { unsafe { let rsa = cvt_p(ffi::RSA_new())?; - RSA_set0_key(rsa, n.as_ptr(), e.as_ptr(), d.as_ptr()); + cvt(RSA_set0_key(rsa, n.as_ptr(), e.as_ptr(), d.as_ptr()))?; mem::forget((n, e, d)); Ok(RsaPrivateKeyBuilder { rsa: Rsa::from_ptr(rsa), @@ -485,12 +485,10 @@ impl RsaPrivateKeyBuilder { /// Sets the factors of the Rsa key. /// /// `p` and `q` are the first and second factors of `n`. - /// - // FIXME should be infallible #[corresponds(RSA_set0_factors)] pub fn set_factors(self, p: BigNum, q: BigNum) -> Result { unsafe { - RSA_set0_factors(self.rsa.as_ptr(), p.as_ptr(), q.as_ptr()); + cvt(RSA_set0_factors(self.rsa.as_ptr(), p.as_ptr(), q.as_ptr()))?; mem::forget((p, q)); } Ok(self) @@ -500,8 +498,6 @@ impl RsaPrivateKeyBuilder { /// /// `dmp1`, `dmq1`, and `iqmp` are the exponents and coefficient for /// CRT calculations which is used to speed up RSA operations. - /// - // FIXME should be infallible #[corresponds(RSA_set0_crt_params)] pub fn set_crt_params( self, @@ -510,12 +506,12 @@ impl RsaPrivateKeyBuilder { iqmp: BigNum, ) -> Result { unsafe { - RSA_set0_crt_params( + cvt(RSA_set0_crt_params( self.rsa.as_ptr(), dmp1.as_ptr(), dmq1.as_ptr(), iqmp.as_ptr(), - ); + ))?; mem::forget((dmp1, dmq1, iqmp)); } Ok(self) diff --git a/boring/src/ssl/test/cert_compressor.rs b/boring/src/ssl/test/cert_compressor.rs index d62ffa879..78124f19a 100644 --- a/boring/src/ssl/test/cert_compressor.rs +++ b/boring/src/ssl/test/cert_compressor.rs @@ -54,7 +54,7 @@ fn server_only_cert_compression() { let mut store = X509StoreBuilder::new().unwrap(); let x509 = X509::from_pem(super::ROOT_CERT).unwrap(); - store.add_cert(x509).unwrap(); + store.add_cert(&x509).unwrap(); let client = server.client(); @@ -67,7 +67,7 @@ fn client_only_cert_compression() { let mut store = X509StoreBuilder::new().unwrap(); let x509 = X509::from_pem(super::ROOT_CERT).unwrap(); - store.add_cert(x509).unwrap(); + store.add_cert(&x509).unwrap(); let mut client = server_builder.client(); client @@ -90,7 +90,7 @@ fn client_and_server_cert_compression() { let mut store = X509StoreBuilder::new().unwrap(); let x509 = X509::from_pem(super::ROOT_CERT).unwrap(); - store.add_cert(x509).unwrap(); + store.add_cert(&x509).unwrap(); let mut client = server.client(); client diff --git a/boring/src/ssl/test/mod.rs b/boring/src/ssl/test/mod.rs index e66d0cc85..9317a7bae 100644 --- a/boring/src/ssl/test/mod.rs +++ b/boring/src/ssl/test/mod.rs @@ -317,18 +317,18 @@ fn test_mutable_store() { let cert2 = X509::from_pem(cert2).unwrap(); let mut ctx = SslContext::builder(SslMethod::tls()).unwrap(); - ctx.cert_store_mut().add_cert(cert.clone()).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(); + 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(); + 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()); diff --git a/boring/src/ssl/test/verify.rs b/boring/src/ssl/test/verify.rs index 5fed57038..b7981e68c 100644 --- a/boring/src/ssl/test/verify.rs +++ b/boring/src/ssl/test/verify.rs @@ -32,7 +32,7 @@ fn trusted_with_set_cert() { let mut store = X509StoreBuilder::new().unwrap(); let x509 = X509::from_pem(super::ROOT_CERT).unwrap(); - store.add_cert(x509).unwrap(); + store.add_cert(&x509).unwrap(); let mut client = server.client(); client.ctx().set_verify(SslVerifyMode::PEER); diff --git a/boring/src/x509/store.rs b/boring/src/x509/store.rs index f4edca034..7b9a2317b 100644 --- a/boring/src/x509/store.rs +++ b/boring/src/x509/store.rs @@ -36,7 +36,7 @@ //! //! let certificate: X509 = builder.build(); //! let mut builder = X509StoreBuilder::new().unwrap(); -//! let _ = builder.add_cert(certificate); +//! let _ = builder.add_cert(&certificate); //! let store: X509Store = builder.build(); //! ``` @@ -44,7 +44,7 @@ use crate::error::ErrorStack; use crate::ffi; use crate::stack::StackRef; use crate::x509::verify::{X509VerifyFlags, X509VerifyParamRef}; -use crate::x509::{X509Object, X509}; +use crate::x509::{X509Object, X509Ref}; use crate::{cvt, cvt_p}; use foreign_types::{ForeignType, ForeignTypeRef}; use openssl_macros::corresponds; @@ -79,9 +79,8 @@ impl X509StoreBuilder { impl X509StoreBuilderRef { /// Adds a certificate to the certificate store. - // FIXME should take an &X509Ref #[corresponds(X509_STORE_add_cert)] - pub fn add_cert(&mut self, cert: X509) -> Result<(), ErrorStack> { + pub fn add_cert(&mut self, cert: &X509Ref) -> Result<(), ErrorStack> { unsafe { cvt(ffi::X509_STORE_add_cert(self.as_ptr(), cert.as_ptr())) } } diff --git a/boring/src/x509/tests/mod.rs b/boring/src/x509/tests/mod.rs index 2a1b3fd56..90e4bf299 100644 --- a/boring/src/x509/tests/mod.rs +++ b/boring/src/x509/tests/mod.rs @@ -451,7 +451,7 @@ fn test_verify_cert() { let chain = Stack::new().unwrap(); let mut store_bldr = X509StoreBuilder::new().unwrap(); - store_bldr.add_cert(ca).unwrap(); + store_bldr.add_cert(&ca).unwrap(); let store = store_bldr.build(); let empty_store = X509StoreBuilder::new().unwrap().build(); @@ -484,7 +484,7 @@ fn test_verify_fails() { let chain = Stack::new().unwrap(); let mut store_bldr = X509StoreBuilder::new().unwrap(); - store_bldr.add_cert(ca).unwrap(); + store_bldr.add_cert(&ca).unwrap(); let store = store_bldr.build(); let mut context = X509StoreContext::new().unwrap(); diff --git a/boring/src/x509/tests/trusted_first.rs b/boring/src/x509/tests/trusted_first.rs index ad660a3b1..3755a876b 100644 --- a/boring/src/x509/tests/trusted_first.rs +++ b/boring/src/x509/tests/trusted_first.rs @@ -75,7 +75,7 @@ fn verify( let mut builder = X509StoreBuilder::new().unwrap(); for cert in trusted { - builder.add_cert((**cert).to_owned()).unwrap(); + builder.add_cert(cert).unwrap(); } builder.build()