GNOME Bugzilla – Bug 727692
gio/gtlscertificate.c -- broken PEM-file processing (affects local CA root stores, for starters)
Last modified: 2014-04-09 14:45:27 UTC
I'm unaware of the full implications of this bug, so I'll bump the severity a notch just in case. I stumbled across this while experimenting with the xombrero Web browser and noticing that it couldn't validate any SSL certificates -- hence it would all-too-quietly fail with a yellow address bar (vs. the all-OK green address bar). Other applications may raise more of an alarm, but I dunno how many there are that rely on the code path in question. Here's the code, which has been present since 2.28 in one form or another: gio/gtlscertificate.c ---------------------------------------------------------------------------- GList * g_tls_certificate_list_new_from_file (const gchar *file, GError **error) { GQueue queue = G_QUEUE_INIT; gchar *contents, *end; const gchar *p; gsize length; if (!g_file_get_contents (file, &contents, &length, error)) return NULL; end = contents + length; p = contents; while (p && *p) { gchar *cert_pem; GTlsCertificate *cert = NULL; cert_pem = parse_next_pem_certificate (&p, end, FALSE, error); if (cert_pem) { cert = g_tls_certificate_new_internal (cert_pem, NULL, error); g_free (cert_pem); } if (!cert) { g_list_free_full (queue.head, g_object_unref); queue.head = NULL; break; } g_queue_push_tail (&queue, cert); } g_free (contents); return queue.head; } ----------------------------------------------------------------------------- This function will successfully build up a list of certificates from the PEM file and then destroy the list. Except in one or two pathological cases (an end-delimiter string followed immediately by '\0' being one such case, if I'm not mistaken), the caller will always get back NULL. In my case, this leads to some empty hash tables in glib-networking's load_anchor_file() (in tls/gnutls/gtlsfiledatabase-gnutls.c), and then g_tls_file_database_gnutls_lookup_certificate_issuer() (in the same file) has nothing with which to hash issuer DNs to subjects. I removed these two lines to get successful SSL validation in the browser: g_list_free_full (queue.head, g_object_unref); queue.head = NULL; It seems OK, but I'm very rusty when it comes to this kind of thing, so reader beware.
Hello, I've created some test cases to show the problem more clearly. This code simply shows the number of certificates discovered in certs1.txt and certs2.txt: ---------- begin gtl.c ---------- #include <stdio.h> #include <glib.h> #include <gio/gio.h> int main(void) { GList *certlist; certlist = g_tls_certificate_list_new_from_file("certs1.txt", NULL); printf("%u\n", (unsigned) g_list_length(certlist)); certlist = g_tls_certificate_list_new_from_file("certs2.txt", NULL); printf("%u\n", (unsigned) g_list_length(certlist)); return 0; } ---------- end gtl.c ---------- The file certs1.txt contains some initial fluff, a GeoTrust certificate, and a bunch of trailing newlines. The trailing newlines are OK, for parse_next_pem_certificate() skips over them. ---------- begin certs1.txt ---------- other stuff -----BEGIN CERTIFICATE----- MIIDVDCCAjygAwIBAgIDAjRWMA0GCSqGSIb3DQEBBQUAMEIxCzAJBgNVBAYTAlVT MRYwFAYDVQQKEw1HZW9UcnVzdCBJbmMuMRswGQYDVQQDExJHZW9UcnVzdCBHbG9i YWwgQ0EwHhcNMDIwNTIxMDQwMDAwWhcNMjIwNTIxMDQwMDAwWjBCMQswCQYDVQQG EwJVUzEWMBQGA1UEChMNR2VvVHJ1c3QgSW5jLjEbMBkGA1UEAxMSR2VvVHJ1c3Qg R2xvYmFsIENBMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA2swYYzD9 9BcjGlZ+W988bDjkcbd4kdS8odhM+KhDtgPpTSEHCIjaWC9mOSm9BXiLnTjoBbdq fnGk5sRgprDvgOSJKA+eJdbtg/OtppHHmMlCGDUUna2YRpIuT8rxh0PBFpVXLVDv iS2Aelet8u5fa9IAjbkU+BQVNdnARqN7csiRv8lVK83Qlz6cJmTM386DGXHKTubU 1XupGc1V3sjs0l44U+VcT4wt/lAjNvxm5suOpDkZALeVAjmRCw7+OC7RHQWa9k0+ bw8HHa8sHo9gOeL6NlMTOdReJivbPagUvTLrGAMoUgRx5aszPeE4uwc2hGKceeoW MPRfwCvocWvk+QIDAQABo1MwUTAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBTA ephojYn7qwVkDBF9qn1luMrMTjAfBgNVHSMEGDAWgBTAephojYn7qwVkDBF9qn1l uMrMTjANBgkqhkiG9w0BAQUFAAOCAQEANeMpauUvXVSOKVCUn5kaFOSPeCpilKIn Z57QzxpeR+nBsqTP3UEaBU6bS+5Kb1VSsyShNwrrZHYqLizz/Tt1kL/6cdjHPTfS tQWVYrmm3ok9Nns4d0iXrKYgjy6myQzCsplFAMfOEVEiIuCl6rYVSAlk6l5PdPcF PseKUgzbFbS9bZvlxrFUaKnjaZC2mqUPuLk/IH2uSrW4nOQdtqvmlKXBx4Ot2/Un hw4EbNX/3aBd7YdStysVAq45pmp06drE57xNNB6pXE0zX5IJL4hmXXeXxx12E6nV 5fEWCRE11azbJHFwLJhWC9kXtNHjUStedejV0NxPNO3CBWaAocvmMw== -----END CERTIFICATE----- ---------- end certs1.txt ---------- The file certs2.txt contains the same initial fluff and the same certificate, but there's a comment at the end. This is the kind of trailing comment that gets added by something like the MAca-bundle.pl tool in the FreeBSD ports collection: http://svnweb.freebsd.org/ports/head/security/ca_root_nss/files/MAca-bundle.pl.in?revision=325572 ---------- begin certs2.txt ---------- other stuff -----BEGIN CERTIFICATE----- MIIDVDCCAjygAwIBAgIDAjRWMA0GCSqGSIb3DQEBBQUAMEIxCzAJBgNVBAYTAlVT MRYwFAYDVQQKEw1HZW9UcnVzdCBJbmMuMRswGQYDVQQDExJHZW9UcnVzdCBHbG9i YWwgQ0EwHhcNMDIwNTIxMDQwMDAwWhcNMjIwNTIxMDQwMDAwWjBCMQswCQYDVQQG EwJVUzEWMBQGA1UEChMNR2VvVHJ1c3QgSW5jLjEbMBkGA1UEAxMSR2VvVHJ1c3Qg R2xvYmFsIENBMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA2swYYzD9 9BcjGlZ+W988bDjkcbd4kdS8odhM+KhDtgPpTSEHCIjaWC9mOSm9BXiLnTjoBbdq fnGk5sRgprDvgOSJKA+eJdbtg/OtppHHmMlCGDUUna2YRpIuT8rxh0PBFpVXLVDv iS2Aelet8u5fa9IAjbkU+BQVNdnARqN7csiRv8lVK83Qlz6cJmTM386DGXHKTubU 1XupGc1V3sjs0l44U+VcT4wt/lAjNvxm5suOpDkZALeVAjmRCw7+OC7RHQWa9k0+ bw8HHa8sHo9gOeL6NlMTOdReJivbPagUvTLrGAMoUgRx5aszPeE4uwc2hGKceeoW MPRfwCvocWvk+QIDAQABo1MwUTAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBTA ephojYn7qwVkDBF9qn1luMrMTjAfBgNVHSMEGDAWgBTAephojYn7qwVkDBF9qn1l uMrMTjANBgkqhkiG9w0BAQUFAAOCAQEANeMpauUvXVSOKVCUn5kaFOSPeCpilKIn Z57QzxpeR+nBsqTP3UEaBU6bS+5Kb1VSsyShNwrrZHYqLizz/Tt1kL/6cdjHPTfS tQWVYrmm3ok9Nns4d0iXrKYgjy6myQzCsplFAMfOEVEiIuCl6rYVSAlk6l5PdPcF PseKUgzbFbS9bZvlxrFUaKnjaZC2mqUPuLk/IH2uSrW4nOQdtqvmlKXBx4Ot2/Un hw4EbNX/3aBd7YdStysVAq45pmp06drE57xNNB6pXE0zX5IJL4hmXXeXxx12E6nV 5fEWCRE11azbJHFwLJhWC9kXtNHjUStedejV0NxPNO3CBWaAocvmMw== -----END CERTIFICATE----- ## Number of certificates: 1 ## End of file. ---------- end certs2.txt ---------- Here's a run of the code: $ ./gtl 1 0 So the assumption in gio is that the certs file terminates with an end-certificate delimiter with some optional newlines. If these two lines get deleted from g_tls_certificate_list_new_from_file(): g_list_free_full (queue.head, g_object_unref); queue.head = NULL; the list won't be trashed when parse_next_pem_certificate() fails to parse the trailing content. Another benefit may be that there won't be an all-or-nothing approach to the certs file. Currently, a single bad certificate wipes out the entire list of good certificates. Removing the two lines means any good certificates processed *before* the bad certificate will be returned to the caller. A slightly more involved fix could allow good certificates *before and after* the bad certificate to be returned to the caller.
(In reply to comment #1) > Another benefit may be that there won't be an all-or-nothing > approach to the certs file. Currently, a single bad certificate wipes out the > entire list of good certificates. That's kind of intentional, given that this is security-related code; if things are exactly as expected, then fail completely, in case it's part of an attack. The problem here was that we weren't distinguishing between parse_next_pem_certificate() returning NULL because there was an error, and returning NULL because it reached end-of-file. Fixed in master and glib-2-40 branch. It should be in glib-2.40.1 next week.