Thread: Re: [BUGS] BUG #6189: libpq: sslmode=require verifies server certificate if root.crt is present

On Wed, Aug 31, 2011 at 11:59, Srinivas Aji <srinivas.aji@emc.com> wrote:
>
> The following bug has been logged online:
>
> Bug reference:      6189
> Logged by:          Srinivas Aji
> Email address:      srinivas.aji@emc.com
> PostgreSQL version: 9.0.4
> Operating system:   Linux
> Description:        libpq: sslmode=require verifies server certificate if
> root.crt is present
> Details:
>
> From the documentation of sslmode values in
> http://www.postgresql.org/docs/9.0/static/libpq-ssl.html ,
> it looks like libpq will not verify the server certificate when the option
> sslmode=require is used, and will perform different levels of certificate
> verification in the cases sslmode=verify-ca and sslmode=verify-full.
>
> The observed behaviour is a bit different. If the ~/.postgresql/root.crt
> file (or any other filename set through sslrootcert option) is found,
> sslmode=require also performs the same level of certificate verification as
> verify-ca. The difference between require and verify-ca is that it is an
> error for the file to not exist when sslmode is verify-ca.

I looked at this again, and I'm pretty sure we did this intentionally.
The idea being that before we had the verify-ca/verify-full options,
adding the root cert would enable the verification. And we didn't want
to turn installations that previously did verify the certificate to
stop doing so in the new version.

So basically, the behaviour that is by design is:
* require: if certificate exists, verify. if certificate doesn't
exist, don't verify.
* verify-ca: if certificate exists, verify. if certificate doesn't
exist, disconnect.

The question is, have we had the new options long enough now that we
should change it so that we don't verify the cert in the case of
cert-exists-but-verification-wasn't-explicitly-asked-for?

Or should we just update the documentation to mention how this works?

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

<div style="font-family: Verdana; font-size: 12pt; color: #000000"><b>From: </b>"Magnus Hagander"
<magnus@hagander.net><br/><b>To: </b>"Srinivas Aji" <srinivas.aji@emc.com><br /><b>Cc:
</b>"PostgreSQL-development"<pgsql-hackers@postgresql.org><br /><b>Sent: </b>Friday, September 23, 2011 7:28:09
AM<br/><b>Subject: </b>[HACKERS] Re: [BUGS] BUG #6189: libpq: sslmode=require verifies server certificate if root.crt
ispresent<br /><br />On Wed, Aug 31, 2011 at 11:59, Srinivas Aji <srinivas.aji@emc.com> wrote:<br />><br
/>>The following bug has been logged online:<br />><br />> Bug reference:      6189<br />> Logged by:      
  Srinivas Aji<br />> Email address:      srinivas.aji@emc.com<br />> PostgreSQL version: 9.0.4<br />>
Operatingsystem:   Linux<br />> Description:        libpq: sslmode=require verifies server certificate if<br />>
root.crtis present<br />> Details:<br />><br />...<br />><br />> The observed behaviour is a bit different.
Ifthe ~/.postgresql/root.crt<br />> file (or any other filename set through sslrootcert option) is found,<br />>
sslmode=requirealso performs the same level of certificate verification as<br />> verify-ca. The difference between
requireand verify-ca is that it is an<br />> error for the file to not exist when sslmode is verify-ca.<br /><br />I
lookedat this again, and I'm pretty sure we did this intentionally.<br />The idea being that before we had the
verify-ca/verify-fulloptions,<br />adding the root cert would enable the verification. And we didn't want<br />to turn
installationsthat previously did verify the certificate to<br />stop doing so in the new version.<br /><br />So
basically,the behaviour that is by design is:<br />* require: if certificate exists, verify. if certificate doesn't<br
/>exist,don't verify.<br />* verify-ca: if certificate exists, verify. if certificate doesn't<br />exist,
disconnect.<br/><br />The question is, have we had the new options long enough now that we<br />should change it so
thatwe don't verify the cert in the case of<br />cert-exists-but-verification-wasn't-explicitly-asked-for?<br /><br
/>Orshould we just update the documentation to mention how this works?<br /><br />-- <br /> Magnus Hagander<br /> Me:
http://www.hagander.net/<br/> Work: http://www.redpill-linpro.com/<br /><br />Magnus, If you're accepting votes on
this:I would say 'yes' - change the behavior to the most logically consistent ones; ie, isolate the verification bits a
bitmore explicitly. And, in documentation, indicate the deprecation of the old behavior.<br /><br />Our mileage, in
practicalterms, is that the perceived inconsistencies create a minor support hassle - we don't want to present any -
eventrivial - hurdle to adoption of SSL to our clients.<br /><br />Lou Picciano<br /></div> 

On Fri, Sep 23, 2011 at 14:35, Lou Picciano <loupicciano@comcast.net> wrote:
>
> On Wed, Aug 31, 2011 at 11:59, Srinivas Aji <srinivas.aji@emc.com> wrote:
>>
>> The following bug has been logged online:
>>
>> Bug reference:      6189
>> Logged by:          Srinivas Aji
>> Email address:      srinivas.aji@emc.com
>> PostgreSQL version: 9.0.4
>> Operating system:   Linux
>> Description:        libpq: sslmode=require verifies server certificate if
>> root.crt is present
>> Details:
>>
> ...
>>
>> The observed behaviour is a bit different. If the ~/.postgresql/root.crt
>> file (or any other filename set through sslrootcert option) is found,
>> sslmode=require also performs the same level of certificate verification
>> as
>> verify-ca. The difference between require and verify-ca is that it is an
>> error for the file to not exist when sslmode is verify-ca.
>
> I looked at this again, and I'm pretty sure we did this intentionally.
> The idea being that before we had the verify-ca/verify-full options,
> adding the root cert would enable the verification. And we didn't want
> to turn installations that previously did verify the certificate to
> stop doing so in the new version.
>
> So basically, the behaviour that is by design is:
> * require: if certificate exists, verify. if certificate doesn't
> exist, don't verify.
> * verify-ca: if certificate exists, verify. if certificate doesn't
> exist, disconnect.
>
> The question is, have we had the new options long enough now that we
> should change it so that we don't verify the cert in the case of
> cert-exists-but-verification-wasn't-explicitly-asked-for?
>
> Or should we just update the documentation to mention how this works?
>
> Magnus, If you're accepting votes on this: I would say 'yes' - change the
> behavior to the most logically consistent ones; ie, isolate the verification
> bits a bit more explicitly. And, in documentation, indicate the deprecation
> of the old behavior.
>
> Our mileage, in practical terms, is that the perceived inconsistencies
> create a minor support hassle - we don't want to present any - even trivial
> - hurdle to adoption of SSL to our clients.

There are really two options to this as well - we can backpatch such a
change, or we can change it only in 9.2. I'm leaning towards a "no" on
the backport, because that will change things for existing users. So
probably a doc change in backbranches and a behaviour change in 9.2
would be the reasonable choice in this case.


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

On Fri, Sep 23, 2011 at 8:38 AM, Magnus Hagander <magnus@hagander.net> wrote:
> On Fri, Sep 23, 2011 at 14:35, Lou Picciano <loupicciano@comcast.net> wrote:
>> On Wed, Aug 31, 2011 at 11:59, Srinivas Aji <srinivas.aji@emc.com> wrote:
>>>
>>> The following bug has been logged online:
>>>
>>> Bug reference:      6189
>>> Logged by:          Srinivas Aji
>>> Email address:      srinivas.aji@emc.com
>>> PostgreSQL version: 9.0.4
>>> Operating system:   Linux
>>> Description:        libpq: sslmode=require verifies server certificate if
>>> root.crt is present
>>> Details:
>>>
>> ...
>>>
>>> The observed behaviour is a bit different. If the ~/.postgresql/root.crt
>>> file (or any other filename set through sslrootcert option) is found,
>>> sslmode=require also performs the same level of certificate verification
>>> as
>>> verify-ca. The difference between require and verify-ca is that it is an
>>> error for the file to not exist when sslmode is verify-ca.
>>
>> I looked at this again, and I'm pretty sure we did this intentionally.
>> The idea being that before we had the verify-ca/verify-full options,
>> adding the root cert would enable the verification. And we didn't want
>> to turn installations that previously did verify the certificate to
>> stop doing so in the new version.
>>
>> So basically, the behaviour that is by design is:
>> * require: if certificate exists, verify. if certificate doesn't
>> exist, don't verify.
>> * verify-ca: if certificate exists, verify. if certificate doesn't
>> exist, disconnect.
>>
>> The question is, have we had the new options long enough now that we
>> should change it so that we don't verify the cert in the case of
>> cert-exists-but-verification-wasn't-explicitly-asked-for?
>>
>> Or should we just update the documentation to mention how this works?
>>
>> Magnus, If you're accepting votes on this: I would say 'yes' - change the
>> behavior to the most logically consistent ones; ie, isolate the verification
>> bits a bit more explicitly. And, in documentation, indicate the deprecation
>> of the old behavior.
>>
>> Our mileage, in practical terms, is that the perceived inconsistencies
>> create a minor support hassle - we don't want to present any - even trivial
>> - hurdle to adoption of SSL to our clients.
>
> There are really two options to this as well - we can backpatch such a
> change, or we can change it only in 9.2. I'm leaning towards a "no" on
> the backport, because that will change things for existing users. So
> probably a doc change in backbranches and a behaviour change in 9.2
> would be the reasonable choice in this case.

I definitely don't think we should back-patch a behavior change that
silently weakens security.  That's not good.

But what about not doing it in master, either?  It seems to me that we
could avoid ever breaking backward compatibility by adding a new
option "require-no-verify".

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Magnus Hagander <magnus@hagander.net> writes:
> I looked at this again, and I'm pretty sure we did this intentionally.

Yeah, we did.

> Or should we just update the documentation to mention how this works?

+1 for doc change only.  I think the behavior was thought through
carefully, and the wording of the docs maybe not so much.
        regards, tom lane

<div style="font-family: Verdana; font-size: 12pt; color: #000000"><b>From: </b>"Magnus Hagander"
<magnus@hagander.net><br/><b>To: </b>"Lou Picciano" <loupicciano@comcast.net><br /><b>Cc:
</b>"PostgreSQL-development"<pgsql-hackers@postgresql.org>, "Srinivas Aji" <srinivas.aji@emc.com><br
/><b>Sent:</b>Friday, September 23, 2011 8:38:00 AM<br /><b>Subject: </b>Re: [HACKERS] Re: [BUGS] BUG #6189: libpq:
sslmode=requireverifies server certificate if root.crt is present<br /><br />On Fri, Sep 23, 2011 at 14:35, Lou
Picciano<loupicciano@comcast.net> wrote:<br />><br />> On Wed, Aug 31, 2011 at 11:59, Srinivas Aji
<srinivas.aji@emc.com>wrote:<br />>><br />>> The following bug has been logged online:<br
/>>><br/>>> Bug reference:      6189<br />>> Logged by:          Srinivas Aji<br />>> Email
address:     srinivas.aji@emc.com<br />>> PostgreSQL version: 9.0.4<br />>> Operating system:   Linux<br
/>>>Description:        libpq: sslmode=require verifies server certificate if<br />>> root.crt is
present<br/>>> Details:<br />>><br />> ...<br />>><br />>> The observed behaviour is a bit
different.If the ~/.postgresql/root.crt<br />>> file (or any other filename set through sslrootcert option) is
found,<br/>>> sslmode=require also performs the same level of certificate verification<br />>> as<br
/>>>verify-ca. The difference between require and verify-ca is that it is an<br />>> error for the file to
notexist when sslmode is verify-ca.<br />><br />> I looked at this again, and I'm pretty sure we did this
intentionally.<br/>> The idea being that before we had the verify-ca/verify-full options,<br />> adding the root
certwould enable the verification. And we didn't want<br />> to turn installations that previously did verify the
certificateto<br />> stop doing so in the new version.<br />><br />> So basically, the behaviour that is by
designis:<br />> * require: if certificate exists, verify. if certificate doesn't<br />> exist, don't verify.<br
/>>* verify-ca: if certificate exists, verify. if certificate doesn't<br />> exist, disconnect.<br />><br
/>>The question is, have we had the new options long enough now that we<br />> should change it so that we don't
verifythe cert in the case of<br />> cert-exists-but-verification-wasn't-explicitly-asked-for?<br />><br />>
Orshould we just update the documentation to mention how this works?<br />><br />> Magnus, If you're accepting
voteson this: I would say 'yes' - change the<br />> behavior to the most logically consistent ones; ie, isolate the
verification<br/>> bits a bit more explicitly. And, in documentation, indicate the deprecation<br />> of the old
behavior.<br/>><br />> Our mileage, in practical terms, is that the perceived inconsistencies<br />> create a
minorsupport hassle - we don't want to present any - even trivial<br />> - hurdle to adoption of SSL to our
clients.<br/><br />There are really two options to this as well - we can backpatch such a<br />change, or we can change
itonly in 9.2. I'm leaning towards a "no" on<br />the backport, because that will change things for existing users.
So<br/>probably a doc change in backbranches and a behaviour change in 9.2<br />would be the reasonable choice in this
case.<br/><br />Again, if you were soliciting votes, I'd take the aggressive stance: +1 for the backport to 9.1.<br
/><br/>Of the population using SSL, you'd be pulling out the subset getting all the way down into PKI implementation,
then,those actually doing apps teasing out these differences in verification behavior...  Among _that_ group, you're
onlyconcerned with recent adopters of 9.1, and only those who wouldn't be in a position to adapt pretty quickly.
Probablya pretty small cohort for something this esoteric.<br /><br />In our case, we do run into it - for our new
clients.We find ourselves in something of a support role regarding pqlib's SSL capabilities!<br /><br />Lou Picciano<br
/></div>

On Fri, Sep 23, 2011 at 14:49, Robert Haas <robertmhaas@gmail.com> wrote:
> On Fri, Sep 23, 2011 at 8:38 AM, Magnus Hagander <magnus@hagander.net> wrote:
>> On Fri, Sep 23, 2011 at 14:35, Lou Picciano <loupicciano@comcast.net> wrote:
>>> On Wed, Aug 31, 2011 at 11:59, Srinivas Aji <srinivas.aji@emc.com> wrote:
>>>>
>>>> The following bug has been logged online:
>>>>
>>>> Bug reference:      6189
>>>> Logged by:          Srinivas Aji
>>>> Email address:      srinivas.aji@emc.com
>>>> PostgreSQL version: 9.0.4
>>>> Operating system:   Linux
>>>> Description:        libpq: sslmode=require verifies server certificate if
>>>> root.crt is present
>>>> Details:
>>>>
>>> ...
>>>>
>>>> The observed behaviour is a bit different. If the ~/.postgresql/root.crt
>>>> file (or any other filename set through sslrootcert option) is found,
>>>> sslmode=require also performs the same level of certificate verification
>>>> as
>>>> verify-ca. The difference between require and verify-ca is that it is an
>>>> error for the file to not exist when sslmode is verify-ca.
>>>
>>> I looked at this again, and I'm pretty sure we did this intentionally.
>>> The idea being that before we had the verify-ca/verify-full options,
>>> adding the root cert would enable the verification. And we didn't want
>>> to turn installations that previously did verify the certificate to
>>> stop doing so in the new version.
>>>
>>> So basically, the behaviour that is by design is:
>>> * require: if certificate exists, verify. if certificate doesn't
>>> exist, don't verify.
>>> * verify-ca: if certificate exists, verify. if certificate doesn't
>>> exist, disconnect.
>>>
>>> The question is, have we had the new options long enough now that we
>>> should change it so that we don't verify the cert in the case of
>>> cert-exists-but-verification-wasn't-explicitly-asked-for?
>>>
>>> Or should we just update the documentation to mention how this works?
>>>
>>> Magnus, If you're accepting votes on this: I would say 'yes' - change the
>>> behavior to the most logically consistent ones; ie, isolate the verification
>>> bits a bit more explicitly. And, in documentation, indicate the deprecation
>>> of the old behavior.
>>>
>>> Our mileage, in practical terms, is that the perceived inconsistencies
>>> create a minor support hassle - we don't want to present any - even trivial
>>> - hurdle to adoption of SSL to our clients.
>>
>> There are really two options to this as well - we can backpatch such a
>> change, or we can change it only in 9.2. I'm leaning towards a "no" on
>> the backport, because that will change things for existing users. So
>> probably a doc change in backbranches and a behaviour change in 9.2
>> would be the reasonable choice in this case.
>
> I definitely don't think we should back-patch a behavior change that
> silently weakens security.  That's not good.
>
> But what about not doing it in master, either?  It seems to me that we
> could avoid ever breaking backward compatibility by adding a new
> option "require-no-verify".

Hmm. Intersting. and we could then deprecate the "require" option and
kill it off 4 releases later or so, I guess...

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

Excerpts from Magnus Hagander's message of vie sep 23 10:39:46 -0300 2011:
> On Fri, Sep 23, 2011 at 14:49, Robert Haas <robertmhaas@gmail.com> wrote:
> > On Fri, Sep 23, 2011 at 8:38 AM, Magnus Hagander <magnus@hagander.net> wrote:
> >> On Fri, Sep 23, 2011 at 14:35, Lou Picciano <loupicciano@comcast.net> wrote:
> >>> On Wed, Aug 31, 2011 at 11:59, Srinivas Aji <srinivas.aji@emc.com> wrote:
> >>>>
> >>>> The following bug has been logged online:
> >>>>
> >>>> Bug reference:      6189
> >>>> Logged by:          Srinivas Aji
> >>>> Email address:      srinivas.aji@emc.com
> >>>> PostgreSQL version: 9.0.4
> >>>> Operating system:   Linux
> >>>> Description:        libpq: sslmode=require verifies server certificate if
> >>>> root.crt is present

> >>> So basically, the behaviour that is by design is:
> >>> * require: if certificate exists, verify. if certificate doesn't
> >>> exist, don't verify.
> >>> * verify-ca: if certificate exists, verify. if certificate doesn't
> >>> exist, disconnect.

> > I definitely don't think we should back-patch a behavior change that
> > silently weakens security.  That's not good.
> >
> > But what about not doing it in master, either?  It seems to me that we
> > could avoid ever breaking backward compatibility by adding a new
> > option "require-no-verify".
> 
> Hmm. Intersting. and we could then deprecate the "require" option and
> kill it off 4 releases later or so, I guess...

So we would have
sslmode=verify-ca / require-no-verify / verify-full / disable / allow / prefer
?

This seems strange to me.  Why not have a second option to let the user
indicate the desired SSL verification?

sslmode=disable/allow/prefer/require
sslverify=none/ca-if-present/ca/full

(ca-if-present being the current "require" sslmode behavior).

We could then deprecate sslmode=verify and verify-full and have them be
synonyms of sslmode=require and corresponding sslverify.

-- 
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

On Fri, Sep 23, 2011 at 15:55, Alvaro Herrera
<alvherre@commandprompt.com> wrote:
>
> Excerpts from Magnus Hagander's message of vie sep 23 10:39:46 -0300 2011:
>> On Fri, Sep 23, 2011 at 14:49, Robert Haas <robertmhaas@gmail.com> wrote:
>> > On Fri, Sep 23, 2011 at 8:38 AM, Magnus Hagander <magnus@hagander.net> wrote:
>> >> On Fri, Sep 23, 2011 at 14:35, Lou Picciano <loupicciano@comcast.net> wrote:
>> >>> On Wed, Aug 31, 2011 at 11:59, Srinivas Aji <srinivas.aji@emc.com> wrote:
>> >>>>
>> >>>> The following bug has been logged online:
>> >>>>
>> >>>> Bug reference:      6189
>> >>>> Logged by:          Srinivas Aji
>> >>>> Email address:      srinivas.aji@emc.com
>> >>>> PostgreSQL version: 9.0.4
>> >>>> Operating system:   Linux
>> >>>> Description:        libpq: sslmode=require verifies server certificate if
>> >>>> root.crt is present
>
>> >>> So basically, the behaviour that is by design is:
>> >>> * require: if certificate exists, verify. if certificate doesn't
>> >>> exist, don't verify.
>> >>> * verify-ca: if certificate exists, verify. if certificate doesn't
>> >>> exist, disconnect.
>
>> > I definitely don't think we should back-patch a behavior change that
>> > silently weakens security.  That's not good.
>> >
>> > But what about not doing it in master, either?  It seems to me that we
>> > could avoid ever breaking backward compatibility by adding a new
>> > option "require-no-verify".
>>
>> Hmm. Intersting. and we could then deprecate the "require" option and
>> kill it off 4 releases later or so, I guess...
>
> So we would have
> sslmode=verify-ca / require-no-verify / verify-full / disable / allow / prefer
> ?
>
> This seems strange to me.  Why not have a second option to let the user
> indicate the desired SSL verification?
>
> sslmode=disable/allow/prefer/require
> sslverify=none/ca-if-present/ca/full
>
> (ca-if-present being the current "require" sslmode behavior).
>
> We could then deprecate sslmode=verify and verify-full and have them be
> synonyms of sslmode=require and corresponding sslverify.

Hmm. I agree that the other suggestion was a bit weird, but I'm not
sure I like the multiple-options approach either. That's going to
require redesign of all software that deals with it at all today :S

Maybe we should just update the docs and be done with it :-)

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

Excerpts from Magnus Hagander's message of vie sep 23 11:31:37 -0300 2011:
> 
> On Fri, Sep 23, 2011 at 15:55, Alvaro Herrera
> <alvherre@commandprompt.com> wrote:

> > This seems strange to me.  Why not have a second option to let the user
> > indicate the desired SSL verification?
> >
> > sslmode=disable/allow/prefer/require
> > sslverify=none/ca-if-present/ca/full
> >
> > (ca-if-present being the current "require" sslmode behavior).
> >
> > We could then deprecate sslmode=verify and verify-full and have them be
> > synonyms of sslmode=require and corresponding sslverify.
> 
> Hmm. I agree that the other suggestion was a bit weird, but I'm not
> sure I like the multiple-options approach either. That's going to
> require redesign of all software that deals with it at all today :S

Why?  They could continue to use the existing options; or switch to the
new options if they wanted different behavior, as is the case of the OP.

> Maybe we should just update the docs and be done with it :-)

That's another option, sure ... :-)

-- 
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

On Fri, Sep 23, 2011 at 16:44, Alvaro Herrera
<alvherre@commandprompt.com> wrote:
>
> Excerpts from Magnus Hagander's message of vie sep 23 11:31:37 -0300 2011:
>>
>> On Fri, Sep 23, 2011 at 15:55, Alvaro Herrera
>> <alvherre@commandprompt.com> wrote:
>
>> > This seems strange to me.  Why not have a second option to let the user
>> > indicate the desired SSL verification?
>> >
>> > sslmode=disable/allow/prefer/require
>> > sslverify=none/ca-if-present/ca/full
>> >
>> > (ca-if-present being the current "require" sslmode behavior).
>> >
>> > We could then deprecate sslmode=verify and verify-full and have them be
>> > synonyms of sslmode=require and corresponding sslverify.
>>
>> Hmm. I agree that the other suggestion was a bit weird, but I'm not
>> sure I like the multiple-options approach either. That's going to
>> require redesign of all software that deals with it at all today :S
>
> Why?  They could continue to use the existing options; or switch to the
> new options if they wanted different behavior, as is the case of the OP.

I guess. I was mostly thinking in the terms of anything that has
connection things that look anything like the one in pgadmin for
example - which will now suddenly need more than one dropdown box, for
what really should be a simple setting. But I guess that can be
considered an UI thing, and jus thave said application map a single
dropdown to multiple options in the connection string.


>> Maybe we should just update the docs and be done with it :-)
>
> That's another option, sure ... :-)

I've applied a docs fix for this now. We can keep discussing how to
make a more extensive fix in head :)


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