From 4c91cf6bb8fa70b48d7cbd889c60f3bbdd3e6172 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20F=C3=A4rnstrand?= Date: Sun, 8 Nov 2020 22:04:56 +0100 Subject: [PATCH 1/2] Don't assume memory layout of std::net::SocketAddr --- src/sockaddr.rs | 119 +++++++++++++++++++++++++++++++++++---------- src/sys/unix.rs | 44 +++++++++-------- src/sys/windows.rs | 69 +++++++++++++++++--------- 3 files changed, 161 insertions(+), 71 deletions(-) diff --git a/src/sockaddr.rs b/src/sockaddr.rs index c1b8abdd..9eee70d8 100644 --- a/src/sockaddr.rs +++ b/src/sockaddr.rs @@ -3,9 +3,11 @@ use std::net::{SocketAddr, SocketAddrV4, SocketAddrV6}; use std::{fmt, ptr}; use crate::sys::{ - c_int, sa_family_t, sockaddr, sockaddr_in, sockaddr_in6, sockaddr_storage, socklen_t, AF_INET, + sa_family_t, sockaddr, sockaddr_in, sockaddr_in6, sockaddr_storage, socklen_t, AF_INET, AF_INET6, }; +#[cfg(windows)] +use winapi::shared::ws2ipdef::SOCKADDR_IN6_LH_u; /// The address of a socket. /// @@ -24,7 +26,7 @@ impl SockAddr { /// It is up to the user to ensure the `addr` pointer and `len` length are /// correct. pub unsafe fn from_raw_parts(addr: *const sockaddr, len: socklen_t) -> SockAddr { - let mut storage = MaybeUninit::::uninit(); + let mut storage = MaybeUninit::::zeroed(); ptr::copy_nonoverlapping( addr as *const _ as *const u8, storage.as_mut_ptr() as *mut u8, @@ -33,7 +35,7 @@ impl SockAddr { SockAddr { // This is safe as we written the address to `storage` above. storage: storage.assume_init(), - len: len, + len, } } @@ -55,30 +57,50 @@ impl SockAddr { /// Returns this address as a `SocketAddr` if it is in the `AF_INET` (IP v4) /// or `AF_INET6` (IP v6) family, otherwise returns `None`. pub fn as_std(&self) -> Option { - self.as_inet() - .map(|a| a.into()) - .or_else(|| self.as_inet6().map(|a| a.into())) + if self.storage.ss_family == AF_INET as sa_family_t { + // Safety: if the ss_family field is AF_INET then storage must be a sockaddr_in. + let addr = unsafe { &*(&self.storage as *const _ as *const sockaddr_in) }; + + let ip = crate::sys::from_in_addr(addr.sin_addr); + let port = u16::from_be(addr.sin_port); + Some(SocketAddr::V4(SocketAddrV4::new(ip, port))) + } else if self.storage.ss_family == AF_INET6 as sa_family_t { + // Safety: if the ss_family field is AF_INET6 then storage must be a sockaddr_in6. + let addr = unsafe { &*(&self.storage as *const _ as *const sockaddr_in6) }; + + let ip = crate::sys::from_in6_addr(addr.sin6_addr); + let port = u16::from_be(addr.sin6_port); + Some(SocketAddr::V6(SocketAddrV6::new( + ip, + port, + addr.sin6_flowinfo, + #[cfg(unix)] + addr.sin6_scope_id, + #[cfg(windows)] + unsafe { + *addr.u.sin6_scope_id() + }, + ))) + } else { + None + } } /// Returns this address as a `SocketAddrV4` if it is in the `AF_INET` /// family. pub fn as_inet(&self) -> Option { - if self.storage.ss_family as c_int == AF_INET { - let storage: *const sockaddr_storage = (&self.storage) as *const _; - Some(unsafe { *(storage as *const sockaddr_in as *const _) }) - } else { - None + match self.as_std() { + Some(SocketAddr::V4(addr)) => Some(addr), + _ => None, } } /// Returns this address as a `SocketAddrV6` if it is in the `AF_INET6` /// family. pub fn as_inet6(&self) -> Option { - if self.storage.ss_family as c_int == AF_INET6 { - let storage: *const sockaddr_storage = (&self.storage) as *const _; - Some(unsafe { *(storage as *const sockaddr_in6 as *const _) }) - } else { - None + match self.as_std() { + Some(SocketAddr::V6(addr)) => Some(addr), + _ => None, } } } @@ -94,22 +116,67 @@ impl From for SockAddr { impl From for SockAddr { fn from(addr: SocketAddrV4) -> SockAddr { - unsafe { - SockAddr::from_raw_parts( - &addr as *const _ as *const _, - mem::size_of::() as socklen_t, - ) + let sockaddr_in = sockaddr_in { + sin_family: AF_INET as sa_family_t, + sin_port: addr.port().to_be(), + sin_addr: crate::sys::to_in_addr(&addr.ip()), + sin_zero: [0; 8], + #[cfg(any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd" + ))] + sin_len: 0, + }; + let mut storage = MaybeUninit::::zeroed(); + // Safety: A `sockaddr_in` is memory compatible with a `sockaddr_storage` + unsafe { (storage.as_mut_ptr() as *mut sockaddr_in).write(sockaddr_in) }; + SockAddr { + storage: unsafe { storage.assume_init() }, + len: mem::size_of::() as socklen_t, } } } impl From for SockAddr { fn from(addr: SocketAddrV6) -> SockAddr { - unsafe { - SockAddr::from_raw_parts( - &addr as *const _ as *const _, - mem::size_of::() as socklen_t, - ) + #[cfg(windows)] + let u = unsafe { + let mut u = mem::zeroed::(); + *u.sin6_scope_id_mut() = addr.scope_id(); + u + }; + + let sockaddr_in6 = sockaddr_in6 { + sin6_family: AF_INET6 as sa_family_t, + sin6_port: addr.port().to_be(), + sin6_addr: crate::sys::to_in6_addr(addr.ip()), + sin6_flowinfo: addr.flowinfo(), + #[cfg(unix)] + sin6_scope_id: addr.scope_id(), + #[cfg(windows)] + u, + #[cfg(any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd" + ))] + sin6_len: 0, + #[cfg(any(target_os = "solaris", target_os = "illumos"))] + __sin6_src_id: 0, + }; + let mut storage = MaybeUninit::::zeroed(); + // Safety: A `sockaddr_in6` is memory compatible with a `sockaddr_storage` + unsafe { (storage.as_mut_ptr() as *mut sockaddr_in6).write(sockaddr_in6) }; + SockAddr { + storage: unsafe { storage.assume_init() }, + len: mem::size_of::() as socklen_t, } } } diff --git a/src/sys/unix.rs b/src/sys/unix.rs index af9ac276..0c984fc7 100644 --- a/src/sys/unix.rs +++ b/src/sys/unix.rs @@ -23,7 +23,7 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::time::Duration; use std::{cmp, fmt, io, mem}; -use libc::{self, c_void, ssize_t}; +use libc::{self, c_void, in6_addr, in_addr, ssize_t}; #[cfg(not(target_os = "redox"))] use crate::RecvFlags; @@ -787,13 +787,12 @@ impl Socket { unsafe { let imr_interface: libc::in_addr = self.getsockopt(libc::IPPROTO_IP, libc::IP_MULTICAST_IF)?; - Ok(from_s_addr(imr_interface.s_addr)) + Ok(from_in_addr(imr_interface)) } } pub fn set_multicast_if_v4(&self, interface: &Ipv4Addr) -> io::Result<()> { - let interface = to_s_addr(interface); - let imr_interface = libc::in_addr { s_addr: interface }; + let imr_interface = to_in_addr(interface); unsafe { self.setsockopt(libc::IPPROTO_IP, libc::IP_MULTICAST_IF, imr_interface) } } @@ -833,11 +832,9 @@ impl Socket { } pub fn join_multicast_v4(&self, multiaddr: &Ipv4Addr, interface: &Ipv4Addr) -> io::Result<()> { - let multiaddr = to_s_addr(multiaddr); - let interface = to_s_addr(interface); let mreq = libc::ip_mreq { - imr_multiaddr: libc::in_addr { s_addr: multiaddr }, - imr_interface: libc::in_addr { s_addr: interface }, + imr_multiaddr: to_in_addr(multiaddr), + imr_interface: to_in_addr(interface), }; unsafe { self.setsockopt(libc::IPPROTO_IP, libc::IP_ADD_MEMBERSHIP, mreq) } } @@ -852,11 +849,9 @@ impl Socket { } pub fn leave_multicast_v4(&self, multiaddr: &Ipv4Addr, interface: &Ipv4Addr) -> io::Result<()> { - let multiaddr = to_s_addr(multiaddr); - let interface = to_s_addr(interface); let mreq = libc::ip_mreq { - imr_multiaddr: libc::in_addr { s_addr: multiaddr }, - imr_interface: libc::in_addr { s_addr: interface }, + imr_multiaddr: to_in_addr(multiaddr), + imr_interface: to_in_addr(interface), }; unsafe { self.setsockopt(libc::IPPROTO_IP, libc::IP_DROP_MEMBERSHIP, mreq) } } @@ -1248,21 +1243,28 @@ fn timeval2dur(raw: libc::timeval) -> Option { } } -fn to_s_addr(addr: &Ipv4Addr) -> libc::in_addr_t { - let octets = addr.octets(); - u32::from_ne_bytes(octets) +pub(crate) fn to_in_addr(addr: &Ipv4Addr) -> in_addr { + // `s_addr` is stored as BE on all machines, and the array is in BE order. + // So the native endian conversion method is used so that it's never swapped. + in_addr { + s_addr: u32::from_ne_bytes(addr.octets()), + } } -fn from_s_addr(in_addr: libc::in_addr_t) -> Ipv4Addr { - in_addr.to_be().into() +pub(crate) fn from_in_addr(in_addr: in_addr) -> Ipv4Addr { + Ipv4Addr::from(in_addr.s_addr.to_ne_bytes()) } -fn to_in6_addr(addr: &Ipv6Addr) -> libc::in6_addr { +pub(crate) fn to_in6_addr(addr: &Ipv6Addr) -> libc::in6_addr { let mut ret: libc::in6_addr = unsafe { mem::zeroed() }; ret.s6_addr = addr.octets(); return ret; } +pub(crate) fn from_in6_addr(in6_addr: in6_addr) -> Ipv6Addr { + Ipv6Addr::from(in6_addr.s6_addr) +} + #[cfg(target_os = "android")] fn to_ipv6mr_interface(value: u32) -> c_int { value as c_int @@ -1297,12 +1299,12 @@ fn dur2linger(dur: Option) -> libc::linger { #[test] fn test_ip() { let ip = Ipv4Addr::new(127, 0, 0, 1); - assert_eq!(ip, from_s_addr(to_s_addr(&ip))); + assert_eq!(ip, from_in_addr(to_in_addr(&ip))); let ip = Ipv4Addr::new(127, 34, 4, 12); let want = 127 << 0 | 34 << 8 | 4 << 16 | 12 << 24; - assert_eq!(to_s_addr(&ip), want); - assert_eq!(from_s_addr(want), ip); + assert_eq!(to_in_addr(&ip).s_addr, want); + assert_eq!(from_in_addr(in_addr { s_addr: want }), ip); } #[test] diff --git a/src/sys/windows.rs b/src/sys/windows.rs index b8f68963..207780cb 100644 --- a/src/sys/windows.rs +++ b/src/sys/windows.rs @@ -615,13 +615,12 @@ impl Socket { pub fn multicast_if_v4(&self) -> io::Result { unsafe { let imr_interface: IN_ADDR = self.getsockopt(IPPROTO_IP, IP_MULTICAST_IF)?; - Ok(from_s_addr(imr_interface.S_un)) + Ok(from_in_addr(imr_interface)) } } pub fn set_multicast_if_v4(&self, interface: &Ipv4Addr) -> io::Result<()> { - let interface = to_s_addr(interface); - let imr_interface = IN_ADDR { S_un: interface }; + let imr_interface = to_in_addr(interface); unsafe { self.setsockopt(IPPROTO_IP, IP_MULTICAST_IF, imr_interface) } } @@ -655,11 +654,9 @@ impl Socket { } pub fn join_multicast_v4(&self, multiaddr: &Ipv4Addr, interface: &Ipv4Addr) -> io::Result<()> { - let multiaddr = to_s_addr(multiaddr); - let interface = to_s_addr(interface); let mreq = IP_MREQ { - imr_multiaddr: IN_ADDR { S_un: multiaddr }, - imr_interface: IN_ADDR { S_un: interface }, + imr_multiaddr: to_in_addr(multiaddr), + imr_interface: to_in_addr(interface), }; unsafe { self.setsockopt(IPPROTO_IP, IP_ADD_MEMBERSHIP, mreq) } } @@ -674,11 +671,9 @@ impl Socket { } pub fn leave_multicast_v4(&self, multiaddr: &Ipv4Addr, interface: &Ipv4Addr) -> io::Result<()> { - let multiaddr = to_s_addr(multiaddr); - let interface = to_s_addr(interface); let mreq = IP_MREQ { - imr_multiaddr: IN_ADDR { S_un: multiaddr }, - imr_interface: IN_ADDR { S_un: interface }, + imr_multiaddr: to_in_addr(multiaddr), + imr_interface: to_in_addr(interface), }; unsafe { self.setsockopt(IPPROTO_IP, IP_DROP_MEMBERSHIP, mreq) } } @@ -1050,19 +1045,19 @@ fn ms2dur(raw: DWORD) -> Option { } } -fn to_s_addr(addr: &Ipv4Addr) -> in_addr_S_un { - let octets = addr.octets(); - let res = u32::from_ne_bytes(octets); - let mut new_addr: in_addr_S_un = unsafe { mem::zeroed() }; - unsafe { *(new_addr.S_addr_mut()) = res }; - new_addr +pub(crate) fn to_in_addr(addr: &Ipv4Addr) -> IN_ADDR { + let mut s_un: in_addr_S_un = unsafe { mem::zeroed() }; + // `S_un` is stored as BE on all machines, and the array is in BE order. + // So the native endian conversion method is used so that it's never swapped. + unsafe { *(s_un.S_addr_mut()) = u32::from_ne_bytes(addr.octets()) }; + IN_ADDR { S_un: s_un } } -fn from_s_addr(in_addr: in_addr_S_un) -> Ipv4Addr { - unsafe { *in_addr.S_addr() }.to_be().into() +pub(crate) fn from_in_addr(in_addr: IN_ADDR) -> Ipv4Addr { + Ipv4Addr::from(unsafe { *in_addr.S_un.S_addr() }.to_ne_bytes()) } -fn to_in6_addr(addr: &Ipv6Addr) -> in6_addr { +pub(crate) fn to_in6_addr(addr: &Ipv6Addr) -> in6_addr { let mut ret_addr: in6_addr_u = unsafe { mem::zeroed() }; unsafe { *(ret_addr.Byte_mut()) = addr.octets() }; let mut ret: in6_addr = unsafe { mem::zeroed() }; @@ -1070,6 +1065,10 @@ fn to_in6_addr(addr: &Ipv6Addr) -> in6_addr { ret } +pub(crate) fn from_in6_addr(in6_addr: in6_addr) -> Ipv6Addr { + Ipv6Addr::from(*unsafe { in6_addr.u.Byte() }) +} + fn linger2dur(linger_opt: sock::linger) -> Option { if linger_opt.l_onoff == 0 { None @@ -1092,16 +1091,38 @@ fn dur2linger(dur: Option) -> sock::linger { } #[test] -fn test_ip() { +fn test_ipv4() { let ip = Ipv4Addr::new(127, 0, 0, 1); - assert_eq!(ip, from_s_addr(to_s_addr(&ip))); + assert_eq!(ip, from_in_addr(to_in_addr(&ip))); let ip = Ipv4Addr::new(127, 34, 4, 12); let want = 127 << 0 | 34 << 8 | 4 << 16 | 12 << 24; - assert_eq!(unsafe { *to_s_addr(&ip).S_addr() }, want); + assert_eq!(unsafe { *to_in_addr(&ip).S_un.S_addr() }, want); let mut addr: in_addr_S_un = unsafe { mem::zeroed() }; unsafe { *(addr.S_addr_mut()) = want }; - assert_eq!(from_s_addr(addr), ip); + assert_eq!(from_in_addr(IN_ADDR { S_un: addr }), ip); +} + +#[test] +fn test_ipv6() { + let ip = Ipv6Addr::new(0x2000, 1, 2, 3, 4, 5, 6, 7); + assert_eq!(ip, from_in6_addr(to_in6_addr(&ip))); + + let ip = Ipv6Addr::new(0x2000, 1, 2, 3, 4, 5, 6, 7); + let want = [ + 0x2000u16.to_be(), + 1u16.to_be(), + 2u16.to_be(), + 3u16.to_be(), + 4u16.to_be(), + 5u16.to_be(), + 6u16.to_be(), + 7u16.to_be(), + ]; + assert_eq!(unsafe { *to_in6_addr(&ip).u.Word() }, want); + let mut addr: in6_addr_u = unsafe { mem::zeroed() }; + unsafe { *(addr.Word_mut()) = want }; + assert_eq!(from_in6_addr(IN6_ADDR { u: addr }), ip); } #[test] From 86fd2dd06ef0ef29949d07663d9db8dcc3072796 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20F=C3=A4rnstrand?= Date: Sun, 8 Nov 2020 22:59:47 +0100 Subject: [PATCH 2/2] Add tests for SockAddr conversion --- src/sockaddr.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/sockaddr.rs b/src/sockaddr.rs index 9eee70d8..f43bb32a 100644 --- a/src/sockaddr.rs +++ b/src/sockaddr.rs @@ -201,3 +201,29 @@ impl fmt::Debug for SockAddr { .finish() } } + +#[cfg(test)] +mod tests { + use super::*; + use std::net::{Ipv4Addr, Ipv6Addr}; + + #[test] + fn conversion_v4() { + let addr = SocketAddrV4::new(Ipv4Addr::new(1, 2, 3, 4), 9876); + let sockaddr = SockAddr::from(addr); + assert_eq!(sockaddr.family(), AF_INET as sa_family_t); + assert!(sockaddr.as_inet6().is_none()); + assert_eq!(sockaddr.as_inet(), Some(addr)); + assert_eq!(sockaddr.as_std(), Some(SocketAddr::V4(addr))); + } + + #[test] + fn conversion_v6() { + let addr = SocketAddrV6::new(Ipv6Addr::new(1, 2, 3, 4, 5, 6, 7, 8), 9876, 11, 12); + let sockaddr = SockAddr::from(addr); + assert_eq!(sockaddr.family(), AF_INET6 as sa_family_t); + assert!(sockaddr.as_inet().is_none()); + assert_eq!(sockaddr.as_inet6(), Some(addr)); + assert_eq!(sockaddr.as_std(), Some(SocketAddr::V6(addr))); + } +}