Hide all raw pointers manipulation by users behind an opt-in feature flag

6 files changed, 50 insertions, 43 deletions

diff --git a/rustables/src/batch.rs b/rustables/src/batch.rs
index 24eca8e..2af1a7c 100644
--- a/rustables/src/batch.rs
+++ b/rustables/src/batch.rs
@@ -27,11 +27,6 @@ pub struct Batch {
     is_empty: bool,
 }
 
-// Safety: It should be safe to pass this around and *read* from it
-// from multiple threads
-unsafe impl Send for Batch {}
-unsafe impl Sync for Batch {}
-
 impl Batch {
     /// Creates a new nftnl batch with the [default page size].
     ///
@@ -120,11 +115,13 @@ impl Batch {
         self.next();
     }
 
+    #[cfg(feature = "unsafe-raw-handles")]
     /// Returns the raw handle.
     pub fn as_ptr(&self) -> *const sys::nftnl_batch {
         self.batch as *const sys::nftnl_batch
     }
 
+    #[cfg(feature = "unsafe-raw-handles")]
     /// Returns a mutable version of the raw handle.
     pub fn as_mut_ptr(&mut self) -> *mut sys::nftnl_batch {
         self.batch
@@ -151,7 +148,7 @@ pub struct FinalizedBatch {
 impl FinalizedBatch {
     /// Returns the iterator over byte buffers to send to netlink.
     pub fn iter(&mut self) -> Iter<'_> {
-        let num_pages = unsafe { sys::nftnl_batch_iovec_len(self.batch.as_mut_ptr()) as usize };
+        let num_pages = unsafe { sys::nftnl_batch_iovec_len(self.batch.batch) as usize };
         let mut iovecs = vec![
             libc::iovec {
                 iov_base: ptr::null_mut(),
@@ -161,7 +158,7 @@ impl FinalizedBatch {
         ];
         let iovecs_ptr = iovecs.as_mut_ptr() as *mut [u8; 0];
         unsafe {
-            sys::nftnl_batch_iovec(self.batch.as_mut_ptr(), iovecs_ptr, num_pages as u32);
+            sys::nftnl_batch_iovec(self.batch.batch, iovecs_ptr, num_pages as u32);
         }
         Iter {
             iovecs: iovecs.into_iter(),
@@ -184,11 +181,6 @@ pub struct Iter<'a> {
     _marker: ::std::marker::PhantomData<&'a ()>,
 }
 
-// Safety: It should be safe to pass this around and *read* from it
-// from multiple threads.
-unsafe impl<'a> Send for Iter<'a> {}
-unsafe impl<'a> Sync for Iter<'a> {}
-
 impl<'a> Iterator for Iter<'a> {
     type Item = &'a [u8];
 
diff --git a/rustables/src/chain.rs b/rustables/src/chain.rs
index 516536c..a5e732e 100644
--- a/rustables/src/chain.rs
+++ b/rustables/src/chain.rs
@@ -1,11 +1,11 @@
 use crate::{MsgType, Table};
 use rustables_sys::{self as sys, libc};
-use std::sync::Arc;
 use std::{
     convert::TryFrom,
     ffi::{c_void, CStr, CString},
     fmt,
     os::raw::c_char,
+    rc::Rc,
 };
 
 pub type Priority = i32;
@@ -71,19 +71,14 @@ impl ChainType {
 /// [`set_hook`]: #method.set_hook
 pub struct Chain {
     chain: *mut sys::nftnl_chain,
-    table: Arc<Table>,
+    table: Rc<Table>,
 }
 
-// Safety: It should be safe to pass this around and *read* from it
-// from multiple threads
-unsafe impl Send for Chain {}
-unsafe impl Sync for Chain {}
-
 impl Chain {
     /// Creates a new chain instance inside the given [`Table`] and with the given name.
     ///
     /// [`Table`]: struct.Table.html
-    pub fn new<T: AsRef<CStr>>(name: &T, table: Arc<Table>) -> Chain {
+    pub fn new<T: AsRef<CStr>>(name: &T, table: Rc<Table>) -> Chain {
         unsafe {
             let chain = try_alloc!(sys::nftnl_chain_alloc());
             sys::nftnl_chain_set_u32(
@@ -101,7 +96,7 @@ impl Chain {
         }
     }
 
-    pub unsafe fn from_raw(chain: *mut sys::nftnl_chain, table: Arc<Table>) -> Self {
+    pub unsafe fn from_raw(chain: *mut sys::nftnl_chain, table: Rc<Table>) -> Self {
         Chain { chain, table }
     }
 
@@ -183,15 +178,17 @@ impl Chain {
     /// Returns a reference to the [`Table`] this chain belongs to
     ///
     /// [`Table`]: struct.Table.html
-    pub fn get_table(&self) -> Arc<Table> {
+    pub fn get_table(&self) -> Rc<Table> {
         self.table.clone()
     }
 
+    #[cfg(feature = "unsafe-raw-handles")]
     /// Returns the raw handle.
     pub fn as_ptr(&self) -> *const sys::nftnl_chain {
         self.chain as *const sys::nftnl_chain
     }
 
+    #[cfg(feature = "unsafe-raw-handles")]
     /// Returns a mutable version of the raw handle.
     pub fn as_mut_ptr(&mut self) -> *mut sys::nftnl_chain {
         self.chain
@@ -241,7 +238,7 @@ impl Drop for Chain {
 #[cfg(feature = "query")]
 pub fn get_chains_cb<'a>(
     header: &libc::nlmsghdr,
-    (table, chains): &mut (&Arc<Table>, &mut Vec<Chain>),
+    (table, chains): &mut (&Rc<Table>, &mut Vec<Chain>),
 ) -> libc::c_int {
     unsafe {
         let chain = sys::nftnl_chain_alloc();
@@ -285,6 +282,6 @@ pub fn get_chains_cb<'a>(
 }
 
 #[cfg(feature = "query")]
-pub fn list_chains_for_table(table: Arc<Table>) -> Result<Vec<Chain>, crate::query::Error> {
+pub fn list_chains_for_table(table: Rc<Table>) -> Result<Vec<Chain>, crate::query::Error> {
     crate::query::list_objects_with_data(libc::NFT_MSG_GETCHAIN as u16, get_chains_cb, &table, None)
 }
diff --git a/rustables/src/lib.rs b/rustables/src/lib.rs
index c5d53c0..1397245 100644
--- a/rustables/src/lib.rs
+++ b/rustables/src/lib.rs
@@ -48,6 +48,25 @@
 //! See the documentation for the corresponding sys crate for details: [`rustables-sys`].
 //! This crate has the same features as the sys crate, and selecting version works the same.
 //!
+//! # Access to raw handles
+//!
+//! Retrieving raw handles is considered unsafe and should only ever be enabled if you absoluetely
+//! need it. It is disabled by default and hidden behind the feature gate `unsafe-raw-handles`.
+//! The reason for that special treatment is we cannot guarantee the lack of aliasing. For example,
+//! a program using a const handle to a object in a thread and writing through a mutable handle
+//! in another could reach all kind of undefined (and dangerous!) behaviors.
+//! By enabling that feature flag, you acknowledge that guaranteeing the respect of safety
+//! invariants is now your responsibility!
+//! Despite these shortcomings, that feature is still available because it may allow you to perform
+//! manipulations that this library doesn't currently expose. If that is your case, we would
+//! be very happy to hear from you and maybe help you get the necessary functionality upstream.
+//!
+//! Our current lack of confidence in our availability to provide a safe abstraction over the
+//! use of raw handles in the face of concurrency is the reason we decided to settly on `Rc`
+//! pointers instead of `Arc` (besides, this should gives us some nice performance boost, not
+//! that it matters much of course) and why we do not declare the types exposes by the library
+//! as `Send` nor `Sync`.
+//!
 //! [`libnftnl`]: https://netfilter.org/projects/libnftnl/
 //! [`nftables`]: https://netfilter.org/projects/nftables/
 //! [`rustables-sys`]: https://crates.io/crates/rustables-sys
diff --git a/rustables/src/rule.rs b/rustables/src/rule.rs
index cdd1876..6e15db7 100644
--- a/rustables/src/rule.rs
+++ b/rustables/src/rule.rs
@@ -3,24 +3,19 @@ use rustables_sys::{self as sys, libc};
 use std::ffi::{c_void, CStr, CString};
 use std::fmt::Debug;
 use std::os::raw::c_char;
-use std::sync::Arc;
+use std::rc::Rc;
 
 /// A nftables firewall rule.
 pub struct Rule {
     rule: *mut sys::nftnl_rule,
-    chain: Arc<Chain>,
+    chain: Rc<Chain>,
 }
 
-// Safety: It should be safe to pass this around and *read* from it
-// from multiple threads
-unsafe impl Send for Rule {}
-unsafe impl Sync for Rule {}
-
 impl Rule {
     /// Creates a new rule object in the given [`Chain`].
     ///
     /// [`Chain`]: struct.Chain.html
-    pub fn new(chain: Arc<Chain>) -> Rule {
+    pub fn new(chain: Rc<Chain>) -> Rule {
         unsafe {
             let rule = try_alloc!(sys::nftnl_rule_alloc());
             sys::nftnl_rule_set_u32(
@@ -43,7 +38,7 @@ impl Rule {
         }
     }
 
-    pub unsafe fn from_raw(rule: *mut sys::nftnl_rule, chain: Arc<Chain>) -> Self {
+    pub unsafe fn from_raw(rule: *mut sys::nftnl_rule, chain: Rc<Chain>) -> Self {
         Rule { rule, chain }
     }
 
@@ -79,7 +74,7 @@ impl Rule {
     /// Returns a reference to the [`Chain`] this rule lives in.
     ///
     /// [`Chain`]: struct.Chain.html
-    pub fn get_chain(&self) -> Arc<Chain> {
+    pub fn get_chain(&self) -> Rc<Chain> {
         self.chain.clone()
     }
 
@@ -116,11 +111,13 @@ impl Rule {
         }
     }
 
+    #[cfg(feature = "unsafe-raw-handles")]
     /// Returns the raw handle.
     pub fn as_ptr(&self) -> *const sys::nftnl_rule {
         self.rule as *const sys::nftnl_rule
     }
 
+    #[cfg(feature = "unsafe-raw-handles")]
     /// Returns a mutable version of the raw handle.
     pub fn as_mut_ptr(&mut self) -> *mut sys::nftnl_rule {
         self.rule
@@ -169,7 +166,7 @@ impl Drop for Rule {
 #[cfg(feature = "query")]
 pub fn get_rules_cb(
     header: &libc::nlmsghdr,
-    (chain, rules): &mut (&Arc<Chain>, &mut Vec<Rule>),
+    (chain, rules): &mut (&Rc<Chain>, &mut Vec<Rule>),
 ) -> libc::c_int {
     unsafe {
         let rule = sys::nftnl_rule_alloc();
@@ -189,7 +186,7 @@ pub fn get_rules_cb(
 }
 
 #[cfg(feature = "query")]
-pub fn list_rules_for_chain(chain: &Arc<Chain>) -> Result<Vec<Rule>, crate::query::Error> {
+pub fn list_rules_for_chain(chain: &Rc<Chain>) -> Result<Vec<Rule>, crate::query::Error> {
     crate::query::list_objects_with_data(
         libc::NFT_MSG_GETRULE as u16,
         get_rules_cb,
diff --git a/rustables/src/set.rs b/rustables/src/set.rs
index 9791e7f..d8c84d6 100644
--- a/rustables/src/set.rs
+++ b/rustables/src/set.rs
@@ -99,11 +99,13 @@ impl<'a, K> Set<'a, K> {
         SetElemsIter::new(self)
     }
 
+    #[cfg(feature = "unsafe-raw-handles")]
     /// Returns the raw handle.
     pub fn as_ptr(&self) -> *const sys::nftnl_set {
         self.set as *const sys::nftnl_set
     }
 
+    #[cfg(feature = "unsafe-raw-handles")]
     /// Returns a mutable version of the raw handle.
     pub fn as_mut_ptr(&self) -> *mut sys::nftnl_set {
         self.set
@@ -177,7 +179,9 @@ pub struct SetElemsIter<'a, K> {
 
 impl<'a, K> SetElemsIter<'a, K> {
     fn new(set: &'a Set<'a, K>) -> Self {
-        let iter = try_alloc!(unsafe { sys::nftnl_set_elems_iter_create(set.as_ptr()) });
+        let iter = try_alloc!(unsafe {
+            sys::nftnl_set_elems_iter_create(set.set as *const sys::nftnl_set)
+        });
         SetElemsIter {
             set,
             iter,
diff --git a/rustables/src/table.rs b/rustables/src/table.rs
index 447d035..dc09b5e 100644
--- a/rustables/src/table.rs
+++ b/rustables/src/table.rs
@@ -1,7 +1,8 @@
 use crate::{MsgType, ProtoFamily};
 use rustables_sys::{self as sys, libc};
+#[cfg(feature = "query")]
+use std::convert::TryFrom;
 use std::{
-    convert::TryFrom,
     ffi::{c_void, CStr, CString},
     fmt::Debug,
     os::raw::c_char,
@@ -16,11 +17,6 @@ pub struct Table {
     family: ProtoFamily,
 }
 
-// Safety: It should be safe to pass this around and *read* from it
-// from multiple threads
-unsafe impl Send for Table {}
-unsafe impl Sync for Table {}
-
 impl Table {
     /// Creates a new table instance with the given name and protocol family.
     pub fn new<T: AsRef<CStr>>(name: &T, family: ProtoFamily) -> Table {
@@ -84,11 +80,13 @@ impl Table {
         }
     }
 
+    #[cfg(feature = "unsafe-raw-handles")]
     /// Returns the raw handle.
     pub fn as_ptr(&self) -> *const sys::nftnl_table {
         self.table as *const sys::nftnl_table
     }
 
+    #[cfg(feature = "unsafe-raw-handles")]
     /// Returns a mutable version of the raw handle.
     pub fn as_mut_ptr(&self) -> *mut sys::nftnl_table {
         self.table