Extract client_id from client cert

deps/rabbit/src/rabbit_ssl.erl

@@ -10,7 +10,7 @@
 -include_lib("public_key/include/public_key.hrl").
 
 -export([peer_cert_issuer/1, peer_cert_subject/1, peer_cert_validity/1]).
--export([peer_cert_subject_items/2, peer_cert_auth_name/1]).
+-export([peer_cert_subject_items/2, peer_cert_auth_name/1, peer_cert_auth_name/2]).
 -export([cipher_suites_erlang/2, cipher_suites_erlang/1,
          cipher_suites_openssl/2, cipher_suites_openssl/1,
          cipher_suites/1]).
@@ -18,7 +18,7 @@
 
 %%--------------------------------------------------------------------------
 
--export_type([certificate/0]).
+-export_type([certificate/0, ssl_cert_login_type/0]).
 
 % Due to API differences between OTP releases.
 -dialyzer(no_missing_calls).
@@ -109,28 +109,51 @@ peer_cert_subject_alternative_names(Cert, Type) ->
 peer_cert_validity(Cert) ->
     rabbit_cert_info:validity(Cert).
 
+-type ssl_cert_login_type() :: 
+    {subject_alternative_name | subject_alt_name, atom(), integer()} | 
+    {distinguished_name | common_name, undefined, undefined }.
+
+-spec extract_ssl_cert_login_settings() -> none | ssl_cert_login_type().
+extract_ssl_cert_login_settings() ->
+    case application:get_env(rabbit, ssl_cert_login_from) of 
+        {ok, Mode} ->
+            case Mode of 
+                subject_alternative_name -> extract_san_login_type(Mode);
+                subject_alt_name -> extract_san_login_type(Mode);
+                _ -> {Mode, undefined, undefined}
+            end;
+        undefined -> none 
+    end.
+
+extract_san_login_type(Mode) ->
+    {Mode,
+        application:get_env(rabbit, ssl_cert_login_san_type, dns),
+        application:get_env(rabbit, ssl_cert_login_san_index, 0)
+    }.
+
 %% Extract a username from the certificate
 -spec peer_cert_auth_name(certificate()) -> binary() | 'not_found' | 'unsafe'.
 peer_cert_auth_name(Cert) ->
-    {ok, Mode} = application:get_env(rabbit, ssl_cert_login_from),
+    case extract_ssl_cert_login_settings() of 
-    peer_cert_auth_name(Mode, Cert).
+        none -> 'not_found';
+        Settings -> peer_cert_auth_name(Settings, Cert)        
+    end.
 
--spec peer_cert_auth_name(atom(), certificate()) -> binary() | 'not_found' | 'unsafe'.
+-spec peer_cert_auth_name(ssl_cert_login_type(), certificate()) -> binary() | 'not_found' | 'unsafe'.
-peer_cert_auth_name(distinguished_name, Cert) ->
+peer_cert_auth_name({distinguished_name, _, _}, Cert) ->
     case auth_config_sane() of
         true  -> iolist_to_binary(peer_cert_subject(Cert));
         false -> unsafe
     end;
 
-peer_cert_auth_name(subject_alt_name, Cert) ->
+peer_cert_auth_name({subject_alt_name, Type, Index0}, Cert) ->
-    peer_cert_auth_name(subject_alternative_name, Cert);
+    peer_cert_auth_name({subject_alternative_name, Type, Index0}, Cert);
 
-peer_cert_auth_name(subject_alternative_name, Cert) ->
+peer_cert_auth_name({subject_alternative_name, Type, Index0}, Cert) ->
     case auth_config_sane() of
         true  ->
-            Type   = application:get_env(rabbit, ssl_cert_login_san_type,  dns),
             %% lists:nth/2 is 1-based
-            Index  = application:get_env(rabbit, ssl_cert_login_san_index, 0) + 1,
+            Index  = Index0 + 1,
             OfType = peer_cert_subject_alternative_names(Cert, otp_san_type(Type)),
             rabbit_log:debug("Peer certificate SANs of type ~ts: ~tp, index to use with lists:nth/2: ~b", [Type, OfType, Index]),
             case length(OfType) of
@@ -152,7 +175,7 @@ peer_cert_auth_name(subject_alternative_name, Cert) ->
         false -> unsafe
     end;
 
-peer_cert_auth_name(common_name, Cert) ->
+peer_cert_auth_name({common_name, _, _}, Cert) ->
     %% If there is more than one CN then we join them with "," in a
     %% vaguely DN-like way. But this is more just so we do something
     %% more intelligent than crashing, if you actually want to escape

deps/rabbitmq_ct_helpers/tools/tls-certs/openssl.cnf.in

@@ -49,6 +49,7 @@ keyUsage = keyCertSign, cRLSign
 [ client_ca_extensions ]
 basicConstraints = CA:false
 keyUsage         = digitalSignature,keyEncipherment
+subjectAltName   = @client_alt_names
 
 [ server_ca_extensions ]
 basicConstraints = CA:false
@@ -59,3 +60,6 @@ subjectAltName   = @server_alt_names
 [ server_alt_names ]
 DNS.1 = @HOSTNAME@
 DNS.2 = localhost
+
+[ client_alt_names ]
+DNS.1 = rabbit_client_id

deps/rabbitmq_mqtt/BUILD.bazel

@@ -135,7 +135,7 @@ rabbitmq_integration_suite(
         "test/rabbit_auth_backend_mqtt_mock.beam",
         "test/util.beam",
     ],
-    shard_count = 14,
+    shard_count = 18,
     runtime_deps = [
         "@emqtt//:erlang_app",
         "@meck//:erlang_app",

deps/rabbitmq_mqtt/priv/schema/rabbitmq_mqtt.schema

@@ -156,6 +156,20 @@ end}.
     {datatype, {enum, [true, false]}}]}.
 
 
+{mapping, "mqtt.ssl_cert_client_id_from", "rabbitmq_mqtt.ssl_cert_client_id_from", [
+    {datatype, {enum, [distinguished_name, subject_alternative_name]}}
+]}.
+
+{mapping, "mqtt.ssl_cert_login_san_type", "rabbitmq_mqtt.ssl_cert_login_san_type", [
+    {datatype, {enum, [dns, ip, email, uri, other_name]}}
+]}.
+
+{mapping, "mqtt.ssl_cert_login_san_index", "rabbitmq_mqtt.ssl_cert_login_san_index", [
+    {datatype, integer}, {validators, ["non_negative_integer"]}
+]}.
+
+
+
 %% TCP/Socket options (as per the broker configuration).
 %%
 %% {tcp_listen_options, [{backlog,   128},

deps/rabbitmq_mqtt/src/rabbit_mqtt_processor.erl

@@ -182,9 +182,9 @@ process_connect(
     Result0 =
     maybe
         ok ?= check_extended_auth(ConnectProps),
-        {ok, ClientId} ?= ensure_client_id(ClientId0, CleanStart, ProtoVer),
+        {ok, ClientId1} ?= extract_client_id_from_certificate(ClientId0, Socket),
+        {ok, ClientId} ?= ensure_client_id(ClientId1, CleanStart, ProtoVer),
         {ok, Username1, Password} ?= check_credentials(Username0, Password0, SslLoginName, PeerIp),
-
         {VHostPickedUsing, {VHost, Username2}} = get_vhost(Username1, SslLoginName, Port),
         ?LOG_DEBUG("MQTT connection ~s picked vhost using ~s", [ConnName0, VHostPickedUsing]),
         ok ?= check_vhost_exists(VHost, Username2, PeerIp),
@@ -642,6 +642,26 @@ check_credentials(Username, Password, SslLoginName, PeerIp) ->
             {error, ?RC_BAD_USER_NAME_OR_PASSWORD}
     end.
 
+%% Extract client_id from the certificate provided it was configured to do so and
+%% it is possible to extract it else returns the client_id passed as parameter
+-spec extract_client_id_from_certificate(client_id(), rabbit_net:socket()) -> {ok, client_id()} | {error, reason_code()}.
+extract_client_id_from_certificate(Client0, Socket) ->
+    case extract_ssl_cert_client_id_settings() of
+        none -> {ok, Client0};
+        SslClientIdSettings ->
+            case ssl_client_id(Socket, SslClientIdSettings) of
+                none ->
+                    {ok, Client0};
+                V when V == Client0 ->
+                    {ok, Client0};
+                V ->
+                    ?LOG_ERROR(
+                        "MQTT login failed: client_id in cert (~p) does not match client_id in protocol (~p)",
+                        [V, Client0]),
+                    {error, ?RC_CLIENT_IDENTIFIER_NOT_VALID}
+            end
+    end.
+
 -spec ensure_client_id(client_id(), boolean(), protocol_version()) ->
     {ok, client_id()} | {error, reason_code()}.
 ensure_client_id(<<>>, _CleanStart = false, ProtoVer)
@@ -1029,16 +1049,9 @@ check_vhost_alive(VHost) ->
     end.
 
 check_user_login(VHost, Username, Password, ClientId, PeerIp, ConnName) ->
-    AuthProps = case Password of
+    AuthProps = [{vhost, VHost},
-                    none ->
+                  {client_id, ClientId},
-                        %% SSL user name provided.
+                  {password, Password}],
-                        %% Authenticating using username only.
-                        [];
-                    _ ->
-                        [{password, Password},
-                         {vhost, VHost},
-                         {client_id, ClientId}]
-                end,
     case rabbit_access_control:check_user_login(Username, AuthProps) of
         {ok, User = #user{username = Username1}} ->
             notify_auth_result(user_authentication_success, Username1, ConnName),
@@ -2292,6 +2305,37 @@ ssl_login_name(Sock) ->
         nossl                -> none
     end.
 
+-spec extract_ssl_cert_client_id_settings() -> none | rabbit_ssl:ssl_cert_login_type().
+extract_ssl_cert_client_id_settings() ->
+    case application:get_env(?APP_NAME, ssl_cert_client_id_from) of
+        {ok, Mode} ->
+            case Mode of
+                subject_alternative_name -> extract_client_id_san_type(Mode);
+                _ -> {Mode, undefined, undefined}
+            end;
+        undefined -> none
+    end.
+
+extract_client_id_san_type(Mode) ->
+    {Mode,
+        application:get_env(?APP_NAME, ssl_cert_client_id_san_type, dns),
+        application:get_env(?APP_NAME, ssl_cert_client_id_san_index, 0)
+    }.
+
+
+-spec ssl_client_id(rabbit_net:socket(), rabbit_ssl:ssl_cert_login_type()) ->
+    none | binary().
+ssl_client_id(Sock, SslClientIdSettings) ->
+    case rabbit_net:peercert(Sock) of
+        {ok, C}              -> case rabbit_ssl:peer_cert_auth_name(SslClientIdSettings, C) of
+                                    unsafe    -> none;
+                                    not_found -> none;
+                                    Name      -> Name
+                                end;
+        {error, no_peercert} -> none;
+        nossl                -> none
+    end.
+
 -spec proto_integer_to_atom(protocol_version()) -> protocol_version_atom().
 proto_integer_to_atom(3) ->
     ?MQTT_PROTO_V3;

deps/rabbitmq_mqtt/test/auth_SUITE.erl

@@ -68,6 +68,13 @@ sub_groups() ->
        ssl_user_vhost_parameter_mapping_vhost_does_not_exist,
        ssl_user_cert_vhost_mapping_takes_precedence_over_port_vhost_mapping
       ]},
+     {ssl_user_with_client_id_in_cert_san_dns, [], 
+       [client_id_from_cert_san_dns,
+        invalid_client_id_from_cert_san_dns
+       ]},
+     {ssl_user_with_client_id_in_cert_dn, [], 
+       [client_id_from_cert_dn
+       ]},  
      {no_ssl_user, [shuffle],
       [anonymous_auth_failure,
        user_credentials_auth,
@@ -194,14 +201,27 @@ mqtt_config(no_ssl_user) ->
 mqtt_config(client_id_propagation) ->
     {rabbitmq_mqtt, [{ssl_cert_login,  true},
                      {allow_anonymous, true}]};
+mqtt_config(ssl_user_with_client_id_in_cert_san_dns) ->
+    {rabbitmq_mqtt, [{ssl_cert_login,  true},
+                     {allow_anonymous, false}, 
+                     {ssl_cert_client_id_from, subject_alternative_name},
+                     {ssl_cert_client_id_san_type, dns}]};
+mqtt_config(ssl_user_with_client_id_in_cert_dn) ->
+    {rabbitmq_mqtt, [{ssl_cert_login,  true},
+                     {allow_anonymous, false}, 
+                     {ssl_cert_client_id_from, distinguished_name}
+                     ]};
 mqtt_config(_) ->
     undefined.
 
-auth_config(client_id_propagation) ->
+auth_config(T) when T == client_id_propagation; 
+                    T == ssl_user_with_client_id_in_cert_san_dns;
+                    T == ssl_user_with_client_id_in_cert_dn ->
     {rabbit, [
             {auth_backends, [rabbit_auth_backend_mqtt_mock]}
           ]
     };
+
 auth_config(_) ->
     undefined.
 
@@ -292,9 +312,24 @@ init_per_testcase(T, Config)
         v4 -> {skip, "Will Delay Interval is an MQTT 5.0 feature"};
         v5 -> testcase_started(Config, T)
     end;
+init_per_testcase(T, Config) 
+  when T =:= client_id_propagation;
+       T =:= invalid_client_id_from_cert_san_dns;
+       T =:= client_id_from_cert_san_dns;
+       T =:= client_id_from_cert_dn ->
+    SetupProcess = setup_rabbit_auth_backend_mqtt_mock(Config),
+    rabbit_ct_helpers:set_config(Config, {mock_setup_process, SetupProcess});
+       
 init_per_testcase(Testcase, Config) ->
     testcase_started(Config, Testcase).
 
+get_client_cert_subject(Config) ->
+    CertsDir = ?config(rmq_certsdir, Config),
+    CertFile = filename:join([CertsDir, "client", "cert.pem"]),
+    {ok, CertBin} = file:read_file(CertFile),
+    [{'Certificate', Cert, not_encrypted}] = public_key:pem_decode(CertBin),
+    iolist_to_binary(rpc(Config, 0, rabbit_ssl, peer_cert_subject, [Cert])).
+    
 set_cert_user_on_default_vhost(Config) ->
     CertsDir = ?config(rmq_certsdir, Config),
     CertFile = filename:join([CertsDir, "client", "cert.pem"]),
@@ -404,6 +439,15 @@ end_per_testcase(T, Config) when T == queue_bind_permission;
     file:write_file(?config(log_location, Config), <<>>),
 
     rabbit_ct_helpers:testcase_finished(Config, T);
+
+end_per_testcase(T, Config) 
+   when T =:= client_id_propagation;
+       T =:= invalid_client_id_from_cert_san_dns;
+       T =:= client_id_from_cert_san_dns;
+       T =:= client_id_from_cert_dn ->
+    SetupProcess = ?config(mock_setup_process, Config),
+    SetupProcess ! stop;
+    
 end_per_testcase(Testcase, Config) ->
     rabbit_ct_helpers:testcase_finished(Config, Testcase).
 
@@ -455,6 +499,36 @@ user_credentials_auth(Config) ->
         fun(Conf) -> connect_user(<<"non-existing-vhost:guest">>, <<"guest">>, Conf) end,
         Config).
 
+client_id_from_cert_san_dns(Config) ->    
+    ExpectedClientId = <<"rabbit_client_id">>, % Found in the client's certificate as SAN type CLIENT_ID 
+    MqttClientId = ExpectedClientId,
+    {ok, C} = connect_ssl(MqttClientId, Config),
+    {ok, _} = emqtt:connect(C),
+    [{authentication, AuthProps}] = rpc(Config, 0,
+                                        rabbit_auth_backend_mqtt_mock,
+                                        get,
+                                        [authentication]),
+    ?assertEqual(ExpectedClientId, proplists:get_value(client_id, AuthProps)),
+    ok = emqtt:disconnect(C).
+
+client_id_from_cert_dn(Config) ->    
+    ExpectedClientId = get_client_cert_subject(Config), % subject = distinguished_name
+    MqttClientId = ExpectedClientId,
+    {ok, C} = connect_ssl(MqttClientId, Config),
+    {ok, _} = emqtt:connect(C),
+    [{authentication, AuthProps}] = rpc(Config, 0,
+                                        rabbit_auth_backend_mqtt_mock,
+                                        get,
+                                        [authentication]),
+    ?assertEqual(ExpectedClientId, proplists:get_value(client_id, AuthProps)),
+    ok = emqtt:disconnect(C).
+
+invalid_client_id_from_cert_san_dns(Config) ->    
+    MqttClientId = <<"other_client_id">>,
+    {ok, C} = connect_ssl(MqttClientId, Config),
+    ?assertMatch({error, _}, emqtt:connect(C)),
+    unlink(C).
+
 ssl_user_vhost_parameter_mapping_success(Config) ->
     expect_successful_connection(fun connect_ssl/1, Config).
 
@@ -506,6 +580,9 @@ connect_anonymous(Config, ClientId) ->
                       {proto_ver, ?config(mqtt_version, Config)}]).
 
 connect_ssl(Config) ->
+    connect_ssl(<<"simpleClient">>, Config).
+
+connect_ssl(ClientId, Config) ->
     CertsDir = ?config(rmq_certsdir, Config),
     SSLConfig = [{cacertfile, filename:join([CertsDir, "testca", "cacert.pem"])},
                  {certfile, filename:join([CertsDir, "client", "cert.pem"])},
@@ -514,12 +591,12 @@ connect_ssl(Config) ->
     P = rabbit_ct_broker_helpers:get_node_config(Config, 0, tcp_port_mqtt_tls),
     emqtt:start_link([{host, "localhost"},
                       {port, P},
-                      {clientid, <<"simpleClient">>},
+                      {clientid, ClientId},
                       {proto_ver, ?config(mqtt_version, Config)},
                       {ssl, true},
                       {ssl_opts, SSLConfig}]).
 
-client_id_propagation(Config) ->
+setup_rabbit_auth_backend_mqtt_mock(Config) ->
     ok = rabbit_ct_broker_helpers:add_code_path_to_all_nodes(Config,
                                                              rabbit_auth_backend_mqtt_mock),
     %% setup creates the ETS table required for the mqtt auth mock
@@ -530,11 +607,13 @@ client_id_propagation(Config) ->
                   rpc(Config, 0, rabbit_auth_backend_mqtt_mock, setup, [Self])
           end),
     %% the setup process will notify us
-    SetupProcess = receive
+    receive
         {ok, SP} -> SP
     after
         3000 -> ct:fail("timeout waiting for rabbit_auth_backend_mqtt_mock:setup/1")
-    end,
+    end.
+
+client_id_propagation(Config) ->
     ClientId = <<"client-id-propagation">>,
     {ok, C} = connect_user(<<"fake-user">>, <<"fake-password">>,
                            Config, ClientId),
@@ -565,11 +644,8 @@ client_id_propagation(Config) ->
     VariableMap = maps:get(variable_map, TopicContext),
     ?assertEqual(ClientId, maps:get(<<"client_id">>, VariableMap)),
 
-    ok = emqtt:disconnect(C),
+    emqtt:disconnect(C).
     
-    SetupProcess ! stop,
-
-    ok.
 
 %% These tests try to cover all operations that are listed in the
 %% table in https://www.rabbitmq.com/access-control.html#authorisation

deps/rabbitmq_mqtt/test/config_schema_SUITE_data/rabbitmq_mqtt.snippets

@@ -95,6 +95,22 @@
   "ssl_cert_login_from = common_name",
   [{rabbit,[{ssl_cert_login_from,common_name}]}],
   [rabbitmq_mqtt]},
+ {ssl_cert_client_id_from_common_name,
+  "mqtt.ssl_cert_client_id_from = distinguished_name",
+  [{rabbitmq_mqtt,[{ssl_cert_client_id_from,distinguished_name}]}],
+  [rabbitmq_mqtt]},
+ {ssl_cert_login_dns_san_type,
+  "mqtt.ssl_cert_login_san_type = dns",
+  [{rabbitmq_mqtt,[{ssl_cert_login_san_type,dns}]}],
+  [rabbitmq_mqtt]},
+ {ssl_cert_login_other_name_san_type,
+  "mqtt.ssl_cert_login_san_type = other_name",
+  [{rabbitmq_mqtt,[{ssl_cert_login_san_type,other_name}]}],
+  [rabbitmq_mqtt]},    
+ {ssl_cert_login_san_index,
+  "mqtt.ssl_cert_login_san_index = 0",
+  [{rabbitmq_mqtt,[{ssl_cert_login_san_index,0}]}],
+  [rabbitmq_mqtt]},   
  {proxy_protocol,
   "listeners.tcp.default = 5672
    mqtt.allow_anonymous  = true