Skip to content

Commit

Permalink
ssl: Use Box instead of Arc for SSL
Browse files Browse the repository at this point in the history
It seems that we can be more strict here - CassSSL type do not
require shared ownership.
  • Loading branch information
Lorak-mmk authored and muzarski committed Nov 25, 2024
1 parent 82dfe26 commit 3b7a2b9
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 17 deletions.
2 changes: 1 addition & 1 deletion scylla-rust-wrapper/src/cluster.rs
Original file line number Diff line number Diff line change
Expand Up @@ -818,7 +818,7 @@ pub unsafe extern "C" fn cass_cluster_set_retry_policy(
#[no_mangle]
pub unsafe extern "C" fn cass_cluster_set_ssl(cluster: *mut CassCluster, ssl: *mut CassSsl) {
let cluster_from_raw = BoxFFI::as_mut_ref(cluster);
let cass_ssl = ArcFFI::cloned_from_ptr(ssl);
let cass_ssl = BoxFFI::as_ref(ssl);

let ssl_context_builder = SslContextBuilder::from_ptr(cass_ssl.ssl_context);
// Reference count is increased as tokio_openssl will try to free `ssl_context` when calling `SSL_free`.
Expand Down
24 changes: 8 additions & 16 deletions scylla-rust-wrapper/src/ssl.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::argconv::ArcFFI;
use crate::argconv::BoxFFI;
use crate::cass_error::CassError;
use crate::types::size_t;
use libc::{c_int, strlen};
Expand All @@ -12,14 +12,13 @@ use openssl_sys::{
use std::convert::TryInto;
use std::os::raw::c_char;
use std::os::raw::c_void;
use std::sync::Arc;

pub struct CassSsl {
pub(crate) ssl_context: *mut SSL_CTX,
pub(crate) trusted_store: *mut X509_STORE,
}

impl ArcFFI for CassSsl {}
impl BoxFFI for CassSsl {}

pub const CASS_SSL_VERIFY_NONE: i32 = 0x00;
pub const CASS_SSL_VERIFY_PEER_CERT: i32 = 0x01;
Expand All @@ -45,16 +44,9 @@ pub unsafe extern "C" fn cass_ssl_new_no_lib_init() -> *const CassSsl {
trusted_store,
};

ArcFFI::into_ptr(Arc::new(ssl))
BoxFFI::into_ptr(Box::new(ssl))
}

// This is required for the type system to impl Send + Sync for Arc<CassSsl>.
// Otherwise, clippy complains about using Arc where Rc would do.
// In our case, though, we need to use Arc because we potentially do share
// the Arc between threads, so employing Rc here would lead to races.
unsafe impl Send for CassSsl {}
unsafe impl Sync for CassSsl {}

impl Drop for CassSsl {
fn drop(&mut self) {
unsafe {
Expand All @@ -65,7 +57,7 @@ impl Drop for CassSsl {

#[no_mangle]
pub unsafe extern "C" fn cass_ssl_free(ssl: *mut CassSsl) {
ArcFFI::free(ssl);
BoxFFI::free(ssl);
}

unsafe extern "C" fn pem_password_callback(
Expand Down Expand Up @@ -112,7 +104,7 @@ pub unsafe extern "C" fn cass_ssl_add_trusted_cert_n(
cert: *const c_char,
cert_length: size_t,
) -> CassError {
let ssl = ArcFFI::cloned_from_ptr(ssl);
let ssl = BoxFFI::as_mut_ref(ssl);
let bio = BIO_new_mem_buf(cert as *const c_void, cert_length.try_into().unwrap());

if bio.is_null() {
Expand Down Expand Up @@ -140,7 +132,7 @@ pub unsafe extern "C" fn cass_ssl_add_trusted_cert_n(

#[no_mangle]
pub unsafe extern "C" fn cass_ssl_set_verify_flags(ssl: *mut CassSsl, flags: i32) {
let ssl = ArcFFI::cloned_from_ptr(ssl);
let ssl = BoxFFI::as_mut_ref(ssl);

match flags {
CASS_SSL_VERIFY_NONE => {
Expand Down Expand Up @@ -178,7 +170,7 @@ pub unsafe extern "C" fn cass_ssl_set_cert_n(
cert: *const c_char,
cert_length: size_t,
) -> CassError {
let ssl = ArcFFI::cloned_from_ptr(ssl);
let ssl = BoxFFI::as_mut_ref(ssl);
let bio = BIO_new_mem_buf(cert as *const c_void, cert_length.try_into().unwrap());

if bio.is_null() {
Expand Down Expand Up @@ -271,7 +263,7 @@ pub unsafe extern "C" fn cass_ssl_set_private_key_n(
password: *mut c_char,
_password_length: size_t,
) -> CassError {
let ssl = ArcFFI::cloned_from_ptr(ssl);
let ssl = BoxFFI::as_mut_ref(ssl);
let bio = BIO_new_mem_buf(key as *const c_void, key_length.try_into().unwrap());

if bio.is_null() {
Expand Down

0 comments on commit 3b7a2b9

Please sign in to comment.