Skip to content
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

fix: enter unreachable when discarding HBH Option with multicast destination address #996

Merged
merged 2 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions src/iface/interface/ipv6.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,12 +288,13 @@ impl InterfaceInner {
Ipv6OptionFailureType::DiscardSendAll => {
return HopByHopResponse::Discard(param_problem());
}
Ipv6OptionFailureType::DiscardSendUnicast
if !ipv6_repr.dst_addr.is_multicast() =>
{
return HopByHopResponse::Discard(param_problem());
Ipv6OptionFailureType::DiscardSendUnicast => {
if !ipv6_repr.dst_addr.is_multicast() {
return HopByHopResponse::Discard(param_problem());
} else {
return HopByHopResponse::Discard(None);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we still have to return here if the sender is multicast, to discard the packet, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, indeed, I misread this in the RFC. I thought only the option needs to be discarded, but like you say, the full packet needs to be discarded, without sending an ICMP parameter problem back.

}
_ => unreachable!(),
}
}
}
Expand Down
50 changes: 36 additions & 14 deletions src/wire/ipv6option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,19 +51,42 @@ impl RouterAlert {
pub const DATA_LEN: u8 = 2;
}

enum_with_unknown! {
/// Action required when parsing the given IPv6 Extension
/// Header Option Type fails
pub enum FailureType(u8) {
/// Skip this option and continue processing the packet
Skip = 0b00000000,
/// Discard the containing packet
Discard = 0b01000000,
/// Discard the containing packet and notify the sender
DiscardSendAll = 0b10000000,
/// Discard the containing packet and only notify the sender
/// if the sender is a unicast address
DiscardSendUnicast = 0b11000000,
/// Action required when parsing the given IPv6 Extension
/// Header Option Type fails
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Hash)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
pub enum FailureType {
/// Skip this option and continue processing the packet
Skip = 0b00000000,
/// Discard the containing packet
Discard = 0b01000000,
/// Discard the containing packet and notify the sender
DiscardSendAll = 0b10000000,
/// Discard the containing packet and only notify the sender
/// if the sender is a unicast address
DiscardSendUnicast = 0b11000000,
}

impl From<u8> for FailureType {
fn from(value: u8) -> FailureType {
match value & 0b11000000 {
0b00000000 => FailureType::Skip,
0b01000000 => FailureType::Discard,
0b10000000 => FailureType::DiscardSendAll,
0b11000000 => FailureType::DiscardSendUnicast,
_ => unreachable!(),
}
}
}

impl From<FailureType> for u8 {
fn from(value: FailureType) -> Self {
match value {
FailureType::Skip => 0b00000000,
FailureType::Discard => 0b01000000,
FailureType::DiscardSendAll => 0b10000000,
FailureType::DiscardSendUnicast => 0b11000000,
}
}
}

Expand All @@ -74,7 +97,6 @@ impl fmt::Display for FailureType {
FailureType::Discard => write!(f, "discard"),
FailureType::DiscardSendAll => write!(f, "discard and send error"),
FailureType::DiscardSendUnicast => write!(f, "discard and send error if unicast"),
FailureType::Unknown(id) => write!(f, "Unknown({id})"),
}
}
}
Expand Down
Loading