-
Notifications
You must be signed in to change notification settings - Fork 443
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TCP sockets can transmit packets with wrong source IP #466
Comments
I am currently running into needing to fix this issue for my application. My approach would absolutely be to implement
I am quite new to the |
I think I've managed to check for a mismatched src addr in Next up is catching this in Which way would be the most correct to go about this? |
This diff essentially does what I want it to functionally, but I would suspect this is not the proper way to solve it? @Dirbaio ? diff --git a/src/iface/interface/mod.rs b/src/iface/interface/mod.rs
index 7d6bdc8..235d9dc 100644
--- a/src/iface/interface/mod.rs
+++ b/src/iface/interface/mod.rs
@@ -599,6 +599,7 @@ impl Interface {
enum EgressError {
Exhausted,
+ MismatchingSrcIp,
Dispatch(DispatchError),
}
@@ -614,6 +615,12 @@ impl Interface {
let mut neighbor_addr = None;
let mut respond = |inner: &mut InterfaceInner, meta: PacketMeta, response: Packet| {
neighbor_addr = Some(response.ip_repr().dst_addr());
+
+ if !inner.has_ip_addr(response.ip_repr().src_addr()) {
+ net_debug!("failed to transmit IP: mismatched src addr");
+ return Err(EgressError::MismatchingSrcIp);
+ }
+
let t = device.transmit(inner.now).ok_or_else(|| {
net_debug!("failed to transmit IP: device exhausted");
EgressError::Exhausted
@@ -663,13 +670,19 @@ impl Interface {
})
}
#[cfg(feature = "socket-tcp")]
- Socket::Tcp(socket) => socket.dispatch(&mut self.inner, |inner, (ip, tcp)| {
+ Socket::Tcp(socket) => match socket.dispatch(&mut self.inner, |inner, (ip, tcp)| {
respond(
inner,
PacketMeta::default(),
Packet::new(ip, IpPayload::Tcp(tcp)),
)
- }),
+ }) {
+ Err(EgressError::MismatchingSrcIp) => {
+ socket.set_state(tcp::State::Closed);
+ Err(EgressError::MismatchingSrcIp)
+ }
+ r => r,
+ },
#[cfg(feature = "socket-dhcpv4")]
Socket::Dhcpv4(socket) => {
socket.dispatch(&mut self.inner, |inner, (ip, udp, dhcp)| {
@@ -702,6 +715,7 @@ impl Interface {
neighbor_addr.expect("non-IP response packet"),
);
}
+ Err(EgressError::MismatchingSrcIp) => {}
Ok(()) => {}
}
}
diff --git a/src/socket/tcp.rs b/src/socket/tcp.rs
index d7b85ab..cf6f42d 100644
--- a/src/socket/tcp.rs
+++ b/src/socket/tcp.rs
@@ -1204,7 +1204,7 @@ impl<'a> Socket<'a> {
self.rx_buffer.len()
}
- fn set_state(&mut self, state: State) {
+ pub fn set_state(&mut self, state: State) {
if self.state != state {
tcp_trace!("state={}=>{}", self.state, state);
} |
If there's a TCP socket open, and then the smoltcp changes IP address (due to eg being plugged into a different Ethernet network), the socket stays established and still transmits packets with the old source IP address.
No matter what, a host should never transmit packets with a source IP that's not its own :)
In the packet capture below you can see the DHCP server gives the address
10.42.0.59
to the smoltcp host, but it continues transmitting packets for the old192.168.1.37
address. Eventually the socket times out and the application establishes a new one, which then gets the right source IP and communication returns.I think the same bug can be triggered by explicitly calling
tcp_socket.connect
with a wrong source address. Maybe with UDP and other sockets too.Not sure what the best solution is.
Interface
that we have the source address, and fail emitting if not? The socket would still have to timeout, but at least we don't emit garbage into the network.This is somewhat related with #458
The text was updated successfully, but these errors were encountered: