Merge pull request #16258 from amazon-mq/fix/rabbitmq-server-16255

Fix `auth_backend_cache` cache miss on reconnect (#16255)

Author: michaelklishin

deps/rabbit/src/rabbit_auth_mechanism_plain.erl

@@ -52,7 +52,9 @@ handle_response(Response, _State) ->
 
 
 build_auth_props(Pass, Socket) ->
-    [{password, Pass}, {sockOrAddr, Socket}].
+    %% Pass a precomputed boolean, not the raw socket, so that AuthProps is
+    %% stable across reconnections of the same client. See issue #16255.
+    [{password, Pass}, {is_loopback, rabbit_net:is_loopback(Socket)}].
 
 extract_user_pass(Response) ->
     case extract_elem(Response) of

deps/rabbitmq_auth_backend_cache/test/rabbit_auth_backend_cache_SUITE.erl

@@ -6,7 +6,8 @@
 %%
 -module(rabbit_auth_backend_cache_SUITE).
 
--include_lib("rabbit_common/include/rabbit.hrl").
+-include_lib("amqp_client/include/amqp_client.hrl").
+-include_lib("common_test/include/ct.hrl").
 
 -compile(export_all).
 
@@ -17,6 +18,7 @@ all() ->
     access_response,
     cache_expiration,
     cache_expiration_topic,
+    cache_hit_across_reconnections,
     expiry_timestamp_delegation
     ].
 
@@ -40,6 +42,19 @@ init_per_testcase(access_response, Config) ->
         <<"guest">>, <<"/">>, <<"amq.topic">>, <<"^a">>, <<"^b">>, <<"acting-user">>
     ]),
     Config;
+init_per_testcase(cache_hit_across_reconnections, Config) ->
+    ok = rabbit_ct_broker_helpers:add_code_path_to_all_nodes(
+           Config, rabbit_auth_backend_cache_counting_mock),
+    ok = rpc(Config, rabbit_auth_backend_cache_counting_mock, init, []),
+    ok = rpc(
+      Config, application, set_env,
+      [rabbitmq_auth_backend_cache, cached_backend,
+       rabbit_auth_backend_cache_counting_mock]),
+    ok = rpc(
+      Config, application, set_env,
+      [rabbit, auth_backends, [rabbit_auth_backend_cache]]),
+    ok = rpc(Config, rabbit_auth_backend_cache, clear_cache, []),
+    Config;
 init_per_testcase(_TestCase, Config) ->
     Config.
 
@@ -53,6 +68,18 @@ end_per_testcase(cache_expiration, Config) ->
     rabbit_ct_broker_helpers:add_user(Config, <<"guest">>),
     rabbit_ct_broker_helpers:set_full_permissions(Config, <<"/">>),
     Config;
+end_per_testcase(cache_hit_across_reconnections, Config) ->
+    %% Restore the default cached_backend defined in the application's
+    %% PROJECT_ENV so that subsequent test cases are unaffected.
+    ok = rpc(
+      Config, application, set_env,
+      [rabbitmq_auth_backend_cache, cached_backend,
+       rabbit_auth_backend_internal]),
+    ok = rpc(
+      Config, application, set_env,
+      [rabbit, auth_backends, [rabbit_auth_backend_internal]]),
+    ok = rpc(Config, rabbit_auth_backend_cache, clear_cache, []),
+    Config;
 end_per_testcase(_TestCase, Config) ->
     Config.
 
@@ -163,6 +190,27 @@ cache_expiration_topic(Config) ->
     false = rpc(Config,rabbit_auth_backend_internal, check_topic_access, [Auth, TopicResource, write, RestrictedTopicContext]),
     false = rpc(Config,rabbit_auth_backend_cache, check_topic_access, [Auth, TopicResource, write, RestrictedTopicContext]).
 
+cache_hit_across_reconnections(Config) ->
+    %% Regression test for issue #16255: the cache backend used to miss on
+    %% every reconnection because the underlying socket, which varies per
+    %% connection, was part of AuthProps and therefore part of the cache
+    %% key. Two sequential AMQP 0-9-1 connections with the same credentials
+    %% should result in exactly one call to the underlying authentication
+    %% backend.
+    ok = rpc(Config, rabbit_auth_backend_cache_counting_mock, reset, []),
+    Host = ?config(rmq_hostname, Config),
+    Port = rabbit_ct_broker_helpers:get_node_config(Config, 0, tcp_port_amqp),
+    Params = #amqp_params_network{host = Host,
+                                  port = Port,
+                                  username = <<"guest">>,
+                                  password = <<"guest">>},
+    {ok, Conn1} = amqp_connection:start(Params),
+    ok = amqp_connection:close(Conn1),
+    {ok, Conn2} = amqp_connection:start(Params),
+    ok = amqp_connection:close(Conn2),
+    1 = rpc(Config, rabbit_auth_backend_cache_counting_mock,
+            authentication_call_count, []).
+
 expiry_timestamp_delegation(Config) ->
     {ok, Auth} = rpc(Config,rabbit_auth_backend_internal, user_login_authentication, [<<"guest">>, [{password, <<"guest">>}]]),
     %% rabbit_auth_backend_internal returns `never` for expiry_timestamp

deps/rabbitmq_auth_backend_cache/test/rabbit_auth_backend_cache_counting_mock.erl

@@ -0,0 +1,55 @@
+%% This Source Code Form is subject to the terms of the Mozilla Public
+%% License, v. 2.0. If a copy of the MPL was not distributed with this
+%% file, You can obtain one at https://mozilla.org/MPL/2.0/.
+%%
+%% Copyright (c) 2007-2026 Broadcom. All Rights Reserved. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved.
+%%
+
+%% Counting authentication and authorization backend used by
+%% rabbit_auth_backend_cache_SUITE to verify that the cache backend hits
+%% across reconnections that share the same credentials.
+
+-module(rabbit_auth_backend_cache_counting_mock).
+-include_lib("rabbit_common/include/rabbit.hrl").
+
+-behaviour(rabbit_authn_backend).
+-behaviour(rabbit_authz_backend).
+
+-export([user_login_authentication/2, user_login_authorization/2,
+         check_vhost_access/3, check_resource_access/4, check_topic_access/4,
+         expiry_timestamp/1]).
+
+-export([init/0, reset/0, authentication_call_count/0]).
+
+-define(KEY, {?MODULE, authentication_calls}).
+
+init() ->
+    reset().
+
+reset() ->
+    persistent_term:put(?KEY, counters:new(1, [])),
+    ok.
+
+authentication_call_count() ->
+    counters:get(persistent_term:get(?KEY), 1).
+
+user_login_authentication(Username, _AuthProps) ->
+    counters:add(persistent_term:get(?KEY), 1, 1),
+    {ok, #auth_user{username = Username,
+                    tags = [],
+                    impl = fun() -> none end}}.
+
+user_login_authorization(_Username, _AuthProps) ->
+    {ok, fun() -> none end, []}.
+
+check_vhost_access(#auth_user{}, _VHostPath, _AuthzData) ->
+    true.
+
+check_resource_access(#auth_user{}, #resource{}, _Permission, _AuthzContext) ->
+    true.
+
+check_topic_access(#auth_user{}, #resource{}, _Permission, _TopicContext) ->
+    true.
+
+expiry_timestamp(_) ->
+    never.

deps/rabbitmq_auth_backend_http/src/rabbit_auth_backend_http.erl

@@ -98,10 +98,17 @@ is_internal_none_password(_, _) -> false.
 is_sockOrAddr(sockOrAddr) -> true;
 is_sockOrAddr(_) -> false.
 
+%% is_loopback is derived from the connection's peer address and is
+%% internal context, not a public credential, so it must not be sent to
+%% the external HTTP auth server.
+is_connection_context(is_loopback) -> true;
+is_connection_context(_) -> false.
+
 extract_other_credentials(AuthProps) ->
   PublicAuthProps = [{K,V} || {K,V} <-AuthProps, not is_internal_property(K) and
                                                   not is_internal_none_password(K, V) and
-                                                  not is_sockOrAddr(K)],
+                                                  not is_sockOrAddr(K) and
+                                                  not is_connection_context(K)],
   case PublicAuthProps of
     [] -> resolve_using_persisted_credentials(AuthProps);
     _ -> PublicAuthProps

deps/rabbitmq_auth_backend_internal_loopback/src/rabbit_auth_backend_internal_loopback.erl

@@ -78,42 +78,41 @@ user_login_authentication(Username, []) ->
 %% For cases when we do have a set of credentials. rabbit_auth_mechanism_plain:handle_response/2
 %% performs initial validation.
 user_login_authentication(Username, AuthProps) ->
-    case proplists:lookup(sockOrAddr, AuthProps) of
-        none -> {refused, ?NO_SOCKET_OR_ADDRESS_REJECTION_MESSAGE, [Username]};      % sockOrAddr doesn't exist
-        {sockOrAddr, SockOrAddr} ->
-            case rabbit_net:is_loopback(SockOrAddr) of
-                true ->
-                    case lists:keyfind(password, 1, AuthProps) of
-                        {password, <<"">>} ->
-                            {refused, ?BLANK_PASSWORD_REJECTION_MESSAGE,
-                             [Username]};
-                        {password, ""} ->
-                            {refused, ?BLANK_PASSWORD_REJECTION_MESSAGE,
-                             [Username]};
-                        {password, none} -> %% For cases when authenticating using an x.509 certificate
-                            internal_check_user_login(Username, fun(_) -> true end);
-                        {password, Cleartext} ->
-                            internal_check_user_login(
-                              Username,
-                              fun(User) ->
-                                  case internal_user:get_password_hash(User) of
-                                      <<Salt:4/binary, Hash/binary>> ->
-                                          Hash =:= rabbit_password:salted_hash(
-                                              hashing_module_for_user(User), Salt, Cleartext);
-                                      _ ->
-                                          false
-                                  end
-                              end);
-                        false ->
-                            case proplists:get_value(rabbit_auth_backend_internal, AuthProps, undefined) of
-                                undefined -> {refused, ?BLANK_PASSWORD_REJECTION_MESSAGE, [Username]};
-                                _ -> internal_check_user_login(Username, fun(_) -> true end)
-                            end
-                    end;
+    case proplists:lookup(is_loopback, AuthProps) of
+        none ->
+            %% is_loopback is not set, so we cannot tell whether the
+            %% connection is from localhost. Refuse.
+            {refused, ?NO_SOCKET_OR_ADDRESS_REJECTION_MESSAGE, [Username]};
+        {is_loopback, true} ->
+            case lists:keyfind(password, 1, AuthProps) of
+                {password, <<"">>} ->
+                    {refused, ?BLANK_PASSWORD_REJECTION_MESSAGE,
+                     [Username]};
+                {password, ""} ->
+                    {refused, ?BLANK_PASSWORD_REJECTION_MESSAGE,
+                     [Username]};
+                {password, none} -> %% For cases when authenticating using an x.509 certificate
+                    internal_check_user_login(Username, fun(_) -> true end);
+                {password, Cleartext} ->
+                    internal_check_user_login(
+                      Username,
+                      fun(User) ->
+                          case internal_user:get_password_hash(User) of
+                              <<Salt:4/binary, Hash/binary>> ->
+                                  Hash =:= rabbit_password:salted_hash(
+                                      hashing_module_for_user(User), Salt, Cleartext);
+                              _ ->
+                                  false
+                          end
+                      end);
                 false ->
-                    {refused, ?NOT_LOOPBACK_REJECTION_MESSAGE, [Username]}
-            end
-
+                    case proplists:get_value(rabbit_auth_backend_internal, AuthProps, undefined) of
+                        undefined -> {refused, ?BLANK_PASSWORD_REJECTION_MESSAGE, [Username]};
+                        _ -> internal_check_user_login(Username, fun(_) -> true end)
+                    end
+            end;
+        {is_loopback, false} ->
+            {refused, ?NOT_LOOPBACK_REJECTION_MESSAGE, [Username]}
     end.
 
 

deps/rabbitmq_auth_backend_internal_loopback/test/rabbit_auth_backend_internal_loopback_SUITE.erl

@@ -65,9 +65,9 @@ end_per_suite(Config) ->
 init_per_group(localhost_connection, Config) ->
     ok = rabbit_ct_broker_helpers:add_user(Config, maps:get(username, ?LOOPBACK_USER)),
     ok = rabbit_ct_broker_helpers:add_user(Config, maps:get(username, ?NONLOOPBACK_USER)),
-    [{sockOrAddr, ?LOCALHOST_ADDR} | Config];
+    [{peer_addr, ?LOCALHOST_ADDR} | Config];
 init_per_group(nonlocalhost_connection, Config) ->
-    [{sockOrAddr, ?NONLOCALHOST_ADDR} | Config];
+    [{peer_addr, ?NONLOCALHOST_ADDR} | Config];
 init_per_group(_, Config) ->
     Config.
 
@@ -99,5 +99,5 @@ login_from_nonlocalhost_with_nonloopback_user(Config) ->
 rpc(Config, M, F, A) ->
     rabbit_ct_broker_helpers:rpc(Config, 0, M, F, A).
 
-build_auth_props(Pass, Socket) ->
-    [{password, Pass}, {sockOrAddr, Socket}].
+build_auth_props(Pass, Addr) ->
+    [{password, Pass}, {is_loopback, rabbit_net:is_loopback(Addr)}].

deps/rabbitmq_web_dispatch/src/rabbit_web_dispatch_access_control.erl

@@ -138,10 +138,12 @@ is_authorized(ReqData, Context, Username, Password, ErrorMsg, Fun, AuthConfig, R
              end,
     {IP, _} = cowboy_req:peer(ReqData),
 
-    AuthProps = [{password, Password}, {sockOrAddr, IP}] ++ case vhost(ReqData) of
-        VHost when is_binary(VHost) -> [{vhost, VHost}];
-        _                           -> []
-    end,
+    AuthProps = [{password, Password},
+                 {is_loopback, rabbit_net:is_loopback(IP)}] ++
+                case vhost(ReqData) of
+                    VHost when is_binary(VHost) -> [{vhost, VHost}];
+                    _                           -> []
+                end,
 
 	{ok, AuthBackends} = get_auth_backends(),