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

Reply to mails in pgsql-bugs:

http://www.postgresql.org/message-id/CAH8yC8mc_2J2UY0Q42WQdWFyaoqT3onG+83Fr=vN46J5+ML94g@mail.gmail.com

and

http://www.postgresql.org/message-id/CAH8yC8nZVUyCQznkQd8=ELMM4k_=uXJRjt8YF9V22Cy2x_dDjQ@mail.gmail.com

* Default ciphersuite

> I would argue nothing should be left to chance, and the project should
> take control of everything. But I don't really have a dog in the fight ;)

Indeed, on my own servers I've stopped bothering with pattern-based
ciphersuite strings, now I am listing ciphers explicitly.

But the discussion here is about default suite for admins who don't
know or care about TLS. Also, it would be good if it does not
need to be tuned yearly to stay good.

* SSL_get_verify_result

I think Postgres uses SSL_VERIFY_PEER + SSL_set_verify() callback instead.
At least for me, the psql -d "dbname=foo sslmode=verify-ca" fails
when cert does not match.

* SNI (Server Name Indication extension).

It might be proper to configure it for connections, but it's unlikely
to be useful as the many-domains-few-ips-one-port problem really does
not apply to databases. And from my experience, even the non-SNI
hostname check is underused (or even - unusable) in many database
setups.

* TLSv1.2

That's the remaining problem with Postgres SSL - new SHA2/AESGCM
ciphersuites will not be used even when both client and server
support them. Also CBC-mode fixes in TLSv1.1 will be missed.

It's a client-side OpenSSL problem and caused indeed
by following line in fe-secure.c:

SSL_context = SSL_CTX_new(TLSv1_method());

It's an ugly problem, because TLSv1_method() actually *should* mean
"TLSv1.0 and higher", but due to problems with various broken
SSL implementations, it's disabled.

I see various ways it can improve:

1) OpenSSL guys change default back to TLSv1.0+.
2) OpenSSL guys give switch to make TLSv1_method() mean TLSv1.0+.
3) libpq starts using TLSv1_2_method() by default.
4) libpq will give switch to users to request TLSv1.2.

I see 1) and 3) as unlikely to happen. As it's not an urgent problem,
we could watch if 2) happens and go with 4) otherwise.

I tried your suggested OP_ALL patch and it does not work. And it's
even harmful to use as it disables few security workarounds.
It's mainly for servers for compat with legacy browsers.

I also tried to clear OP_NO_TLSv1_x to see if there is some default
OPs in TLSv1_method() that we could change, but that also did not work.
My conclusion is that currently there is no switch to make TLSv1.0+
work. (OpenSSL v1.0.1 / 1.1.0-git).

--
marko

Hi Marko,

Forgive me for cherry picking two of these...

> I think Postgres uses SSL_VERIFY_PEER + SSL_set_verify() callback instead.
> At least for me, the psql -d "dbname=foo sslmode=verify-ca" fails
> when cert does not match.
I can't comment on the use of psql. My apologies for my ignorance.
However, here's what I see in fe-secure.c around line 695:

static int
verify_cb(int ok, X509_STORE_CTX *ctx)
{
return ok;
}

If `ok` is 0, then validation fails. To learn of the failure, a
program must call SSL_get_verify_result to fetch the error code. Error
codes are listed at https://www.openssl.org/docs/apps/verify.html.

If `ok` is always 1, then validation succeeds. To learn of the
success, a program must call SSL_get_verify_result and ensure
X509_V_OK is returned.

I know of no other ways to check the result of OpenSSL's chain
validation. The open question (for me) is where are
SSL_get_verify_result/X509_V_OK checked? Neither show up in the
Postgres sources.

> 1) OpenSSL guys change default back to TLSv1.0+.
> 2) OpenSSL guys give switch to make TLSv1_method() mean TLSv1.0+.
Well, I don't think that's going to happen, but I could be wrong :)

For what its worth, I agree with you. I want a TLSv1.0+ option and
even had this discussion with Tim Hudson offline.

> 3) libpq starts using TLSv1_2_method() by default.
> 4) libpq will give switch to users to request TLSv1.2.
This might have negative effects on non-TLSv1.2 clients. For example,
an Android 2.3 device can only do TLSv1.0 (IIRC). I think there's a
similar limitation on a lot of Windows XP clients (depending on the IE
version and SChannel version). And OpenSSL-based clients prior to
1.0.0h (released 14 Mar 2012) will have trouble (if I am reading the
change log correctly).

I believe the "standard" way of achieving TLS1.0 and above is to use
the SSLv23_client_method() and then remove the SSL protocols with
SSL_OP_NO_SSLv2 and SSL_OP_NO_SSLv3. I have to use handwaiving around
"standard" because I don't believe its documented anywhere (one of the
devs told me its the standard way to do it.).

Jeff

On Fri, Nov 29, 2013 at 3:19 PM, Marko Kreen <markokr(at)gmail(dot)com> wrote:
> Reply to mails in pgsql-bugs:
>
> http://www.postgresql.org/message-id/CAH8yC8mc_2J2UY0Q42WQdWFyaoqT3onG+83Fr=vN46J5+ML94g@mail.gmail.com
>
> and
>
> http://www.postgresql.org/message-id/CAH8yC8nZVUyCQznkQd8=ELMM4k_=uXJRjt8YF9V22Cy2x_dDjQ@mail.gmail.com
>
>
> * Default ciphersuite
>
>> I would argue nothing should be left to chance, and the project should
>> take control of everything. But I don't really have a dog in the fight ;)
>
> Indeed, on my own servers I've stopped bothering with pattern-based
> ciphersuite strings, now I am listing ciphers explicitly.
>
> But the discussion here is about default suite for admins who don't
> know or care about TLS. Also, it would be good if it does not
> need to be tuned yearly to stay good.
>
>
> * SSL_get_verify_result
>
> I think Postgres uses SSL_VERIFY_PEER + SSL_set_verify() callback instead.
> At least for me, the psql -d "dbname=foo sslmode=verify-ca" fails
> when cert does not match.
>
>
> * SNI (Server Name Indication extension).
>
> It might be proper to configure it for connections, but it's unlikely
> to be useful as the many-domains-few-ips-one-port problem really does
> not apply to databases. And from my experience, even the non-SNI
> hostname check is underused (or even - unusable) in many database
> setups.
>
>
> * TLSv1.2
>
> That's the remaining problem with Postgres SSL - new SHA2/AESGCM
> ciphersuites will not be used even when both client and server
> support them. Also CBC-mode fixes in TLSv1.1 will be missed.
>
> It's a client-side OpenSSL problem and caused indeed
> by following line in fe-secure.c:
>
> SSL_context = SSL_CTX_new(TLSv1_method());
>
> It's an ugly problem, because TLSv1_method() actually *should* mean
> "TLSv1.0 and higher", but due to problems with various broken
> SSL implementations, it's disabled.
>
> I see various ways it can improve:
>
> 1) OpenSSL guys change default back to TLSv1.0+.
> 2) OpenSSL guys give switch to make TLSv1_method() mean TLSv1.0+.
> 3) libpq starts using TLSv1_2_method() by default.
> 4) libpq will give switch to users to request TLSv1.2.
>
> I see 1) and 3) as unlikely to happen. As it's not an urgent problem,
> we could watch if 2) happens and go with 4) otherwise.
>
>
> I tried your suggested OP_ALL patch and it does not work. And it's
> even harmful to use as it disables few security workarounds.
> It's mainly for servers for compat with legacy browsers.
>
> I also tried to clear OP_NO_TLSv1_x to see if there is some default
> OPs in TLSv1_method() that we could change, but that also did not work.
> My conclusion is that currently there is no switch to make TLSv1.0+
> work. (OpenSSL v1.0.1 / 1.1.0-git).
>

On Fri, Nov 29, 2013 at 06:01:01PM -0500, Jeffrey Walton wrote:
> I know of no other ways to check the result of OpenSSL's chain
> validation. The open question (for me) is where are
> SSL_get_verify_result/X509_V_OK checked? Neither show up in the
> Postgres sources.

According to SSL_set_verify manpage, you are perhaps talking about
SSL_VERIFY_NONE case? Which has suggestion that you should call
SSL_get_verify_result if you want to know if cert was valid.

But if SSL_VERIFY_PEER is used, this is not needed.

> > 3) libpq starts using TLSv1_2_method() by default.
> > 4) libpq will give switch to users to request TLSv1.2.
> This might have negative effects on non-TLSv1.2 clients. For example,
> an Android 2.3 device can only do TLSv1.0 (IIRC). I think there's a
> similar limitation on a lot of Windows XP clients (depending on the IE
> version and SChannel version). And OpenSSL-based clients prior to
> 1.0.0h (released 14 Mar 2012) will have trouble (if I am reading the
> change log correctly).

Note we are talking about client-side settings here. So the negative
effect would be that clients with TLSv1.2+ libpq cannot connect to
old servers.

> I believe the "standard" way of achieving TLS1.0 and above is to use
> the SSLv23_client_method() and then remove the SSL protocols with
> SSL_OP_NO_SSLv2 and SSL_OP_NO_SSLv3. I have to use handwaiving around
> "standard" because I don't believe its documented anywhere (one of the
> devs told me its the standard way to do it.).

Indeed - Python ssl module seems to achieve TLSv1.1 and it uses
SSLv23_method(). But still no TLSv1.2.

I'll play with it a bit to see whether it can have any negative effects.

--
marko

> According to SSL_set_verify manpage, you are perhaps talking about
> SSL_VERIFY_NONE case? Which has suggestion that you should call
> SSL_get_verify_result if you want to know if cert was valid.
>
> But if SSL_VERIFY_PEER is used, this is not needed.
Oh, man.... I missed that detail.

Please accept my apologies.

Jeff

On Fri, Nov 29, 2013 at 7:14 PM, Marko Kreen <markokr(at)gmail(dot)com> wrote:
> On Fri, Nov 29, 2013 at 06:01:01PM -0500, Jeffrey Walton wrote:
>> I know of no other ways to check the result of OpenSSL's chain
>> validation. The open question (for me) is where are
>> SSL_get_verify_result/X509_V_OK checked? Neither show up in the
>> Postgres sources.
>
> According to SSL_set_verify manpage, you are perhaps talking about
> SSL_VERIFY_NONE case? Which has suggestion that you should call
> SSL_get_verify_result if you want to know if cert was valid.
>
> But if SSL_VERIFY_PEER is used, this is not needed.
>
>> > 3) libpq starts using TLSv1_2_method() by default.
>> > 4) libpq will give switch to users to request TLSv1.2.
>> This might have negative effects on non-TLSv1.2 clients. For example,
>> an Android 2.3 device can only do TLSv1.0 (IIRC). I think there's a
>> similar limitation on a lot of Windows XP clients (depending on the IE
>> version and SChannel version). And OpenSSL-based clients prior to
>> 1.0.0h (released 14 Mar 2012) will have trouble (if I am reading the
>> change log correctly).
>
> Note we are talking about client-side settings here. So the negative
> effect would be that clients with TLSv1.2+ libpq cannot connect to
> old servers.
>
>> I believe the "standard" way of achieving TLS1.0 and above is to use
>> the SSLv23_client_method() and then remove the SSL protocols with
>> SSL_OP_NO_SSLv2 and SSL_OP_NO_SSLv3. I have to use handwaiving around
>> "standard" because I don't believe its documented anywhere (one of the
>> devs told me its the standard way to do it.).
>
> Indeed - Python ssl module seems to achieve TLSv1.1 and it uses
> SSLv23_method(). But still no TLSv1.2.
>
> I'll play with it a bit to see whether it can have any negative effects.
>

Hi Marko,

Sorry to go offlist....

>> I believe the "standard" way of achieving TLS1.0 and above is to use
>> the SSLv23_client_method() and then remove the SSL protocols with
>> SSL_OP_NO_SSLv2 and SSL_OP_NO_SSLv3. I have to use handwaiving around
>> "standard" because I don't believe its documented anywhere (one of the
>> devs told me its the standard way to do it.).
>
> Indeed - Python ssl module seems to achieve TLSv1.1 and it uses
> SSLv23_method(). But still no TLSv1.2.
It sounds like they are using the TLSv1_1_method(). You can check it
with Wireshark. The Client Hello will advertise the highest version of
the protocol supported. See http://postimg.org/image/e4mk3nhhl/.

If Python is not advertising TLS 1.2, then they should use the
SSLv23_method() with SSL_OP_NO_SSLv2, SSL_OP_NO_SSLv3 and
SSL_OP_NO_TLSv1. That will get them TLS 1.1 and above. From ssl.h,
around line 605:

#define SSL_OP_NO_SSLv2 0x01000000L
#define SSL_OP_NO_SSLv3 0x02000000L
#define SSL_OP_NO_TLSv1 0x04000000L
#define SSL_OP_NO_TLSv1_2 0x08000000L
#define SSL_OP_NO_TLSv1_1 0x10000000L

If you only want TLS 1.1 and 1.2, you can further trim your preferred
cipher list. TLS 1.1 did not add any ciphers, so your list might look
like (the TLS 1.0 ciphers can be used in TLS 1.1):

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 and above */
"DHE-DSS-AES256-SHA:"
"DHE-RSA-AES256-SHA:"
"DHE-DSS-AES128-SHA:"
"DHE-RSA-AES128-SHA:"

Personally, I'd like to drop TLS 1.0 (even though the complaints are
mainly academic). But I think its still needed for interop. I've never
rolled a system without it enabled.

Jeff

On Fri, Nov 29, 2013 at 7:14 PM, Marko Kreen <markokr(at)gmail(dot)com> wrote:
> On Fri, Nov 29, 2013 at 06:01:01PM -0500, Jeffrey Walton wrote:
>> I know of no other ways to check the result of OpenSSL's chain
>> validation. The open question (for me) is where are
>> SSL_get_verify_result/X509_V_OK checked? Neither show up in the
>> Postgres sources.
>
> According to SSL_set_verify manpage, you are perhaps talking about
> SSL_VERIFY_NONE case? Which has suggestion that you should call
> SSL_get_verify_result if you want to know if cert was valid.
>
> But if SSL_VERIFY_PEER is used, this is not needed.
>
>> > 3) libpq starts using TLSv1_2_method() by default.
>> > 4) libpq will give switch to users to request TLSv1.2.
>> This might have negative effects on non-TLSv1.2 clients. For example,
>> an Android 2.3 device can only do TLSv1.0 (IIRC). I think there's a
>> similar limitation on a lot of Windows XP clients (depending on the IE
>> version and SChannel version). And OpenSSL-based clients prior to
>> 1.0.0h (released 14 Mar 2012) will have trouble (if I am reading the
>> change log correctly).
>
> Note we are talking about client-side settings here. So the negative
> effect would be that clients with TLSv1.2+ libpq cannot connect to
> old servers.
>
>> I believe the "standard" way of achieving TLS1.0 and above is to use
>> the SSLv23_client_method() and then remove the SSL protocols with
>> SSL_OP_NO_SSLv2 and SSL_OP_NO_SSLv3. I have to use handwaiving around
>> "standard" because I don't believe its documented anywhere (one of the
>> devs told me its the standard way to do it.).
>
> Indeed - Python ssl module seems to achieve TLSv1.1 and it uses
> SSLv23_method(). But still no TLSv1.2.
>
> I'll play with it a bit to see whether it can have any negative effects.

On Sat, Nov 30, 2013 at 03:46:06AM -0500, Jeffrey Walton wrote:
> >> I believe the "standard" way of achieving TLS1.0 and above is to use
> >> the SSLv23_client_method() and then remove the SSL protocols with
> >> SSL_OP_NO_SSLv2 and SSL_OP_NO_SSLv3. I have to use handwaiving around
> >> "standard" because I don't believe its documented anywhere (one of the
> >> devs told me its the standard way to do it.).
> >
> > Indeed - Python ssl module seems to achieve TLSv1.1 and it uses
> > SSLv23_method(). But still no TLSv1.2.
> It sounds like they are using the TLSv1_1_method(). You can check it
> with Wireshark. The Client Hello will advertise the highest version of
> the protocol supported. See http://postimg.org/image/e4mk3nhhl/.

No, they are using SSLv23_method(). And I can confirm - I did small
C program with SSLv23_method and SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3,
and it requests up to TLSv1.1.

> If Python is not advertising TLS 1.2, then they should use the
> SSLv23_method() with SSL_OP_NO_SSLv2, SSL_OP_NO_SSLv3 and
> SSL_OP_NO_TLSv1. That will get them TLS 1.1 and above. From ssl.h,
> around line 605:
>
> #define SSL_OP_NO_SSLv2 0x01000000L
> #define SSL_OP_NO_SSLv3 0x02000000L
> #define SSL_OP_NO_TLSv1 0x04000000L
> #define SSL_OP_NO_TLSv1_2 0x08000000L
> #define SSL_OP_NO_TLSv1_1 0x10000000L
>
> If you only want TLS 1.1 and 1.2, you can further trim your preferred
> cipher list. TLS 1.1 did not add any ciphers, so your list might look
> like (the TLS 1.0 ciphers can be used in TLS 1.1):

I could not get TLSv1.1+ with that. But I'm working against
Ubuntu 12.04 default OpenSSL. I'll try with other versions too.

> Personally, I'd like to drop TLS 1.0 (even though the complaints are
> mainly academic). But I think its still needed for interop. I've never
> rolled a system without it enabled.

Good thing in about libpq is that it knows server is OpenSSL. Bad thing
is that server may be old, so we need to support servers down to
OpenSSL 0.9.7. Which means TLSv1.0.

--
marko

> I could not get TLSv1.1+ with that. But I'm working against
> Ubuntu 12.04 default OpenSSL. I'll try with other versions too.
That looks like a Ubuntu 12.04 limitation: http://postimg.org/image/3ju4fu0y1/

I would bet the 1.0.0 version of OpenSSL is less that 1.0.0h:

$ ldd /usr/lib/x86_64-linux-gnu/libssl.so
linux-vdso.so.1 => (0x00007fffd9d84000)
libcrypto.so.1.0.0 => /lib/x86_64-linux-gnu/libcrypto.so.1.0.0
(0x00007f1e0691e000)
libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f1e0655e000)
libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f1e06359000)
libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007f1e06142000)
/lib64/ld-linux-x86-64.so.2 (0x00007f1e06f6d000)

Gotta love back patching and broken versioning ;)

Jeff

On Sat, Nov 30, 2013 at 4:12 AM, Marko Kreen <markokr(at)gmail(dot)com> wrote:
> On Sat, Nov 30, 2013 at 03:46:06AM -0500, Jeffrey Walton wrote:
>> >> I believe the "standard" way of achieving TLS1.0 and above is to use
>> >> the SSLv23_client_method() and then remove the SSL protocols with
>> >> SSL_OP_NO_SSLv2 and SSL_OP_NO_SSLv3. I have to use handwaiving around
>> >> "standard" because I don't believe its documented anywhere (one of the
>> >> devs told me its the standard way to do it.).
>> >
>> > Indeed - Python ssl module seems to achieve TLSv1.1 and it uses
>> > SSLv23_method(). But still no TLSv1.2.
>> It sounds like they are using the TLSv1_1_method(). You can check it
>> with Wireshark. The Client Hello will advertise the highest version of
>> the protocol supported. See http://postimg.org/image/e4mk3nhhl/.
>
> No, they are using SSLv23_method(). And I can confirm - I did small
> C program with SSLv23_method and SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3,
> and it requests up to TLSv1.1.
>
>> If Python is not advertising TLS 1.2, then they should use the
>> SSLv23_method() with SSL_OP_NO_SSLv2, SSL_OP_NO_SSLv3 and
>> SSL_OP_NO_TLSv1. That will get them TLS 1.1 and above. From ssl.h,
>> around line 605:
>>
>> #define SSL_OP_NO_SSLv2 0x01000000L
>> #define SSL_OP_NO_SSLv3 0x02000000L
>> #define SSL_OP_NO_TLSv1 0x04000000L
>> #define SSL_OP_NO_TLSv1_2 0x08000000L
>> #define SSL_OP_NO_TLSv1_1 0x10000000L
>>
>> If you only want TLS 1.1 and 1.2, you can further trim your preferred
>> cipher list. TLS 1.1 did not add any ciphers, so your list might look
>> like (the TLS 1.0 ciphers can be used in TLS 1.1):
>
> I could not get TLSv1.1+ with that. But I'm working against
> Ubuntu 12.04 default OpenSSL. I'll try with other versions too.
>
>> Personally, I'd like to drop TLS 1.0 (even though the complaints are
>> mainly academic). But I think its still needed for interop. I've never
>> rolled a system without it enabled.
>
> Good thing in about libpq is that it knows server is OpenSSL. Bad thing
> is that server may be old, so we need to support servers down to
> OpenSSL 0.9.7. Which means TLSv1.0.

As pointed out by Jeffrey Walton, only SSLv23_method() will negotiate
higher TLS versions than immediately requested. All other _method()
functions will negotiate only one specific version.

Attached are two patches:

* libpq.tls11plus.diff

Use SSLv23_method() instead TLSv1_method in fe-secure.c. Also
disable SSLv2 and SSLv3 so TLSv1 continues to be minimum
TLS version negotiated. This means TLSv1.1 or TLSv1.2 will be
used as protocol if both sides can speak it.

* psql.conninfo.tlsver.diff

Make psql \conninfo show TLS protocol version. It's really hard
to see what version is actually used otherwise without using
network dumps.

Random findings
---------------

- TLSv1.1 and TLSv1.2 are implemented in OpenSSL 1.0.1.

- All OpenSSL 1.0.1+ versions negotiate TLSv1.2 with SSLv23_method()
by default. This is default also in OpenSSL git branches (1.0.1-stable,
1.0.2-stable, master).

- OpenSSL versions up to 0.9.7f send SSLv2 ClientHello from SSLv23_method()
even when SSLv2 and SSLv3 are disabled. They still manage to
negotiate TLSv1. Since version 0.9.7h, OpenSSL sends TLSv1 ClientHello
in those circumstances.

- Ubuntu uses compilation flag that disables TLSv1.2 in SSLv23_method().

- Ubuntu also uses compilation flag to cut TLSv1.2 cipher list to first 25.
This means only AES256 usable. This also disables secure renegotation
as that is signaled with extra ciphersuite. And because of previous flag,
it affects only programs using TLSv1_2_method() directly.
I see signs of confusion here.

- Debian and Fedora OpenSSL 1.0.1+ packages do not mess with TLSv1.2,
so I assume everything is fine there.

Because the patches are small and look safe, it might be better if they
are handled together with other SSL patches in this commitfest. That
would give them more mileage before release, to see if any problems
pop out. But I can add them to next commitfest if that is not OK.

--
marko

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.

[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