GNOME Bugzilla – Bug 724708
glib: no support for multiple tls connection certificates
Last modified: 2014-08-30 19:25:50 UTC
It seems it is not possible to specify a list of certificates to create the server TLS connection, only one certificate is allowed. This also gets propagated to glib-networking. Imagine the scenario where you have a Root CA, some intermediate-CA and finally the server certificate. The client only has the Root CA. In TLS it is possible to send multiple certificates in the handshaking, so the client could receive all the intermediate CAs and verify the server certificate. It would be great if the a list of certificates could be specified in g_tls_connection_set_certificate(). I will start working on this tomorrow, but with how I understand this is currently implemented I see a few API changes. I am not very familiar with the code, a part from a fix in glib-networking, so any suggestions would be appreciated.
If I understand correctly, I think this also applies to the client side. http://tools.ietf.org/html/rfc5246#section-7.4.6
(In reply to comment #0) > It would be great if the a list of certificates could be specified in > g_tls_connection_set_certificate(). > > I will start working on this tomorrow, but with how I understand this is > currently implemented I see a few API changes. GTlsCertificate has an "issuer" property, which is a pointer to the GTlsCertificate that issued/signed the cert. So you just need to make sure that the certificate you set as the GTlsConnection's certificate has its issuer pointing to the intermediate certificate, which in turn has its issuer pointing to the top-level CA. And then fix the gnutls implementation in glib-networking to actually send the whole chain rather than only the first certificate. It's not totally trivial to get the GTlsCertificate issuers set up right though... If you just use g_tls_certificate_new_from_file() or new_from_pem() though, then I don't think there's currently any way to set the issuer. But if you have the whole cert chain in a PEM file, and create a GTlsFileDatabase from that file, and then fetch the server cert from that database, then glib should set up the chain correctly. But maybe we do need some better API here.
(In reply to comment #2) > > GTlsCertificate has an "issuer" property, which is a pointer to the > GTlsCertificate that issued/signed the cert. So you just need to make sure that > the certificate you set as the GTlsConnection's certificate has its issuer > pointing to the intermediate certificate, which in turn has its issuer pointing > to the top-level CA. And then fix the gnutls implementation in glib-networking > to actually send the whole chain rather than only the first certificate. > > It's not totally trivial to get the GTlsCertificate issuers set up right > though... If you just use g_tls_certificate_new_from_file() or new_from_pem() > though, then I don't think there's currently any way to set the issuer. But if > you have the whole cert chain in a PEM file, and create a GTlsFileDatabase from > that file, and then fetch the server cert from that database, then glib should > set up the chain correctly. But maybe we do need some better API here. Thank you for the feedback. The issuer property makes things a little bit easier. About a different API, may be GTlsCertificate* g_tls_certificate_chain_new_from_file (const gchar *file); The first certificate should be the server one, the others could be guessed automatically. This could use g_tls_certificate_list_new_from_file() internally. If there's no chain, the function would return NULL. This way there's no need to change the API, only adding a function and updating glib-networking. WDYT?
(In reply to comment #3) > > About a different API, may be > > GTlsCertificate* g_tls_certificate_chain_new_from_file (const gchar *file); > This is actually not that easy I think. But it seems like exposing the build_certificate_chain from gtlsdatabase-gnutls.c. So may be an easier option would be: 1. load first certificate from the file with the chain using g_tls_certificate_new_from_file. by default it loads the first one. 2. load the same file with g_tls_file_database_new. 3. create a new function g_tls_database_build_chain that would be given the certificate form 1 and the database from 2.
(In reply to comment #4) > 3. create a new function g_tls_database_build_chain that would be given the > certificate form 1 and the database from 2. No need for a new function g_tls_database_verify_chain should be enough.
Created attachment 269865 [details] [review] use all certificate chain in gnutls_retr2_st This and bug 724842 and bug 724843 fix this problem.
Created attachment 269944 [details] [review] use all certificate chain in gnutls_retr2_st Fixed a couple of object references issues.
Ping.
Comment on attachment 269944 [details] [review] use all certificate chain in gnutls_retr2_st Do you have a git.gnome.org account by the way or will you need me to commit this? >+ /* >+ * We will do this loop twice, but it's probably much more efficient >+ * than re-allocating memory. >+ */ Tiny style nitpick: Start the comment on the "/*" line, not on the next line. >+ num_certs += 1; "num_certs++". (Likewise below with st->ncerts.) >+ g_object_get (chain, "issuer", &chain, NULL); You can use "G_TLS_CERTIFICATE_GNUTLS_GET_PRIVATE (chain)->issuer". You know the whole chain consists of GTlsCertificateGnutls. objects. (Likewise below.)
(In reply to comment #6) > Created an attachment (id=269865) [details] [review] > use all certificate chain in gnutls_retr2_st > > This and bug 724842 and bug 724843 fix this problem. As mentioned there though, I think bug 724843 is probably wrong. So we'll need some other API instead.
(In reply to comment #9) > (From update of attachment 269944 [details] [review]) > Do you have a git.gnome.org account by the way or will you need me to commit > this? > (In reply to comment #9) > (From update of attachment 269944 [details] [review]) > Do you have a git.gnome.org account by the way or will you need me to commit > this? > Thanks for the review! No, I don't have an account, should I?
Created attachment 273195 [details] [review] send full certificate chain during handshake Patch with fixes from the review. I end up using: chain = chain->priv->issuer; because G_TLS_CERTIFICATE_GNUTLS_GET_PRIVATE is not available.
(In reply to comment #9) > (From update of attachment 269944 [details] [review]) > Do you have a git.gnome.org account by the way or will you need me to commit > this? > Btw, I would be really happy to have one. My question was more under what circumstances a person should have one (which I have found online). In any case, I wouldn't commit anything unless reviewed. Should I proceed with the form in? https://wiki.gnome.org/action/show/AccountsTeam/NewAccounts?action=show&redirect=NewAccount
Already have a GNOME git accounts. Thanks! Should I go ahead and commit my last patch? (comment 12).
Comment on attachment 273195 [details] [review] send full certificate chain during handshake yes, looks good
commit 995dbe7cb108215a5733ce6a1b07673fd5741325 Author: Aleix Conchillo Flaqué <aleix@oblong.com> Date: Thu Feb 20 17:47:22 2014 -0800 tlscertificate: copy all certificate chain We now copy all the certificate chain into gnutls_retr2_st structure. This way the whole chain is sent during the TLS handshaking. https://bugzilla.gnome.org/show_bug.cgi?id=724708
Review of attachment 273195 [details] [review]: Committed.
Hm, I think we need some documentation here. A couple months ago (after this patch was committed) I was trying to test to see if I could import multiple certificates into a GTlsCertificate using g_tls_certificate_new_from_file(), but I had no luck. How exactly is this supposed to work? The documentation [1] doesn't currently even mention this possibility. [1] https://developer.gnome.org/gio/unstable/GTlsCertificate.html
Also, it's pretty confusing that g_tls_certificate_list_new_from_file() exists if it can't be used to send a list of certificates....
ah, sorry, this is just the support for having GTlsConnection send the complete certificate chain that you set on it; you still need to construct the chain manually (by setting issuer properties on the certs). bug 729739 is about adding new API for more easily constructing certificate chains