Remove should_offer/1.

7 files changed, 42 insertions, 57 deletions

diff --git a/include/rabbit_auth_mechanism_spec.hrl b/include/rabbit_auth_mechanism_spec.hrl
index 56c8faf6..93aa40bd 100644
--- a/include/rabbit_auth_mechanism_spec.hrl
+++ b/include/rabbit_auth_mechanism_spec.hrl
@@ -31,7 +31,6 @@
 -ifdef(use_specs).
 
 -spec(description/0 :: () -> [{atom(), any()}]).
--spec(should_offer/1 :: (rabbit_net:socket()) -> boolean()).
 -spec(init/1 :: (rabbit_net:socket()) -> any()).
 -spec(handle_response/2 :: (binary(), any()) ->
                                 {'ok', rabbit_types:user()} |
diff --git a/src/rabbit_auth_mechanism.erl b/src/rabbit_auth_mechanism.erl
index d8410c84..1258cb8d 100644
--- a/src/rabbit_auth_mechanism.erl
+++ b/src/rabbit_auth_mechanism.erl
@@ -38,10 +38,6 @@ behaviour_info(callbacks) ->
      %% A description.
      {description, 0},
 
-     %% If this mechanism is enabled, should it be offered for a given socket?
-     %% (primarily so EXTERNAL can be SSL-only)
-     {should_offer, 1},
-
      %% Called before authentication starts. Should create a state
      %% object to be passed through all the stages of authentication.
      {init, 1},
diff --git a/src/rabbit_auth_mechanism_amqplain.erl b/src/rabbit_auth_mechanism_amqplain.erl
index 0207e6c6..5d51d904 100644
--- a/src/rabbit_auth_mechanism_amqplain.erl
+++ b/src/rabbit_auth_mechanism_amqplain.erl
@@ -34,7 +34,7 @@
 
 -behaviour(rabbit_auth_mechanism).
 
--export([description/0, should_offer/1, init/1, handle_response/2]).
+-export([description/0, init/1, handle_response/2]).
 
 -include("rabbit_auth_mechanism_spec.hrl").
 
@@ -53,9 +53,6 @@ description() ->
     [{name, <<"AMQPLAIN">>},
      {description, <<"QPid AMQPLAIN mechanism">>}].
 
-should_offer(_Sock) ->
-    true.
-
 init(_Sock) ->
     [].
 
diff --git a/src/rabbit_auth_mechanism_cr_demo.erl b/src/rabbit_auth_mechanism_cr_demo.erl
index fe77021a..0e4b7a85 100644
--- a/src/rabbit_auth_mechanism_cr_demo.erl
+++ b/src/rabbit_auth_mechanism_cr_demo.erl
@@ -34,7 +34,7 @@
 
 -behaviour(rabbit_auth_mechanism).
 
--export([description/0, should_offer/1, init/1, handle_response/2]).
+-export([description/0, init/1, handle_response/2]).
 
 -include("rabbit_auth_mechanism_spec.hrl").
 
@@ -58,9 +58,6 @@ description() ->
      {description, <<"RabbitMQ Demo challenge-response authentication "
                      "mechanism">>}].
 
-should_offer(_Sock) ->
-    true.
-
 init(_Sock) ->
     #state{}.
 
diff --git a/src/rabbit_auth_mechanism_external.erl b/src/rabbit_auth_mechanism_external.erl
index a5977264..c0531bcc 100644
--- a/src/rabbit_auth_mechanism_external.erl
+++ b/src/rabbit_auth_mechanism_external.erl
@@ -34,7 +34,7 @@
 
 -behaviour(rabbit_auth_mechanism).
 
--export([description/0, should_offer/1, init/1, handle_response/2]).
+-export([description/0, init/1, handle_response/2]).
 
 -include("rabbit_auth_mechanism_spec.hrl").
 
@@ -57,35 +57,28 @@ description() ->
     [{name, <<"EXTERNAL">>},
      {description, <<"SASL EXTERNAL authentication mechanism">>}].
 
-should_offer(Sock) ->
-    case peer_subject(Sock) of
-        none ->
-            false;
-        _ ->
-            {ok, Opts} = application:get_env(ssl_options),
-            case {proplists:get_value(fail_if_no_peer_cert, Opts),
-                  proplists:get_value(verify, Opts)} of
-                {true, verify_peer} ->
-                    true;
-                {F, V} ->
-                    rabbit_log:warning("EXTERNAL mechanism disabled, "
-                                       "fail_if_no_peer_cert=~p; "
-                                       "verify=~p~n", [F, V]),
-                    false
-            end
-    end.
-
 init(Sock) ->
-    {ok, C} = rabbit_net:peercert(Sock),
-    CN = case rabbit_ssl:peer_cert_subject_item(C, ?'id-at-commonName') of
-             not_found -> not_found;
-             CN0       -> list_to_binary(CN0)
-         end,
-    #state{username = CN}.
+    Username = case rabbit_net:peercert(Sock) of
+                   {ok, C} ->
+                       CN = case rabbit_ssl:peer_cert_subject_item(
+                                   C, ?'id-at-commonName') of
+                                not_found -> not_found;
+                                CN0       -> list_to_binary(CN0)
+                            end,
+                       case config_sane() of
+                           true  -> CN;
+                           false -> not_found
+                       end;
+                   {error, no_peercert} ->
+                       not_found;
+                   nossl ->
+                       not_found
+               end,
+    #state{username = Username}.
 
 handle_response(_Response, #state{username = Username}) ->
     case Username of
-        not_found -> {refused, Username};
+        not_found -> {refused, "CN not found"};
         _         -> case rabbit_access_control:lookup_user(Username) of
                          {ok, User}         -> {ok, User};
                          {error, not_found} -> {refused, Username}
@@ -94,9 +87,15 @@ handle_response(_Response, #state{username = Username}) ->
 
 %%--------------------------------------------------------------------------
 
-peer_subject(Sock) ->
-    case rabbit_net:peercert(Sock) of
-        nossl                -> none;
-        {error, no_peercert} -> none;
-        {ok, C}              -> rabbit_ssl:peer_cert_subject(C)
+config_sane() ->
+    {ok, Opts} = application:get_env(ssl_options),
+    case {proplists:get_value(fail_if_no_peer_cert, Opts),
+          proplists:get_value(verify, Opts)} of
+        {true, verify_peer} ->
+            true;
+        {F, V} ->
+            rabbit_log:warning("EXTERNAL mechanism disabled, "
+                               "fail_if_no_peer_cert=~p; "
+                               "verify=~p~n", [F, V]),
+            false
     end.
diff --git a/src/rabbit_auth_mechanism_plain.erl b/src/rabbit_auth_mechanism_plain.erl
index 7de61976..8758f85f 100644
--- a/src/rabbit_auth_mechanism_plain.erl
+++ b/src/rabbit_auth_mechanism_plain.erl
@@ -34,7 +34,7 @@
 
 -behaviour(rabbit_auth_mechanism).
 
--export([description/0, should_offer/1, init/1, handle_response/2]).
+-export([description/0, init/1, handle_response/2]).
 
 -include("rabbit_auth_mechanism_spec.hrl").
 
@@ -52,9 +52,6 @@ description() ->
     [{name, <<"PLAIN">>},
      {description, <<"SASL PLAIN authentication mechanism">>}].
 
-should_offer(_Sock) ->
-    true.
-
 init(_Sock) ->
     [].
 
diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl
index 05ca1df5..57d82d80 100644
--- a/src/rabbit_reader.erl
+++ b/src/rabbit_reader.erl
@@ -687,7 +687,7 @@ start_connection({ProtocolMajor, ProtocolMinor, _ProtocolRevision},
       version_major = ProtocolMajor,
       version_minor = ProtocolMinor,
       server_properties = server_properties(),
-      mechanisms = auth_mechanisms_binary(Sock),
+      mechanisms = auth_mechanisms_binary(),
       locales = <<"en_US">> },
     ok = send_on_channel0(Sock, Start, Protocol),
     switch_callback(State#v1{connection = Connection#connection{
@@ -739,7 +739,7 @@ handle_method0(#'connection.start_ok'{mechanism = Mechanism,
                State0 = #v1{connection_state = starting,
                             connection       = Connection,
                             sock             = Sock}) ->
-    AuthMechanism = auth_mechanism_to_module(Mechanism, Sock),
+    AuthMechanism = auth_mechanism_to_module(Mechanism),
     State = State0#v1{auth_mechanism   = AuthMechanism,
                       auth_state       = AuthMechanism:init(Sock),
                       connection_state = securing,
@@ -833,14 +833,14 @@ handle_method0(_Method, #v1{connection_state = S}) ->
 send_on_channel0(Sock, Method, Protocol) ->
     ok = rabbit_writer:internal_send_command(Sock, 0, Method, Protocol).
 
-auth_mechanism_to_module(TypeBin, Sock) ->
+auth_mechanism_to_module(TypeBin) ->
     case rabbit_registry:binary_to_type(TypeBin) of
         {error, not_found} ->
             rabbit_misc:protocol_error(
               command_invalid, "unknown authentication mechanism '~s'",
               [TypeBin]);
         T ->
-            case {lists:member(T, auth_mechanisms(Sock)),
+            case {lists:member(T, auth_mechanisms()),
                   rabbit_registry:lookup_module(auth_mechanism, T)} of
                 {true, {ok, Module}} ->
                     Module;
@@ -851,15 +851,15 @@ auth_mechanism_to_module(TypeBin, Sock) ->
             end
     end.
 
-auth_mechanisms(Sock) ->
+auth_mechanisms() ->
     {ok, Configured} = application:get_env(auth_mechanisms),
-    [Name || {Name, Module} <- rabbit_registry:lookup_all(auth_mechanism),
-             Module:should_offer(Sock), lists:member(Name, Configured)].
+    [Name || {Name, _Module} <- rabbit_registry:lookup_all(auth_mechanism),
+             lists:member(Name, Configured)].
 
-auth_mechanisms_binary(Sock) ->
+auth_mechanisms_binary() ->
     list_to_binary(
             string:join(
-              [atom_to_list(A) || A <- auth_mechanisms(Sock)], " ")).
+              [atom_to_list(A) || A <- auth_mechanisms()], " ")).
 
 auth_phase(Response,
            State = #v1{auth_mechanism = AuthMechanism,