CHANGELOG.md

@@ -2,12 +2,6 @@
 
 This project follows [semantic versioning](http://semver.org).
 
-### Unreleased
-
-- [added] Added feature to disable special NAT support
-- [changed] Improved port forwarding on quirky routers
-- [changed] Reduced peer timeout to 5min to work better with NAT
-
 ### v1.2.1 (2019-12-22)
 
 - [fixed] Fixed a problem with service restrictions

Cargo.toml

@@ -24,7 +24,7 @@ rand = "0.7"
 fnv = "1"
 net2 = "0.2"
 yaml-rust = "0.4"
-igd = { version = "0.9", optional = true }
+igd = "0.9"
 siphasher = "0.3"
 daemonize = "0.4"
 ring = "0.16"
@@ -38,9 +38,8 @@ pkg-config = "0.3"
 tempfile = "3"
 
 [features]
-default = ["nat"]
+default = []
 bench = []
-nat = ["igd"]
 
 [profile.release]
 lto = true

assets/example.net.disabled

@@ -21,7 +21,7 @@
 
 # Peer timeout in seconds. The peers will exchange information periodically
 # and drop peers that are silent for this period of time.
-#peer_timeout: 600
+#peer_timeout: 1800
 
 # Switch table entry timeout in seconds. This parameter is only used in switch
 # mode. Addresses that have not been seen for the given period of time  will

src/cloud.rs

@@ -19,7 +19,7 @@ use rand::{prelude::*, random, thread_rng};
 
 use super::{
     beacon::BeaconSerializer,
-    config::{Config, DEFAULT_PEER_TIMEOUT},
+    config::Config,
     crypto::Crypto,
     device::Device,
     net::Socket,
@@ -86,7 +86,7 @@ impl<TS: TimeSource> PeerList<TS> {
     }
 
     pub fn min_peer_timeout(&self) -> u16 {
-        self.peers.iter().map(|p| p.1.peer_timeout).min().unwrap_or(DEFAULT_PEER_TIMEOUT)
+        self.peers.iter().map(|p| p.1.peer_timeout).min().unwrap_or(1800)
     }
 
     #[inline]
@@ -257,7 +257,12 @@ impl<D: Device, P: Protocol, T: Table, S: Socket, TS: TimeSource> GenericCloud<D
             Err(err) => fail!("Failed to open ipv6 address ::{}: {}", config.port, err)
         };
         let now = TS::now();
-        let update_freq = config.get_keepalive() as u16;
+        let update_freq = if socket4.detect_nat() && config.get_keepalive() > 120 {
+            info!("Private IP detected, setting keepalive interval to 120s");
+            120
+        } else {
+            config.get_keepalive() as u16
+        };
         let mut res = GenericCloud {
             magic: config.get_magic(),
             node_id: random(),

src/config.rs

@@ -16,7 +16,6 @@ use std::hash::{Hash, Hasher};
 
 
 const HASH_PREFIX: &str = "hash:";
-pub const DEFAULT_PEER_TIMEOUT: u16 = 600;
 
 
 #[derive(Deserialize, Debug, PartialEq, Clone)]
@@ -60,7 +59,7 @@ impl Default for Config {
             magic: None,
             port: 3210,
             peers: vec![],
-            peer_timeout: DEFAULT_PEER_TIMEOUT as Duration,
+            peer_timeout: 1800,
             keepalive: None,
             beacon_store: None,
             beacon_load: None,
@@ -297,7 +296,7 @@ port: 3210
 peers:
   - remote.machine.foo:3210
   - remote.machine.bar:3210
-peer_timeout: 600
+peer_timeout: 1800
 keepalive: 840
 dst_timeout: 300
 beacon_store: /run/vpncloud.beacon.out
@@ -323,7 +322,7 @@ stats_file: /var/log/vpncloud.stats
         magic: Some("0123ABCD".to_string()),
         port: Some(3210),
         peers: Some(vec!["remote.machine.foo:3210".to_string(), "remote.machine.bar:3210".to_string()]),
-        peer_timeout: Some(600),
+        peer_timeout: Some(1800),
         keepalive: Some(840),
         beacon_store: Some("/run/vpncloud.beacon.out".to_string()),
         beacon_load: Some("/run/vpncloud.beacon.in".to_string()),
@@ -353,7 +352,7 @@ fn config_merge() {
         magic: Some("0123ABCD".to_string()),
         port: Some(3210),
         peers: Some(vec!["remote.machine.foo:3210".to_string(), "remote.machine.bar:3210".to_string()]),
-        peer_timeout: Some(600),
+        peer_timeout: Some(1800),
         keepalive: Some(840),
         beacon_store: Some("/run/vpncloud.beacon.out".to_string()),
         beacon_load: Some("/run/vpncloud.beacon.in".to_string()),
@@ -378,7 +377,7 @@ fn config_merge() {
         shared_key: Some("mysecret".to_string()),
         port: 3210,
         peers: vec!["remote.machine.foo:3210".to_string(), "remote.machine.bar:3210".to_string()],
-        peer_timeout: 600,
+        peer_timeout: 1800,
         keepalive: Some(840),
         dst_timeout: 300,
         beacon_store: Some("/run/vpncloud.beacon.out".to_string()),

src/net.rs

@@ -6,7 +6,7 @@ use std::{
     sync::atomic::{AtomicBool, Ordering}
 };
 
-use super::util::{MockTimeSource, Time, TimeSource};
+use super::util::{get_internal_ip, MockTimeSource, Time, TimeSource};
 
 use net2::UdpBuilder;
 
@@ -17,6 +17,7 @@ pub trait Socket: AsRawFd + Sized {
     fn receive(&mut self, buffer: &mut [u8]) -> Result<(usize, SocketAddr), io::Error>;
     fn send(&mut self, data: &[u8], addr: SocketAddr) -> Result<usize, io::Error>;
     fn address(&self) -> Result<SocketAddr, io::Error>;
+    fn detect_nat(&self) -> bool;
 }
 
 impl Socket for UdpSocket {
@@ -39,6 +40,9 @@ impl Socket for UdpSocket {
     fn address(&self) -> Result<SocketAddr, io::Error> {
         self.local_addr()
     }
+    fn detect_nat(&self) -> bool {
+        get_internal_ip().is_private()
+    }
 }
 
 thread_local! {
@@ -125,4 +129,7 @@ impl Socket for MockSocket {
     fn address(&self) -> Result<SocketAddr, io::Error> {
         Ok(self.address)
     }
+    fn detect_nat(&self) -> bool {
+        self.nat
+    }
 }

src/port_forwarding.rs

@@ -2,27 +2,24 @@
 // Copyright (C) 2015-2019  Dennis Schwerdel
 // This software is licensed under GPL-3 or newer (see LICENSE.md)
 
-#[cfg(feature = "nat")]
-mod internal {
+use std::{io, net::SocketAddrV4};
 
-    use std::{io, net::SocketAddrV4};
+use igd::*;
 
-    use igd::{search_gateway, AddAnyPortError, AddPortError, Gateway, PortMappingProtocol, SearchError};
+use super::util::{get_internal_ip, SystemTimeSource, Time, TimeSource};
 
-    use crate::util::{get_internal_ip, SystemTimeSource, Time, TimeSource};
+const LEASE_TIME: u32 = 1800;
+const DESCRIPTION: &str = "VpnCloud";
 
-    const LEASE_TIME: u32 = 1800;
 
-    const DESCRIPTION: &str = "VpnCloud";
-
-    pub struct PortForwarding {
+pub struct PortForwarding {
     pub internal_addr: SocketAddrV4,
     pub external_addr: SocketAddrV4,
-        gateway: Gateway,
+    pub gateway: Gateway,
     pub next_extension: Option<Time>
-    }
+}
 
-    impl PortForwarding {
+impl PortForwarding {
     pub fn new(port: u16) -> Option<Self> {
         // Get the gateway
         let gateway = match search_gateway(Default::default()) {
@@ -49,75 +46,54 @@ mod internal {
                 return None
             }
         };
-            if let Ok((port, timeout)) = Self::get_any_forwarding(&gateway, internal_addr, port) {
+        // Try to activate the port forwarding
+        // - First with external port = internal port and timeout
+        // - If the port is used, request any port
+        // - If timeout is denied, try permanent forwarding
         info!("Port-forwarding: external IP is {}", external_ip);
-                let external_addr = SocketAddrV4::new(external_ip, port);
-                info!("Port-forwarding: sucessfully activated port forward on {}, timeout: {}", external_addr, timeout);
-                let next_extension =
-                    if timeout > 0 { Some(SystemTimeSource::now() + Time::from(timeout) - 60) } else { None };
-                Some(PortForwarding { internal_addr, external_addr, gateway, next_extension })
-            } else {
-                None
-            }
-        }
-
-        fn get_any_forwarding(gateway: &Gateway, addr: SocketAddrV4, port: u16) -> Result<(u16, u32), ()> {
-            if let Ok(a) = Self::get_forwarding(gateway, addr, port) {
-                return Ok(a)
-            }
-            if let Ok(a) = Self::get_forwarding(gateway, addr, 0) {
-                return Ok(a)
-            }
-            for i in 1..5 {
-                if let Ok(a) = Self::get_forwarding(gateway, addr, port + i) {
-                    return Ok(a)
-                }
-            }
-            for _ in 0..5 {
-                if let Ok(a) = Self::get_forwarding(gateway, addr, rand::random()) {
-                    return Ok(a)
-                }
-            }
-            Err(())
-        }
-
-        fn get_forwarding(gateway: &Gateway, addr: SocketAddrV4, port: u16) -> Result<(u16, u32), ()> {
-            info!("Trying external port {}", port);
-            if port == 0 {
-                match gateway.add_any_port(PortMappingProtocol::UDP, addr, LEASE_TIME, DESCRIPTION) {
-                    Ok(port) => Ok((port, LEASE_TIME)),
+        let (external_addr, timeout) = match gateway.add_port(
+            PortMappingProtocol::UDP,
+            internal_addr.port(),
+            internal_addr,
+            LEASE_TIME,
+            DESCRIPTION
+        ) {
+            Ok(()) => (SocketAddrV4::new(external_ip, internal_addr.port()), LEASE_TIME),
+            Err(AddPortError::PortInUse) => {
+                match gateway.add_any_port(PortMappingProtocol::UDP, internal_addr, LEASE_TIME, DESCRIPTION) {
+                    Ok(port) => (SocketAddrV4::new(external_ip, port), LEASE_TIME),
                     Err(AddAnyPortError::OnlyPermanentLeasesSupported) => {
-                        match gateway.add_any_port(PortMappingProtocol::UDP, addr, 0, DESCRIPTION) {
-                            Ok(port) => Ok((port, 0)),
+                        match gateway.add_any_port(PortMappingProtocol::UDP, internal_addr, 0, DESCRIPTION) {
+                            Ok(port) => (SocketAddrV4::new(external_ip, port), 0),
                             Err(err) => {
                                 error!("Port-forwarding: failed to activate port forwarding: {}", err);
-                                Err(())
+                                return None
                             }
                         }
                     }
                     Err(err) => {
                         error!("Port-forwarding: failed to activate port forwarding: {}", err);
-                        Err(())
+                        return None
+                    }
                 }
             }
-            } else {
-                match gateway.add_port(PortMappingProtocol::UDP, port, addr, LEASE_TIME, DESCRIPTION) {
-                    Ok(()) => Ok((port, LEASE_TIME)),
             Err(AddPortError::OnlyPermanentLeasesSupported) => {
-                        match gateway.add_port(PortMappingProtocol::UDP, port, addr, 0, DESCRIPTION) {
-                            Ok(()) => Ok((port, 0)),
+                match gateway.add_port(PortMappingProtocol::UDP, internal_addr.port(), internal_addr, 0, DESCRIPTION) {
+                    Ok(()) => (SocketAddrV4::new(external_ip, internal_addr.port()), 0),
                     Err(err) => {
                         error!("Port-forwarding: failed to activate port forwarding: {}", err);
-                                Err(())
+                        return None
                     }
                 }
             }
             Err(err) => {
                 error!("Port-forwarding: failed to activate port forwarding: {}", err);
-                        Err(())
-                    }
-                }
+                return None
             }
+        };
+        info!("Port-forwarding: sucessfully activated port forward on {}, timeout: {}", external_addr, timeout);
+        let next_extension = if timeout > 0 { Some(SystemTimeSource::now() + Time::from(timeout) - 60) } else { None };
+        Some(PortForwarding { internal_addr, external_addr, gateway, next_extension })
     }
 
     pub fn check_extend(&mut self) {
@@ -147,29 +123,10 @@ mod internal {
             Err(err) => error!("Port-forwarding: failed to deactivate port forwarding: {}", err)
         }
     }
-    }
+}
 
-    impl Drop for PortForwarding {
+impl Drop for PortForwarding {
     fn drop(&mut self) {
         self.deactivate()
     }
-    }
 }
-
-#[cfg(not(feature = "nat"))]
-mod internal {
-    pub struct PortForwarding;
-
-    impl PortForwarding {
-        pub fn new(_port: u16) -> Option<Self> {
-            warn!("Compiled without feature 'nat', skipping port forwarding.");
-            None
-        }
-
-        pub fn check_extend(&mut self) {
-            unreachable!()
-        }
-    }
-}
-
-pub use internal::*;

src/tests/peers.rs

@@ -18,24 +18,24 @@ fn connect_v4() {
     node1.connect("2.3.4.5:6789").unwrap();
 
     // Node 1 -> Node 2: Init 0
-    assert_message4!(node1, node1_addr, node2, node2_addr, Message::Init(0, node1.node_id(), vec![], 600));
+    assert_message4!(node1, node1_addr, node2, node2_addr, Message::Init(0, node1.node_id(), vec![], 1800));
     assert_clean!(node1);
     assert!(node2.peers().contains_node(&node1.node_id()));
 
     // Node 2 -> Node 1: Init 1 | Node 2 -> Node 1: Peers
-    assert_message4!(node2, node2_addr, node1, node1_addr, Message::Init(1, node2.node_id(), vec![], 600));
+    assert_message4!(node2, node2_addr, node1, node1_addr, Message::Init(1, node2.node_id(), vec![], 1800));
     assert!(node1.peers().contains_node(&node2.node_id()));
     assert_message4!(node2, node2_addr, node1, node1_addr, Message::Peers(vec![node1_addr]));
     assert_clean!(node2);
 
     // Node 1 -> Node 2: Peers | Node 1 -> Node 1: Init 0
     assert_message4!(node1, node1_addr, node2, node2_addr, Message::Peers(vec![node2_addr]));
-    assert_message4!(node1, node1_addr, node1, node1_addr, Message::Init(0, node1.node_id(), vec![], 600));
+    assert_message4!(node1, node1_addr, node1, node1_addr, Message::Init(0, node1.node_id(), vec![], 1800));
     assert!(node1.own_addresses().contains(&node1_addr));
     assert_clean!(node1);
 
     // Node 2 -> Node 2: Init 0
-    assert_message4!(node2, node2_addr, node2, node2_addr, Message::Init(0, node2.node_id(), vec![], 600));
+    assert_message4!(node2, node2_addr, node2, node2_addr, Message::Init(0, node2.node_id(), vec![], 1800));
     assert_clean!(node2);
     assert!(node2.own_addresses().contains(&node2_addr));
 
@@ -158,7 +158,7 @@ fn lost_init1() {
     node1.connect("2.3.4.5:6789").unwrap();
 
     // Node 1 -> Node 2: Init 0
-    assert_message4!(node1, node1_addr, node2, node2_addr, Message::Init(0, node1.node_id(), vec![], 600));
+    assert_message4!(node1, node1_addr, node2, node2_addr, Message::Init(0, node1.node_id(), vec![], 1800));
     assert_clean!(node1);
 
     // Node 2 -> Node 1: Init 1 | Node 2 -> Node 1: Peers
@@ -179,7 +179,7 @@ fn wrong_magic() {
     let node2_addr = addr!("2.3.4.5:6789");
     node1.connect("2.3.4.5:6789").unwrap();
 
-    assert_message4!(node1, node1_addr, node2, node2_addr, Message::Init(0, node1.node_id(), vec![], 600));
+    assert_message4!(node1, node1_addr, node2, node2_addr, Message::Init(0, node1.node_id(), vec![], 1800));
 
     assert_clean!(node1, node2);
 

src/udpmessage.rs

@@ -8,7 +8,6 @@ use std::{
 };
 
 use super::{
-    config::DEFAULT_PEER_TIMEOUT,
     crypto::Crypto,
     types::{Error, HeaderMagic, NodeId, Range, NODE_ID_BYTES},
     util::{bytes_to_hex, Encoder}
@@ -191,8 +190,7 @@ pub fn decode<'a>(data: &'a mut [u8], magic: HeaderMagic, crypto: &Crypto) -> Re
                 pos += read;
                 addrs.push(range);
             }
-            let peer_timeout =
-                if data.len() >= pos + 2 { Encoder::read_u16(&data[pos..]) } else { DEFAULT_PEER_TIMEOUT };
+            let peer_timeout = if data.len() >= pos + 2 { Encoder::read_u16(&data[pos..]) } else { 1800 };
             Message::Init(stage, node_id, addrs, peer_timeout)
         }
         3 => Message::Close,

vpncloud.md

@@ -78,7 +78,7 @@ vpncloud(1) -- Peer-to-peer VPN
   * `--peer-timeout <secs>`:
 
     Peer timeout in seconds. The peers will exchange information periodically
-    and drop peers that are silent for this period of time. [default: `600`]
+    and drop peers that are silent for this period of time. [default: `1800`]
 
   * `--keepalive <secs>`:
 
@@ -338,7 +338,7 @@ port: 3210
 peers:
   - remote.machine.foo:3210
   - remote.machine.bar:3210
-peer_timeout: 600
+peer_timeout: 1800
 mode: normal
 subnets:
   - 10.0.1.0/24