Created attachment 269852 [details] [review]
do not consider intermediate CA anchor certificates

Something like this works now (no error checking in the following code) with intermediate CAs in place and also checking with system database.

  // Load the first certificate in the file.
  GTlsCertificate *cert =
    g_tls_certificate_new_from_files (cert_file, priv_key_file, error);

  // Now load the file again, but this time in a database so all
  // certificates are loaded.
  GTlsDatabase *db =
    g_tls_file_database_new (cert_file, error);

  // Now we will build the chain. If everything is right this will setup
  // the "issuer" property correctly in our GTlsCertificate.
  GTlsCertificateFlags flags =
    g_tls_database_verify_chain (db, cert,
                                 G_TLS_DATABASE_PURPOSE_AUTHENTICATE_SERVER,
                                 NULL, NULL, G_TLS_DATABASE_VERIFY_NONE,
                                 NULL, error);

  // If the certificate is not completely verified, try with the system
  // certificate database.
  if (flags)
    { GTlsBackend *backend = g_tls_backend_get_default ();
      GTlsDatabase *sysdb = g_tls_backend_get_default_database (backend);

      flags =
        g_tls_database_verify_chain (sysdb, cert,
                                     G_TLS_DATABASE_PURPOSE_AUTHENTICATE_SERVER,
                                     NULL, NULL, G_TLS_DATABASE_VERIFY_NONE,
                                     NULL, error);
    }

  if (flags)
    fprintf (stderr, "Server certificate chain not verified (flags: 0x%08X).",
             flags);
  else
    fprintf (stderr, "Server certificate chain verified successfully.");

Patch in comment 1 does not pass "make check". Test uses certificates without Basic Constraints.

Created attachment 269943 [details] [review]
do not consider intermediate CA anchor certificates

Do not report a warning if basic constraints is not found. If it's now found just consider certificate an anchor file as before.

Comment on attachment 269943 [details] [review]
do not consider intermediate CA anchor certificates

Keep thinking  this patches on my commute. I think this is not right. An anchors certificate could be anyone in the database as the old code states.

However, build_certificate_chain should not stop at the very first certificate that it find in the database and that it considers anchors, otherwise the chain is not built correctly. It should keep looking until no issuer is found and that certificate should be the anchor.

Created attachment 270039 [details] [review]
do not consider intermediate CA anchor certificates

OK, let's see if this finally makes any sense. Passes "make check" and also build the full chain with the certificates found in the database.

g_tls_file_database_gnutls_lookup_assertion() used to return TRUE if the given certificate was found in the database. That is, all certificates found were considered anchor certificates.

Now, if the issuer of certificate can also be found in the database, the certificate is not considered an anchor certificate and the chain will continue to be built.

Ping.

I think this is NOTABUG. The GTlsFileDatabase docs are not clear everywhere, but for example, the GTlsFileDatabase:anchors property says:

   * The path to a file containing PEM encoded certificate authority
   * root anchors. The certificates in this file will be treated as
   * root authorities for the purpose of verifying other certificates
   * via the g_tls_database_verify_chain() operation.

And that's the property that's set from the path in g_tls_file_database_new(). So the idea is that you're supposed to pass it a file containing ONLY anchors, with no intermediate certs.

So I think what I claimed in bug 724708 comment 2 was just wrong.

I guess a nice option would be to change g_tls_certificate_new_from_file(s) so they load all certificates in the file building the chain. The returned GTlsCertificate would be the first one.

If a certificate doesn't belong to the chain we could emit a signal.

What do you think?

Created attachment 276030 [details] [review]
load certificate chain

This patch (for glib) loads a certificate chain from a PEM file. It changes the behavior of the following functions:

 g_tls_certificate_new_from_pem
 g_tls_certificate_new_from_file
     (this one now just calls g_tls_certificate_new_from_pem)
 g_tls_certificate_new_from_files

They will work as before if only one certificate is found in the file. If more than one certificate is found it will try to load the chain.

It is assumed that the chain will be in the right order. If the chain cannot be verified an error will be returned.

If the file doesn't contain a chain one could still use g_tls_certificate_list_new_from_file, and only get the first one if that's needed, but I've never had such a use case.

It still lacks documentation updates. "make check" on glib-networking still works.

Dan, if you think the approach is OK I could create a new bug for glib, close this one and move the discussion there.

Marking as NOTABUG.

Created new bug 729739 to add support for loading certificate chains.