improve(cli, gossip): Disallow port in --gossip-host (#335)

* Added IpAddr.parseIpv4 * Added IpAddr.parseIpv6 * Added IpAddr.parse * Parse host using IpAddr.parse * Turn presence of port into error in Ipv4Addr parsing * fmt * Turn presence of port into error in Ipv6Addr parsing * Improve error report * refactor(net): simplify ip parsing and use more informative errors * feat(cmd): log for *any* ip parsing errors --------- Co-authored-by: Drew Nutter <[email protected]>
Syndica · Oct 29, 2024 · a21b812 · a21b812
1 parent 0ac4200
commit a21b812
Show file tree

Hide file tree

Showing 3 changed files with 123 additions and 21 deletions.
diff --git a/src/cmd/cmd.zig b/src/cmd/cmd.zig
@@ -1124,8 +1124,7 @@ const AppBase = struct {
         errdefer entrypoints.deinit();
 
         const ip_echo_data = try getMyDataFromIpEcho(logger, entrypoints.items);
-        // Try to extract the port from the given host. If no port is found, fall back to the configured default port.
-        const my_port = (config.current.gossip.getPortFromHost() orelse config.current.gossip.port) catch config.current.gossip.port;
+        const my_port = config.current.gossip.port;
 
         return .{
             .closed = false,
@@ -1265,7 +1264,14 @@ fn getMyDataFromIpEcho(
         logger.warn().log("could not get a shred version from an entrypoint");
         break :loop 0;
     };
-    const my_ip = try (config.current.gossip.getHost() orelse (my_ip_from_entrypoint orelse IpAddr.newIpv4(127, 0, 0, 1)));
+    const my_ip = (config.current.gossip.getHost() orelse
+        (my_ip_from_entrypoint orelse IpAddr.newIpv4(127, 0, 0, 1))) catch |err| {
+        logger.err().logf(
+            "Failed to parse IP in '--gossip-host {?s}' - {}",
+            .{ config.current.gossip.host, err },
+        );
+        return err;
+    };
     logger.info().logf("my ip: {}", .{my_ip});
     return .{
         .shred_version = my_shred_version,

diff --git a/src/cmd/config.zig b/src/cmd/config.zig
@@ -45,12 +45,12 @@ const GossipConfig = struct {
     dump: bool = false,
     trusted_validators: [][]const u8 = &.{},
 
-    pub fn getHost(config: GossipConfig) ?sig.net.SocketAddr.ParseIpError!IpAddr {
+    pub fn getHost(config: GossipConfig) ?sig.net.IpAddr.ParseIpError!IpAddr {
         const host_str = config.host orelse return null;
-        const socket = try sig.net.SocketAddr.parse(host_str);
+        const socket = try sig.net.IpAddr.parse(host_str);
         return switch (socket) {
-            .V4 => |v4| .{ .ipv4 = v4.ip },
-            .V6 => |v6| .{ .ipv6 = v6.ip },
+            .ipv4 => |v4| .{ .ipv4 = v4 },
+            .ipv6 => |v6| .{ .ipv6 = v6 },
         };
     }
 

diff --git a/src/net/net.zig b/src/net/net.zig
@@ -112,7 +112,7 @@ pub const SocketAddr = union(enum(u8)) {
         if (!parsed_ip) return error.InvalidIpv4;
 
         return Self{ .V4 = .{
-            .ip = Ipv4Addr.init(octs[0], octs[1], octs[2], octs[3]),
+            .ip = Ipv4Addr.init(octs),
             .port = addr_port,
         } };
     }
@@ -125,7 +125,7 @@ pub const SocketAddr = union(enum(u8)) {
             var octets: [4]u8 = undefined;
             random.bytes(&octets);
             return Self{
-                .V4 = .{ .ip = Ipv4Addr.init(octets[0], octets[1], octets[2], octets[3]), .port = pport },
+                .V4 = .{ .ip = Ipv4Addr.init(octets), .port = pport },
             };
         } else {
             var octets: [16]u8 = undefined;
@@ -138,7 +138,7 @@ pub const SocketAddr = union(enum(u8)) {
 
     pub fn initIpv4(octets: [4]u8, portt: u16) Self {
         return Self{
-            .V4 = .{ .ip = Ipv4Addr.init(octets[0], octets[1], octets[2], octets[3]), .port = portt },
+            .V4 = .{ .ip = Ipv4Addr.init(octets), .port = portt },
         };
     }
 
@@ -218,7 +218,7 @@ pub const SocketAddr = union(enum(u8)) {
             .ipv4 => |v4| {
                 return Self{
                     .V4 = SocketAddrV4{
-                        .ip = Ipv4Addr.init(v4.value[0], v4.value[1], v4.value[2], v4.value[3]),
+                        .ip = Ipv4Addr.init(v4.value),
                         .port = endpoint.port,
                     },
                 };
@@ -362,10 +362,8 @@ pub const Ipv4Addr = struct {
 
     const Self = @This();
 
-    pub fn init(a: u8, b: u8, c: u8, d: u8) Self {
-        return Self{
-            .octets = [4]u8{ a, b, c, d },
-        };
+    pub fn init(octets: [4]u8) Self {
+        return Self{ .octets = octets };
     }
 
     pub fn eql(self: *const Self, other: *const Self) bool {
@@ -453,12 +451,40 @@ pub const IpAddr = union(enum(u32)) {
 
     const Self = @This();
 
+    pub const ParseIpError = ParseIpv4Error || ParseIpv6Error;
+
+    pub fn parse(bytes: []const u8) ParseIpError!Self {
+        if (std.mem.indexOfScalar(u8, bytes, '.')) |_| {
+            // Probably IPv4.
+            return parseIpv4(bytes);
+        } else {
+            // Probably IPv6.
+            return parseIpv6(bytes);
+        }
+    }
+
+    pub const ParseIpv4Error = std.net.IPv4ParseError || error{UnexpectedPort};
+
+    pub fn parseIpv4(bytes: []const u8) ParseIpv4Error!Self {
+        if (std.mem.indexOfScalar(u8, bytes, ':')) |_| {
+            return error.UnexpectedPort;
+        }
+        const address = try std.net.Ip4Address.parse(bytes, 0);
+        return Self{ .ipv4 = Ipv4Addr.init(@bitCast(address.sa.addr)) };
+    }
+
+    pub const ParseIpv6Error = std.net.IPv6ParseError || error{UnexpectedPort};
+
+    pub fn parseIpv6(bytes: []const u8) ParseIpv6Error!Self {
+        if (std.mem.indexOfScalar(u8, bytes, ']')) |_| {
+            return error.UnexpectedPort;
+        }
+        const address = try std.net.Ip6Address.parse(bytes, 0);
+        return Self{ .ipv6 = Ipv6Addr.init(address.sa.addr) };
+    }
+
     pub fn newIpv4(a: u8, b: u8, c: u8, d: u8) IpAddr {
-        return .{
-            .ipv4 = Ipv4Addr{
-                .octets = [4]u8{ a, b, c, d },
-            },
-        };
+        return .{ .ipv4 = Ipv4Addr.init(.{ a, b, c, d }) };
     }
 
     pub fn asV4(self: *const Self) [4]u8 {
@@ -524,7 +550,7 @@ test "invalid ipv4 socket parsing" {
 test "valid ipv4 socket parsing" {
     const addr = "127.0.0.1:1234";
     const expected_addr = SocketAddr{ .V4 = SocketAddrV4{
-        .ip = Ipv4Addr.init(127, 0, 0, 1),
+        .ip = Ipv4Addr.init(.{ 127, 0, 0, 1 }),
         .port = 1234,
     } };
     const actual_addr = try SocketAddr.parseIpv4(addr);
@@ -573,3 +599,73 @@ test "parse IPv6 if IPv4 fails" {
         try std.testing.expectEqual(sa.V6, expected.V6);
     }
 }
+
+test "valid ipv4 address without port parsing" {
+    const addr = "127.0.0.1";
+    const expected_addr = IpAddr{ .ipv4 = Ipv4Addr.init(.{ 127, 0, 0, 1 }) };
+    const actual_addr = try IpAddr.parseIpv4(addr);
+    try std.testing.expectEqual(expected_addr, actual_addr);
+}
+
+test "valid ipv4 address with port inclusion parsing" {
+    const addr = "127.0.0.1:1234";
+    const result = IpAddr.parseIpv4(addr);
+    try std.testing.expectError(error.UnexpectedPort, result);
+}
+
+test "invalid ipv4 address without port parsing" {
+    const addr = "127.0.01";
+    const result = IpAddr.parseIpv4(addr);
+    try std.testing.expectError(error.NonCanonical, result);
+}
+
+test "valid ipv6 address without port parsing" {
+    const addr = try IpAddr.parseIpv6("FE38:DCE3:124C:C1A2:BA03:6745:EF1C:683D");
+    const expected = IpAddr{ .ipv6 = Ipv6Addr.init([16]u8{ '\xFE', '\x38', '\xDC', '\xE3', '\x12', '\x4C', '\xC1', '\xA2', '\xBA', '\x03', '\x67', '\x45', '\xEF', '\x1C', '\x68', '\x3D' }) };
+
+    try std.testing.expectEqual(addr, expected);
+}
+
+test "invalid ipv6 address with port inclusion parsing" {
+    const addr = "[FE38:DCE3:124C:C1A2:BA03:6745:EF1C:683D]:8000";
+    const result = IpAddr.parseIpv6(addr);
+    try std.testing.expectError(error.UnexpectedPort, result);
+}
+
+test "invalid ipv6 address without port parsing" {
+    try std.testing.expectError(error.InvalidCharacter, IpAddr.parseIpv6("FE38:DCEq:124C:C1A2:BA03:6745:EF1C683D"));
+}
+
+test "parse IPv6 addreess if IPv4 address fails" {
+    {
+        const addr = try IpAddr.parse("127.0.0.1");
+        const expected = IpAddr{ .ipv4 = Ipv4Addr.init(.{ 127, 0, 0, 1 }) };
+        try std.testing.expectEqual(addr.ipv4, expected.ipv4);
+    }
+
+    {
+        const addr = try IpAddr.parse("FE38:DCE3:124C:C1A2:BA03:6745:EF1C:683D");
+        const expected = IpAddr{ .ipv6 = Ipv6Addr.init([16]u8{ '\xFE', '\x38', '\xDC', '\xE3', '\x12', '\x4C', '\xC1', '\xA2', '\xBA', '\x03', '\x67', '\x45', '\xEF', '\x1C', '\x68', '\x3D' }) };
+
+        try std.testing.expectEqual(addr.ipv6, expected.ipv6);
+    }
+
+    {
+        const addr = try IpAddr.parse("::1");
+        const expected = IpAddr{ .ipv6 = Ipv6Addr.init([16]u8{ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, '\x01' }) };
+
+        try std.testing.expectEqual(addr.ipv6, expected.ipv6);
+    }
+
+    {
+        const addr = try IpAddr.parse("2001:0df8:00f2::06ee:0:0f11");
+        const expected = IpAddr{ .ipv6 = Ipv6Addr.init([16]u8{ '\x20', '\x01', '\x0D', '\xF8', 0, '\xF2', 0, 0, 0, 0, '\x06', '\xEE', 0, 0, '\x0F', '\x11' }) };
+        try std.testing.expectEqual(addr.ipv6, expected.ipv6);
+    }
+
+    {
+        const addr = try IpAddr.parse("2001:0df8:00f2::06ee:0:0f11");
+        const expected = IpAddr{ .ipv6 = Ipv6Addr.init([16]u8{ '\x20', '\x01', '\x0D', '\xF8', 0, '\xF2', 0, 0, 0, 0, '\x06', '\xEE', 0, 0, '\x0F', '\x11' }) };
+        try std.testing.expectEqual(addr.ipv6, expected.ipv6);
+    }
+}