Merge pull request #2984 from rabbitmq/rabbitmq-server-2983

x.509 certificate auth mechanism: support (some) SANS of type otherName

3 files changed, 31 insertions, 8 deletions

diff --git a/deps/rabbit/src/rabbit_ssl.erl b/deps/rabbit/src/rabbit_ssl.erl
index 7535492a0a..7fc0700aff 100644
--- a/deps/rabbit/src/rabbit_ssl.erl
+++ b/deps/rabbit/src/rabbit_ssl.erl
@@ -130,13 +130,12 @@ peer_cert_validity(Cert) ->
     rabbit_cert_info:validity(Cert).
 
 %% Extract a username from the certificate
--spec peer_cert_auth_name
-        (certificate()) -> binary() | 'not_found' | 'unsafe'.
-
+-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),
     peer_cert_auth_name(Mode, Cert).
 
+-spec peer_cert_auth_name(atom(), certificate()) -> binary() | 'not_found' | 'unsafe'.
 peer_cert_auth_name(distinguished_name, Cert) ->
     case auth_config_sane() of
         true  -> iolist_to_binary(peer_cert_subject(Cert));
@@ -158,8 +157,17 @@ peer_cert_auth_name(subject_alternative_name, Cert) ->
                 0                 -> not_found;
                 N when N < Index  -> not_found;
                 N when N >= Index ->
-                    {_, Value} = lists:nth(Index, OfType),
-                    rabbit_data_coercion:to_binary(Value)
+                    Nth = lists:nth(Index, OfType),
+                    case Nth of
+                      %% this is SAN of type otherName; it can be anything, so we simply try to extract the value
+                      %% the best we can and return it. There aren't really any conventions or widely held expectations
+                      %% about the format :(
+                      {otherName, {'AnotherName', _, Value}} ->
+                          rabbit_cert_info:sanitize_other_name(rabbit_data_coercion:to_binary(Value));
+                      %% most SAN types return a pair: DNS, email, URI
+                      {_, Value} ->
+                          rabbit_data_coercion:to_binary(Value)
+                    end
             end;
         false -> unsafe
     end;
@@ -193,3 +201,4 @@ otp_san_type(email)      -> rfc822Name;
 otp_san_type(uri)        -> uniformResourceIdentifier;
 otp_san_type(other_name) -> otherName;
 otp_san_type(Other)      -> Other.
+
diff --git a/deps/rabbit_common/src/rabbit_cert_info.erl b/deps/rabbit_common/src/rabbit_cert_info.erl
index 771209229b..abb2df061c 100644
--- a/deps/rabbit_common/src/rabbit_cert_info.erl
+++ b/deps/rabbit_common/src/rabbit_cert_info.erl
@@ -17,11 +17,13 @@
          extensions/1
 ]).
 
+-export([sanitize_other_name/1]).
+
 %%--------------------------------------------------------------------------
 
 -export_type([certificate/0]).
 
--type certificate() :: binary().
+-type certificate() :: #'OTPCertificate'{}.
 
 %%--------------------------------------------------------------------------
 %% High-level functions used by reader
@@ -103,6 +105,13 @@ find_by_type(Type, {rdnSequence, RDNs}) ->
 %% Formatting functions
 %%--------------------------------------------------------------------------
 
+sanitize_other_name(Bin) when is_binary(Bin) ->
+    %% strip off leading values that seem to be ASN.1 UTF value encoding artifacts
+    case Bin of
+        <<12, 14, Rest/binary>> -> Rest;
+        Other                   -> Other
+    end.
+
 %% Format and rdnSequence as a RFC4514 subject string.
 format_rdn_sequence({rdnSequence, Seq}) ->
     string:join(lists:reverse([format_complex_rdn(RDN) || RDN <- Seq]), ",").
diff --git a/deps/rabbitmq_auth_mechanism_ssl/src/rabbit_auth_mechanism_ssl.erl b/deps/rabbitmq_auth_mechanism_ssl/src/rabbit_auth_mechanism_ssl.erl
index f47742ec3a..337edc5058 100644
--- a/deps/rabbitmq_auth_mechanism_ssl/src/rabbit_auth_mechanism_ssl.erl
+++ b/deps/rabbitmq_auth_mechanism_ssl/src/rabbit_auth_mechanism_ssl.erl
@@ -49,14 +49,19 @@ init(Sock) ->
                        case rabbit_ssl:peer_cert_auth_name(C) of
                            unsafe    -> {refused, none, "TLS configuration is unsafe", []};
                            not_found -> {refused, none, "no name found", []};
-                           Name      -> rabbit_data_coercion:to_binary(Name)
+                           Name      ->
+                               %% strip any leading and trailing newlines, we assume that they are never
+                               %% added intentionally. For SANS of type otherName, the Erlang ASN.1/public key parser
+                               %% seems to add some in certain cases
+                               Val = rabbit_data_coercion:to_binary(Name),
+                               rabbit_log:debug("auth mechanism TLS extracted username '~s' from peer certificate", [Val]),
+                               Val
                        end;
                    {error, no_peercert} ->
                        {refused, none, "connection peer presented no TLS (x.509) certificate", []};
                    nossl ->
                        {refused, none, "not a TLS-enabled connection", []}
                end,
-    rabbit_log:debug("auth mechanism TLS extracted username '~s' from peer certificate", [Username]),
     #state{username = Username}.
 
 handle_response(_Response, #state{username = Username}) ->