Adds IPv6 support to `Sparoid::Server` by antondalgren · Pull Request #17 · 84codes/sparoid

antondalgren · 2026-01-27T12:28:47Z

WHY are these changes introduced?

We want to support ipv6

WHAT is this pull request doing?

Adds support for ipv6 by allowing the ip part of the message to be 16 bytes.

Config changes

nftablesv6-cmd a nftables command to run when a ipv6 packet is accepted.

HOW was this pull request tested?

Specs? Manual steps? Please fill me in.

carlhoerberg · 2026-01-28T05:43:37Z

-    end
+      def initialize(@version)
+        @ts = Time.utc.to_unix_ms
+        @nounce = uninitialized UInt8[16]


I know it's not introduced here but stack allocated variables (that are not copied) are dangerous to use as instances variables, pointer to unallocated stack memory will result in a seg fault

Suggested change

@nounce = uninitialized UInt8[16]

@nounce = Bytes.new 16

carlhoerberg · 2026-01-28T05:56:20Z

+          String.build(39) do |str|
+            8.times do |i|
+              str << ':' unless i == 0
+              str << @ip[i * 2].to_s(16).rjust(2, '0')


This allocated two new strings and then concat it to the third.

Suggested change

str << @ip[i * 2].to_s(16).rjust(2, '0')

str << '0' if @ip[i * 2] < 0x10

@ip[i * 2].to_s(str, 16)

carlhoerberg · 2026-01-28T06:02:10Z

-      self.new(version, ts, nounce, ip)
+    struct V2 < Base
+      getter family : UInt8
+      getter ip : StaticArray(UInt8, 16) | StaticArray(UInt8, 4)


Consider if it should be a Bytes instead

carlhoerberg · 2026-01-28T06:03:09Z

Are both the server and client backward compatible?

antondalgren · 2026-01-28T08:29:19Z

Are both the server and client backward compatible?

The server is backwards compatible.

I started to look at the client but couldn't figure out a good solution for that, so it's still in the works! Would it be reasonable to add a CLI arg to the client to enforce ipv4 (version 1)?

Or maybe always resort to a v1 message fallback at the end of this list? See comment at the end of the method.

    # Send to all resolved IPs for the hostname
    private def self.udp_send(host, port, data) : Array(String)
      host_addresses = Socket::Addrinfo.udp(host, port, Socket::Family::INET)
      socket = Socket.udp(Socket::Family::INET)
      Socket.set_blocking(socket.fd, true)
      host_addresses.each do |addrinfo|
        begin
          socket.send data, to: addrinfo.ip_address
        rescue ex
          STDERR << "Sparoid error sending " << ex.inspect << "\n"
        end
      end
      # Loop over all host_addresses using IPv4
      # Create a V1 message and send to all those hosts
      host_addresses.map &.ip_address.address
    ensure
      socket.close if socket
    end

Copilot

Pull request overview

Adds IPv6-awareness to Sparoid’s UDP message format and client/server plumbing so receivers can distinguish IPv4 vs IPv6 sources and apply different acceptance actions (e.g., nftables rules).

Changes:

Introduces Sparoid::Message versioning with new V2 format (adds IP family + optional CIDR range; supports IPv6).
Updates server callback signature to include Socket::Family, and extends CLI/config to optionally run a separate nftablesv6-cmd.
Updates client to generate/send multiple messages (V1 + V2) and adds IPv6 address discovery helpers + expanded specs/docs.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
src/server.cr	Updates server initialization/binding and passes IP family to accept callback.
src/server-cli.cr	Adds nftables IPv6 command path selection based on accepted packet family.
src/public_ip.cr	Changes public IP discovery to return IPv4/IPv6 strings and caches multiple results.
src/message.cr	Refactors message into versioned formats (V1 + new V2 with family/range + IPv6 string formatting).
src/ipv6.cr	Adds IPv6 interface inspection + CIDR inference via `getifaddrs`.
src/config.cr	Adds `nftablesv6-cmd` config parsing.
src/client.cr	Sends V1+V2 messages, prioritizes IPv6 resolution, and integrates IPv6 discovery.
spec/sparoid_spec.cr	Adjusts expectations for multiple packets per send and updated callback signature.
spec/message_spec.cr	Adds comprehensive tests for V1/V2 parsing/serialization and IPv6 formatting.
README.md	Updates example configuration and nftables rules for IPv6 support.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…low for ipv6 addresses.

oskgu360

Looks good, Server team tested it out on a CloudAMQP machine to verify its backwardscompatible to the old clients (ruby and crystal). And working with ipv6.

My mind got stuck on what a Message really is, so wanted to rename it to something else more decriptive, but messy to change and I have no good suggestions (PortKnockingProtocolPacket? 😀 ). So maybe better to just keep, once I got familiar with the code, it made sense.

I have hard time giving a proper review on the low level stuff, so if you want opinions there, someone else has to hehe. But it works, and this app isn't that performance sensitive, so good to go imo.

Good job! 🚀

baelter · 2026-02-19T15:35:46Z

About the client/server compatibility,

Currently, both the client and server are backwards compatible with each other—a new client can speak to an old server, and vice versa. This increases complexity, since clients must handle both old and new handshake/nonce formats. It can become very complex and limiting if we always want to keep 2-way comparability.

Do we actually need this? If we instead require that servers are always upgraded before clients (which is quite common), we could simplify things so only servers are backwards compatible:

The server accepts both old and new client formats.
The client only needs to support the new (v2) format.
No need for clients to send multiple nonce formats or support handshake fallbacks.
We would just need to document that upgrading the server before the clients is required.

Would this be a reasonable change to make, so clients only need to send the v2 format?

antondalgren · 2026-02-20T07:45:30Z

About the client/server compatibility,

Currently, both the client and server are backwards compatible with each other—a new client can speak to an old server, and vice versa. This increases complexity, since clients must handle both old and new handshake/nonce formats. It can become very complex and limiting if we always want to keep 2-way comparability.

Do we actually need this? If we instead require that servers are always upgraded before clients (which is quite common), we could simplify things so only servers are backwards compatible:

The server accepts both old and new client formats.

The client only needs to support the new (v2) format.

No need for clients to send multiple nonce formats or support handshake fallbacks.

We would just need to document that upgrading the server before the clients is required.

Would this be a reasonable change to make, so clients only need to send the v2 format?

Well, I had that solution in the first iteration of the PR. Can't really seem to figure out why I went with the client keep sending the v1 format if I'm being honest. Maybe the following comment tricked me :D

Are both the server and client backward compatible?

If there is a problem with the client sending the v2 format all it has to do is to downgrade the client version. Let's revert the client changes to only send v2 messages.

Remove v1 message generation from the client. Servers must be upgraded before clients to maintain backwards compatibility.

carlhoerberg · 2026-02-23T09:32:11Z

I pushed a couple of suggestions.

But I'm also thinking if we can simply quite a lot by always sending a IPv6 address (StaticArray(UInt8, 16), or UInt128) in the V2 message. And then on the server side see if it's a ipv6 mapped ipv4 address (starting with ::ffff) or a real ipv6 address.

IPv4 addresses are stored as IPv4-mapped IPv6 (::ffff:x.x.x.x). Family is now a computed property checking the prefix instead of a stored field. Added from_ip overloads for StaticArray(UInt8, 4), StaticArray(UInt8, 16), and StaticArray(UInt16, 8), removing the encode_ip helper from the client.

IPv4 ranges are stored +96 (e.g. /32 becomes /128) to match the IPv4-mapped IPv6 address. The range getter subtracts 96 for IPv4, and serialization always writes family=6 with all 16 IP bytes. Old family=4 messages are still handled in from_io for compat.

Replace Channel-based fiber synchronization with WaitGroup in fdpass. Simplify UDPSocket creation and remove redundant .try on close. Relax public_ip parameter type and add comments to generate_messages.

Move IP string parsing into Message::V2.from_ip(String) so client.cr can pass IP strings directly instead of parsing them first. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Replace complex platform-specific interface enumeration (getifaddrs on macOS, netlink on Linux) with a simple UDP connect to get the OS-selected outgoing IPv6 address. Fall back to icanhazip.com if no global address. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Range/CIDR is no longer needed since we send single IP addresses. Simplifies V2 serialization (46→45 bytes plaintext, still 96 bytes encrypted), removes verify_ip_range from server, and cleans up all from_ip overloads. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Simplify to one message type where IP is 4 or 16 bytes, determined by plaintext size after decryption. Both produce 96-byte encrypted packets. IPv4-mapped IPv6 (::ffff:x.x.x.x) is normalized to plain IPv4 so the correct nftables command is used. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Filter out reserved/non-global ranges per IETF RFC 4291, RFC 6890 in the UDP connect probe. Based on Rust std::net::Ipv6Addr::is_global. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

oskgu360

Looks good to me!

baelter · 2026-03-10T08:29:59Z

Is the PR description still valid?

antondalgren · 2026-03-10T08:33:32Z

Is the PR description still valid?

updated! :)

walro · 2026-03-10T09:35:30Z

Will copilot check after merge? 🧪

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-10T09:45:17Z

+        Array(String).new.tap do |ips|
+          while line = file.gets
+            ips << line
+          end
+        end


read_cache appends lines from the cache file without stripping newlines/whitespace. That can produce values like "1.2.3.4\n", which will fail strict IP parsing later (e.g., Message.from_ip). Strip/chomp each line (and consider skipping blank lines) before pushing to ips.

Copilot · 2026-03-10T09:45:17Z

+        wg.spawn do
+          ipaddr = Socket::IPAddress.new(ip, port)
+          socket = TCPSocket.new ipaddr.family
+          socket.connect(ipaddr, timeout: 10)
          FDPass.send_fd(1, socket.fd)
-          # exit as soon as possible so no other fiber also succefully connects
-          exit 0
-        rescue
-          ch.send(nil)
+          exit 0 # exit as soon as possible so no other fiber also succefully connects
        end


fdpass no longer rescues connection errors or closes sockets on failure. Unhandled exceptions in the spawned fibers can spam logs and a failed connect can leak the socket until GC. Wrap the connect/send in begin/rescue/ensure (closing the socket in ensure) and treat failures as a normal outcome before wg.wait.

Copilot · 2026-03-10T09:45:17Z

+  def self.global?(ip : String) : Bool
+    s = Socket::IPAddress.parse_v6_fields?(ip)
+    return false unless s
+    return false if unspecified?(s) || loopback?(s)
+    return false if ipv4_mapped?(s)
+    return false if ipv4_ipv6_translation?(s)
+    return false if discard_only?(s)
+    return false if ietf_protocol_non_global?(s)
+    return false if sixto4?(s)


IPv6.global? currently returns true for multicast IPv6 ranges (ff00::/8), e.g. ff02::1, because there is no multicast check. Add an explicit multicast exclusion (and any other non-global ranges you intend to match Rust’s Ipv6Addr::is_global) so the predicate matches its documentation.

Copilot · 2026-03-10T09:45:18Z

+    # Send to all resolved IPs for the hostname, prioritizing IPv6
+    private def self.udp_send(host, port, key : String, hmac_key : String, public_ip : String? = nil) : Array(String)
+      host_addresses = Socket::Addrinfo.udp(host, port)
      host_addresses.each do |addrinfo|


The comment says udp_send is "prioritizing IPv6", but Socket::Addrinfo.udp(host, port) relies on resolver/system ordering and doesn’t guarantee IPv6-first. Either enforce the intended ordering (e.g., query INET6 then INET) or adjust the comment to avoid promising behavior the code doesn’t ensure.

Copilot · 2026-03-10T09:45:18Z

+  # Check if an IPv6 address is globally reachable.
+  # Based on Rust std::net::Ipv6Addr::is_global (IETF RFC 4291, RFC 6890, etc.)
+  # ameba:disable Metrics/CyclomaticComplexity
+  def self.global?(ip : String) : Bool
+    s = Socket::IPAddress.parse_v6_fields?(ip)
+    return false unless s
+    return false if unspecified?(s) || loopback?(s)
+    return false if ipv4_mapped?(s)
+    return false if ipv4_ipv6_translation?(s)
+    return false if discard_only?(s)
+    return false if ietf_protocol_non_global?(s)
+    return false if sixto4?(s)
+    return false if documentation?(s)
+    return false if segment_routing?(s)
+    return false if unique_local?(s)
+    return false if link_local?(s)
+    true


New IPv6 classification logic (IPv6.global? and helpers) is non-trivial but has no spec coverage. Add unit specs for representative addresses (multicast, unique-local, link-local, documentation, global unicast) to guard against regressions and ensure the predicate matches the intended RFC/Rust behavior.

Copilot · 2026-03-10T09:45:18Z

+    def self.by_http : Array(String)
+      with_cache do
+        ips = URLS.compact_map do |url|
+          resp = HTTP::Client.get(url)
+          next unless resp.status_code == 200
+          resp.body.chomp
+        rescue
+          nil
+        end
+        raise "No valid response from icanhazip.com" if ips.empty?


PublicIP.by_http fetches the client’s public IP over plain HTTP from icanhazip.com using HTTP::Client.get, and this value is then used by Sparoid::Client to decide which remote address is authorized. An attacker who can MITM or hijack DNS for this HTTP request can forge a different IP address and cause the server to open access for an unauthorized host. Use HTTPS (with certificate validation) or another integrity-protected mechanism for obtaining the public IP before using it in security-sensitive firewall rules.

antondalgren · 2026-03-10T09:51:17Z

@copilot open a new pull request to apply changes based on the comments in this thread

walro · 2026-03-10T13:11:25Z

        end
      end
-      ips.size.times { ch.receive }
+      wg.wait


since we never call #done, won't it get stuck here, or how does it work?

def spawn(*, name : String? = nil, &block) : Fiber
Increment the counter by 1, perform the work inside the block in a separate fiber, decrementing the counter after it completes or raises. Returns the Fiber that was spawned.

wg = WaitGroup.new wg.spawn { do_something } wg.wait

It will wait until all fibers spawned by the waitgroup have finished.

Ah thanks, I was reading https://crystal-lang.org/api/1.19.1/WaitGroup.html#overview, but I see now it has a number in the constructor, is that what makes it different?

When looking at the source code for WaitGroup that looks to be the case. The n is setting a start value for the counter, and spawn is wrapped around a add and done call. So, effectively, the n counter seems redundant 🤔

I think that may have been the case before the #spawn method was introduced!

antondalgren force-pushed the server-ipv6 branch from 3ab1f42 to 437064c Compare January 27, 2026 12:45

antondalgren requested review from baelter, carlhoerberg and spuun January 27, 2026 12:55

carlhoerberg reviewed Jan 28, 2026

View reviewed changes

antondalgren marked this pull request as ready for review February 3, 2026 13:22

antondalgren requested a review from a team as a code owner February 3, 2026 13:22

antondalgren mentioned this pull request Feb 9, 2026

Add IPv6 support 84codes/sparoid.rb#16

Merged

oskgu360 requested a review from Copilot February 10, 2026 12:03

Copilot started reviewing on behalf of oskgu360 February 10, 2026 12:03 View session

Copilot AI reviewed Feb 10, 2026

View reviewed changes

Adds ipv6 support to server. It introduces a new message format to al…

564e140

…low for ipv6 addresses.

antondalgren force-pushed the server-ipv6 branch from bfa0286 to 564e140 Compare February 10, 2026 13:33

antondalgren added 2 commits February 10, 2026 14:40

Update test with proper cidrrange

109a2a9

Validate cidr range, and make nftablesv6 usable on it own

a2434b0

oskgu360 approved these changes Feb 10, 2026

View reviewed changes

Comment thread src/server.cr

Comment thread README.md

Comment thread src/client.cr Outdated

antondalgren and others added 3 commits February 10, 2026 16:44

Better boolean description

3ed4afb

return nil if we cant resolve the ip

7dfa36d

Merge branch 'main' into server-ipv6

a766b00

antondalgren mentioned this pull request Feb 12, 2026

Add IPv6 support 84codes/sparoid-rs#8

Merged

Remove redundant test

1893de2

antondalgren requested a review from carlhoerberg February 16, 2026 12:19

antondalgren added 3 commits February 20, 2026 09:29

Test server backwards compatibility with v1 and v2 messages

31f17a5

Client only sends v2 messages

ec8a24d

Remove v1 message generation from the client. Servers must be upgraded before clients to maintain backwards compatibility.

Lint: use short block syntax

d7a400b

antondalgren requested a review from oskgu360 February 20, 2026 09:05

carlhoerberg and others added 4 commits February 23, 2026 11:04

Simplify client: use WaitGroup, clean up UDP send logic

76244e6

Replace Channel-based fiber synchronization with WaitGroup in fdpass. Simplify UDPSocket creation and remove redundant .try on close. Relax public_ip parameter type and add comments to generate_messages.

Add V2.from_ip(String) overload, simplify client IP handling

7387984

Move IP string parsing into Message::V2.from_ip(String) so client.cr can pass IP strings directly instead of parsing them first. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

antondalgren commented Feb 23, 2026

View reviewed changes

Comment thread src/message.cr Outdated

antondalgren commented Feb 23, 2026

View reviewed changes

Comment thread src/message.cr Outdated

antondalgren and others added 8 commits March 2, 2026 11:53

Strip range from V2 IPv4 ip_string, validate server-side

34f1075

Narrow down msg type

84c8534

If local target host is ipv6, send ipv6 address

4f684ad

Fix formatting and ameba lint

d5b1f9f

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add comprehensive IPv6 global address check

b15a486

Filter out reserved/non-global ranges per IETF RFC 4291, RFC 6890 in the UDP connect probe. Based on Rust std::net::Ipv6Addr::is_global. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

oskgu360 approved these changes Mar 9, 2026

View reviewed changes

antondalgren requested a review from baelter March 10, 2026 07:53

remove interval from nftables config example

7c87b5f

baelter approved these changes Mar 10, 2026

View reviewed changes

antondalgren merged commit 3def118 into main Mar 10, 2026
24 of 25 checks passed

antondalgren deleted the server-ipv6 branch March 10, 2026 09:22

walro requested a review from Copilot March 10, 2026 09:35

Copilot started reviewing on behalf of walro March 10, 2026 09:35 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

antondalgren mentioned this pull request Mar 10, 2026

Address PR review feedback #21

Merged

walro reviewed Mar 10, 2026

View reviewed changes

	str << @ip[i * 2].to_s(16).rjust(2, '0')
	str << '0' if @ip[i * 2] < 0x10
	@ip[i * 2].to_s(str, 16)

Uh oh!

Conversation

antondalgren commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

WHY are these changes introduced?

WHAT is this pull request doing?

Config changes

HOW was this pull request tested?

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carlhoerberg commented Jan 28, 2026

Uh oh!

antondalgren commented Jan 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oskgu360 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

baelter commented Feb 19, 2026

Uh oh!

antondalgren commented Feb 20, 2026

Uh oh!

carlhoerberg commented Feb 23, 2026

Uh oh!

Uh oh!

Uh oh!

oskgu360 left a comment

Choose a reason for hiding this comment

Uh oh!

baelter commented Mar 10, 2026

Uh oh!

antondalgren commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

walro commented Mar 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

antondalgren commented Jan 27, 2026 •

edited

Loading

antondalgren commented Mar 10, 2026 •

edited

Loading