Socket syscall (#247)

* example_PR

* formating unit tests for socket_syscall

Signed-off-by: Yashaswi Makula <yashaswi.makula@gmail.com>

* updating socket syscall tests and adding error checking for socket syscall

Signed-off-by: Yashaswi Makula <yashaswi.makula@gmail.com>

* updating comments and tests

Signed-off-by: Yashaswi Makula <yashaswi.makula@gmail.com>

* update comments

* update blocking comment

* update the comments

* update panics

---------

Signed-off-by: Yashaswi Makula <yashaswi.makula@gmail.com>
Co-authored-by: Yashaswi Makula <yashaswi.makula@gmail.com>

Author: 2 people

src/safeposix/syscalls/net_calls.rs

@@ -152,12 +152,54 @@ impl Cage {
         0
     }
 
+    /// ## `socket_syscall`
+    ///
+    /// ### Description
+    /// This function creates a new socket, ensuring the requested domain, socket type, 
+    /// and protocol are supported by SafePosix. 
+    /// It validates the requested communication domain, socket type, and protocol, permitting only combinations that are known 
+    /// to be safe and secure.
+    ///
+    /// ### Function Arguments
+    /// * `domain`: The communication domain for the socket. Supported values are 
+    ///   `PF_INET` (Internet Protocol) and `PF_UNIX` (Unix domain sockets).
+    /// * `socktype`: The socket type. Supported values are `SOCK_STREAM` (stream sockets) and `SOCK_DGRAM` (datagram sockets).
+    /// * `protocol`: The protocol to use for communication. This defaults to TCP for stream sockets 
+    ///   (`SOCK_STREAM`) and UDP for datagram sockets (`SOCK_DGRAM`).
+    ///
+    /// ### Returns
+    /// * The new file descriptor representing the socket on success.
+    ///
+    /// ### Errors
+    /// * `EOPNOTSUPP(95)`: If an unsupported combination of domain, socket type, or protocol is requested.
+    /// * `EINVAL(22)`: If an invalid combination of flags is provided.
+    /// ### Panics
+    /// There are no panics in this syscall.
     pub fn socket_syscall(&self, domain: i32, socktype: i32, protocol: i32) -> i32 {
         let real_socktype = socktype & 0x7; //get the type without the extra flags, it's stored in the last 3 bits
-        let nonblocking = (socktype & SOCK_NONBLOCK) != 0;
+        let nonblocking = (socktype & SOCK_NONBLOCK) != 0; // Checks if the socket should be non-blocking.
+        //Check blocking status for storage in the file descriptor, we'll need this for calls that don't access the kernel 
+        //socket, unix sockets, and properly directing kernel calls for recv and accept
         let cloexec = (socktype & SOCK_CLOEXEC) != 0;
-
-        match real_socktype {
+        // Checks if the 'close-on-exec' flag is set. This flag ensures the socket is automatically closed if the current
+        // process executes another program, preventing unintended inheritance of the socket by the new program.
+       
+        // additional flags are not supported
+        // filtering out any socktypes with unexpected flags set.
+        // This is important as we dont want to pass down any flags that are not supported by SafePOSIX.
+        // which may potentially cause issues with the underlying libc call. or the socket creation process.
+        // leading to unexpected behavior.
+        if socktype & !(SOCK_NONBLOCK | SOCK_CLOEXEC | 0x7) != 0 {
+            return syscall_error(
+                Errno::EOPNOTSUPP,
+                "socket",
+                "Invalid combination of flags"
+            );
+        }
+        //SafePOSIX intentionally supports only a restricted subset of socket types . This is to make sure that 
+        // applications not creating other socket types which may lead to security issues. 
+        //By using the match statement, SafePOSIX ensures that only these approved socket types are allowed.
+        match real_socktype {// Handles different socket types SOCK_STREAM or SOCK_DGRAM in this cases
             SOCK_STREAM => {
                 //SOCK_STREAM defaults to TCP for protocol, otherwise protocol is unsupported
                 let newprotocol = if protocol == 0 { IPPROTO_TCP } else { protocol };
@@ -169,8 +211,10 @@ impl Cage {
                         "The only SOCK_STREAM implemented is TCP. Unknown protocol input.",
                     );
                 }
-                match domain {
-                    PF_INET | PF_UNIX => {
+                match domain {// Handles different communication domains in this case PF_INET/PF_UNIX 
+                    PF_INET | PF_UNIX => {// Internet Protocol (PF_INET) and Unix Domain Sockets (PF_UNIX)
+                        //PR_INET / AF_INET and PF_UNIX / AF_UNIX are the same
+                        //https://man7.org/linux/man-pages/man2/socket.2.html
                         let sockfdobj = self._socket_initializer(
                             domain,
                             socktype,
@@ -179,14 +223,18 @@ impl Cage {
                             cloexec,
                             ConnState::NOTCONNECTED,
                         );
+                        // Creates a SafePOSIX socket descriptor using '_socket_initializer', a helper function 
+                        // that encapsulates the internal details of socket creation and initialization.
                         return self._socket_inserter(Socket(sockfdobj));
+                        // Inserts the newly created socket descriptor into the cage's file descriptor table,
+                        // making it accessible to the application.Returns the file descriptor representing the socket.
                     }
                     _ => {
                         return syscall_error(
                             Errno::EOPNOTSUPP,
                             "socket",
                             "trying to use an unimplemented domain",
-                        );
+                        );// Returns an error if an unsupported domain is requested.
                     }
                 }
             }
@@ -202,8 +250,13 @@ impl Cage {
                         "The only SOCK_DGRAM implemented is UDP. Unknown protocol input.",
                     );
                 }
-                match domain {
-                    PF_INET | PF_UNIX => {
+                // SafePOSIX intentionally supports only a restricted subset of socket types . This is to make sure
+                // that applications not creating other socket types which may lead to security issues. 
+                //By using the match statement,  SafePOSIX ensures that only these approved socket types are allowed.
+                match domain {// Handles different communication domains in this case PF_INET/PF_UNIX 
+                    PF_INET | PF_UNIX => {// Internet Protocol (PF_INET) and Unix Domain Sockets (PF_UNIX)
+                        //PR_INET / AF_INET and PF_UNIX / AF_UNIX are the same 
+                        //https://man7.org/linux/man-pages/man2/socket.2.html
                         let sockfdobj = self._socket_initializer(
                             domain,
                             socktype,
@@ -212,7 +265,11 @@ impl Cage {
                             cloexec,
                             ConnState::NOTCONNECTED,
                         );
+                        // Creates a SafePOSIX socket descriptor using '_socket_initializer', a helper 
+                        // function that encapsulates the internal details of socket creation and initialization.
                         return self._socket_inserter(Socket(sockfdobj));
+                        // Inserts the newly created socket descriptor into the cage's file descriptor table,making it accessible to the application. 
+                        // Returns the file descriptor (an integer) representing the socket.
                     }
                     _ => {
                         return syscall_error(
@@ -229,7 +286,7 @@ impl Cage {
                     Errno::EOPNOTSUPP,
                     "socket",
                     "trying to use an unimplemented socket type",
-                );
+                );// Returns an error if an unsupported domain is requested.
             }
         }
     }

src/tests/networking_tests.rs

@@ -1921,27 +1921,78 @@ pub mod net_tests {
 
         let cage = interface::cagetable_getref(1);
 
+        // Following checks are inplace to ensure that the socket types are correctly defined
+        // and that the assumptions about the values of the socket types are correct across platforms.
+        // Check that SOCK_STREAM only uses the lowest 3 bits
+        assert_eq!(SOCK_STREAM & !0x7, 0);
+
+        // Check that SOCK_DGRAM only uses the lowest 3 bits
+        assert_eq!(SOCK_DGRAM & !0x7, 0);
+
+        // Check that SOCK_NONBLOCK does not use the lowest 3 bits
+        assert_eq!(SOCK_NONBLOCK & 0x7, 0);
+
+        // Check that SOCK_CLOEXEC does not use the lowest 3 bits
+        assert_eq!(SOCK_CLOEXEC & 0x7, 0);
+
+        //let's check an illegal operation...
+        // RDM is not a valid socket type for SOCK_DGRAM (UDP) as its not implemented yet.
+        let sockfd5 = cage.socket_syscall(AF_INET, SOCK_RDM, 0);
+        assert!(sockfd5 < 0, "Expected an error, got a valid file descriptor");
+
+        //let's check an illegal operation...
+        //invalid protocol for SOCK_STREAM Type.
+        let sockfd6 = cage.socket_syscall(AF_INET, SOCK_STREAM, 999);
+        assert!(sockfd6 < 0, "Expected an error, got a valid file descriptor");
+
+        //let's check an illegal operation...
+        //invalid domain for socket
+        let sockfd7 = cage.socket_syscall(999, SOCK_STREAM, 0);
+        assert!(sockfd7 < 0, "Expected an error, got a valid file descriptor");
+
+        //let's check an illegal operation...
+        //invalid socket type flags combination
+        let sockfd8 = cage.socket_syscall(AF_INET, SOCK_STREAM | 0x100000, 0);
+        assert!(sockfd8 < 0, "Expected an error, got a valid file descriptor");
+
+        //let's check an illegal operation...
+        //invalid socket type protocol combination
+        let sockfd9 = cage.socket_syscall(AF_INET, SOCK_STREAM, IPPROTO_UDP);
+        assert!(sockfd9 < 0, "Expected an error, got a valid file descriptor");
+
+        //let's check an illegal operation...
+        //invalid socket type protocol combination
+        let sockfd10 = cage.socket_syscall(AF_INET, SOCK_DGRAM, IPPROTO_TCP);
+        assert!(sockfd10 < 0, "Expected an error, got a valid file descriptor");
+
+
         let mut sockfd = cage.socket_syscall(AF_INET, SOCK_STREAM, 0);
+        assert!(sockfd > 0, "Expected a valid file descriptor, got error");
+
         let sockfd2 = cage.socket_syscall(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+        assert!(sockfd2 > 0, "Expected a valid file descriptor, got error");
 
         let sockfd3 = cage.socket_syscall(AF_INET, SOCK_DGRAM, 0);
+        assert!(sockfd3 > 0, "Expected a valid file descriptor, got error");
+        
         let sockfd4 = cage.socket_syscall(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
+        assert!(sockfd4 > 0, "Expected a valid file descriptor, got error");
+
+
 
-        //checking that the fd's are correct
-        assert!(sockfd > 0);
-        assert!(sockfd2 > 0);
-        assert!(sockfd3 > 0);
-        assert!(sockfd4 > 0);
 
         //let's check an illegal operation...
+        // let sockfd7 = cage.socket_syscall(AF_INET, !0b111 | 0b001, 0);
+        // assert!(sockfd7 < 0, "Expected an error, got a valid file descriptor");
+
         let sockfddomain = cage.socket_syscall(AF_UNIX, SOCK_DGRAM, 0);
-        assert!(sockfddomain > 0);
+        assert!(sockfddomain > 0, "Expected a valid file descriptor, got error");
 
         sockfd = cage.socket_syscall(AF_INET, SOCK_STREAM, 0);
-        assert!(sockfd > 0);
+        assert!(sockfd > 0, "Expected a valid file descriptor, got error");
 
-        assert_eq!(cage.close_syscall(sockfd), 0);
-        assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS);
+        assert_eq!(cage.close_syscall(sockfd), 0, "Expected successful close, got error");
+        assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS, "Expected successful exit, got error");
         lindrustfinalize();
     }