Re: [HACKERS] [review] libpq: Support TLSv1.1+ (was: fe-secure.c and SSL/TLS)

2014-01-24 Thread Noah Misch
[restored Cc: list]

On Thu, Jan 09, 2014 at 10:12:28PM -0800, Wim Lewis wrote:
 I applied both libpq.tls11plus.diff and the related
 psql.conninfo.tlsver.diff patch to postgresql git head.

psql.conninfo.tlsver.diff is not so essential for debugging purposes since
commit 4cba1f6bbf7c8f956c95e72c43e517a56b97665b, but it still seems like a
perfectly reasonable addition.

 Source review:
 
 The source changes are pretty tiny. Although I think the change
 from TLSv1_method to SSLv23_method is correct, the comment is not
 quite correct:
 
  * SSLv23_method() is only method that negotiates
  * higher protocol versions.  Rest of the methods
  * allow only one specific TLS version.
 
 As I understand it (backed up by a quick glance through the openssl
 source), the TLSv1_method, TLSv1_1_method, and TLSv1_2_method will
 all advertise the corresponding protocol version to the peer, meaning
 that in practice they will negotiate *up to* that TLS version, but
 will still negotiate down to SSLv3. So, using TLSv1_2_method would
 give the right behavior when compiled against a recent openssl.
 However, someday when TLSv1.3 (or 2.0) appears, presumably the
 SSLv23_method will be extended to include it but TLSv1_2_method
 would have to be changed to TLSv1_3_method. Therefore using
 SSLv23_method and disabling older protocol versions with
 SSL_CTX_set_options() should have the desired behavior even in
 future versions. (And it doesn't require autoconf to probe the
 openssl version.)

With OpenSSL 1.0.1f, I see results consistent with the comment.  Using
TLSv1_1_method() gets a TLSv1.1 connection against a server capable of
TLSv1.2, and it fails against a server capable of no more than TLSv1.  The
patch's use of SSLv23_method() gets TLSv1.2 from the first server and TLSv1
from the second server.

 Testing:
 
 I built the patched postgresql against a handful of openssl versions:
 1.0.1 (netbsd, x86-64, supports TLSv1.1); Git head aka 1.0.1f++
 (osx, x86-32, supports TLSv1.2), and 0.9.8y (osx, x86-32, supports
 TLSv1.0). They all built cleanly and passed 'make check'. I also
 built 'contrib' and installed the sslinfo extension. I connected
 between each pair of versions (with psql) and saw that the connection
 negotiated the highest protocol version supported by both ends and
 a corresponding ciphersuite. /conninfo and the sslinfo extension
 agreed on the protocol version and ciphersuite in use.
 
 Things I didn't test:
 
 Client certificates, restricted sets of ciphersuites, MITM
 protocol-downgrade attacks, non-x86 architectures, or 1.0.0* versions
 of openssl.

The level of testing you did made for an excellent review.  Thank you.

I've committed both patches.

-- 
Noah Misch
EnterpriseDB http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] [review] libpq: Support TLSv1.1+ (was: fe-secure.c and SSL/TLS)

2014-01-09 Thread Wim Lewis
I applied both libpq.tls11plus.diff and the related
psql.conninfo.tlsver.diff patch to postgresql git head.

Source review:

The source changes are pretty tiny. Although I think the change
from TLSv1_method to SSLv23_method is correct, the comment is not
quite correct:

 * SSLv23_method() is only method that negotiates
 * higher protocol versions.  Rest of the methods
 * allow only one specific TLS version.

As I understand it (backed up by a quick glance through the openssl
source), the TLSv1_method, TLSv1_1_method, and TLSv1_2_method will
all advertise the corresponding protocol version to the peer, meaning
that in practice they will negotiate *up to* that TLS version, but
will still negotiate down to SSLv3. So, using TLSv1_2_method would
give the right behavior when compiled against a recent openssl.
However, someday when TLSv1.3 (or 2.0) appears, presumably the
SSLv23_method will be extended to include it but TLSv1_2_method
would have to be changed to TLSv1_3_method. Therefore using
SSLv23_method and disabling older protocol versions with
SSL_CTX_set_options() should have the desired behavior even in
future versions. (And it doesn't require autoconf to probe the
openssl version.)


Testing:

I built the patched postgresql against a handful of openssl versions:
1.0.1 (netbsd, x86-64, supports TLSv1.1); Git head aka 1.0.1f++
(osx, x86-32, supports TLSv1.2), and 0.9.8y (osx, x86-32, supports
TLSv1.0). They all built cleanly and passed 'make check'. I also
built 'contrib' and installed the sslinfo extension. I connected
between each pair of versions (with psql) and saw that the connection
negotiated the highest protocol version supported by both ends and
a corresponding ciphersuite. /conninfo and the sslinfo extension
agreed on the protocol version and ciphersuite in use.

Things I didn't test:

Client certificates, restricted sets of ciphersuites, MITM
protocol-downgrade attacks, non-x86 architectures, or 1.0.0* versions
of openssl.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers