Cannot parse IPv6 link-local adresses from the TOML file #596

New Issue

Closed

opened 2023-07-12 18:14:53 +00:00 by PicNoir · 2 comments

PicNoir commented

2023-07-12 18:14:53 +00:00

Contributor

It seems like the TOML parser is struggling parsing (what I think is) a valid IPv6 link-local address:

-07-12T17:08:05.237694Z  INFO garage::server: Loading configuration...
juil. 12 19:08:05 dundies garage[182164]: Error: TOML decode error: TOML parse error at line 4, column 17
juil. 12 19:08:05 dundies garage[182164]:   |
juil. 12 19:08:05 dundies garage[182164]: 4 | rpc_bind_addr = "[fe80::1%wg-garage]:3901"
juil. 12 19:08:05 dundies garage[182164]:   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^
juil. 12 19:08:05 dundies garage[182164]: invalid socket address syntax
juil. 12 19:08:05 dundies systemd[1]: garage.service: Main process exited, code=exited, status=1/FAILURE
juil. 12 19:08:05 dundies systemd[1]: garage.service: Failed with result 'exit-code'.

I tried to investigate this a bit further by creating a minimal Serde reproduction. I get the same behavior:

use std::net::{SocketAddr};
use serde::Deserialize;

#[derive(Debug, Clone, Deserialize)]
struct Config {
  address: SocketAddr
}

fn main() {
    let s = "{\"address\": \"[fe80::2a3f:5804:4337:def8%enp0s13f0u3c2]:8080\"}";

    serde_json::from_str::<Config>(&s).unwrap();
}

/tmp/serdetest (main*) » cargo run 
   Compiling serdetest v0.1.0 (/tmp/serdetest)
    Finished dev [unoptimized + debuginfo] target(s) in 0.18s
     Running `target/debug/serdetest`
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error("invalid socket address syntax", line: 1, column: 60)', src/main.rs:12:40
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Is this really a Serde bug? Or am I doing something very wrong here?

I looked at the serde bug tracker, I couldn't find anybody complaining about this. It'd be really surprising to be the first one stumbling upon a bug on this pretty common IPv6 addr, I feel like I'm missing something obvious here :/

0/ It seems like the TOML parser is struggling parsing (what I think is) a valid IPv6 link-local address: ``` -07-12T17:08:05.237694Z INFO garage::server: Loading configuration... juil. 12 19:08:05 dundies garage[182164]: Error: TOML decode error: TOML parse error at line 4, column 17 juil. 12 19:08:05 dundies garage[182164]: | juil. 12 19:08:05 dundies garage[182164]: 4 | rpc_bind_addr = "[fe80::1%wg-garage]:3901" juil. 12 19:08:05 dundies garage[182164]: | ^^^^^^^^^^^^^^^^^^^^^^^^^^ juil. 12 19:08:05 dundies garage[182164]: invalid socket address syntax juil. 12 19:08:05 dundies systemd[1]: garage.service: Main process exited, code=exited, status=1/FAILURE juil. 12 19:08:05 dundies systemd[1]: garage.service: Failed with result 'exit-code'. ``` I tried to investigate this a bit further by creating a minimal Serde reproduction. I get the same behavior: ```rust use std::net::{SocketAddr}; use serde::Deserialize; #[derive(Debug, Clone, Deserialize)] struct Config { address: SocketAddr } fn main() { let s = "{\"address\": \"[fe80::2a3f:5804:4337:def8%enp0s13f0u3c2]:8080\"}"; serde_json::from_str::<Config>(&s).unwrap(); } ``` ```sh /tmp/serdetest (main*) » cargo run Compiling serdetest v0.1.0 (/tmp/serdetest) Finished dev [unoptimized + debuginfo] target(s) in 0.18s Running `target/debug/serdetest` thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error("invalid socket address syntax", line: 1, column: 60)', src/main.rs:12:40 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace ``` Is this really a Serde bug? Or am I doing something very wrong here? I looked at the serde bug tracker, I couldn't find anybody complaining about this. It'd be really surprising to be the first one stumbling upon a bug on this pretty common IPv6 addr, I feel like I'm missing something obvious here :/

PicNoir commented

2023-07-12 18:57:51 +00:00

Author

Contributor

It seems like Serde currently does not parse the IPv6 scope identifier: 03da66c805/serde/src/de/impls.rs (L1621), the last parameter of net::SocketAddrV6::new https://doc.rust-lang.org/stable/std/net/struct.SocketAddrV6.html#method.new.

I'm really surprised.

This is definitely not a garage-specific bug though. I'll upstream the bug report to Serde.

It seems like Serde currently does not parse the IPv6 scope identifier: https://github.com/serde-rs/serde/blob/03da66c805499a9e79a5b6cb11fa21ef6da636eb/serde/src/de/impls.rs#L1621, the last parameter of `net::SocketAddrV6::new` https://doc.rust-lang.org/stable/std/net/struct.SocketAddrV6.html#method.new. I'm really surprised. This is definitely not a garage-specific bug though. I'll upstream the bug report to Serde.

PicNoir commented

2023-07-12 21:07:03 +00:00

Author

Contributor

Last update:

The Rust parser only supports int scope ids: https://doc.rust-lang.org/stable/src/core/net/parser.rs.html#236. (Serde is re-using this parser)

So, basically, instead of using fe80::1%wg-garage, I should be using fe80::1%9. You can figure out the scope id associated to an interface via:

cat /sys/class/net/wg-garage/ifindex
9

Having a look at the relevant RFC, I learned that:

An implementation SHOULD support at least numerical indices that are non-negative decimal integers as <zone_id>. The default zone index, which should typically be 0 (see Section 6), is included in the integers. When <zone_id> is the default, the delimiter characters "%" and <zone_id> can be omitted. Similarly, if a textual representation of an IPv6 address is given without a zone index, it should be interpreted as <address>%<default ID>, where <default ID> is the default zone index of the scope that <address> has.

An implementation MAY support other kinds of non-null strings as <zone_id>. However, the strings must not conflict with the delimiter character. The precise format and semantics of additional strings is implementation dependent.

One possible candidate for these strings would be interface names, as interfaces uniquely disambiguate any scopes. In particular, interface names can be used as "default identifiers" for interfaces and links, because by default there is a one-to-one mapping between interfaces and each of those scopes as described in Section 6.

It seems like the Rust team MAY have decided not to implement the interface-as-a-zone_id notation. That's unfortunate, it's a really handy notation.

I thought this zone-id notation was a valid IPv6 textual representation. But it seems like the specification is pretty loose and the decision up to the implementer. TIL

In any case, I think it's safe to assume this issue is definitely out of scope as far as garage goes. Let's put an end to this monologue and close it.

Last update: The Rust parser only supports int scope ids: https://doc.rust-lang.org/stable/src/core/net/parser.rs.html#236. (Serde is re-using this parser) So, basically, instead of using `fe80::1%wg-garage`, I should be using `fe80::1%9`. You can figure out the scope id associated to an interface via: ```sh cat /sys/class/net/wg-garage/ifindex 9 ``` Having a look at the [relevant RFC](https://datatracker.ietf.org/doc/html/rfc4007), I learned that: > An implementation SHOULD support at least numerical indices that are non-negative decimal integers as <zone_id>. The default zone index, which should typically be 0 (see Section 6), is included in the integers. When <zone_id> is the default, the delimiter characters "%" and <zone_id> can be omitted. Similarly, if a textual representation of an IPv6 address is given without a zone index, it should be interpreted as \<address>%\<default ID>, where \<default ID> is the default zone index of the scope that \<address> has. > > An implementation MAY support other kinds of non-null strings as <zone_id>. However, the strings must not conflict with the delimiter character. The precise format and semantics of additional strings is implementation dependent. > > One possible candidate for these strings would be interface names, as interfaces uniquely disambiguate any scopes. In particular, interface names can be used as "default identifiers" for interfaces and links, because by default there is a one-to-one mapping between interfaces and each of those scopes as described in Section 6. It seems like the Rust team **MAY** have decided not to implement the interface-as-a-zone_id notation. That's unfortunate, it's a really handy notation. I thought this zone-id notation was a valid IPv6 textual representation. But it seems like the specification is pretty loose and the decision up to the implementer. TIL In any case, I think it's safe to assume this issue is definitely out of scope as far as garage goes. Let's put an end to this monologue and close it.