github-personal/firezone
1
0
Fork 0
mirror of https://github.com/outbackdingo/firezone.git synced 2026-01-27 02:18:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

chore(connlib): clarify Protocol::Icmp as IcmpEcho (#10988)

Browse Source 
Firezone's DNS resource NAT can only route certain ICMP packets. In
particular, we can only route ICMP _echo_ packets as only those have a
stable identifier that we can use in our NAT table.

To clarify that, rename the enum variant to `IcmpEcho` from `Icmp`.
This commit is contained in:
Thomas Eizinger
2025-11-27 15:25:51 +11:00
committed by GitHub
parent e490de7729
commit 3226d434c2
6 changed files with 20 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
rust/libs/connlib/ip-packet/src/icmp_error.rs
View File

@@ -84,7 +84,7 @@ impl FailedPacket {
match self.l4_proto {
Layer4Protocol::Udp { src, .. } => Protocol::Udp(src),
Layer4Protocol::Tcp { src, .. } => Protocol::Tcp(src),
Layer4Protocol::Icmp { id, .. } => Protocol::Icmp(id),
Layer4Protocol::Icmp { id, .. } => Protocol::IcmpEcho(id),
}
}
@@ -181,9 +181,9 @@ fn translate_original_packet_protocol(
inside_proto: Protocol,
) {
let proto_offset = match inside_proto {
Protocol::Tcp(_) => 0, // source port is the first thing in a TCP packet.
Protocol::Udp(_) => 0, // source port is the first thing in a UDP packet.
Protocol::Icmp(_) => 4, // icmp identifier comes after type, code and checksum.
Protocol::Tcp(_) => 0, // source port is the first thing in a TCP packet.
Protocol::Udp(_) => 0, // source port is the first thing in a UDP packet.
Protocol::IcmpEcho(_) => 4, // icmp identifier comes after type, code and checksum.
};
let proto_index = payload_start + proto_offset;

18
rust/libs/connlib/ip-packet/src/lib.rs
View File

@@ -61,8 +61,8 @@ pub enum Protocol {
Tcp(u16),
/// Contains either the source or destination port.
Udp(u16),
/// Contains the `identifier` of the ICMP packet.
Icmp(u16),
/// Contains the `identifier` of the ICMP echo packet.
IcmpEcho(u16),
}
impl Protocol {
@@ -71,7 +71,7 @@ impl Protocol {
(self, other),
(Protocol::Tcp(_), Protocol::Tcp(_))
| (Protocol::Udp(_), Protocol::Udp(_))
| (Protocol::Icmp(_), Protocol::Icmp(_))
| (Protocol::IcmpEcho(_), Protocol::IcmpEcho(_))
)
}
@@ -79,7 +79,7 @@ impl Protocol {
match self {
Protocol::Tcp(v) => *v,
Protocol::Udp(v) => *v,
Protocol::Icmp(v) => *v,
Protocol::IcmpEcho(v) => *v,
}
}
@@ -87,7 +87,7 @@ impl Protocol {
match self {
Protocol::Tcp(_) => Protocol::Tcp(value),
Protocol::Udp(_) => Protocol::Udp(value),
Protocol::Icmp(_) => Protocol::Icmp(value),
Protocol::IcmpEcho(_) => Protocol::IcmpEcho(value),
}
}
}
@@ -305,7 +305,7 @@ impl IpPacket {
.ok_or_else(|| UnsupportedProtocol::UnsupportedIcmpv4Type(p.icmp_type()))?
.id;
return Ok(Protocol::Icmp(id));
return Ok(Protocol::IcmpEcho(id));
}
if let Some(p) = self.as_icmpv6() {
@@ -314,7 +314,7 @@ impl IpPacket {
.ok_or_else(|| UnsupportedProtocol::UnsupportedIcmpv6Type(p.icmp_type()))?
.id;
return Ok(Protocol::Icmp(id));
return Ok(Protocol::IcmpEcho(id));
}
Err(UnsupportedProtocol::UnsupportedIpPayload(
@@ -337,7 +337,7 @@ impl IpPacket {
.ok_or_else(|| UnsupportedProtocol::UnsupportedIcmpv4Type(p.icmp_type()))?
.id;
return Ok(Protocol::Icmp(id));
return Ok(Protocol::IcmpEcho(id));
}
if let Some(p) = self.as_icmpv6() {
@@ -346,7 +346,7 @@ impl IpPacket {
.ok_or_else(|| UnsupportedProtocol::UnsupportedIcmpv6Type(p.icmp_type()))?
.id;
return Ok(Protocol::Icmp(id));
return Ok(Protocol::IcmpEcho(id));
}
Err(UnsupportedProtocol::UnsupportedIpPayload(

4
rust/libs/connlib/tunnel/src/gateway/filter_engine.rs
View File

@@ -67,7 +67,7 @@ impl AllowRules {
match protocol {
Ok(Protocol::Tcp(port)) if self.tcp.contains(&port) => Ok(()),
Ok(Protocol::Udp(port)) if self.udp.contains(&port) => Ok(()),
Ok(Protocol::Icmp(_)) if self.icmp => Ok(()),
Ok(Protocol::IcmpEcho(_)) if self.icmp => Ok(()),
// If ICMP is allowed, we don't care about the specific ICMP type.
// i.e. it doesn't have to be an echo request / reply.
@@ -78,7 +78,7 @@ impl AllowRules {
Ok(Protocol::Tcp(_)) => Err(Filtered::Tcp),
Ok(Protocol::Udp(_)) => Err(Filtered::Udp),
Ok(Protocol::Icmp(_)) => Err(Filtered::Icmp),
Ok(Protocol::IcmpEcho(_)) => Err(Filtered::Icmp),
Err(e) => Err(Filtered::UnsupportedProtocol(e)),
}

4
rust/libs/connlib/tunnel/src/gateway/flow_tracker.rs
View File

@@ -414,7 +414,7 @@ impl FlowTracker {
}
};
}
(Protocol::Icmp(_), Protocol::Icmp(_)) => {}
(Protocol::IcmpEcho(_), Protocol::IcmpEcho(_)) => {}
_ => {
tracing::error!("src and dst protocol must be the same");
}
@@ -508,7 +508,7 @@ impl FlowTracker {
}
};
}
(Protocol::Icmp(_), Protocol::Icmp(_)) => {}
(Protocol::IcmpEcho(_), Protocol::IcmpEcho(_)) => {}
_ => {
tracing::error!("src and dst protocol must be the same");
}

4
rust/libs/connlib/tunnel/src/gateway/nat_table.rs
View File

@@ -218,7 +218,7 @@ impl EntryState {
let ttl = match protocol {
Protocol::Tcp(_) => TCP_TTL,
Protocol::Udp(_) => UDP_TTL,
Protocol::Icmp(_) => ICMP_TTL,
Protocol::IcmpEcho(_) => ICMP_TTL,
};
self.last_packet() + ttl
@@ -381,7 +381,7 @@ mod tests {
let ttl = match src {
Protocol::Tcp(_) => 7200,
Protocol::Udp(_) => 120,
Protocol::Icmp(_) => 120,
Protocol::IcmpEcho(_) => 120,
};
// Assert

2
rust/libs/connlib/tunnel/src/gateway/unroutable_packet.rs
View File

@@ -119,7 +119,7 @@ impl FiveTuple {
dst: MaybeIpOrSocket::Socket(SocketAddr::new(dst_ip, dst_port)),
proto: MaybeProto::Udp,
},
(Ok(Protocol::Icmp(_)), Ok(Protocol::Icmp(_))) => Self {
(Ok(Protocol::IcmpEcho(_)), Ok(Protocol::IcmpEcho(_))) => Self {
src: MaybeIpOrSocket::Ip(src_ip),
dst: MaybeIpOrSocket::Ip(dst_ip),
proto: MaybeProto::Icmp,