ssl_cert_login_san_type = other_name returns a non-standard attribute value

[Thibi2000]

Hello,

I've posted about it before here, but no on responded, so I am trying
here again with a fix.

Context

I want to be able to authenticate users with their otherName of Subject Alternative
Name of their certificates. This is possible using the following
rabbitmq.conf:

listeners.ssl.default = 5671
auth_mechanisms.1 = EXTERNAL
ssl_cert_login_from = subject_alternative_name
ssl_cert_login_san_type = other_name
ssl_cert_login_san_index =0

ssl_options.cacertfile = /path/root-cert.pem
ssl_options.certfile   = /path/server-cert.pem
ssl_options.keyfile    = /path/server.key
ssl_options.verify     = verify_peer
ssl_options.depth  = 1
ssl_options.fail_if_no_peer_cert = true

To create a test situation, you can use the following commands. It
will create the necessary certificates:

# self signed root
openssl req -x509 -newkey rsa:2048 -keyout root-key.pem -out root-cert.pem -days 365 -nodes -subj "/CN=RABBIT TEST ROOT"
# server cert
openssl req -new -out server.csr -newkey rsa:2048 -nodes -sha256 -keyout server.key -subj "/CN=server"
openssl x509 -req -in server.csr -CA root-cert.pem -CAkey root-key.pem -CAcreateserial -out server-cert.pem
# client cert with SAN
openssl req -new -out client.csr -newkey rsa:2048 -nodes -sha256 -keyout client.key -subj "/CN=client"
openssl x509 -req -in client.csr -out client-cert.pem -CA root-cert.pem -CAkey root-key.pem  -CAcreateserial \
        -extensions SAN \
        -extfile <(cat /etc/ssl/openssl.cnf \
                       <(printf "\n[SAN\nsubjectAltName=otherName:1.3.6.1.4.1.54392.5.436;UTF8:username,DNS:username,URI:username"))
openssl pkcs12 -export -in <(cat root-cert.pem client-cert.pem client.key) -password pass:"verystrong" \
        -name "client" -out "client.p12"
keytool -import -alias root -file root-cert.pem \
        -keystore truststore -storepass verystrong \
        -noprompt

Make sure the certificates are stored in the correct paths according
to the configuration.

The username will be username. It is possible to add users during
startup adding the following line to the config:

load_definitions=/path/to/defs.json

This .json file will contain the following:

{
    "users": [
        {
            "tags": "",
            "name": "username"
        }
    ],
    "permissions": [
        {
            "user": "username",
            "vhost": "/",
            "read": ".*",
            "write": ".*",
            "configure": ".*"
        }
    ],
    "vhosts": [
        {
            "name": "/"
        }
    ],
    "bindings": [],
    "exchanges": [],
    "queues": [],
    "policies": [],
    "parameters": []
}

The plugin that makes it possible to login with the certificates is
rabbitmq-auth-mechanism-ssl.

The only thing that is needed is a client program.

char[] keyPassphrase = "verystrong".toCharArray();
KeyStore ks = KeyStore.getInstance("PKCS12");
ks.load(new FileInputStream("client.p12"), keyPassphrase);

KeyManagerFactory kmf = KeyManagerFactory.getInstance("SunX509");
kmf.init(ks, keyPassphrase);

char[] trustPassphrase = "verystrong".toCharArray();
KeyStore tks = KeyStore.getInstance("JKS");
tks.load(new FileInputStream("truststore"), trustPassphrase);

TrustManagerFactory tmf = TrustManagerFactory.getInstance("SunX509");
tmf.init(tks);

SSLContext c = SSLContext.getInstance("TLSv1.2");
c.init(kmf.getKeyManagers(), tmf.getTrustManagers(), null);
factory.useSslProtocol(c);
factory.setUsername(USERNAME);
    factory.setSaslConfig(DefaultSaslConfig.EXTERNAL);

try (Connection connection = factory.newConnection();
     Channel channel = connection.createChannel()) {
    channel.queueDeclare(QUEUE_NAME, false, false, false, null);
    String message = "message";
    channel.basicPublish("", QUEUE_NAME, null, message.getBytes());
}

Expected Behavior

This client should be able to login, and send the message.

Current Behavior

When trying to connect the following error happens:

2021-04-18 17:52:14.855771+02:00 [info] <0.707.0> accepting AMQP connection <0.707.0> (127.0.0.1:47544 -> 127.0.0.1:5671)
2021-04-18 17:52:14.928386+02:00 [debug] <0.707.0> Peer certificate SANs of type other_name: [{otherName,
2021-04-18 17:52:14.928386+02:00 [debug] <0.707.0>                                             {'AnotherName',
2021-04-18 17:52:14.928386+02:00 [debug] <0.707.0>                                              {1,3,6,1,4,1,54392,5,436},
2021-04-18 17:52:14.928386+02:00 [debug] <0.707.0>                                              <<"\f\busername">>}}], index to use with lists:nth/2: 1
2021-04-18T17:52:14.928570+02:00 debug: FORMATTER CRASH: {"auth mechanism TLS extracted username '~s' from peer certificate",[{'AnotherName',{1,3,6,1,4,1,34907},<<"\f\busername">>}]}

The ~s tells us that it expects a string, while an object is
passed.

Test with DNS

Changing ssl_cert_login_san_type to DNS or Uri doesn't result in errors.

2021-04-18 17:54:07.371091+02:00 [debug] <0.641.0> Peer certificate SANs of type dns: [{dNSName,"username"}], index to use with lists:nth/2: 1
2021-04-18 17:54:07.383745+02:00 [info] <0.641.0> connection <0.641.0> (127.0.0.1:47550 -> 127.0.0.1:5671): user 'username' authenticated and granted access to vhost '/'

Fix

When having a look through the code, I found the following relevant
part in deps/rabbit/src/rabbit_ssl.erl:peer_cert_auth_name

{_, Value} = lists:nth(Index, OfType),
rabbit_data_coercion:to_binary(Value)

nth is expected to be of the following format: {name, value}. From the resulting output when using other_name it is
proven that this is not always the case.

I will go on a hunch and assume that every other type that isn't
other_name is of the format {name, value} because that is how
they look inside of the certificate.

My fix is the following:

diff --git a/deps/rabbit/src/rabbit_ssl.erl b/deps/rabbit/src/rabbit_ssl.erl
index 7535492a0a..908a35d6dd 100644
--- a/deps/rabbit/src/rabbit_ssl.erl
+++ b/deps/rabbit/src/rabbit_ssl.erl
@@ -159,7 +159,10 @@ peer_cert_auth_name(subject_alternative_name, Cert) ->
                 N when N < Index  -> not_found;
                 N when N >= Index ->
                     {_, Value} = lists:nth(Index, OfType),
-                    rabbit_data_coercion:to_binary(Value)
+		            if is_tuple(Value) ->
+		    	        string:slice(element(3, Value), 2);
+		                true -> rabbit_data_coercion:to_binary(Value)
+		            end
             end;
         false -> unsafe
     end;

I tested it with both dns and other_name. However, I have no
knowledge of Erlang or this codebase, so I might be mistaken.

I don't really have the time to test everything nor to make a decent
pull request. I hope someone who maintains this, can fix this based
on the information I give here. I am sure it is not such a big deal
for an experience Rabbit Contributor. Forgive me for being lazy.

If I made any mistakes in this message, be sure to let me know so I
can fix it.

[Thibi2000]

A little note on string:slice(element(3, Value), 2), this is because the string value is proceeded by \f\b, so the first 2 characters are irrelevant. I don't know why they're there, and I didn't check if this is the case with every certificate. It's at least like this with certificates generated using openssl and HashiCorp's Vault.

[michaelklishin]

The following openssl.cnf modification to tls-gen produces a certificate with a SAN of type other:

[ client_alt_names ]
DNS.1 = $common_name
DNS.2 = $client_alt_name
DNS.3 = localhost
# examples of more Subject Alternative Names
# DNS.4 = guest
# email = guest@warp10.local
# URI   = amqps://123.client.warp10.local
otherName = 1.3.6.1.5.5.7.8.9;FORMAT:UTF8,UTF8String:nonasciiname.example.com

FWIW I haven't see this type used out there in the wild, perhaps Vault is the only popular tool that does it. So it was added primarily for completeness, not because we actually have seen any demand or examples of such custom OID extensions.

So I have no idea how to test this, I'm afraid our team has limited resources and we would really appreciate if users such as @Thibi2000 could actually test the changes we make. The core team cannot be expected to produce a suitable environment for every use case someone on the Internet has thought of.

[michaelklishin]

With the above change, OpenSSL 1.1.1k CLI tools report that this extension is "unsupported":

            X509v3 Subject Alternative Name:
                DNS:moraine.local, DNS:moraine.local, DNS:localhost, othername:<unsupported>

[michaelklishin]

Besides ASN.1 encoding and decoding, there are no tests in the Erlang PKI or TLS implementation suites that use any values of otherName. The value seems to be, well, completely arbitrary in nature. The only thing I can tell is that it is decoded
as {'AnotherName', Term1, Term2}, which is an unorthodox way of encoding a key=value pair I guess.

I will adjust the code to extract value from it but I have no idea where the prefix is coming from, how can we test this
or even if support for this field type is even realistic. SANs by definition follow certain conventions and otherName is arbitrary data of custom types.

The fact that in order to inspect the value you have to mess with ASN parser commands directly with openssl CLI tools suggests it is really not a commonly used feature.

[michaelklishin]

The \f\b (or <<12, 15>> in Erlang binary) prefix seems to be an artefact of UTF value encoding.

[michaelklishin]

Closed by accident, I actually have a PR ready.