Implement positive/negative tests for peer verification

[vitaly-krugl]

See discussion in issue #464 for additional background.
See also issue #744.

In pika master targeting pika v1.0.0 release, the connection parameters support passing SSLContext and server_hostname via SSLOptions. This enables the app to control all the SSL features called out in the description of this issue. In particular:

1. The user can set context.verify_mode = ssl.CERT_REQUIRED; and
2. The user can set context.check_hostname = True

• This causes SSLSocket.do_handshake() to call ssl.match_hostname(), which is supported since python 2.7.9.

At this point, I am thinking about having the new positive/negative MITM tests in the following acceptance test scripts:

1. https://github.com/pika/pika/blob/master/tests/acceptance/io_services_tests.py - add 3 new create_streaming_connection() tests:
   • With ssl.CERT_REQUIRED, context.check_hostname = True and a server_hostname that matches the one in the server certificate and connects successfully;
   • With ssl.CERT_REQUIRED, context.check_hostname = True and a server_hostname that doesn't match the one in the server certificate and fails to connect as the result of mismatched server_hostname; and
   • With ssl.CERT_REQUIRED and the "server" end not providing a cert resulting in connection failure due to lack of cert from the server end.
2. https://github.com/pika/pika/blob/master/tests/acceptance/async_adapter_tests.py - TestSocketConnectTimeoutWithTinySocketTimeout is an example of a test there that creates a new connection based on the class of the connection created by the test framework. See also the following logic in the test framework: should_use_tls() and _new_tls_connection_params().

NOTE: We presently have some certs in https://github.com/pika/pika/tree/master/testdata/. This would be a good place to store additional cert(s) needed by the tests, should we need more. Also, when creating additional certs for the tests, have the cert expiration date really far out so we don't have to revisit this for a while.

[vitaly-krugl]

@maggyero, given your expertise in contributing #1030, would you mind tackling this issue too? Thanks in advance.

[michaelklishin]

In my experience this is a royal PITA to automate testing of peer verification since standard TLS peer verification usually involves hostname comparison. How about we introduce some examples that use tls-gen instead and learn/document the behavior that way?

[vitaly-krugl]

Hi Michael - I have some ideas for testing it that I want to try first, falling back to your suggestion if needed.

…

On Fri, May 4, 2018, 7:56 PM Michael Klishin ***@***.***> wrote: In my experience this is a royal PITA to automate testing of peer verification since standard TLS peer verification usually involves hostname comparison. How about we introduce some examples that use tls-gen <https://github.com/michaelklishin/tls-gen> instead and learn/document the behavior that way? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1035 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABX9KpJLAHT7JCbw3weCzQVuCQDXpG6dks5tvRTdgaJpZM4TxexH> .

[geryogam]

Hi @vitaly-krugl and @michaelklishin. I'm not a T.L.S. specialist but maybe I could help.

First I noticed that you rely on the commonName field of the Subject section to set the domain name of the X.509 certificates in the pika/testdata/certs directory.

RFC 2818 (published in May 2000) describes two methods to match a domain name against a certificate:

If a subjectAltName extension of type dNSName is present, that MUST be used as the identity. Otherwise, the (most specific) Common Name field in the Subject field of the certificate MUST be used. Although the use of the Common Name is existing practice, it is deprecated and Certification Authorities are encouraged to use the dNSName instead.

So the new standard is to set the domain names in the DNS or IP fields of the X509v3 Subject Alternative Name (SAN) section. Since the version 58, Google Chrome no longer falls back to the commonName field for server certificates lacking a DNS or IP field (it shows an ERR_CERT_COMMON_NAME_INVALID error): see the bug report (2013) and the release (2017). I have indeed got this error while trying to connect to the RabbitMQ Management plugin in H.T.T.P.S. from Google Chrome using a server certificate lacking a proper SAN section.

In order to generate a C.A.-signed certificate with a SAN section, the two key things are:

• setting the SAN section in the req_extensions section of the openssl.cnf file for generating the certificate signing request (e.g., subjectAltName = DNS:localhost, IP:127.0.0.1, or using an environment variable, subjectAltName = $ENV::SAN);
• activating the copy of request extensions in the default_ca section of the openssl.cnf file for generating the C.A.-signed certificate from the certificate signing request (copy_extensions = copy).

This is the best openssl.cnf example file (with all the openssl commands in comments) that I've found so far.

---

In my experience this is a royal PITA to automate testing of peer verification since standard TLS peer verification usually involves hostname comparison.

@michaelklishin The good thing with the SAN section is that, contrary to the Subject section, you can specify multiple domain names (using multiple DNS or IP fields).

[vitaly-krugl]

Thanks @maggyero!

[michaelklishin]

I suggest that Pika adopts tls-gen's basic profile (possibly with some tweaks and improvements) for tests instead of reinventing the wheel :) We already do that in one doc guide, in fact.

[michaelklishin]

@maggyero thanks good to know. That deserves a couple of sections in RabbitMQ's TLS and Management plugin guides, in fact.