fe-secure.c and SSL/TLS - Mailing list pgsql-bugs

I believe fe-secure.c has a few opportunities for improvement. I
believe the first three are features requests/improvements, but the
fourth and fifth could be a security vulnerabilities.

*****

`initialize_SSL` uses the following around line 970 (which is much
better than most):

    SSL_context = SSL_CTX_new(TLSv1_method());

The TLSv1_method locks selection to TLS 1.0. Its good because SSL v2
and v3 are removed; but it could be improved upon to make TLS 1.1 and
1.2 available.

The attached shows a sample connection under Wireshark that uses
TLSv1_method. Notice the Handshake Protocol (not TLSv1 Record)
advertises 1.0 (0x301) as the highest level.

Unfortunately, OpenSSL does not provide a TLSv1_and_above_method. I
believe a better way to do things would be:

    SSL_context = SSL_CTX_new(SSLv23_method());
    flags = SSL_OP_ALL | SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3;
    SSL_CTX_set_options(SSL_context, flags);

SSL_OP_NO_COMPRESSION might also be a good choice.

That will set the minimum to TLS 1.0, and the maximum to TLS 1.2. It
also prefers ciphers like ECDHE-ECDSA-AES256-GCM-SHA384 (the order is
built into OpenSSL).

*****

If you take the opportunity above, then you can trim the ciphers being
used (and improve the security) by providing similar to the following.
It will also remove weak/wounded/unneeded ciphers. (I prefer GCM over
both CBC and RC4. If I have to use one or the other, I think CBC is
the lesser of the evils).

const char* PREFERRED_CIPHERS =

    /* TLS 1.2 only */
    "ECDHE-ECDSA-AES256-GCM-SHA384:"
    "ECDHE-RSA-AES256-GCM-SHA384:"
    "ECDHE-ECDSA-AES128-GCM-SHA256:"
    "ECDHE-RSA-AES128-GCM-SHA256:"

    /* TLS 1.2 only */
    "DHE-DSS-AES256-GCM-SHA384:"
    "DHE-RSA-AES256-GCM-SHA384:"
    "DHE-DSS-AES128-GCM-SHA256:"
    "DHE-RSA-AES128-GCM-SHA256:"

    /* TLS 1.0 only */
    "DHE-DSS-AES256-SHA:"
    "DHE-RSA-AES256-SHA:"
    "DHE-DSS-AES128-SHA:"
    "DHE-RSA-AES128-SHA:"

    res = SSL_set_cipher_list(ssl, PREFERRED_CIPHERS);
    if(res != 1)
        /* handle error */

**********

`pqsecure_open_client` makes the connection to the server. I believe
Server Name Indicator (SNI) could be used to take advantage of the TLS
1.0 feature:

    SSL_set_tlsext_host_name(SSL_context, HOST_NAME);

**********

There are three checks needed to verify a SSL/TLS connection under
OpenSSL. I see two of them, but I don't see the third.

The three checks are:

    (1) non-NULL server certifcate
    (2) SSL_get_verify_result must return X509_V_OK
    (3) hostname must be verified

Items (1) and (3) are present in `open_client_SSL` around line 1500.
(By the way, OpenSSL 1.1.0 will offer to perform the hostname checks -
its in HEAD now).

Item (2) fetches the result from OpenSSL performing the customary
checks like chaining to a valid root and Not_Before/Not_After. If the
verify_callback always return 1, then SSL_get_verify_result will
always return X509_V_OK. If the verify_callback returns a 0, then
SSL_get_verify_result will *not* return X509_V_OK.

I believe this is a vulnerability. A rogue server can present a forged
a certificate that does not forma a valid chain (i.e., not chained to
the root specififed by SSL_CTX_load_verify_locations), and Postgres
will accept it as genuine as long as the name matches because
SSL_get_verify_result is not called.

**********

Looking at the rules in `wildcard_certificate_match` around line 720,
there might be a problem with ccTLDs. Mozilla maintains a list of them
at http://publicsuffix.org/ (alt,
https://mxr.mozilla.org/mozilla-central/source/netwerk/dns/effective_tld_names.dat?raw=1).

The potential problem is a rogue server could present a wildcard
certificate for a ccTLD like '*.com.eu' and the code might match if
some conditions are met. I believe the conditions include tricking the
client into using, for example, 'example.com.eu'. I don't believe its
easy to exploit, but the potential looks like its there.