From bb8cee82f98b0238844a968ed19da8aa32de5785 Mon Sep 17 00:00:00 2001
From: Stephen Akinyemi <appcypher@outlook.com>
Date: Tue, 1 Apr 2025 22:30:40 +0100
Subject: [PATCH] feat(vsock): make subnet optional for Group scope IP
 filtering

When using scope 1 (Group) for IP filtering, subnet specification is now
optional. If no subnet is provided, all connections will be blocked,
matching scope 0 behavior. This provides more flexibility in network
configuration while maintaining security.

- Update IpFilterConfig to make subnet optional for scope 1
- Modify is_valid() to accept scope 1 without subnet
- Update documentation in libkrun.h to clarify scope behaviors
- Improve warning message specificity in VsockMuxer creation
---
 include/libkrun.h                         |  7 ++++++-
 src/devices/src/virtio/vsock/ip_filter.rs | 14 ++++++++------
 src/devices/src/virtio/vsock/muxer.rs     |  2 +-
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/include/libkrun.h b/include/libkrun.h
index a03745183..ffc1c45e7 100644
--- a/include/libkrun.h
+++ b/include/libkrun.h
@@ -325,7 +325,12 @@ int32_t krun_set_port_map(uint32_t ctx_id, const char *const port_map[]);
  *  "ctx_id"   - the configuration context ID.
  *  "c_ip"     - an optional null-terminated string representing the guest's static IPv4 address.
  *  "c_subnet" - an optional null-terminated string representing the guest's subnet in CIDR notation (e.g., "192.168.1.0/24").
- *  "scope"    - an integer specifying the scope (0-3). Refer to TSI documentation for details.
+ *               If scope is 1 and subnet is not provided, all connections will be blocked.
+ *  "scope"    - an integer specifying the scope (0-3):
+ *               0: None - Block all IP communication
+ *               1: Group - Allow within subnet (if specified; otherwise, block all like scope 0)
+ *               2: Public - Allow public IPs
+ *               3: Any - Allow any IP
  *
  * Returns:
  *  Zero on success or a negative error number on failure.
diff --git a/src/devices/src/virtio/vsock/ip_filter.rs b/src/devices/src/virtio/vsock/ip_filter.rs
index 6eed6382c..4fb81fe18 100644
--- a/src/devices/src/virtio/vsock/ip_filter.rs
+++ b/src/devices/src/virtio/vsock/ip_filter.rs
@@ -10,7 +10,7 @@ use std::net::Ipv4Addr;
 pub struct IpFilterConfig {
     /// Defines the scope of allowed connections/bindings.
     /// 0: None (Block all IP communication)
-    /// 1: Group (Allow within `subnet`, bind only to `ip` if specified)
+    /// 1: Group (Allow within `subnet` if specified, otherwise behaves like scope 0)
     /// 2: Public (Allow public IPs, bind only to `ip` if specified)
     /// 3: Any (Allow any IP, bind only to `ip` if specified)
     pub scope: u8,
@@ -19,7 +19,8 @@ pub struct IpFilterConfig {
     /// (ignored if scope is 0).
     pub ip: Option<Ipv4Addr>,
 
-    /// The allowed subnet for Scope 1 (Group). Required if scope is 1.
+    /// The allowed subnet for Scope 1 (Group). Optional - if not provided when scope is 1,
+    /// all connections will be blocked (same as scope 0).
     pub subnet: Option<Ipv4Network>,
 }
 
@@ -31,9 +32,8 @@ impl IpFilterConfig {
     /// Checks if the configuration is logically valid.
     pub fn is_valid(&self) -> bool {
         match self.scope {
-            0 | 2 | 3 => true, // Scopes 0, 2, 3 are valid without extra checks (ip is optional)
-            1 => self.subnet.is_some(), // Scope 1 requires a subnet
-            _ => false, // Invalid scope number
+            0 | 1 | 2 | 3 => true, // All valid scopes (subnet is optional for scope 1)
+            _ => false,            // Invalid scope number
         }
     }
 
@@ -57,13 +57,14 @@ impl IpFilterConfig {
             0 => false, // Scope 0: Deny all connections
             1 => {
                 // Scope 1: Group - Allow connection only if dest_ip is within the specified subnet
+                // If no subnet is specified, behaves like scope 0 (deny all)
                 self.subnet.map_or(false, |subnet| subnet.contains(dest_ip))
             }
             2 => {
                 // Scope 2: Public - Allow connection only if dest_ip is NOT private
                 !Self::is_private(dest_ip)
             }
-            3 => true, // Scope 3: Any - Allow connection to any IP
+            3 => true,  // Scope 3: Any - Allow connection to any IP
             _ => false, // Invalid scope
         }
     }
@@ -82,6 +83,7 @@ impl IpFilterConfig {
         // No specific IP specified, check based on scope rules for the bind_ip itself
         match self.scope {
             // Scope 1: Group - Allow binding within the subnet if no specific IP given
+            // If no subnet is specified, behaves like scope 0 (deny all)
             1 => self.subnet.map_or(false, |subnet| subnet.contains(bind_ip)),
             // Scope 2: Public - Allow binding to public IPs if no specific IP given
             2 => !Self::is_private(bind_ip),
diff --git a/src/devices/src/virtio/vsock/muxer.rs b/src/devices/src/virtio/vsock/muxer.rs
index d44a22db0..75e947bd8 100644
--- a/src/devices/src/virtio/vsock/muxer.rs
+++ b/src/devices/src/virtio/vsock/muxer.rs
@@ -127,7 +127,7 @@ impl VsockMuxer {
         ip_filter: IpFilterConfig,
     ) -> Self {
         if !ip_filter.is_valid() {
-            warn!("Invalid IpFilterConfig provided during VsockMuxer creation: {:?}. Check configuration.", ip_filter);
+            warn!("Invalid IpFilterConfig provided during VsockMuxer creation: {:?}. Scope value must be between 0 and 3.", ip_filter);
         }
 
         VsockMuxer {