Created attachment 310216 [details] [review]
WIP not working don't push: attempt to add certificate chain tests

They fail because only one certificate is sent. I'm still debugging this. I'm posting it here just because I've written a patch similar to this and lost my copy at least twice before today, and I don't want to lose it again. :)

To run the tests you'll need to remove the GMainLoop I run for manual testing with gnutls-cli in start_async_server_and_connect_to_it(), and uncomment the tests in main(). If you don't, you can connect to the server to see what's going on like so:

$ gnutls-cli -V 127.0.0.1 -p 57257

Created attachment 310217 [details] [review]
WIP not working don't push: attempt to add certificate chain tests

Just kidding, the port is decided randomly. This patch prints the port to connect to if you run with G_MESSAGES_DEBUG=all in your environment.

Next step: figure out why only one certificate is sent.

Created attachment 310226 [details] [review]
WIP not working don't push: attempt to add certificate chain tests

In bug #754264 I fixed the problem with parse_and_create_certificate_list() that was causing only one certificate to be sent, and copied the fix into this patch. Now the GTlsCertificate is being generated correctly and properly sent to the client. By uncommenting the test call to g_main_loop_run() in start_async_server_and_connect_to_it(), you can verify with gnutls-cli that all certificates are now being sent.

The test is still broken though. The client rejects the connection with the following error:

GnuTLS error: Public key signature verification has failed.

Further investigation required (and help welcome ;)

Created attachment 310264 [details] [review]
fixup: use the right server key in the chain files

Created attachment 310265 [details] [review]
fixup: the code assumes the server cert's commonName is "server.example.com"

At this point (after regenerating the certs) it gets an "Unacceptable TLS certificate" because G_TLS_CERTIFICATE_UNKNOWN_CA. I didn't investigate why yet.

(In reply to Dan Winship from comment #6)
> At this point (after regenerating the certs) it gets an "Unacceptable TLS
> certificate" because G_TLS_CERTIFICATE_UNKNOWN_CA. I didn't investigate why
> yet.

That's strange, because now all the tests pass for me (after regenerating the certificates and replacing the first certificate in ca-roots.pem and ca-roots-bad.pem).

Created attachment 310268 [details] [review]
Add test for out-of-order certificate chain handling

Made possible by help from Dan Winship.

This passes for me, but fails if 0e08f17396287d00a69bbbcbec3b364b98cbcace is reverted, so seems good. It doesn't test the new behavior in bug #750457, but it's an important first step.

(In reply to Michael Catanzaro from comment #7)
> That's strange, because now all the tests pass for me (after regenerating
> the certificates and replacing the first certificate in ca-roots.pem and
> ca-roots-bad.pem).

Yeah, I missed that last step... I really ought to automate that.


Need to do something about the huge amount of cut+pasting though. It's only needed for one test...

Instead of reading the chain from a single file, you could do:

  intermediate = g_tls_certificate_new_from_file("intermediate-ca.pem")

  cert_data = g_file_get_data("ca.pem")
  ca = g_object_new (cert_type,
                     "issuer", intermediate,
                     "certificate-pem", cert_data,
                     NULL)

  cert_data = g_file_get_data("server-intermediate.pem")
  key_data = g_file_get_data("server-intermediate-key.pem")
  server_cert = g_object_new (cert_type,
                              "issuer", ca,
                              "certificate-pem", cert_data,
                              "private-key-pem", key_data,
                              NULL)

and now you have a Server -> CA -> Intermediate chain

Created attachment 310276 [details] [review]
Add tests for client certificate chain handling

OK, removed the huge copypaste, and replaced it with more tests. :)

Comment on attachment 310276 [details] [review]
Add tests for client certificate chain handling

yay

Attachment 310276 [details] pushed as 01631a0 - Add tests for client certificate chain handling