Skip to content

feat: implement native Unix Domain Socket support#16877

Closed
Piyush0049 wants to merge 14 commits into
rabbitmq:mainfrom
Piyush0049:feature/unix-domain-sockets
Closed

feat: implement native Unix Domain Socket support#16877
Piyush0049 wants to merge 14 commits into
rabbitmq:mainfrom
Piyush0049:feature/unix-domain-sockets

Conversation

@Piyush0049

@Piyush0049 Piyush0049 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor
  • Extend listeners config to natively support UDS paths
  • Bypass DNS resolution for local socket paths
  • Fix ntoa usage across all core modules and plugins to prevent crashes
  • Identify UDS as a secure loopback connection to permit guest login
  • Add integration tests for connection lifecycles and pub/sub routing

Proposed Changes

This PR introduces native support for Unix Domain Sockets (UDS) across the RabbitMQ core and plugin ecosystem, closing Issue #7311.

As noted by @michaelklishin and @jvary in #7311, the primary roadblock to implementing UDS previously was that various monitoring paths, CLI tools, and plugins implicitly assumed a (hostname, port) pair, causing crashes when they received a {local, Path} tuple.

To resolve this without breaking the ecosystem, this PR systematically audits and hardens those paths:

  1. Core Networking: Added {local, _} as a recognized loopback connection in rabbit_net:is_loopback/1, allowing the default guest user to securely authenticate over local sockets.
  2. Monitoring & Web Server: Replaced inet_parse:ntoa with the UDS-aware rabbit_misc:ntoa inside rabbit_cowboy_stream_h.erl. This ensures the Management UI and Prometheus endpoints no longer crash Cowboy when access logs are generated for a UDS client.
  3. CLI Diagnostics: Rewrote interface formatting in the Elixir CLI codebase (listeners.ex) to gracefully handle non-IP interfaces, preventing rabbitmq-diagnostics listeners and report from crashing.
  4. Plugins: Safely bypassed DNS resolution and replaced inet:ntoa assumptions in MQTT, Web STOMP, and HTTP Auth backends.
  5. Testing: Added rigorous end-to-end data plane testing (test_uds_publish_consume) in unix_domain_socket_SUITE.erl to guarantee payload integrity over local sockets.

Maintainers: As this touches several core networking and monitoring formatting paths, your review and inputs are heavily appreciated! I'm fully open to iterating on this based on your feedback.

Configuration Syntax

A UDS listener is configured with the same listeners.tcp.* (or listeners.ssl.*) option used for TCP listeners:

listeners.tcp.1 = unix:/var/run/rabbitmq/rabbitmq.sock:0

The unix: (or equivalent local:) prefix and the trailing :0 are both required. This is a consequence of how the value is parsed by cuttlefish:

  • listeners.tcp.$name uses the datatype union [integer, ip, domain_socket], and cuttlefish tries each converter in order. A bare path cannot be positively identified as a socket, so cuttlefish_datatypes:validate_uds/1 keys on the unix:/local: prefix to disambiguate a socket path from an IP address.
  • The domain_socket converter reuses the same address:port splitting as the IP and FQDN converters, so a :port component must be present. A Unix domain socket has no port, so the only accepted value is 0 (mirroring the inet local_address type, where the port is always 0).

The mandatory :0 is a cuttlefish parsing limitation, tracked upstream at Kyorai/cuttlefish#83.

Types of Changes

What types of changes does your code introduce to this project?
Put an x in the boxes that apply

  • Bug fix (non-breaking change which fixes issue #NNNN)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause an observable behavior change in existing systems)
  • Documentation improvements (corrections, new content, etc)
  • Cosmetic change (whitespace, formatting, etc)
  • Build system and/or CI

Checklist

Put an x in the boxes that apply.
You can also fill these out after creating the PR.
This is simply a reminder of what we are going to look for before merging your code.

@mergify

mergify Bot commented Jul 5, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@Piyush0049 Piyush0049 mentioned this pull request Jul 5, 2026
@jvary

jvary commented Jul 6, 2026

Copy link
Copy Markdown

Hello! Thanks @Piyush0049 for taking over.
I'm surprised by how little code change was needed to squeeze the feature in. Clever tricks!

My Erlang is a bit rusty (and even more so the RabbitMQ codebase), but LGTM.

@lukebakken
lukebakken force-pushed the feature/unix-domain-sockets branch from ddb4662 to f14cb94 Compare July 6, 2026 20:18
@lukebakken lukebakken self-assigned this Jul 6, 2026

@lukebakken lukebakken left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: PR #16877 - native Unix Domain Socket support

Note: this review was produced by an AI agent with guidance and direction from @lukebakken. The findings below were verified by running code against real Unix Domain Sockets on Erlang/OTP 27, and @lukebakken has reviewed the conclusions before posting.

The AI verified the central claims by running the actual logic against real UDS sockets on Erlang/OTP 27, plus the real cuttlefish and Elixir code. The change does not currently achieve its primary goal, and it needs a different foundation before it can be merged.

Summary

The PR is built on the premise that a UDS peer address reaches RabbitMQ as the tuple {local, Path}, and it adds {local, _} handling to formatting and loopback code on that basis. In practice, inet:peername/1 for an accepted UDS socket returns {ok, {local, <<>>}}, and rabbit_net:socket_ends/2 decomposes that as Address = local (a bare atom) and Port = <<>>. Every fix in the PR targets the {local, Path} tuple shape, which sits downstream of a decomposition that has already destroyed that shape. The core networking path (socket_ends/2) is untouched, so guest-over-UDS does not work, and the test suite masks the failure by disabling the loopback check.

Blocking findings

B1. Guest login over UDS does not work, so the PR's primary goal is not met

The new is_loopback({local, _}) -> true clause in rabbit_net.erl is effectively dead code on the connection-authentication path.

check_user_loopback(Username, PeerHost) is called with PeerHost produced by rabbit_net:socket_ends/2 in all three connection readers: rabbit_reader.erl:1553 (AMQP 0-9-1), rabbit_amqp_reader.erl:869 (AMQP 1.0), and rabbit_mqtt_processor.erl:204 (MQTT). socket_ends/2 matches the peername result with {ok, {FromAddress, FromPort}}. For UDS, peername returns {ok, {local, <<>>}}, so FromAddress binds to the atom local. is_loopback(local) matches none of the {local, _} or IP clauses and falls through to is_loopback(_) -> false.

End-to-end reproduction using the real is_loopback/1 and socket_ends/2 logic against a live UDS socket:

is_loopback(AcceptedSocket) = false
socket_ends -> PeerHost=local PeerPort=<<>> Host=local Port=<<"/tmp/uds_test_real.sock">>
is_loopback(PeerHost) = false

With the default loopback_users = [<<"guest">>] (verified in deps/rabbit/ebin/rabbit.app), the check evaluates false orelse not lists:member(<<"guest">>, [<<"guest">>]), which is false, so it returns not_allowed. Guest is rejected over UDS, the opposite of the stated goal.

Note there are two distinct paths. The web/HTTP path (rabbit_web_dispatch_access_control.erl:142) calls is_loopback(IP) with IP from cowboy_req:peer/1, which does yield a {local, _} tuple, so the new clause works there. But the AMQP/MQTT connection path, which is what this PR is really about, does not. That asymmetry is why the change looks plausible but does not work as intended.

B2. The test suite hides B1 by disabling the loopback check

unix_domain_socket_SUITE.erl lines 47 and 72 call set_env(rabbit, loopback_users, []) before connecting, which globally disables the loopback restriction, so the tests pass regardless of B1. The test that would actually exercise the feature (guest connecting over UDS with the default loopback_users) is exactly the one that would fail.

Additional test issues:

  • set_env(rabbit, loopback_users, []) mutates a node-global setting and never restores it, leaking state into any later suite on the same node
  • Hardcoded /tmp/*.sock paths ignore the rabbit_ct_helpers temp-dir conventions and the 108-character sun_path limit; there is no end_per_testcase cleanup, so a mid-test failure leaks both the socket file and the dynamically started listener
  • timer:sleep(500) to wait for listener startup is racy

B3. Config regression: the string catch-all turns invalid listener config into a boot crash

rabbit.schema changes listeners.tcp.$name and listeners.ssl.$name from [integer, ip] to [integer, ip, string]. Cuttlefish tries datatypes left to right and takes the first that succeeds (cuttlefish_generator:foldm_either/3), and from_string(String, string) accepts any list, so string becomes an unconditional catch-all.

Verified against real cuttlefish plus the real tcp_listener_addresses/1 clauses:

  • Before: listeners.tcp.1 = localhost was rejected with a config conversion error ({error,{conversion,{"localhost",'IP'}}})
  • After: it parses as the string "localhost", reaches tcp_listener_addresses("localhost"), matches no clause (not integer, not a tuple, does not start with / or .), and crashes at boot with a function_clause error

So a typo in a listener value now crashes the node instead of producing a clean validation error. Cuttlefish already ships a purpose-built domain_socket datatype (cuttlefish_datatypes.erl:428, producing {local, Path, Port} with validation) that would be the idiomatic choice, though using it requires a matching tcp_listener_addresses clause.

B4. Address representation is inconsistent (string at config time, binary at runtime)

rabbit_misc:ntoa({local, Path}) -> Path returns Path unchanged, but Path is a string when it comes from config (listener record ip_address = {local, "..."}) and a binary when it comes from inet:peername/sockname or cowboy_req:peer (OTP's returned_non_ip_address() :: {'local', binary()}, see otp/lib/kernel/src/inet.erl:444).

Any call of the form list_to_binary(rabbit_misc:ntoa(X)) crashes if X is {local, BinaryPath} (verified: list_to_binary(<<"x">>) raises badarg). The call sites the PR audits happen to receive the string form today (for example rabbit_mgmt_wm_health_check_certificate_expiration.erl:113 and rabbit_mgmt_format.erl:197 read the listener record's string path), so they do not crash now, but the mixed representation is a latent trap. rabbit_misc:ntoa/1 should normalize {local, Path} to a consistent type rather than passing the path through untouched.

Non-blocking and minor

  • M1 (dead code, no effect): rabbit_mqtt_util.erl's ip_address_to_binary/1 is exported and imported into rabbit_mqtt_processor, but has zero call sites anywhere in the tree (the unused import predates this PR). The change is harmless but does nothing.
  • M2 (garbled but non-crashing output): because socket_ends/2 yields the atom local, AMQP connection strings for UDS render as local:<<>> -> local:<<"/path">> (via rabbit_net:connection_string/2 and maybe_ntoab/1, which passes atoms through). Auth-attempt metrics record the address as <<"local">> (rabbit_core_metrics.erl false branch, to_binary(local)), losing the socket path.
  • M3 (latent crash if reverse DNS is enabled): with reverse_dns_lookups = true, rabbit_net:rdns(local) calls tcp_host(local), which calls inet:gethostbyaddr(local) and errors; the result then flows into formatting. Not the default, but unhandled.
  • M4 (non-idiomatic guard): the new tcp_listener_addresses/1 guard uses length(Path) > 0 (an O(n) traversal) where Path =/= [] suffices, and the inline hd(Path) =:= $/ orelse hd(Path) =:= $. is unusual for this module. Not a bug (the length > 0 guard does correctly protect hd).
  • M5 (cosmetic): UDS listeners report port: 0 in rabbitmq-diagnostics listeners and the HTTP API. Harmless but potentially confusing.

What is correct in the PR

  • The data plane works. gen_tcp:connect({local, Path}, 0, [local | RABBIT_TCP_OPTS]) connects and transfers data over UDS with the exact amqp_client options, including {nodelay, true} (OTP silently ignores nodelay on UDS, no error). This is why the loopback-disabled test passes.
  • The Elixir CLI format_interface({:local, path}) is correct; path is a bound variable and it handles both string and binary paths.
  • Ranch natively supports {ip, {local, Path}} and cleans up the socket file (ranch_tcp.erl:279).
  • The listener registration path (tcp_host/1, tcp_name/1) handles {local, StringPath} without crashing, and the management listener-listing path works because it uses the string form.

Suggested direction

The fix needs to live at the source of the address, not at the formatting leaves:

  1. Teach rabbit_net:socket_ends/2 (and the is_loopback(Sock) socket clause) to recognize peername/sockname returning {local, Binary} and preserve it as a {local, Path} value rather than decomposing it into {Address, Port}
  2. Make the {PeerHost, PeerPort, Host, Port} consumers in rabbit_reader, rabbit_amqp_reader, and rabbit_mqtt_processor prepared for a UDS peer
  3. Normalize rabbit_misc:ntoa/1 to return a consistent type for {local, Path} regardless of whether the path arrived as a string or a binary
  4. Prefer cuttlefish's existing domain_socket datatype (with a matching tcp_listener_addresses clause) over a string catch-all, to preserve config validation
  5. Add a test that connects as guest over UDS with the default loopback_users, and restores any env it changes

I have converted this PR to draft as we begin the process of refining it. Once it's more ready, we can change the status.

@lukebakken
lukebakken marked this pull request as draft July 6, 2026 21:19
@michaelklishin

Copy link
Copy Markdown
Collaborator

It's a good question whether UDS support makes much sense for RabbitMQ.

@Piyush0049

Piyush0049 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Hi @michaelklishin and @lukebakken,

Thanks a lot for taking the time to do such a deep review, really appreciate the detailed breakdown of the edge cases.

To answer the question about whether UDS support makes sense for RabbitMQ, here are the main use cases I had in mind:

  1. Sidecar/Proxy setups: In Kubernetes deployments, it's pretty common to have local sidecars (Envoy, HAProxy, custom agents) running alongside the broker. Using a Unix socket instead of TCP avoids exposing localhost ports entirely, which helps prevent port exhaustion and conflicts
  2. OS-level security: UDS lets you control access through standard file permissions (chmod/chown) and user groups, giving you a solid security boundary for local clients without needing to set up TLS certificates just for local traffic
  3. Less overhead: Skipping the TCP/IP stack (no routing, checksums, or window management) means lower CPU usage and latency for local publishers/consumers

On Luke's technical findings, you're absolutely right. Patching the formatting functions at the leaves (like ntoa) was the wrong approach since the core socket_ends/2 decomposition was already stripping the {local, Path} tuple into a bare local atom before anything downstream could use it. The guest loopback auth was broken under the hood and the tests were hiding it by disabling loopback_users.

I understand it may not be a universal requirement, but for containerized/local-only deployments these patterns are very common, and supporting UDS cleanly would make RabbitMQ more composable in those environments. I fully agree with the suggested direction. I'll rework this from the source:

  • Fix rabbit_net:socket_ends/2 to preserve {local, Path} instead of decomposing it
  • Update the downstream readers (rabbit_reader, rabbit_amqp_reader, rabbit_mqtt_processor) to handle UDS peers
  • Normalize rabbit_misc:ntoa/1 to return a consistent type
  • Switch to Cuttlefish's domain_socket datatype instead of the string catch-all
  • Write a proper test that validates guest login over UDS with default loopback_users

I'm fully committed to seeing this through, happy to invest the time needed to get this right.

@michaelklishin

Copy link
Copy Markdown
Collaborator

@Piyush0049 sure, I understand when USD makes sense. You don't have to sell it hard.

Consider how many RabbitMQ clients can use UDS, though. It's a niche feature all things considered. I am not against it but I don't want to overpromise, our team may decide to not accept this PR.

@Piyush0049

Copy link
Copy Markdown
Contributor Author

@michaelklishin Apologies, definitely not trying to oversell it. Like I mentioned, I completely understand it's a niche feature and not a universal requirement for most clients.
I'm totally fine with it if the team ultimately decides not to merge. Either way, I'm ready to work on it and contribute to fixing the architecture based on review, even if it's just for the learning experience.

@lukebakken
lukebakken force-pushed the feature/unix-domain-sockets branch from f14cb94 to 606a47f Compare July 11, 2026 11:34
@Piyush0049

Copy link
Copy Markdown
Contributor Author

Hi @lukebakken, I've pushed an update in this PR.

Here is a summary:

  • Reworked rabbit_net:socket_ends/2 to preserve the {local, Path} tuple instead of decomposing it, which natively fixes the is_loopback check so guest login works as expected.
  • Swapped the string catch-all in rabbit.schema for the Cuttlefish domain_socket datatype to ensure proper config validation.
  • Normalized rabbit_misc:ntoa/1 and ntoab/1 to guarantee consistent string and binary returns.
  • Added a guard to rabbit_net:rdns/1 to bypass lookups for local paths and prevent the reverse DNS crash.
  • Rewrote the test suite to remove the global loopback_users override. It also now cleanly handles the 108-character sun_path limit dynamically and properly cleans up the listeners and socket files in end_per_testcase.
  • Cleaned up the minor style nit and removed the unused ip_address_to_binary/1 function.

@lhoguin lhoguin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lhoguin

lhoguin commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Please follow the instructions around the error at https://github.com/rabbitmq/rabbitmq-server/actions/runs/29192409204/job/87321204187?pr=16877 and I will relaunch CI.

@mergify mergify Bot added the make label Jul 15, 2026
@Piyush0049

Piyush0049 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the approval, @lhoguin!

I have pushed a fix. I registered the new unix_domain_socket test suite in the PARALLEL_CT_SET_1_A variable inside deps/rabbit/Makefile.

Please do let me know if any other changes are required.

@lhoguin
lhoguin requested a review from lukebakken July 15, 2026 11:19
@michaelklishin

Copy link
Copy Markdown
Collaborator

So far there does not seem to be much pushback on our team. At least conceptually.

Which means spending more time on this PR can be justified :)

@Piyush0049

Piyush0049 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

So far there does not seem to be much pushback on our team. At least conceptually.

Which means spending more time on this PR can be justified :)

I’m very glad to hear that 😊. Please do let me know if any changes are required; I’d be happy to contribute.

@lukebakken
lukebakken force-pushed the feature/unix-domain-sockets branch from de52af8 to 8b7fb81 Compare July 15, 2026 16:07
@lukebakken

Copy link
Copy Markdown
Collaborator

I just rebased Piyush0049:feature/unix-domain-sockets on main and will be doing one more review with the 🧞

@lukebakken lukebakken left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a review by Claude Code, guided by @lukebakken. The findings below were reproduced against a running node built from this branch (single node, default loopback_users, UDS listener plus 5672).

Requesting changes.

What works: the socket_ends/2 change correctly carries the {local, Path} tuple into the AMQP 0-9-1 reader, so on a default install guest can connect over a Unix socket and authenticate. That path is solid.

The problems are on paths that use a raw socket or a non-default configuration:

  1. rabbit_net.erl: the new is_loopback({local, _}) clause is dead when called on a socket. is_loopback/1 calls peername, which on an accepted UDS socket returns {ok, {local, <<>>}}, and the existing {ok, {Addr, _Port}} clause binds Addr to the atom local. rabbit_auth_mechanism_plain calls is_loopback/1 on the raw socket, so with the internal_loopback backend a guest UDS connection is refused ("not from loopback/localhost"). Reproduced live.

  2. rabbit_core_metrics.erl: with track_auth_attempt_source enabled, every UDS connection crashes in update_auth_attempt (rabbit_data_coercion:to_binary/1 has no clause for the {local, Path} tuple). The crash is on the auth success branch, so the connection dies right after authenticating. Reproduced live.

  3. rabbit_auth_backend_http.erl: removing the einval fallback regresses non-IP peeraddrs. Both the UDS atom local and reverse-DNS hostname binaries make ntoa/1 return {error, einval}, which then crashes quote_plus/1. Reproduced live.

  4. rabbit_networking.erl / rabbit.schema: the raw string datatype shadows the existing domain_socket datatype, and the string listener clause only accepts paths starting with / or ., so other string values crash at boot. Reproduced live: listeners.tcp.badstring = myapp.sock fails startup with function_clause in tcp_listener_addresses/1. Dropping string and relying on domain_socket addresses both.

  5. unix_domain_socket_SUITE.erl: the CT broker runs loopback_users=[], so the feature's core guarantee (UDS is loopback, guest may connect) is never actually exercised. Setting loopback_users=[<<"guest">>] and asserting acceptance would have caught item 1.

Non-blocking:

  1. The {local, Path} case is special-cased across roughly eight formatters rather than normalized once; items 2 and 3 are missed instances of that pattern. Normalizing at the socket_ends/2 boundary would remove the class.
  2. rabbit_mqtt_processor.erl still imports the removed rabbit_mqtt_util:ip_address_to_binary/1.
  3. Test nits (inline): missing MPL header and executable file mode, an unnecessary timer:sleep, and a failure test that asserts only {error, _}.

Comment thread deps/rabbit_common/src/rabbit_net.erl
Comment thread deps/rabbit_common/src/rabbit_core_metrics.erl
Comment thread deps/rabbitmq_auth_backend_http/src/rabbit_auth_backend_http.erl Outdated
Comment thread deps/rabbit/src/rabbit_networking.erl Outdated
Comment thread deps/rabbit/priv/schema/rabbit.schema Outdated
Comment thread deps/rabbit/test/unix_domain_socket_SUITE.erl
Comment thread deps/rabbit/test/unix_domain_socket_SUITE.erl Outdated
Comment thread deps/rabbit/test/unix_domain_socket_SUITE.erl Outdated
Comment thread deps/rabbit/test/unix_domain_socket_SUITE.erl
@lhoguin

lhoguin commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

@lukebakken Perhaps you can ask Claude to add tests for the issues it found and confirmed as well? Many of your inline comments would qualify ("verified live") and I don't think it'd cost much more tokens.

@lukebakken
lukebakken force-pushed the feature/unix-domain-sockets branch from dea6407 to f5c6cf9 Compare July 15, 2026 23:29
@lukebakken

Copy link
Copy Markdown
Collaborator

Following up on the review: I pushed f5c6cf9 with regression tests that reproduce the confirmed issues. They are expected to fail against the current branch and should pass once the fixes land. Written by Claude Code, guided by @lukebakken.

The new cases in unix_domain_socket_SUITE.erl:

  • test_uds_is_loopback_on_raw_socket, test_uds_guest_with_loopback_users, test_uds_internal_loopback_backend - is_loopback returning false for a UDS peer (finding 1)
  • test_uds_auth_attempt_source_tracking - the to_binary({local, Path}) crash with track_auth_attempt_source enabled (finding 2)
  • test_uds_ntoa_local_atom - rabbit_misc:ntoa crashing on the bare atom local from a UDS peeraddr (finding 3)
  • test_uds_tcp_listener_addresses - tcp_listener_addresses/1 crashing on a string path that does not start with / or . (finding 4)

The commit also adds rabbitmq_auth_backend_internal_loopback to TEST_DEPS, adds the MPL header, and fixes the file mode (755 to 644). The inline comments above still capture the requested changes; this just adds the failing tests that pin them.

@Piyush0049

Copy link
Copy Markdown
Contributor Author

Hey @lukebakken @lhoguin, I've pushed the changes after reading the review. The is_loopback(local) clause, the rabbit_core_metrics crash, the einval fallback in the HTTP auth backend, the schema/networking string issue, and the loose test assertion are all addressed. All 9 test cases in the suite pass locally including the regression tests you added.
Let me know if anything else needs changing.

- Extend listeners config to natively support UDS paths
- Bypass DNS resolution for local socket paths
- Fix ntoa usage across all core modules and plugins to prevent crashes
- Identify UDS as a secure loopback connection to permit guest login
- Add integration tests for connection lifecycles and pub/sub routing
Piyush0049 and others added 5 commits July 16, 2026 05:29
- Fix socket_ends/2 tuple decomposition causing is_loopback failures
- Normalise string/binary conversions in ntoa/1
- Use Cuttlefish domain_socket datatype for robust config validation
- Safely handle 108-character sun_path limits in CT test suites
- Ensure reliable listener teardown on mid-test crashes
- Remove unused ip_address_to_binary/1
Add tests that exercise the five confirmed issues with the Unix Domain
Socket implementation:

- is_loopback must return true for the {local, _} tuple (finding rabbitmq#1)
- guest must connect over UDS with loopback_users=[<<"guest">>] (rabbitmq#1)
- internal_loopback backend must accept guest over UDS (rabbitmq#1)
- track_auth_attempt_source must not crash UDS connections (rabbitmq#2)
- rabbit_misc:ntoa must handle the bare atom `local` (rabbitmq#3)
- tcp_listener_addresses must handle arbitrary string paths (rabbitmq#4)

Also: add rabbitmq_auth_backend_internal_loopback to TEST_DEPS, add
the MPL copyright header, fix executable file mode (755 -> 644), and
remove unnecessary timer:sleep calls and trivial comments.

These tests are expected to FAIL against the current PR code, proving
the bugs exist. They will pass once the fixes are applied.
- Fix socket_ends/2 tuple decomposition causing is_loopback failures (Finding rabbitmq#1)
- Restore {error, enoent} assertion in uds_connection_failure tests (Finding rabbitmq#2)
- Handle the bare atom 'local' in rabbit_misc:ntoa/1 (Finding rabbitmq#3)
- Drop overly strict hd(Path) guard from tcp_listener_addresses/1 (Finding rabbitmq#4)
- Remove 'string' from tcp and ssl listener schema datatypes (Finding rabbitmq#5)
- Restore {error, einval} fallback in rabbit_auth_backend_http
- Normalise track_auth_attempt_source tuple extraction in rabbit_core_metrics
@lukebakken
lukebakken force-pushed the feature/unix-domain-sockets branch 2 times, most recently from c3c3bd1 to ac252e0 Compare July 16, 2026 14:01
The Unix domain socket support left the bare atom `local` leaking out of
two `rabbit_net:peername/1` destructure sites. An accepted UDS socket
reports `{ok, {local, <<>>}}`, but both sites matched `{ok, {Addr, _Port}}`,
collapsing the tuple into the atom `local`. That atom then reached
`is_loopback/1` and the auth data path, so scattered `local` atom clauses
had been added downstream to compensate.

Fix the two destructure sites to preserve the `{local, _}` tuple:
`rabbit_net:is_loopback/1` (socket clause) and
`rabbit_access_control:get_authz_data_from({socket, _})`. With the tuple
preserved, remove the now-dead symptom clauses `is_loopback(local)`,
`ntoa(local)` and `ntoab(local)`.

Widen the `rabbit_net:peername/1` and `sockname/1` specs to include
`inet:returned_non_ip_address()`, mirroring OTP, since an accepted UDS
socket genuinely returns `{ok, {local, Path}}`. This lets Dialyzer accept
the new `{ok, {local, _}}` clause, and the `socket_ends/2` Dialyzer
suppression is no longer needed.

Render a `{local, Path}` address in the CLI `list_connections` output: add
a `format_info_item/2` clause for it, mirroring the existing IPv4 and IPv6
tuple clauses, so the socket path is shown instead of the raw
`{local, ...}` tuple. This matches the HTTP API, which already formats it
via `rabbit_misc:ntoa/1`.

Correct the `ntoab/1` spec to `string() | binary()`, add `{local, string()}`
to the `peeraddr` type, and make `rabbit_mgmt_format:ipb/1` use
`iolist_to_binary/1` so a `{local, _}` address cannot crash it.

A UDS listener is configured with `local:/path:0` or `unix:/path:0`, which
the cuttlefish `domain_socket` datatype parses into `{local, Path, 0}`.

Split the test suite into `with_listener` and `without_listener` groups and
give the listener a single owner: `init_per_group`/`end_per_group` start and
stop it once for the group, per-testcase application environment moves into
`init_per_testcase`/`end_per_testcase`, and the test bodies only open
connections and assert. No test body or teardown calls `stop_tcp_listener`
on a listener whose existence it cannot guarantee, which removes the
`{error, not_found}` teardown crash seen under the parallel CT runner.
@lukebakken
lukebakken force-pushed the feature/unix-domain-sockets branch from ac252e0 to 6e9fe98 Compare July 16, 2026 16:48
The example config covered TCP and TLS listeners but not Unix domain
socket listeners, which the `listeners.tcp.$name` schema now accepts.

Add a commented example for the `unix:` and `local:` prefixed forms, both
of which take the socket path followed by `:0` as the port.
Adding rabbitmq_auth_backend_internal_loopback to rabbit's TEST_DEPS
introduced a dependency cycle: that plugin declares DEPS = rabbit_common
rabbit, so building it as part of rabbit's test build recompiled rabbit
without -DTEST. That stripped test-only code from rabbit's ebin and made
every rabbit parallel-ct set fail with {undef, ...} on functions that
exist in the source, not just the set containing the UDS suite.

Remove the TEST_DEPS entry and drop test_uds_internal_loopback_backend,
the only case that needed the plugin. Coverage is retained elsewhere:
test_uds_is_loopback_on_raw_socket proves is_loopback/1 returns true for
an accepted UDS socket, test_uds_guest_with_loopback_users exercises
loopback-user acceptance over UDS, and the plugin's own
rabbit_auth_backend_internal_loopback_SUITE covers its accept and refuse
logic.
rabbit_misc:ntoa/1 and ntoab/1 render a {local, Path} address by calling
rabbit_data_coercion:to_list/1 and to_binary/1 respectively. Those helpers
mishandle Unix domain socket paths that are not plain ASCII:

  - A config-defined listener path arrives from cuttlefish as a Unicode
    charlist (codepoints). to_binary/1 (and list_to_binary/1 in the
    callers) raises badarg on any codepoint above 255, crashing the
    management listener listing and the certificate-expiration health
    check.
  - An accepted connection's peer path arrives as a UTF-8 binary (the
    accepted socket's sockname). to_list/1 turns it into a raw byte
    list, so it is no longer distinguishable from a codepoint charlist,
    and no single caller-side wrapper is correct for both shapes.

Fix the conversion at the source. ntoa/1 now uses to_unicode_charlist/1,
which yields a proper codepoint charlist for both the charlist and the
UTF-8 binary shape. ntoab/1 uses to_utf8_binary/1, which yields a correct
UTF-8 binary for both shapes and falls back to latin1 for paths that are
not valid UTF-8. Callers that need a binary (rabbit_mgmt_format:ip/1, the
certificate-expiration health check, and rabbit_core_metrics auth attempt
tracking) now build it with to_utf8_binary/1 over ntoa/1, and
rabbit_networking:tcp_host/1 routes the {local, _} case through ntoab/1
instead of duplicating the conversion.
rabbit_json:encode_value/2 has dedicated clauses for IPv4 and IPv6 address
tuples but none for the {local, Path} address shape that a Unix domain
socket connection now yields. Such a value falls through to the generic
clause, where json:encode_value/2 cannot serialize the tuple and raises
{unsupported_type, {local, _}}.

Add a {local, Path} clause that renders the address through
rabbit_misc:ntoab/1, matching how the IP tuple clauses render their
addresses and yielding a UTF-8 binary for both the charlist and binary
path shapes.
Two leftovers from the initial Unix domain socket work advertise or guard
shapes that never occur:

  - listener_config() gained a bare string() alternative, but
    tcp_listener_addresses/1 has no clause for a bare string and the
    suite asserts it raises function_clause. The real UDS listener config
    is {local, Path, Port}, so document that shape instead of the string()
    that the code rejects.
  - rabbit_net:rdns/1 gained a {local, _} clause, but socket_ends/2
    handles a local-on-both-ends connection in its own clause and never
    calls rdns for it; the remaining rdns call sites only receive
    non-local {Addr, Port} shapes or proxy header addresses. The clause
    is unreachable.
The test-case comments referenced "finding rabbitmq#1".."finding rabbitmq#4", ephemeral
PR-review artifacts that mean nothing to a future reader of the tree.
Reword each comment to describe what the test verifies, and drop the
incidental quote_plus reference from the ntoa comment.

Switch the copyright header from ASCII quotes to the curly quotes used by
the sibling test files.
The UTF-8 handling for Unix domain socket paths in `ntoa/1`, `ntoab/1`
and `rabbit_json:encode/1` was verified only manually. A socket path
reaches these functions in two shapes: a Unicode charlist from a
config-defined listener and a UTF-8 binary from an accepted connection's
sockname. An earlier revision re-encoded the binary byte-by-byte, which
double-encoded non-ASCII paths, and `rabbit_json` had no clause for a
`{local, Path}` address at all, crashing on the empty-binary sockname of
an accepted UDS socket.

Add two cases to `unix_domain_socket_SUITE`. `test_uds_ntoa_non_ascii_path`
asserts a non-ASCII path normalises identically through `ntoa/1` and
`ntoab/1` for both input shapes. `test_uds_json_encode_local_address`
asserts `rabbit_json:encode/1` handles the empty-binary path and
non-ASCII paths in both shapes.
@lukebakken
lukebakken marked this pull request as ready for review July 16, 2026 22:11
@michaelklishin

Copy link
Copy Markdown
Collaborator

Moved to #16963.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants