Punchnet (sdlan-lib-rs) Code Review Report

This report provides a detailed, comprehensive review of the punchnet SD-LAN (Software-Defined LAN / VPN) client repository. It covers the overall architecture, highlights key design strengths, details security and code quality risks, and suggests practical refactoring opportunities.

1. Repository Overview & Architecture

punchnet is a virtual networking client written in Rust designed to establish secure, peer-to-peer Software-Defined LAN connections.

The architecture consists of two main planes:

Control Plane (QUIC): Communicates with a centralized supernode over QUIC (via quinn) to handle registration (RegisterSuper), policy exchange, peer discovery, and NAT type discovery.
Data Plane (UDP): Encrypted virtual network traffic is sent directly between peer nodes using UDP. It performs NAT traversal (using STUN-like hole punching) to establish direct P2P connections, falling back to relaying via the supernode if direct routing fails.

Core Components

graph TD
    A[OS Virtual NIC: TUN/TAP] <-->|Read/Write L3/L2 Packets| B[Packet Routing Engine: packet.rs]
    B <-->|Encrypt/Decrypt: AES or ChaCha20| C[UDP Socket Engine: socks.rs]
    D[Control Plane: quic.rs] -->|Session Keys / ACL Rules| B
    D <-->|QUIC Protocol| E[Supernode Server]
    C <-->|P2P Encrypted Data| F[Remote Peers]
    B -->|Local Domain Filter| G[DNS Hijacking Proxy: dns.rs]
    G <-->|Proxy Queries| H[AliDNS 223.5.5.5 / Supernode DNS]

Platform-specific NIC Adapters (src/network/tun_win.rs & src/network/tun_linux.rs): Handles raw packet read/write. Windows implements L3 TUN using the Wintun driver (relying on wintun.dll), simulating Ethernet frames (L2) locally. Linux binds directly to /dev/net/tun via ioctl (supported by helper C-code tuntap.c).
Routing & Encapsulation Engine (src/network/packet.rs & src/network/route.rs): Identifies packet destination MACs and resolves them to peer IP/ports using a custom ARP table. Packs IP/Ethernet payloads into protobuf messages (SdlData), encrypts them, and forwards them. Uses a custom lock-free IP Trie (src/utils/system_action.rs) for prefix route resolution.
DNS Proxy Engine (src/utils/dns.rs): Automatically intercepts DNS queries. Queries for private virtual domains (e.g. *.network_domain) are sent to the supernode's DNS server (server_ip:15353), while other public queries are proxied to AliDNS (223.5.5.5:53) over dynamically rotated UDP sockets.

2. Key Design Highlights (Strengths)

Lock-Free Concurrent Routing Lookups: Outbound lookups on the routing table (RouteTable2) happen on every single packet. The project handles this very efficiently by implementing an IpTrie wrapped in an ArcSwap. Write operations (inserting or clearing routes) clone the trie and perform an atomic pointer swap, enabling read threads to query routes concurrently without locking.
Dynamic DNS Socket Rotation: To prevent firewalls or stateful NAT routers from blocking persistent DNS proxy connections, DynamicDNSClient rotates its local UDP socket either every 60 seconds or after 100 queries. It spawns a new receiver thread on a fresh port and allows the old socket to time out and close gracefully, which is a highly resilient approach to DNS proxying.
Clean Separation of Control and Data Planes: Using QUIC for signaling and standard UDP for data plane routing provides a robust control connection (resilient to head-of-line blocking and network changes) while keeping the overhead of data forwarding as low as possible.

3. Security Vulnerabilities & Risks

Caution

1. ACL/Firewall Rules Completely Bypassed

In acl_session.rs, the core firewall verification method is_identity_ok unconditionally returns (true, false):
pub fn is_identity_ok(&self, allow_routing: bool, identity: IdentityID, info: FiveTuple) -> (bool, ShouldRenew) {
    if allow_routing {
        return (true, false);
    }
    return (true, false);

    error!("is identity ok? {:?}", info);
    // ... remaining validation logic is dead code ...
}
Risk: Any peer on the SD-LAN network can send traffic to any port of the client, bypassing all security group policies pushed from the supernode server.

Warning

2. Hardcoded Signature Key for HTTP API

In api/mod.rs, the signing key for REST APIs is hardcoded:
const DIGEST_KEY: &'static str = "H6p*2RfEu4ITcL";
Risk: This key is used to sign requests sent to the central controller via HMAC-MD5 (sent as X-sign). Since the key is hardcoded in the client, an attacker could reverse-engineer the binary, retrieve the key, and forge authenticated API requests.

Warning

3. Weak Directory Permissions for Local Credentials

In node.rs and main.rs, the client directory .client is created inside the base directory to hold the generated RSA key pair (id_rsa/id_rsa.pub). The code does not restrict folder permissions on Unix. Risk: If the app is run as root (needed to configure TUN/TAP) and saves files in a shared base directory (e.g. /usr/local/punchnet), other non-root local users might be able to read the private key file id_rsa if the default umask is weak.

4. Code Quality & Potential Bugs

1. DNS Settings Corruption on Improper Exit

In main.rs, the code restores system DNS settings on ctrl_c by invoking restore_dns.

Bug: If the process crashes, panics, is killed via kill -9 (SIGKILL), or system shutdown happens without triggering the tokio runtime signal handler, the system's DNS settings (e.g., /etc/resolv.conf on Linux or static DNS addresses on Windows) will remain pointed to the hijacked address 100.100.100.100. This will break the host machine's internet connectivity until manually fixed.

2. PID File Race Conditions & False Positives

In main.rs, is_process_running() checks for the PID file /tmp/punchnet.pid and verifies whether that process ID exists in the system:

fn is_pid_running(pid: u32) -> bool {
    let mut sys = sysinfo::System::new_all();
    sys.refresh_all();
    sys.process(sysinfo::Pid::from_u32(pid)).is_some()
}

Bug: If punchnet crashes, the PID file is left over. If another completely unrelated application starts later and gets assigned the exact same process ID, punchnet will refuse to launch, erroneously reporting "process is running, aborting...". It is safer to verify if the process name matches "punchnet" or check the binary executable path.

3. Blocking Operations on Async Thread Pool

In async_main.rs and node.rs, there are multiple instances where filesystem operations (fs::read_to_string, fs::write) or platform calls (Command::new("netsh"), Command::new("route")) are executed synchronously within async tokio contexts instead of using tokio::fs or spawn_blocking.

Performance Impact: These blocking system operations can stall the Tokio worker threads, reducing throughput and leading to increased latency in packet forwarding.

5. Suggestions for Improvement & Refactoring

Fix the ACL Shortcut: Remove the early return (true, false) from is_identity_ok in src/utils/acl_session.rs and fully integrate the rules received from the server to secure the client against unauthorized peer access.
Move Secret Keys to Environment/Config: Remove DIGEST_KEY from source files. Instead, use a key derivation mechanism or load it from a secure local environment variable or configuration file.

Ensure Atomic & Safe File Permissions: When creating folders or writing key pairs on Unix, restrict the directory permissions explicitly to 0700 and files to 0600 using standard library extensions:

#[cfg(unix)]
{
    use std::os::unix::fs::DirBuilderExt;
    let mut builder = std::fs::DirBuilder::new();
    builder.mode(0o700);
    builder.create(rsa_path)?;
}

Use Subprocess Wrappers and Async Routines: Migrate blocking file I/O and process spawns to async counterparts (e.g. tokio::fs and tokio::process::Command) to prevent worker thread starvation.
Graceful Recovery of System DNS Settings: Consider writing a secondary daemon / sentinel wrapper, or register a shell-level script trap to ensure that system DNS settings are automatically restored even under fatal panics/crashes.

8.8 KiB Raw Blame History