node: Use banning for repeated offenders

When the penalty score for a node reaches a certain threshold, we ban
the node.

Banned nodes are not allowed to connect to us.

radicle-node/src/service.rs

@@ -1162,6 +1162,14 @@ where
         if self.sessions.inbound().count() >= self.config.limits.connection.inbound {
             return false;
         }
+        match self.db.addresses().is_banned(&addr) {
+            Ok(banned) => {
+                if banned {
+                    return false;
+                }
+            }
+            Err(e) => error!(target: "service", "Error querying ban status for {addr}: {e}"),
+        }
         let host: HostName = addr.into();
 
         if self.limiter.limit(
@@ -2185,7 +2193,7 @@ where
                 // even if it's in a disconnected state. Those sessions are re-attempted automatically.
                 let mut peers = entries
                     .filter(|entry| !entry.address.banned)
-                    .filter(|entry| !entry.penalty.is_threshold_reached())
+                    .filter(|entry| !entry.penalty.is_connect_threshold_reached())
                     .filter(|entry| !self.sessions.contains_key(&entry.node))
                     .filter(|entry| !self.config.external_addresses.contains(&entry.address.addr))
                     .filter(|entry| &entry.node != self.nid())

radicle/src/node.rs

@@ -53,7 +53,9 @@ pub const DEFAULT_TIMEOUT: time::Duration = time::Duration::from_secs(30);
 /// Maximum length in bytes of a node alias.
 pub const MAX_ALIAS_LENGTH: usize = 32;
 /// Penalty threshold at which point we avoid connecting to this node.
-pub const PENALTY_THRESHOLD: u8 = 32;
+pub const PENALTY_CONNECT_THRESHOLD: u8 = 32;
+/// Penalty threshold at which point we ban this node.
+pub const PENALTY_BAN_THRESHOLD: u8 = 64;
 /// Filename of node database under the node directory.
 pub const NODE_DB_FILE: &str = "node.db";
 /// Filename of policies database under the node directory.
@@ -162,8 +164,12 @@ pub struct Penalty(u8);
 impl Penalty {
     /// If the penalty threshold is reached, at which point we should just avoid
     /// connecting to this node.
-    pub fn is_threshold_reached(&self) -> bool {
+    pub fn is_connect_threshold_reached(&self) -> bool {
-        self.0 >= PENALTY_THRESHOLD
+        self.0 >= PENALTY_CONNECT_THRESHOLD
+    }
+
+    pub fn is_ban_threshold_reached(&self) -> bool {
+        self.0 >= PENALTY_BAN_THRESHOLD
     }
 }
 

radicle/src/node/address.rs

@@ -132,6 +132,8 @@ pub struct Node {
     pub timestamp: Timestamp,
     /// Node connection penalty.
     pub penalty: Penalty,
+    /// Whether the node is banned.
+    pub banned: bool,
 }
 
 /// A known address.

radicle/src/node/address/store.rs

@@ -63,6 +63,9 @@ pub trait Store {
     fn is_empty(&self) -> Result<bool, Error> {
         self.len().map(|l| l == 0)
     }
+    /// Check if an address is banned. Also returns `true` if the node this address belongs
+    /// to is banned.
+    fn is_banned(&self, addr: &Address) -> Result<bool, Error>;
     /// Get the address entries in the store.
     fn entries(&self) -> Result<Box<dyn Iterator<Item = AddressEntry>>, Error>;
     /// Mark a node as attempted at a certain time.
@@ -80,10 +83,9 @@ pub trait Store {
 
 impl Store for Database {
     fn get(&self, node: &NodeId) -> Result<Option<Node>, Error> {
-        let mut stmt = self
+        let mut stmt = self.db.prepare(
-            .db
+            "SELECT features, alias, pow, penalty, banned, timestamp FROM nodes WHERE id = ?",
-            .prepare("SELECT features, alias, pow, penalty, timestamp FROM nodes WHERE id = ?")?;
+        )?;
-
         stmt.bind((1, node))?;
 
         if let Some(Ok(row)) = stmt.into_iter().next() {
@@ -93,6 +95,7 @@ impl Store for Database {
             let pow = row.read::<i64, _>("pow") as u32;
             let penalty = row.read::<i64, _>("penalty").min(u8::MAX as i64);
             let penalty = Penalty(penalty as u8);
+            let banned = row.read::<i64, _>("banned").is_positive();
             let addrs = self.addresses_of(node)?;
 
             Ok(Some(Node {
@@ -102,12 +105,34 @@ impl Store for Database {
                 timestamp,
                 penalty,
                 addrs,
+                banned,
             }))
         } else {
             Ok(None)
         }
     }
 
+    fn is_banned(&self, addr: &Address) -> Result<bool, Error> {
+        let mut stmt = self.db.prepare(
+            "SELECT a.banned, n.banned
+             FROM addresses AS a
+             JOIN nodes AS n ON a.node = n.id
+             WHERE value = ?1 AND type =?2",
+        )?;
+        stmt.bind((1, addr))?;
+        stmt.bind((2, AddressType::from(addr)))?;
+
+        if let Some(row) = stmt.into_iter().next() {
+            let row = row?;
+            let addr_banned = row.read::<i64, _>(0).is_positive();
+            let node_banned = row.read::<i64, _>(1).is_positive();
+
+            Ok(node_banned || addr_banned)
+        } else {
+            Ok(false)
+        }
+    }
+
     fn addresses_of(&self, node: &NodeId) -> Result<Vec<KnownAddress>, Error> {
         let mut addrs = Vec::new();
         let mut stmt = self.db.prepare(
@@ -306,17 +331,29 @@ impl Store for Database {
         _addr: &Address,
         severity: Severity,
     ) -> Result<(), Error> {
-        let mut stmt = self.db.prepare(
+        transaction(&self.db, |db| {
-            "UPDATE `nodes`
+            let mut stmt = self.db.prepare(
-             SET penalty = penalty + ?2
+                "UPDATE `nodes`
-             WHERE id = ?1",
+                 SET penalty = penalty + ?2
-        )?;
+                 WHERE id = ?1",
+            )?;
+            stmt.bind((1, nid))?;
+            stmt.bind((2, severity as i64))?;
+            stmt.next()?;
 
-        stmt.bind((1, nid))?;
+            // If the ban threshold is reached, we ban the node and its addresses.
-        stmt.bind((2, severity as i64))?;
+            let node = self.get(nid)?.ok_or(Error::NoRows)?;
-        stmt.next()?;
+            if node.penalty.is_ban_threshold_reached() {
+                let mut stmt = db.prepare("UPDATE `nodes` SET banned = 1 WHERE id = ?1")?;
+                stmt.bind((1, nid))?;
+                stmt.next()?;
 
-        Ok(())
+                let mut stmt = db.prepare("UPDATE `addresses` SET banned = 1 WHERE node = ?1")?;
+                stmt.bind((1, nid))?;
+                stmt.next()?;
+            }
+            Ok(())
+        })
     }
 }
 
@@ -684,4 +721,46 @@ mod test {
         let node = cache.get(&alice).unwrap().unwrap();
         assert_eq!(node.penalty, Penalty(4));
     }
+
+    #[test]
+    fn test_disconnected_ban() {
+        let alice = arbitrary::gen::<NodeId>(1);
+        let ka1 = arbitrary::gen::<KnownAddress>(1);
+        let ka2 = arbitrary::gen::<KnownAddress>(1);
+        let mut db = Database::memory().unwrap();
+        let features = node::Features::SEED;
+        let timestamp = Timestamp::from(LocalTime::now());
+
+        db.insert(
+            &alice,
+            features,
+            Alias::new("alice"),
+            16,
+            timestamp,
+            [ka1.clone(), ka2.clone()],
+        )
+        .unwrap();
+        let node = db.get(&alice).unwrap().unwrap();
+        assert_eq!(node.penalty, Penalty::default());
+
+        for _ in 0..7 {
+            db.disconnected(&alice, &ka1.addr, Severity::High).unwrap();
+            let node = db.get(&alice).unwrap().unwrap();
+
+            assert!(!node.penalty.is_ban_threshold_reached());
+            assert!(!node.banned);
+        }
+
+        db.disconnected(&alice, &ka1.addr, Severity::High).unwrap();
+        let node = db.get(&alice).unwrap().unwrap();
+
+        assert!(node.penalty.is_ban_threshold_reached());
+        assert!(node.banned);
+
+        for addr in node.addrs {
+            assert!(addr.banned);
+        }
+        assert!(db.is_banned(&ka1.addr).unwrap());
+        assert!(db.is_banned(&ka2.addr).unwrap()); // Banned because node is banned.
+    }
 }

radicle/src/test/arbitrary.rs

@@ -18,8 +18,8 @@ use crate::identity::{
     project::Project,
     Did,
 };
-use crate::node::address::AddressType;
+use crate::node::address::{AddressType, Source};
-use crate::node::{Address, Alias, Timestamp};
+use crate::node::{Address, Alias, KnownAddress, Timestamp};
 use crate::storage;
 use crate::storage::refs::{Refs, RefsAt, SignedRefs};
 use crate::test::storage::{MockRepository, MockStorage};
@@ -303,6 +303,12 @@ impl Arbitrary for Address {
     }
 }
 
+impl Arbitrary for KnownAddress {
+    fn arbitrary(g: &mut qcheck::Gen) -> Self {
+        KnownAddress::new(Address::arbitrary(g), Source::Peer)
+    }
+}
+
 impl Arbitrary for Alias {
     fn arbitrary(g: &mut qcheck::Gen) -> Self {
         let s = g