Re: default SSL compression (was: libpq compression)

On Thu, Aug 30, 2012 at 11:41 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:

> On Sun, Jun 17, 2012 at 11:45:54PM +0800, Magnus Hagander wrote:
> > On Sun, Jun 17, 2012 at 11:42 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > > Magnus Hagander <magnus(at)hagander(dot)net> writes:
> > >> Is there a reason why we don't have a parameter on the client
> > >> mirroring ssl_ciphers?
> > >
> > > Dunno, do we need one? I am not sure what the cipher negotiation
> process
> > > looks like or which side has the freedom to choose.
> >
> > I haven't looked into the details, but it seems reasonable that
> > *either* side should be able to at least define a list of ciphers it
> > *doens't* want to talk with.
> >
> > Do we need it - well, it makes sense for the client to be able to say
> > "I won't trust 56-bit encryption" before it sends over the password,
> > imo..
> >
> >
> > >> That, or just have DEFAULT as being the default (which in current
> > >> openssl means ALL:!aNULL:!eNULL.
> > >
> > > If our default isn't the same as the underlying default, I have to
> > > question why not.
> >
> > Yeah, that's exaclty what I'm questioning here..
> >
> > > But are you sure this "!" notation will work with
> > > all openssl versions?
> >
> > Uh. We have the ! notation in our default *now*. What openssl also
> > supports is the text "DEFAULT", which is currently the equivalent of
> > "ALL!aNULL!eNULL". The question, which is valid of course, should be
> > if "DEFAULT" works with all openssl versions.
> >
> > It would seem reasonable it does, but I haven't investigated.
>
> Do we want to change our ssl_ciphers default to 'DEFAULT'? Currently it
> is 'ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH'.
>
>
Did we ever get anywhere with this? Is this a change we want to do for 9.3?
Since nobody seems to have come up with a motivation for not following the
openssl default, we probably should?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Tue, Jan 01, 2013 at 04:29:35PM +0100, Magnus Hagander wrote:
> On Thu, Aug 30, 2012 at 11:41 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> > On Sun, Jun 17, 2012 at 11:45:54PM +0800, Magnus Hagander wrote:
> > > Uh. We have the ! notation in our default *now*. What openssl also
> > > supports is the text "DEFAULT", which is currently the equivalent of
> > > "ALL!aNULL!eNULL". The question, which is valid of course, should be
> > > if "DEFAULT" works with all openssl versions.
> > >
> > > It would seem reasonable it does, but I haven't investigated.

The oldest version readily available for download (0.9.1c, 1998) has it.

> > Do we want to change our ssl_ciphers default to 'DEFAULT'? Currently it
> > is 'ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH'.
> >
> Did we ever get anywhere with this? Is this a change we want to do for 9.3?
> Since nobody seems to have come up with a motivation for not following the
> openssl default, we probably should?

+1 for doing that. I'm not aware of a PostgreSQL-specific selection criterion
for SSL cipher suites.

Noah Misch <noah(at)leadboat(dot)com> writes:
> On Tue, Jan 01, 2013 at 04:29:35PM +0100, Magnus Hagander wrote:
>> On Thu, Aug 30, 2012 at 11:41 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>>> Do we want to change our ssl_ciphers default to 'DEFAULT'? Currently it
>>> is 'ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH'.

>> Did we ever get anywhere with this? Is this a change we want to do for 9.3?
>> Since nobody seems to have come up with a motivation for not following the
>> openssl default, we probably should?

> +1 for doing that. I'm not aware of a PostgreSQL-specific selection criterion
> for SSL cipher suites.

I did a bit of digging in the commit logs. The current default was
introduced in commit 17386ac45345fe38a10caec9d6e63afa3ce31bb9. So far
as I can find, the only discussion leading up to that patch was the
thread starting at
http://archives.postgresql.org/pgsql-interfaces/2003-04/msg00075.php
which only discusses the key renegotiation interval, not anything about
selecting the allowed ciphers. What's more, one might be forgiven for
suspecting that the cipherset string wasn't too carefully researched
after noticing that it wasn't even spelled correctly in that commit.

So +1 for changing it to "DEFAULT" from me, too. There's no reason to
think we know more about this than the OpenSSL authors.

regards, tom lane

On Wed, Jan 2, 2013 at 1:15 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Noah Misch <noah(at)leadboat(dot)com> writes:
> > On Tue, Jan 01, 2013 at 04:29:35PM +0100, Magnus Hagander wrote:
> >> On Thu, Aug 30, 2012 at 11:41 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> >>> Do we want to change our ssl_ciphers default to 'DEFAULT'? Currently it
> >>> is 'ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH'.
>
> >> Did we ever get anywhere with this? Is this a change we want to do for 9.3?
> >> Since nobody seems to have come up with a motivation for not following the
> >> openssl default, we probably should?
>
> > +1 for doing that. I'm not aware of a PostgreSQL-specific selection criterion
> > for SSL cipher suites.
>
> I did a bit of digging in the commit logs. The current default was
> introduced in commit 17386ac45345fe38a10caec9d6e63afa3ce31bb9. So far
> as I can find, the only discussion leading up to that patch was the
> thread starting at
> http://archives.postgresql.org/pgsql-interfaces/2003-04/msg00075.php
> which only discusses the key renegotiation interval, not anything about
> selecting the allowed ciphers. What's more, one might be forgiven for
> suspecting that the cipherset string wasn't too carefully researched
> after noticing that it wasn't even spelled correctly in that commit.

Yeah, clearly seems that way.

> So +1 for changing it to "DEFAULT" from me, too. There's no reason to
> think we know more about this than the OpenSSL authors.
>

The DEFAULT value in OpenSSL 1.0 means "ALL:!aNULL:!eNULL".

Researching some more, this might cause a problem actually, which
would explain some of the things that are in our default. For example,
an ADH algorithm doesn't use certificates - but it uses DH parameters,
so it likely won't work anyway. EDH uses certs, but also requires DH
parameters.

Maybe what we nede is "DEFAULT:!ADH:@STRENGTH" as the default?

The other difference is that our current string denies 40 and 56 bit
encryptions (low and export strenghts). Do we stll want to do that?

Finally we deny MD5 - I have no idea why we do that.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Wed, Jan 02, 2013 at 02:03:20PM +0100, Magnus Hagander wrote:
> On Wed, Jan 2, 2013 at 1:15 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > So +1 for changing it to "DEFAULT" from me, too. There's no reason to
> > think we know more about this than the OpenSSL authors.
>
> The DEFAULT value in OpenSSL 1.0 means "ALL:!aNULL:!eNULL".
>
> Researching some more, this might cause a problem actually, which
> would explain some of the things that are in our default. For example,
> an ADH algorithm doesn't use certificates - but it uses DH parameters,
> so it likely won't work anyway. EDH uses certs, but also requires DH
> parameters.
>
> Maybe what we nede is "DEFAULT:!ADH:@STRENGTH" as the default?

I understand aNULL to include ADH.

> The other difference is that our current string denies 40 and 56 bit
> encryptions (low and export strenghts). Do we stll want to do that?

On the one hand, those seem bad to permit by default in 2013. On the other
hand, if so, why hasn't OpenSSL removed them from DEFAULT? Perhaps it has
backward-compatibility concerns that wouldn't apply to us by virtue of having
disabled them for some time. Sounds reasonable to continue disabling them.

On Wed, Jan 2, 2013 at 3:15 PM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> On Wed, Jan 02, 2013 at 02:03:20PM +0100, Magnus Hagander wrote:
>> On Wed, Jan 2, 2013 at 1:15 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> > So +1 for changing it to "DEFAULT" from me, too. There's no reason to
>> > think we know more about this than the OpenSSL authors.
>>
>> The DEFAULT value in OpenSSL 1.0 means "ALL:!aNULL:!eNULL".
>>
>> Researching some more, this might cause a problem actually, which
>> would explain some of the things that are in our default. For example,
>> an ADH algorithm doesn't use certificates - but it uses DH parameters,
>> so it likely won't work anyway. EDH uses certs, but also requires DH
>> parameters.
>>
>> Maybe what we nede is "DEFAULT:!ADH:@STRENGTH" as the default?
>
> I understand aNULL to include ADH.

Hmm. Seems you're right when I run a test on it, I was reading it wrong.

>> The other difference is that our current string denies 40 and 56 bit
>> encryptions (low and export strenghts). Do we stll want to do that?
>
> On the one hand, those seem bad to permit by default in 2013. On the other
> hand, if so, why hasn't OpenSSL removed them from DEFAULT? Perhaps it has
> backward-compatibility concerns that wouldn't apply to us by virtue of having
> disabled them for some time. Sounds reasonable to continue disabling them.

So then the default would be "DEFAULT:!LOW:!EXP:@STRENGTH"

(the @STRENGTH part is the sorting key for preference, which the
default one seems not to have)

The biggest difference being that we start from DEFAULT rather than ALL.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Wed, Jan 2, 2013 at 10:03 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> Finally we deny MD5 - I have no idea why we do that.

Because it's broken, same motivation as in the thread for implementing
ZK authentication.

Also, I seem to have missed something because the thread subject
mentions compression whereas I've seen no mention of it in the
discussion (maybe I just missed it). But, in any case, I would like to
point out this[0]. I'm not sure it's readily applicable to PG's wire
protocol, but just in case I mention it.

[0] http://news.idg.no/cw/art.cfm?id=976ED08F-CB4A-23DA-FFDA0419B8750B72

On Wed, Jan 2, 2013 at 3:17 PM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> On Wed, Jan 2, 2013 at 3:15 PM, Noah Misch <noah(at)leadboat(dot)com> wrote:
>> On Wed, Jan 02, 2013 at 02:03:20PM +0100, Magnus Hagander wrote:
>>> On Wed, Jan 2, 2013 at 1:15 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> > So +1 for changing it to "DEFAULT" from me, too. There's no reason to
>>> > think we know more about this than the OpenSSL authors.
>>>
>>> The DEFAULT value in OpenSSL 1.0 means "ALL:!aNULL:!eNULL".
>>>
>>> Researching some more, this might cause a problem actually, which
>>> would explain some of the things that are in our default. For example,
>>> an ADH algorithm doesn't use certificates - but it uses DH parameters,
>>> so it likely won't work anyway. EDH uses certs, but also requires DH
>>> parameters.
>>>
>>> Maybe what we nede is "DEFAULT:!ADH:@STRENGTH" as the default?
>>
>> I understand aNULL to include ADH.
>
> Hmm. Seems you're right when I run a test on it, I was reading it wrong.
>
>
>>> The other difference is that our current string denies 40 and 56 bit
>>> encryptions (low and export strenghts). Do we stll want to do that?
>>
>> On the one hand, those seem bad to permit by default in 2013. On the other
>> hand, if so, why hasn't OpenSSL removed them from DEFAULT? Perhaps it has
>> backward-compatibility concerns that wouldn't apply to us by virtue of having
>> disabled them for some time. Sounds reasonable to continue disabling them.
>
> So then the default would be "DEFAULT:!LOW:!EXP:@STRENGTH"
>
> (the @STRENGTH part is the sorting key for preference, which the
> default one seems not to have)
>
> The biggest difference being that we start from DEFAULT rather than ALL.

I've applied a change that does this, including still rejecting MD5.
Meaning that the difference is we start from DEFAULT instead of ALL
(and the ADH rule is removed, since !aNULL is already in the default).

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/