Proper SSL connection verification

[wilmer]

I just realized the SSL modules aren't really doing any certificate verification when setting up SSL connections. Certificates aren't checked, and it doesn't compare the CN of the certificate with the known hostname. It'd probably be good to do at least the hostname verification.

I'll do this at least for GnuTLS, probably not for all SSL modules.

[ilf@…]

A bump for importance :)

[anonymous]

Bump. It also doesn't check to see whether the certificate is issued by a trusted CA; moreover it seems that even if it is a trusted CA sometimes there are issues (e.g. if it's not from a big name root CA; e.g. CACert when connecting to jabber.ccc.de)

[anonymous]

+1 I'd also like to see this being implemented.

[anonymous]

3 years ? Really...

[wilmer]

Are you being a fucking wiseass for a product you didn't pay a single cent for? Really...

If you think this is so important then WHERE IS MY PATCH?

I don't often get cranky on this bug tracker, but please, your attitude is beyond broken. Yes, I have plans to fix this. No, it's not simple. And note how there are SEVEN HUNDRED bugs in this bug tracker. Sorry, this one just happens to not be the most important one for this one man show called BitlBee development. There's hardly a team. There's just this one dude who has a demanding daytime job and spends the little amount of spare time he does have on things he enjoys.

[AopicieR]

Personally I use stunnel[1] for every program that doesn't support SSL or doesn't do proper certificate verification. I have explained how to do this under [2]. Feel free to contact me if you have problems setting it up with BitlBee.

[1] ​http://www.stunnel.org/ [2] ​https://aopicier.wordpress.com/

[wilmer]

​http://www.gnu.org/software/gnutls/manual/html_node/Verifying-peer_0027s-certificate.html#Verifying-peer_0027s-certificate has example code. It'd be best to just add a "strict SSL" mode. Let the user set a CApath in bitlbee.conf, and just drop SSL connections that seem unreliable.

This can be GnuTLS-only, it's always been the best supported SSL module for BitlBee.

[AopicieR]

Certificate verification for starttls with xmpp/jabber

[AopicieR]

The proposed patch adds certificate verficiation for the xmpp/jabber protocol and TLS. It adds a setting "cafile" in bitlbee.conf which points to a file containing a list of trusted CAs and defaults to "/etc/ssl/certs/ca-certificates.crt". It also adds an accountsetting "tlsverify" which, when set to true, causes connections to fail in case the certificate cannot be verified. The verification only works if BitlBee is built against GnuTLS.

I'd be very happy about feedback and criticism. The patch also really needs some testing. In case the patch turns out to be reasonable I might give standard SSL and therefore also other procotols a shot.

[AopicieR]

The first patch contained an error. This should fix it. This fix needs to be applied on top of the first patch.

[AopicieR]

Sorry, the fix didn't apply cleanly. This patch combines the other two, so only use this one.

[anonymous]

Hi AopicieR,

I patched bitlbee from bzr on Ubuntu 11.10, and the patch seems to work well. However I'm unable to connect to my Google Apps accounts with tlsverify set to true. Have you tried testing it on talk.google.com with a Google Apps account in a non @google.com domain?

[Wilmer van der Gaast <wilmer@…>]

I wonder what hostname has to be used for cert verification. talk.google.com, I assume? Is the code maybe passing a different name?

[anonymous]

jd->server = strchr( jd->username, '@');

... somewhere else ...

jd->ssl = ssl_starttls( jd->fd, jd->server, set_getbool(&ic->acc->set, "tlsverify"), jabber_connected_ssl, ic );

[Wilmer van der Gaast <wilmer@…>]

Ok, that might be wrong, see the output of

openssl s_client -connect talk.google.com:5223

Although someone should check standards on which approach is right. Requiring the part behind @ could be complicated when hosting many domains on one IP.

[AopicieR]

Hi, thanks for trying the patch.

I think connecting on 5223 leads to false conclusions. I haven't checked standards yet (sorry for that) but I thought that the CN in the certificate should be the value of the 'to' field of the XMPP handshake, which is exactly 'jd->server'. Try this:

$ gnutls-cli -s talk.google.com -p 5222

Then paste the line

<?xml version='1.0'?><stream:stream to='gmail.com' xmlns='jabber:client' xmlns:stream='http://etherx.jabber.org/streams' version='1.0'><starttls xmlns='urn:ietf:params:xml:ns:xmpp-tls'/>

and hit CTRL-D. As part of the certificate output I get

- subject `C=US,ST=California,L=Mountain View,O=Google Inc.,CN=gmail.com', issuer `C=US,O=Equifax,OU=Equifax Secure Certificate Authority', RSA key 1024 bits, signed using RSA-SHA1, activated `2007-04-11 17:17:38 UTC', expires `2012-04-10 17:17:38 UTC', SHA-1 fingerprint `9ff83bda2ca3125524d5b9d6fc49698f0a91d8cd'

But when using

<?xml version='1.0'?><stream:stream to='googlemail.com' xmlns='jabber:client' xmlns:stream='http://etherx.jabber.org/streams' version='1.0'><starttls xmlns='urn:ietf:params:xml:ns:xmpp-tls'/>

I get

- subject `C=US,ST=California,L=Mountain View,O=Google Inc.,CN=googlemail.com', issuer `C=US,O=Equifax,OU=Equifax Secure Certificate Authority', RSA key 1024 bits, signed using RSA-SHA1, activated `2007-04-11 17:18:01 UTC', expires `2012-04-10 17:18:01 UTC', SHA-1 fingerprint `20bf4896e520dc99fb122e87a1eee51617b01819'

But okay, it looks like it just falls back to 'talk.google.com' in case the value of the 'to' field is something strange:

<?xml version='1.0'?><stream:stream to='foo' xmlns='jabber:client' xmlns:stream='http://etherx.jabber.org/streams' version='1.0'><starttls xmlns='urn:ietf:params:xml:ns:xmpp-tls'/>

yields

- subject `C=US,ST=California,L=Mountain View,O=Google Inc.,CN=talk.google.com', issuer `C=US,O=Equifax,OU=Equifax Secure Certificate Authority', RSA key 1024 bits, signed using RSA-SHA1, activated `2007-04-11 17:20:16 UTC', expires `2012-04-10 17:20:16 UTC', SHA-1 fingerprint `953fbe4d549b7e700ec14782c68cd09f9b512bce'

Other servers just abort the connection in case the value of the 'to' field is unfamiliar, but not Google.

@anonymous: What's the domain of the username in your case? You are trying to connect to talk.google.com, yes?

[anonymous]

Hi AopicieR, the domain I'm trying to connect to is something like @mydomain.com - it's a domain that I have hosted at Google apps. I'm connecting to talk.google.com, and it's through my Google Apps account. Maybe there is no way to verify TLS for Google Apps accounts? I'm not sure how Pidgin (for example) behaves, but I'll test that out this evening.

BTW, the patch works perfectly for the rest of my accounts. Thanks!

[Wilmer van der Gaast <wilmer@…>]

Yeps, that's what I thought, Google's definitely not going to have a cert for your GAFYD domain.

But that's messy. I wonder how this is supposed to work then..

[ilf@…]

On the first encounter, present the fingerprint to the user and let her ack/deny it?

[anonymous]

I just tested in Pidgin, and there is no cert popup with my Google Apps account. Looking through the Libpurple code for cert verification, it'll verify the connect server if it's defined. Otherwise, it uses the domain from the Jabber username/account.

[AopicieR]

This version of the patch copies the behavior of Pidgin: If the user specifies a server it is used for the hostname verification, in any other case the domain of the username is used. It also improves the error messages. Please test it.

[AopicieR]

The third patch contained a (really stupid) mistake, please only use this one. Sorry about that ... As a side note: Currently the patch needs gnutls >= 2.10.0. If the patch turns out to be reasonable I will try to make it compatible with older versions of gnutls.

[AopicieR]

This patch now also compiles with older versions of gnutls. I've tested it on an Ubuntu with gnutls 2.8.5, so it should also work with the version from Debian stable, but I don't have access to a Debian stable installation to verify that. The patch now has all the functionality I am personally interested in, so I will only continue to work on it if someone asks me to. :)

[AopicieR]

A bit of documentation for the options added by the patch.

[anonymous]

Thanks for the updated patch AopicieR, I can now access my Google Apps accounts with TLS verification enabled. I've been running it the past two weeks - no issues to speak of!

[ilf@…]

Replying to ilf@…:

On the first encounter, present the fingerprint to the user and let her ack/deny it?

I want to vote for this again. Given the recent X.509 breakages (Comodo, DigiNotar), I think we should not rely on the certificate chain and a hostname check.

Let's present the certificate to the user on the first encounter. Then use only that certificate with the ack'd fingerprint. If the cert changes, show a warning and let the user decide.

This also works around the problem of exact domain names not being in the certificate.

[wilmer]

Finally looking at this patch now. I think I quite like the implementation. Once merged, I'll get http_client to use it too.

One thing I really dislike though, is how error codes are converted to strings. This puts awareness of SSL libs inside the Jabber module. Instead, what about having an ssl_strerror() function or something like that? It can return a long string, a list of strings, doesn't really matter.

Also, I'd like to keep this disabled by default but easy to enable by uncommenting a line in bitlbee.conf, so that people can do this if they have a usable /etc/ssl/certs/ca-certificates.crt and know what they're doing (and know that they're using GnuTLS and not one of the others).

Can you make the change I mentioned above, or shall I?

[wilmer]

Weird. My jabberd didn't have its cert updated for three years or so and I never noticed apparently..

And for some reason GnuTLS' code didn't complain about that all, but it did complain about the cert using MD5 signatures.

Anyway, with a new cert (using SHA1) things are good now. :-) This is pretty neat, thanks!

[wilmer]

Merged: changeset:devel,859.

This works with just GnuTLS and for now one needs to enable it by hand. If anyone wants to fix the other SSL libs, be my guest. In the meantime I might just put more emphasis on the "use GnuTLS or forget about getting support". :-)

[wilmer]

Crap. This still breaks compatibility with some older version of GnuTLS (2.4.2, which comes with Debian Lenny):

/tmp/buildd/bitlbee-3.0.4+z+20111224+devel+860/lib/ssl_gnutls.c: In function 'verify_certificate_callback':
/tmp/buildd/bitlbee-3.0.4+z+20111224+devel+860/lib/ssl_gnutls.c:168: error: 'GNUTLS_CERT_NOT_ACTIVATED' undeclared (first use in this function)
/tmp/buildd/bitlbee-3.0.4+z+20111224+devel+860/lib/ssl_gnutls.c:168: error: (Each undeclared identifier is reported only once
/tmp/buildd/bitlbee-3.0.4+z+20111224+devel+860/lib/ssl_gnutls.c:168: error: for each function it appears in.)
/tmp/buildd/bitlbee-3.0.4+z+20111224+devel+860/lib/ssl_gnutls.c:171: error: 'GNUTLS_CERT_EXPIRED' undeclared (first use in this function)

Looks like these are just failure conditions so it's easy to skip the checks, at least. But I sure hope 2.4.2 did already support checking for cert expiry? Would it report those problems in some other way?

[wilmer]

Yikes. Looks like before ​https://issues.rpath.com/secure/attachment/16651/gnutls-cli-expire-checks.patch GnuTLS did not do any time checks yet?

Ah, brilliant: ​http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2009-1417

That's some quality software.

[wilmer]

Uh oh...

wilmer   27381  0.0  0.0  48948  2844 pts/2    S+   20:06   0:00  |   \_ ./bitlbee
wilmer   27445  1.2  0.1  51928  5784 pts/8    S+   20:07   0:00  |   \_ ./bitlbee

Guess what's the difference between those two processes...

I wonder where this is leaking. I discovered this after turning on this functionality on testing.bitlbee.org. Will have to switch it off there again for now.

[AopicieR]

I can reproduce an apparently constant increase of memory usage which seems to be related to loading CAfile. Here are some VSZ RSS values:

Revision 858 (before the tls_verify merge):

Initial start of BitlBee:

45264  1164

One user connected to the server with all accounts initially disabled:

45432  1656

One user connected and one account enabled:

47696  2708

Revision 861 with a tiny CAfile (only three certs or so) in the same cases as above:

45264  1168

45432  1660

47628  2692

Revision 861 with a huge CAfile (150 certs or so) in the same cases as above:

45264  1164

45432  1656

50284  5268

So looks like this is just the CAfile which sits in memory after at least one certificate verification has been performed. Not sure what one can do about that. Is this really an issue if it is a constant amount of memory and does not grow with users/connections/enabled accounts (I cannot really test if it does but it really shouldn't)?

I could look into whether there is a way of freeing this memory after the verification has been completed. I'll check how other programs using GnuTLS perform in this respect.

[AopicieR]

This patch moves the allocation of the GnuTLS credentials to the inital ssl_init. The credentials should then be shared among the different connections and even among forked processes.

[wilmer]

Applied, but sticking to atexit() and not changing the other SSL files. Thanks again!

[AopicieR]

Some small changes: Currently we set GNUTLS_VERIFY_ALLOW_X509_V1_CA_CRT but only because I had copied the example code from an old version of the GnuTLS documentation. The current version of the documentation does not set this anymore and doing an apt-cache rdepends libgnutls26 and a bit of browsing through the source code of some of the results suggests that the only people who set this flag are the ones who have also based their code on the example code from the documentation ... Secondly the comment which the patch removes is a blunt lie, I must have been asleep while writing it. We do want this check. Thirdly we need to include bitlbee.h in ssl_openssl.c to get global.conf->cafile which is used to determine if verification was enabled.

[AopicieR]

I've done some more digging related to GNUTLS_VERIFY_ALLOW_X509_V1_CA_CRT: According to the NEWS file this is enabled by default since GnuTLS 2.11.5 (from 2010-11-26), so that's why it is not explicitly set anymore in the example code from the recent documentation. And a quick search for "Version: 1" in /usr/share/ca-certificates yields 15 results. So we might want to keep this after all for those cases where BitlBee is built against an older version of GnuTLS.

[Wilmer van der Gaast <wilmer@…>]

Oh yes, we definitely need to support GnuTLS <2.11. For example Debian Squeeze (latest stable release) has GnuTLS 2.8.6.