Conversation
190cb60 to
7cfef73
Compare
7cfef73 to
1fd0949
Compare
The c-ares integration created every UDP socket as AF_INET and rejected any AF_INET6 sockaddr with "No ipv6 yet", so a host whose resolv.conf lists an IPv6 nameserver could not resolve anything: c-ares reported ARES_ECONNREFUSED for every server. That is what an IPv6-only Kubernetes pod looks like (CoreDNS over IPv6), and what an IPv6-only VM with a link-local or ULA resolver looks like. - do_socket creates the datagram channel with the family c-ares asked for instead of hardcoding AF_INET. - sock_addr accepts AF_INET6 and checks the sockaddr length. - do_recvfrom copies the source address by its real length; sockaddr is only wide enough for AF_INET, so an IPv6 source was being truncated and its length misreported. - options.servers is installed with ares_set_servers_csv after channel creation; ARES_OPT_SERVERS carries in_addr only. - ARES_ECONNREFUSED is rendered with c-ares' own text, "Could not contact DNS servers", so it stops reading like a refused TCP connect. IPv4 nameservers take the same AF_INET branches as before. Adds a unit test that answers a query from a mock nameserver bound to [::1].
get_host_by_name short-circuited numeric literals only when no address
family was requested. With a family set the literal went to c-ares,
which cannot parse a scope id ("fe80::1%eth0" turned into a real DNS
query that failed with ARES_ENOTFOUND) and which returns an IPv4 node
for an AF_INET6 request, so the family was not a filter for literals.
Parse literals with inet_address::parse_numerical regardless of the
requested family. A literal of the other family fails with
ARES_EBADFAMILY, matching what the resolver does for names that only
have records of the other family.
The error category kept a hand-written copy of c-ares' error table
that stopped at ARES_ECANCELLED, so ARES_ESERVICE and ARES_ENOSERVER
("No DNS servers were configured", the result of every configured
server being unusable) rendered as "Unknown error". Use ares_strerror.
ares_socket_functions_ex had aif_nametoindex/aif_indextoname set to
null. c-ares uses them to resolve the %iface suffix of link-local
nameservers and silently drops such servers without them, so a
resolv.conf of "nameserver fe80::1%eth0" lost its only server.
The mock nameserver only ever answered A records, so make_hostent's AF_INET6 branches, reverse lookups of 16-byte addresses, the TCP query path over an IPv6 transport and numeric-literal handling with an explicit family had no coverage. Drive the mock by the question type (A, AAAA, PTR) and add: - test_resolve_aaaa_from_ipv6_nameserver - test_reverse_lookup_ipv6_from_ipv6_nameserver (ip6.arpa PTR) - test_resolve_tcp_ipv6_nameserver - test_resolve_numeric_with_family (literals never reach c-ares; a literal of the other family fails; a %scope suffix is preserved) All IPv6 cases skip when the host has no IPv6 loopback.
Whether a wildcard [::] listener also accepts IPv4 clients (as IPv4-mapped IPv6 peers) has so far followed the host's net.ipv6.bindv6only default, and a [::]:P listener could never share a port with a 0.0.0.0:P one. Add listen_options::ipv6_only. When set, posix_listen applies it with IPV6_V6ONLY before bind on AF_INET6 sockets; other families are untouched and the default (unset) keeps today's behaviour. Tested by a listener on [::]:0 reached from 127.0.0.1 with the option false (accepted, peer ::ffff:127.0.0.1) and true (ECONNREFUSED).
An IPv4 client of a dual-stack [::] listener is reported by the kernel as ::ffff:a.b.c.d. inet_address already unwrapped that form in its in_addr conversion but nowhere else: operator==, hashing and is_loopback() treated it as a different address from a.b.c.d, and http_server / rpc::server handed the mapped form to handlers and filter_connection, so address-keyed policies written as IPv4 never matched. Add inet_address::is_ipv4_mapped() and unmapped() (the in_addr conversion now reuses them), socket_address::unmapped(), and normalise the peer and local addresses http_server and rpc::server record. Non-mapped addresses of either family are returned unchanged.
…INFO The control buffer posix_datagram_channel hands to recvmsg was declared as `struct cmsghdrcmh;` — a forward declaration of a nested type, not a cmsghdr member — so it was 20 bytes where CMSG_SPACE(in_pktinfo) is 28 and CMSG_SPACE(in6_pktinfo) is 36. The kernel truncated every pktinfo message (MSG_CTRUNC, never checked) and receive() then read a whole in_pktinfo past the end of the buffer. On top of that only IP_PKTINFO was ever enabled, so an AF_INET6 socket never saw the IPv6 destination at all, and msg_namelen was neither reset between receives nor copied back into the source address, whose length() stayed at sizeof(sockaddr_storage). Size the buffer with CMSG_SPACE, reset msg_namelen/msg_controllen before each recvmsg, skip cmsg parsing on MSG_CTRUNC, request IPV6_RECVPKTINFO on AF_INET6 sockets (IP_PKTINFO stays for IPv4-mapped traffic) and record the source address length the kernel reports. datagram::get_dst() is now asserted unconditionally in udp_packet_test, and two new cases receive on a wildcard socket of each family and check the destination and source-length the pktinfo message provides.
inet_address::invalid_scope (0xffffffff) is an in-memory "no zone" marker, but socket_address wrote it straight into sin6_scope_id, and addr() copied the kernel's sin6_scope_id (0 for the common no-zone case) straight back into _scope. Every accepted IPv6 peer therefore printed as [addr%0]:port and reported scope() == 0, while connect()/sendto() were handed a bogus zone for link-local destinations. Translate at the boundary: write 0 for invalid_scope, read 0 as invalid_scope, and key the link-local zone lookup in resolve_outgoing_address on sin6_scope_id == 0. Also fix the inet_pton result check in the /proc/net/ipv6_route parser (0 means 'not an address', not < 0), pin the sin_port/sin6_port aliasing that port() relies on with a static_assert, and document that socket_address equality deliberately ignores the zone.
…rts must fit
inet_address::parse_numerical dropped a %zone that matched no interface
on the box, leaving the literal silently unscoped, and accepted a zone on
an IPv4 literal. Follow RFC 4007 §11 instead: an interface name that
matches nothing makes the literal invalid, and a numeric zone is kept as
given so literals written for another host survive.
ipv6_addr(const std::string&) converted the port with std::stoul and a
bare uint16_t cast, so "[::1]:70000" became port 4464. Parse the
bracketed form explicitly, range-check the port and reject stray text
after the bracket; an unbracketed string is the address alone ("::1:9092"
is a valid address, so it cannot carry a port). Zoned literals still
throw: ipv6_addr has no field for a zone, which the header now says.
tls_options::server_name is the name verification checks the certificate against, and callers that connect by address pass the address. Both backends copied it verbatim into the server_name extension, which RFC 6066 §3 forbids for IPv4 and IPv6 literals; servers that validate the extension reject the ClientHello. Skip the extension for a literal (bracketed or not) in both the OpenSSL and the GnuTLS backend. Verification is unchanged: the literal is still matched against the certificate's IP SANs. The GnuTLS call's return value is now checked instead of dropped. Tested by a plain socket that reads the ClientHello a client sends and looks for the extension: DNS names appear, IPv4/IPv6 literals do not.
The GnuTLS backend passes server_name to gnutls_certificate_verify_peers3, so a certificate that chains to a trusted CA but is issued for a different host (or a different IP SAN) fails verification. The OpenSSL backend only ever checked the chain: SSL_VERIFY_PEER with no expected host, so any trusted certificate was accepted for any server_name. Add tls_options::verify_server_name. On OpenSSL it registers the expected identity on the session's X509_VERIFY_PARAM — an IP literal (brackets and zone stripped) via X509_VERIFY_PARAM_set1_ip_asc, a DNS name via SSL_set1_host without partial wildcards — so the mismatch surfaces through SSL_get_verify_result and the existing verify() path. It defaults to false: clients that connect by address to servers whose certificates carry no IP SAN keep working until they opt in. GnuTLS keeps its always-on check; the header documents the difference. The test certificate gains an IP SAN for ::1 next to 127.0.0.1, and test_alt_names asserts both values. New cases run the echo test with the flag set: matching DNS name and IP literals (127.0.0.1, ::1, [::1]) pass, a wrong DNS name and wrong addresses raise verification_error.
Every IPv6 test gates on engine().net().supports_ipv6() and skips with a log line when it is false. supports_ipv6() is a probe that swallows every exception, so a regression in IPv6 socket setup turns the whole IPv6 part of the suite into silent passes — which is exactly what happened while developing the previous commits: an int-sized socket option passed as a bool made the probe fail and hid the breakage behind green runs. Add tests/unit/ipv6_support.hh with ipv6_available_or_skip(): the same skip by default, a failure when SEASTAR_TEST_REQUIRE_IPV6 is set in the environment, for CI and containers that are known to have IPv6. Use it at every gate in ipv6_test, dns_test, httpd_test and tls_test.
ipv4_addr has operator==, std::hash and make_ipv4_address(); ipv6_addr had none of them, so a caller working in IPv6 could not put an address in a hash map or spell a wildcard without knowing that socket_address(port) is IPv4-only. Add operator== and std::hash for ipv6_addr, make_ipv6_address(), and socket_address::wildcard(family, port) for an any-address of an explicit family. Delete std::hash<::sockaddr_in> and operator==(sockaddr_in, sockaddr_in) from reactor.hh: nothing uses them, and they read only the IPv4 arm of what callers hold as a union. Also drop the stale FIXME on network_stack::connect — the local address has not assumed IPv4 since the ephemeral-port loop started taking its family from the destination.
posix_network_stack::listen rewrote a default-constructed (AF_UNSPEC)
socket_address to the IPv4 wildcard, so listen(socket_address{}) — "any
address, any port" — bound 0.0.0.0 and was unreachable on an IPv6-only
host, with no way to ask for any family.
Bind the IPv6 wildcard instead where the stack has IPv6, with IPV6_V6ONLY
off so IPv4 peers still arrive, and keep the IPv4 wildcard on a host
without IPv6. A caller who wants exactly one family passes that family's
wildcard (socket_address::wildcard) or sets listen_options::ipv6_only,
which is honoured here if it was set explicitly.
Tested by listening on socket_address{} and connecting from 127.0.0.1:
the listener is AF_INET6 and on the any-address, and the peer reads as
IPv4 after unmapping.
The socket_address to ipv4_addr conversion is implicit and was noexcept, but it delegates to ipv4_addr(const inet_address&, uint16_t), which throws for an IPv6 address that is not IPv4-mapped — so passing an IPv6 socket_address anywhere an ipv4_addr is expected terminated the process instead of raising std::invalid_argument. Drop the noexcept. The only caller inside a noexcept function is socket_address::is_wildcard(), in its AF_INET arm, where the conversion cannot throw; that is now commented. The native stack's make_bound_datagram_channel checked nothing and reached an assert on a non-AF_INET address, while its unbound counterpart throws: make it throw too.
request::make takes the Host header as a string, so a caller with a
socket_address formats it, and the obvious "{}:{}" of address and port
produces "fd00::5:8080" for IPv6 — a different address, not an authority.
Seastar had no formatter to reach for; Redpanda's own http client has the
same defect while its RPC client brackets correctly, so the two disagree.
Add http::internal::format_authority(), which brackets an IPv6 address per
RFC 3986 §3.2.2 and drops a zone index (an authority cannot carry one),
and request::make overloads taking a socket_address.
bba3b2b to
7b496e2
Compare
|
Thanks David. I think things like "commit 12" should include instead the commit message, or the number should be in the message since otherwise it's hard to know what commit it refers to (and will change as the work is re-written). So is this currently tested in GHA in this repo? I.e., does GHA support IPv6? |
|
I've mainly tested IPV6 against real clusters but I can check if its happening via GHA in this repo. I'll include commit number in the message no problem. |
I think this is an API change, we probably need to leave this. "in tree" is not a sufficient search as sesatar is a library used by other applications which may use these. Though Avi may disagree and say we should not provide these methods on "3rd party types". |
|
Closing this in favour of a stack of ten smaller PRs on current upstream master, one feature each. Each PR is based on the one before it, so its diff shows only its own commits:
What changed from this PR, including the review here:
|
Why
Users want to run on IPv6 VPCs: a VPC has far more IPv6 addresses than IPv4. On Kubernetes every pod takes one, so a large cluster runs out of private IPv4 space. AWS gives each VPC an IPv6 /56 and EKS can run IPv6-only pods; GKE and AKS do dual-stack.
To form a cluster there, a node has to resolve its peers through an IPv6 nameserver and connect over IPv6. That does not work today: the resolver opens
AF_INETsockets and refuses an IPv6 nameserver, so every lookup fails and the cluster never forms. We hit it with Redpanda on IPv6-only EKS (redpanda-operator#1855). The later commits cover dual-stack, where clients stay on IPv4 while the cluster talks IPv6.This started on Kubernetes, but nothing here is Kubernetes- or Redpanda-specific. The changes are in Seastar's networking layer and depend only on the address families involved, so they are tested on IPv6-only VMs as well as on IPv6-only Kubernetes.
What
Preview of a series for
scylladb/seastar: Seastar's networking layer works over IPv6 only where the code path was exercised — TCP listen/connect/accept on[::]/::1— and stops at the first hostname lookup, dual-stack listener or IP-literal TLS name. This is one PR with one small, independently revertable feature per commit, in dependency order; every commit carries its own tests and its message says what was broken and how it was checked.DNS (c-ares glue)
674838457) — the resolver opened its UDP/TCP sockets asAF_INETand threw "No ipv6 yet" for anAF_INET6server address, so every lookup on an IPv6-only host failed withARES_ECONNREFUSED. Sockets follow the nameserver's family; servers are handed to c-ares viaares_set_servers_csv, which accepts both families.c288f02ed) — the literal shortcut ran only when no family was requested; with one,fe80::1%eth0reached c-ares and failed. The requested family now acts as a filter (ARES_EBADFAMILYon mismatch).dafabe7f9) —ares_strerrorinstead of a hand-written table that stopped atARES_ECANCELLED;aif_nametoindex/aif_indextonameso link-local nameservers survive.654fa05b9) — AAAA answers, ip6.arpa PTR, TCP transport to[::1], literals per family; a QTYPE-driven mock nameserver.Sockets and addresses
bdef5c04e) —listen_options::ipv6_only; a[::]listener no longer inheritsnet.ipv6.bindv6only.c4593cef5) —inet_address::is_ipv4_mapped()/unmapped(),socket_address::unmapped();http_serverandrpc::serverreport IPv4 clients of a dual-stack listener asa.b.c.d, not::ffff:a.b.c.d, so address-keyed policies match.b1fbb45ff) —struct cmsghdrcmh;was a forward declaration, not a member: a 20-byte control buffer,MSG_CTRUNCon every datagram, an 8-byte over-read inreceive(), and no IPv6 destination ever.datagram::get_dst()is now correct for both families andget_src().length()is the real length.e3835a1e3) — accepted peers printed as[addr%0]:port; the in-memory sentinel was written intosin6_scope_id.f4a5233b0) — unknown%zonenames are rejected instead of silently dropped, numeric zones are kept (RFC 4007 §11),[::1]:70000no longer becomes port 4464.TLS
c050fda5c) — RFC 6066 §3, both backends; verification still matches literals against IP SANs, andserver_nameis normalized once (brackets and zone stripped) for both of them.787387aa6) —tls_options::verify_server_name: OpenSSL only checked the chain, GnuTLS always checked the name but was handedserver_nameverbatim, so a bracketed[::1]never matched an IP SAN; default off so clients connecting by address to certs without IP SANs are unaffected. Test cert gains anIP:::1SAN.ce560b379) —supports_ipv6()swallows exceptions, so a broken IPv6 socket setup made every IPv6 test skip and pass; caught abool-sizedsetsockopt(IPV6_RECVPKTINFO)in an earlier version of commit 7.Address and HTTP API gaps the audit turned up alongside
fb41e5462) —operator==andstd::hashforipv6_addr,make_ipv6_address(),socket_address::wildcard(family, port); deletes the unusedstd::hash<::sockaddr_in>/operator==pair, which read only the IPv4 arm of a union, and the staleconnect()FIXME.705f4598f) —listen(socket_address{})rewrote AF_UNSPEC to0.0.0.0and was unreachable on an IPv6-only host; it now binds[::]withIPV6_V6ONLYoff (IPv4 peers still arrive), or0.0.0.0where the stack has no IPv6.46c788775) — the implicit conversion wasnoexceptbut delegates to a throwing constructor, so an IPv6socket_addressreaching anipv4_addrparameter calledstd::terminate. Also makes the native stack'smake_bound_datagram_channelreject a non-AF_INET address like its unbound counterpart, instead of reaching an assert.7b496e252) —http::internal::format_authority()(RFC 3986 §3.2.2 brackets, zone dropped) andrequest::makeoverloads taking asocket_address, so callers stop producingfd00::5:8080.IPv4 compatibility
An IPv4-only deployment sees no behavioural change. Every new knob is off by default (
listen_options::ipv6_onlyisstd::optional<bool>, unset;tls_options::verify_server_nameisfalse), the IPv4 arms ofinet_address'soperator==,std::hash,is_loopback(),is_addr_any()andsocket_address::addr()are untouched,unmapped()is the identity for anything that is not::ffff:a.b.c.d,IP_PKTINFOis still requested on every INET socket, andipv4_addr(const std::string&)is not touched at all. The whole diff to IPv4-reachable code is:IP_PKTINFOset with anintinstead of abool(same value, correct width), and the IPv4 wildcard now spelledsocket_address::wildcard(AF_INET, port).Four behaviour changes are intentional, all reachable only by asking for something that was previously wrong or ill-defined:
INET6ARES_EBADFAMILY[::1]:70000uint16_twrap)std::invalid_argumentfe80::1%no-such-ifaceserver_nameTwo changes to the API surface, both source-level:
std::hash<::sockaddr_in>andoperator==(::sockaddr_in, ::sockaddr_in)are deleted fromcore/reactor.hh. Nothing in the tree uses them and they read only the IPv4 arm of what callers hold as a union; say the word and they can stay.ipv4_addr(const socket_address&)losesnoexcept, because it always could throw — it delegates to a throwing constructor, so an IPv6 argument calledstd::terminateinstead of raising. Calls are unaffected; only code that requires the conversion to benoexceptwould notice.listen_optionsandtls_optionseach gain a field, which is an ABI change but not a source one.Dual-stack (
[::]) listeners do change: an IPv4 peer is now reported tohttp_serverandrpc::serverhandlers asa.b.c.drather than::ffff:a.b.c.d,listen(socket_address{})binds[::]instead of0.0.0.0where the host has IPv6, and an accepted IPv6 peer no longer formats with a%0scope. That is the point of the series, and each of those has a test.How to review
The commits are in dependency order and each builds and tests on its own, so
git log -p --reversereads top to bottom. If you'd rather sample:src/net/dns.ccis where a family-agnostic resolver has to be, and the mock nameserver in commit 4 is how it is tested without a network.recvmsgcontrol buffer and enables a new socket option.struct cmsghdrcmh;on the old line 955 was a forward declaration, not a member — worth confirming that reading, since it means the buffer was 20 bytes whereCMSG_SPACE(in_pktinfo)is 28 and the code then read a wholein_pktinfoout of it.verify_server_name). It is off by default; the question for you is whether OpenSSL should instead match GnuTLS and verify wheneverserver_nameis set. I kept the flag because turning it on unconditionally would break clients that connect by IP to certificates without an IP SAN.noexceptremoval described above.To run it:
./configure.py --mode=release --c++-standard=23 # + --cook fmt on distros with fmt < 10 ninja -C build/release tests/unit/{dns,ipv6,socket,rpc,httpd,tls,network_interface,websocket}_test SEASTAR_TEST_REQUIRE_IPV6=1 ./build/release/tests/unit/dns_test -- -c2SEASTAR_TEST_REQUIRE_IPV6=1(commit 12) turns "this host has no IPv6, skipping" into a failure, which is what you want on a machine that does have IPv6 — without it the IPv6 half of the suite can pass by not running.tls_testneeds-c2:test_reload_certificates_with_only_shard0_notifysubmits to shard 1 and hangs on a single shard (pre-existing, not from this series). For the TLS backend not built by default, configure a second tree with-DSeastar_OPENSSL=ON -DSeastar_GNUTLS=OFF.To see a commit actually fix something, revert its source hunk and keep its test: the resolver tests fail with
std::invalid_argument: Servers must be ipv4 addresses, and dropping theexpect_peer_name()call in commit 11 makestest_verify_server_name_dns_mismatchreport "Should have gotten validation error" — i.e. today OpenSSL accepts any trusted certificate for any name.Testing
Unit suites on Ubuntu 24.04 / GCC 14 / C++23 with
-c2,SEASTAR_TEST_REQUIRE_IPV6=1so no IPv6 case may skip, on both TLS backends:ipv6_testnetwork_interface_testdns_testsocket_testrpc_testwebsocket_testhttpd_testtls_test0 skips is the point of commit 12: with
SEASTAR_TEST_REQUIRE_IPV6set, an IPv6 case that cannot run is a failure, not a log line.tls_testneeds-c2:test_reload_certificates_with_only_shard0_notifydoessmp::submit_to(1, …)and hangs on a single shard — pre-existing, unrelated to this series.Negative checks: the resolver tests fail on unpatched master (
Servers must be ipv4 addresses×4,familymessage ×1); the OpenSSL name-mismatch tests fail with the verification code removed ("Should have gotten validation error").Deliberately not here
Native stack IPv6 beyond rejecting what it cannot do, the IPv4-only collectd exporter, the native-stack YAML
ip_cfgsurface, and the CI workflow change (--sysctl net.ipv6…so a container is guaranteed to have IPv6 — commit 12 makes its absence loud rather than silent).supports_ipv6()still probes by binding::1and caches the answer forever.