Skip to content

Commit 668a1a6

Browse files
committed
ndp: validate router solicitations
Validate Router Solicitation messages before replying: enforce minimum ICMP payload length, validate option structure, reject multicast sources and reject SLLA options when the source is the unspecified address. Continue to accept solicitations with no options, valid SLLA options, or unknown (yet complete) options per RFC 4861. When replying to a valid solicitation, send solicited RAs to all-nodes multicast for unspecified sources and unicast them back to link-local sources. Keep receive-loop storm backoff, but only after successfully received packets so idle timeouts are not delayed. Signed-off-by: Trey Aspelund <trey@oxidecomputer.com>
1 parent 585eaf8 commit 668a1a6

2 files changed

Lines changed: 288 additions & 6 deletions

File tree

ndp/src/manager.rs

Lines changed: 185 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,9 @@ fn rx_loop(
146146
if let Ok(ra) = Icmp6RouterAdvertisement::from_wire(buf) {
147147
handle_ra(&ifx, &state, &log, ra, src);
148148
}
149-
if let Ok(rs) = Icmp6RouterSolicitation::from_wire(buf) {
149+
if let Ok(rs) =
150+
Icmp6RouterSolicitation::from_wire_with_source(buf, src)
151+
{
150152
handle_rs(&s, &ifx, &state, &log, rs, src);
151153
}
152154
}
@@ -220,16 +222,34 @@ fn handle_rs(
220222
_rs: Icmp6RouterSolicitation,
221223
src: Ipv6Addr,
222224
) {
225+
let Some(dst) = solicited_ra_destination(src) else {
226+
debug!(
227+
log,
228+
"ignoring RS from non-link-local source {src} on {}", ifx.name,
229+
);
230+
return;
231+
};
232+
223233
send_ra(
224234
sk,
225235
ifx.ip,
226-
Some(src),
236+
dst,
227237
ifx.scope_id,
228238
state.get_tx_router_lifetime(),
229239
log,
230240
);
231241
}
232242

243+
fn solicited_ra_destination(src: Ipv6Addr) -> Option<Option<Ipv6Addr>> {
244+
if src.is_unspecified() {
245+
Some(None)
246+
} else if src.is_unicast_link_local() {
247+
Some(Some(src))
248+
} else {
249+
None
250+
}
251+
}
252+
233253
/// Check to see if the reachable time for our current peer (if any) is expired.
234254
/// If so, remove the peer.
235255
fn check_expired(state: &RouterDiscoveryState) {
@@ -409,6 +429,145 @@ mod test {
409429
Icmp6RouterAdvertisement::from_wire(&data)
410430
.expect_err("RS should not parse as RA");
411431
Icmp6RouterSolicitation::from_wire(&data).expect("parsed solicitation");
432+
Icmp6RouterSolicitation::from_wire_with_source(
433+
&data,
434+
"fe80::1".parse().unwrap(),
435+
)
436+
.expect("parsed solicitation with link-local source");
437+
}
438+
439+
#[test]
440+
fn router_solicitation_from_unspecified_rejects_slla() {
441+
let data: [u8; 16] = [
442+
133, 0, 11, 22, // type, code checksum
443+
0, 0, 0, 0, // reserved
444+
1, 1, 0xab, 0xbb, // source link-layer addr option
445+
0xcc, 0xdd, 0xee, 0xff,
446+
];
447+
448+
assert!(matches!(
449+
Icmp6RouterSolicitation::from_wire_with_source(
450+
&data,
451+
Ipv6Addr::UNSPECIFIED,
452+
),
453+
Err(crate::packet::Icmp6RsParseError::SllaFromUnspecifiedSource),
454+
));
455+
}
456+
457+
#[test]
458+
fn router_solicitation_rejects_multicast_source() {
459+
let data: [u8; 8] = [133, 0, 11, 22, 0, 0, 0, 0];
460+
let source = "ff02::1".parse().unwrap();
461+
462+
assert!(matches!(
463+
Icmp6RouterSolicitation::from_wire_with_source(&data, source),
464+
Err(crate::packet::Icmp6RsParseError::MulticastSource(
465+
err_source,
466+
)) if err_source == source,
467+
));
468+
}
469+
470+
#[test]
471+
fn router_solicitation_rejects_truncated_payload() {
472+
let data: [u8; 7] = [133, 0, 11, 22, 0, 0, 0];
473+
474+
assert!(matches!(
475+
Icmp6RouterSolicitation::from_wire(&data),
476+
Err(crate::packet::Icmp6RsParseError::TooShort(7)),
477+
));
478+
}
479+
480+
#[test]
481+
fn router_solicitation_rejects_truncated_option_header() {
482+
let data: [u8; 9] = [
483+
133, 0, 11, 22, // type, code, checksum
484+
0, 0, 0, 0, // reserved
485+
1, // truncated source link-layer address option header
486+
];
487+
488+
assert!(matches!(
489+
Icmp6RouterSolicitation::from_wire(&data),
490+
Err(crate::packet::Icmp6RsParseError::Option(
491+
crate::packet::Icmp6RsOptionParseError::TruncatedHeader {
492+
remaining: 1,
493+
},
494+
)),
495+
));
496+
}
497+
498+
#[test]
499+
fn router_solicitation_rejects_zero_length_option() {
500+
let data: [u8; 16] = [
501+
133, 0, 11, 22, // type, code checksum
502+
0, 0, 0, 0, // reserved
503+
1, 0, 0xab, 0xbb, // zero-length source link-layer addr option
504+
0xcc, 0xdd, 0xee, 0xff,
505+
];
506+
507+
assert!(matches!(
508+
Icmp6RouterSolicitation::from_wire(&data),
509+
Err(crate::packet::Icmp6RsParseError::Option(
510+
crate::packet::Icmp6RsOptionParseError::ZeroLength {
511+
option_type: 1,
512+
},
513+
)),
514+
));
515+
}
516+
517+
#[test]
518+
fn router_solicitation_rejects_truncated_option_body() {
519+
let data: [u8; 15] = [
520+
133, 0, 11, 22, // type, code checksum
521+
0, 0, 0, 0, // reserved
522+
1, 1, 0xab, 0xbb, // truncated source link-layer addr option
523+
0xcc, 0xdd, 0xee,
524+
];
525+
526+
assert!(matches!(
527+
Icmp6RouterSolicitation::from_wire(&data),
528+
Err(crate::packet::Icmp6RsParseError::Option(
529+
crate::packet::Icmp6RsOptionParseError::TruncatedOption {
530+
option_type: 1,
531+
option_len: 8,
532+
remaining: 7,
533+
},
534+
)),
535+
));
536+
}
537+
538+
#[test]
539+
fn router_solicitation_rejects_trailing_partial_option() {
540+
let data: [u8; 17] = [
541+
133, 0, 11, 22, // type, code checksum
542+
0, 0, 0, 0, // reserved
543+
1, 1, 0xab, 0xbb, // complete source link-layer addr option
544+
0xcc, 0xdd, 0xee, 0xff,
545+
5, // trailing partial unknown option header
546+
];
547+
548+
assert!(matches!(
549+
Icmp6RouterSolicitation::from_wire(&data),
550+
Err(crate::packet::Icmp6RsParseError::Option(
551+
crate::packet::Icmp6RsOptionParseError::TruncatedHeader {
552+
remaining: 1,
553+
},
554+
)),
555+
));
556+
}
557+
558+
#[test]
559+
fn router_solicitation_accepts_complete_unknown_option() {
560+
let data: [u8; 24] = [
561+
133, 0, 11, 22, // type, code checksum
562+
0, 0, 0, 0, // reserved
563+
1, 1, 0xab, 0xbb, // complete source link-layer addr option
564+
0xcc, 0xdd, 0xee, 0xff, 253, 1, 0,
565+
0, // complete unknown option
566+
0, 0, 0, 0,
567+
];
568+
569+
Icmp6RouterSolicitation::from_wire(&data)
570+
.expect("parsed solicitation with complete unknown option");
412571
}
413572

414573
#[test]
@@ -431,6 +590,30 @@ mod test {
431590
Icmp6RouterSolicitation::from_wire(&data).expect("parsed solicitation");
432591
}
433592

593+
#[test]
594+
fn router_solicitation_from_unspecified_replies_to_all_nodes() {
595+
assert_eq!(
596+
super::solicited_ra_destination(Ipv6Addr::UNSPECIFIED),
597+
Some(None),
598+
);
599+
}
600+
601+
#[test]
602+
fn router_solicitation_from_link_local_replies_to_sender() {
603+
let src = "fe80::1".parse().unwrap();
604+
605+
assert_eq!(super::solicited_ra_destination(src), Some(Some(src)),);
606+
}
607+
608+
#[test]
609+
fn router_solicitation_from_non_link_local_is_ignored() {
610+
let global = "2001:db8::1".parse().unwrap();
611+
let loopback = Ipv6Addr::LOCALHOST;
612+
613+
assert_eq!(super::solicited_ra_destination(global), None,);
614+
assert_eq!(super::solicited_ra_destination(loopback), None,);
615+
}
616+
434617
#[test]
435618
fn minimum_router_advertisement() {
436619
// Attempt to parse a minimal ICMPv6 router advertisement as a

ndp/src/packet.rs

Lines changed: 103 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
// Copyright 2026 Oxide Computer Company
88

99
use serde::{Deserialize, Serialize};
10+
use std::net::Ipv6Addr;
1011
use std::time::Duration;
1112

1213
/// ICMP6 router advertisement
@@ -122,14 +123,46 @@ pub struct Icmp6RouterSolicitation {
122123
impl Icmp6RouterSolicitation {
123124
const TYPE: u8 = 133;
124125
const CODE: u8 = 0;
126+
const MIN_PAYLOAD_LEN: usize = 8;
127+
// Source Link-Layer Address option.
128+
const OPTION_SLLA: u8 = 1;
129+
130+
#[cfg(test)]
131+
pub(crate) fn from_wire(buf: &[u8]) -> Result<Self, Icmp6RsParseError> {
132+
Self::from_wire_impl(buf, None)
133+
}
134+
135+
pub fn from_wire_with_source(
136+
buf: &[u8],
137+
source: Ipv6Addr,
138+
) -> Result<Self, Icmp6RsParseError> {
139+
Self::from_wire_impl(buf, Some(source))
140+
}
141+
142+
fn from_wire_impl(
143+
buf: &[u8],
144+
source: Option<Ipv6Addr>,
145+
) -> Result<Self, Icmp6RsParseError> {
146+
// Per RFC 4861 Section 6.1.1: a valid RS has an ICMP payload of >= 8b.
147+
if buf.len() < Self::MIN_PAYLOAD_LEN {
148+
return Err(Icmp6RsParseError::TooShort(buf.len()));
149+
}
125150

126-
pub fn from_wire(buf: &[u8]) -> Result<Self, Icmp6RsFromWireError> {
127151
let s: Self = ispf::from_bytes_be(buf)?;
128152
if s.typ != Self::TYPE {
129-
return Err(Icmp6RsFromWireError::WrongType(s.typ));
153+
return Err(Icmp6RsParseError::WrongType(s.typ));
130154
}
131155
if s.code != Self::CODE {
132-
return Err(Icmp6RsFromWireError::WrongCode(s.code));
156+
return Err(Icmp6RsParseError::WrongCode(s.code));
157+
}
158+
let has_slla = validate_nd_options(&buf[Self::MIN_PAYLOAD_LEN..])?;
159+
if let Some(source) = source {
160+
if source.is_multicast() {
161+
return Err(Icmp6RsParseError::MulticastSource(source));
162+
}
163+
if source.is_unspecified() && has_slla {
164+
return Err(Icmp6RsParseError::SllaFromUnspecifiedSource);
165+
}
133166
}
134167
Ok(s)
135168
}
@@ -146,14 +179,80 @@ impl Default for Icmp6RouterSolicitation {
146179
}
147180
}
148181

182+
fn validate_nd_options(buf: &[u8]) -> Result<bool, Icmp6RsOptionParseError> {
183+
let mut offset = 0;
184+
let mut has_slla = false;
185+
while offset < buf.len() {
186+
if buf.len() - offset < 2 {
187+
return Err(Icmp6RsOptionParseError::TruncatedHeader {
188+
remaining: buf.len() - offset,
189+
});
190+
}
191+
192+
let option_type = buf[offset];
193+
let length = buf[offset + 1];
194+
if length == 0 {
195+
return Err(Icmp6RsOptionParseError::ZeroLength { option_type });
196+
}
197+
198+
let option_len = usize::from(length) * 8;
199+
let next = offset + option_len;
200+
if next > buf.len() {
201+
return Err(Icmp6RsOptionParseError::TruncatedOption {
202+
option_type,
203+
option_len,
204+
remaining: buf.len() - offset,
205+
});
206+
}
207+
208+
if option_type == Icmp6RouterSolicitation::OPTION_SLLA {
209+
has_slla = true;
210+
}
211+
offset = next;
212+
}
213+
214+
Ok(has_slla)
215+
}
216+
149217
#[derive(Debug, thiserror::Error)]
150-
pub enum Icmp6RsFromWireError {
218+
pub enum Icmp6RsParseError {
151219
#[error("deserialization error: {0}")]
152220
Ispf(#[from] ispf::Error),
153221

222+
#[error("too short: {0} octets, expected at least 8")]
223+
TooShort(usize),
224+
225+
#[error("invalid option: {0}")]
226+
Option(#[from] Icmp6RsOptionParseError),
227+
228+
/// Source Link-Layer Address option is invalid from an unspecified source.
229+
#[error("SLLA option is invalid from unspecified source")]
230+
SllaFromUnspecifiedSource,
231+
232+
#[error("multicast source address is invalid for router solicitation: {0}")]
233+
MulticastSource(Ipv6Addr),
234+
154235
#[error("wrong type: expected {expected}, got {0}", expected = Icmp6RouterSolicitation::TYPE)]
155236
WrongType(u8),
156237

157238
#[error("wrong code: expected {expected}, got {0}", expected = Icmp6RouterSolicitation::CODE)]
158239
WrongCode(u8),
159240
}
241+
242+
#[derive(Debug, thiserror::Error)]
243+
pub enum Icmp6RsOptionParseError {
244+
#[error("truncated option header: {remaining} octets remaining")]
245+
TruncatedHeader { remaining: usize },
246+
247+
#[error("option {option_type} has zero length")]
248+
ZeroLength { option_type: u8 },
249+
250+
#[error(
251+
"option {option_type} length {option_len} exceeds {remaining} remaining octets"
252+
)]
253+
TruncatedOption {
254+
option_type: u8,
255+
option_len: usize,
256+
remaining: usize,
257+
},
258+
}

0 commit comments

Comments
 (0)