broker 1.5 conf use_identity_as_username true TLS is broken

[idcrook]

On a raspberry pi using raspbian package Version: 1.5-0mosquitto1, in the broker logfile, the logged username no longer matches the Common Name found in the client certificate.

output from: sudo less /var/log/mosquitto/mosquitto.log

1527190662: New client connected from 10.0.1.88 as 389IB4d3dWUllZpTLXUKcI (c1, k600, u'^E').
[...]
1527192380: New client connected from 10.0.1.51 as mosqpub/2404-rpi1 (c1, k60, u'^D').

the u'^E' and u'^D' in the log message above used to reflect the Common Name in the certificate, e.g. in output from 1.4.15 it is u'rpip3'

1526905503: New client connected from 10.0.1.88 as 389IB4d3dWUllZpTLXUKcI (c1, k600, u'rpip3').

[ptjm]

I think the problem isn't the logging but the actual user name is set incorrectly. At least, I changed my ACL file to have "User ^N" (with an actual control-N), and the client started working again. I'm seeing this on a NetBSD system.

[ptjm]

I haven't prepared a pull request for this but the problem is in handle_connect.c. In version 1.4.15, the commonName was extracted like this:

i = X509_NAME_get_index_by_NID(name, NID_commonName, -1);
name_entry = X509_NAME_get_entry(name, i);
context->username = _mosquitto_strdup((char *)ASN1_STRING_data(X509_NAME_ENTRY_get_data(name_entry)));

but in 1.5, it's like this:

i = X509_NAME_get_index_by_NID(name, NID_commonName, -1);
name_entry = X509_NAME_get_entry(name, i);
context->username = mosquitto__strdup((char *)X509_NAME_ENTRY_get_data(name_entry));

These appear to be independent fixes to the same issue, but the fix in 1.4.15 (commit fff7416) is correct in this case. I've verified that 1.5 works correctly with a call to ASN1_STRING_data() inserted as in 1.4.15.

[bricewge]

I got the same issue.
1.4.14:

1530095225: New client connected from 192.168.10.153 as esp32_4B0094 (c1, k60, u'device.test').

1.5:

1530092237: New client connected from 192.168.10.153 as esp32_4B0094 (c1, k60, u'
                                                                                 ').

The culprit seems to be the commit 9c6a5f3#diff-72af1417df06c9efe29630bd226a62b3.
So reverting the change to src/handle_connect.c may broke Windows builds by regressing on #656.

[ptjm]

@bricewge the problem with your change is that it won't work with openssl before version 1.1. The essential change from the previous commit is using X509_NAME_ENTRY_get_data() rather than accessing the structure members directly, so I think using the code from 1.4.15 is probably the way to go until older versions of openssl all go away.
On another note, there's a well-known security flaw with handling x509 certificates exhibited by this code. The common name in the certificate can have embedded nulls, so it's possible to spoof clients who don't expect that. e.g., if you can get a cert signed with common name [email protected]\[email protected], mosquitto will treat that as if it were just [email protected]. Ideally, there should be some code in there to compare ASN1_STRING_length() to the strlen of ASN1_STRING_data().

[toast-uz]

Note there is a comment as bellow in ChangeLog.txt on mosquitto v1.5.

- Support for openssl versions 1.0.0 and 1.0.1 has been removed as these are
  no longer supported by openssl.

[karlp]

@toast-uz don't forget that openssl 1.0.2 is LTS, and very much still supported. What exactly is the "information" you feel is required here given your tagging?

[toast-uz]

@karlp sorry the discussion moved to the PR #868 .

[ralight]

The PR #868 is now merged into the fixes branch, if you get the chance please take a look and close the issue if you are happy. Thanks for the report!

[bricewge]

It's good with me, you can close the issue (I can't do it myself).

[gblewis1]

@ralight I think there should be a CVE for this so older distros, e.g. Ubuntu 16.04, get this fix backported. I recently ran into this issue on a fairly recently updated Ubuntu 16.04 server install, and I see that they do backport CVEs, but I don't see a CVE for this listed under https://mosquitto.org/security/ -- correct me if I'm wrong?