Skip to content

Commit ba55373

Browse files
hawkwolix0r
andcommitted
addr: fix to_http_authority panic with IPv6 (#976)
Currently, the `Addr::to_http_authority` method panics when called on a `SocketAddr` which is an IPv6 address with port 80. This method does not panic when called with IPv4 addresses, or with IPv6 addresses whose ports are *not* port 80. This was initially caught by oss-fuzz; see [here][1] for details. The panic occurs because when an IPv6+ address occurs in an authority, it must be within square brackets, as per [RFC3986, Section 3.2][2]. The square brackets distinguish between colons in the IPv6 address and the colon separating the address and port. When the `SocketAddr`'s port is not port 80, we format it including the port, and the `fmt::Display` output from IPv6 `SocketAddr`s includes the square brackets as expected. However, when the socket's port *is* port 80, we have special logic for eliding the port from the authority. This works fine for IPv4, where we can just call `addr.ip().to_string()` to nicely format the address. However, with IPv6 addresses, this only formats the address itself, *not* the square brackets. According to RFC3986, square brackets are mandatory for *all* IPv6 addresses, even when port 80 is elided. This branch fixes the panic by changing `Addr::to_http_authority` to include square brackets when formatting IPv6 `SocketAddr`s with port 80. I've also improved on @olix0r's original test cases from dbf898a to include IPv6 addrs with and without shorthand, and to test ports that are and are not port 80. These tests helped catch the panic, and may be useful to guard against future regressions. Fixes linkerd/linkerd2#6020 [1]: https://oss-fuzz.com/testcase-detail/6502844766224384 [2]: https://tools.ietf.org/html/rfc3986#section-3.2 Signed-off-by: Eliza Weisman <[email protected]> Co-authored-by: Oliver Gould <[email protected]>
1 parent 372a804 commit ba55373

File tree

1 file changed

+78
-4
lines changed

1 file changed

+78
-4
lines changed

linkerd/addr/src/lib.rs

Lines changed: 78 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,26 @@ impl Addr {
8282
match self {
8383
Addr::Name(n) => n.as_http_authority(),
8484
Addr::Socket(ref a) if a.port() == 80 => {
85-
http::uri::Authority::from_str(&a.ip().to_string())
86-
.expect("SocketAddr must be valid authority")
85+
let ip = if a.is_ipv4() {
86+
a.ip().to_string()
87+
} else {
88+
// When IPv6 or later addresses are used in an authority,
89+
// they must be within square brackets. See
90+
// https://tools.ietf.org/html/rfc3986#section-3.2 for
91+
// details. The `fmt::Display` implementation of the
92+
// `Ipv6Addr` type does not include brackets, so we must add
93+
// them ourselves.
94+
format!("[{}]", a.ip())
95+
};
96+
http::uri::Authority::from_str(&ip).unwrap_or_else(|err| {
97+
panic!("SocketAddr ({}) must be valid authority: {}", a, err)
98+
})
99+
}
100+
Addr::Socket(a) => {
101+
http::uri::Authority::from_str(&a.to_string()).unwrap_or_else(|err| {
102+
panic!("SocketAddr ({}) must be valid authority: {}", a, err)
103+
})
87104
}
88-
Addr::Socket(a) => http::uri::Authority::from_str(&a.to_string())
89-
.expect("SocketAddr must be valid authority"),
90105
}
91106
}
92107

@@ -257,6 +272,65 @@ mod tests {
257272
assert_eq!(a.is_loopback(), *expected_result, "{:?}", host)
258273
}
259274
}
275+
276+
fn test_to_http_authority(cases: &[&str]) {
277+
let width = cases.iter().map(|s| s.len()).max().unwrap_or(0);
278+
for host in cases {
279+
print!("trying {:1$} ... ", host, width);
280+
Addr::from_str(host).unwrap().to_http_authority();
281+
println!("ok");
282+
}
283+
}
284+
285+
#[test]
286+
fn to_http_authority_names_port_80() {
287+
test_to_http_authority(&[
288+
"localhost:80",
289+
"localhost.:80",
290+
"LocalhOsT.:80",
291+
"mlocalhost.:80",
292+
"localhost1.:80",
293+
])
294+
}
295+
296+
#[test]
297+
fn to_http_authority_names() {
298+
test_to_http_authority(&[
299+
"localhost:9090",
300+
"localhost.:9090",
301+
"LocalhOsT.:9090",
302+
"mlocalhost.:9090",
303+
"localhost1.:9090",
304+
])
305+
}
306+
307+
#[test]
308+
fn to_http_authority_ipv4_port_80() {
309+
test_to_http_authority(&["10.7.0.42:80", "127.0.0.1:80"])
310+
}
311+
312+
#[test]
313+
fn to_http_authority_ipv4() {
314+
test_to_http_authority(&["10.7.0.42:9090", "127.0.0.1:9090"])
315+
}
316+
317+
#[test]
318+
fn to_http_authority_ipv6_port_80() {
319+
test_to_http_authority(&[
320+
"[2001:0db8:0000:0000:0000:8a2e:0370:7334]:80",
321+
"[2001:db8::8a2e:370:7334]:80",
322+
"[::1]:80",
323+
])
324+
}
325+
326+
#[test]
327+
fn to_http_authority_ipv6() {
328+
test_to_http_authority(&[
329+
"[2001:0db8:0000:0000:0000:8a2e:0370:7334]:9090",
330+
"[2001:db8::8a2e:370:7334]:9090",
331+
"[::1]:9090",
332+
])
333+
}
260334
}
261335

262336
#[cfg(fuzzing)]

0 commit comments

Comments
 (0)