Re: [BUGS] libpq 8.4 beta1: $PGHOST complains about missing root.crt

2009-04-13 Thread Magnus Hagander
Bruce Momjian wrote:
> Magnus Hagander wrote:
>>> One random idea is to fold both of these settings into sslmode, with  
>>> the
>>> following progression:
>>>
>>> disable, allow, prefer, require, require-cert, require-cn
>>>
>>> And then set the default to "disable", because as you say "prefer"  
>>> is pretty
>>> silly.  And then users can explictly choose which level of SSL-ness  
>>> they want.
>> This is a different way to do bruces suggestion of a different  
>> default. That's possibly even clearer. So I can definitely go with  
>> this, but I think two different parameters makes it more clear and is  
>> better.
>>
>> And +1 for changing the default sslmode regardless of how we configure  
>> ssl verification.
> 
> I like Peter's idea too.  Having _three_ SSL settings is overkill, and I
> like the idea of doing it with one parameter.  As already pointed out,
> it makes no sense to do server certificate verification unless the
> sslmode is 'require', and having require-cert and require-cn are very
> clear.
> 
> I disagree with Magnus that having two parameters is better --- I think
> there is just too much risk of misconfiguration with two parameters.

Very well. One important part of having that would be to enable it by
default when you do "require", but there are other ways to accomplish
that of course.


> I would actually call the two parameters 'verify-cert' and 'verify-cn',
> and document that they also have "require" behavior.  Obviously you
> can't verify certificates unless you require SSL.

I would prefer having "verify", "verify-no-cn" and "no-verify" or
something like that. Making it the "default choice" to have verification
enabled, and very clear that you're turning something off if you're not.
And then just map require to verify. Or they could be "require-no-cn"
and "require-no-cert" perhaps?

("default choice" only for those using ssl of course - we'd still have
"disable" as the default *value* of the parameter)

//Magnus


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


Re: [BUGS] libpq 8.4 beta1: $PGHOST complains about missing root.crt

2009-04-13 Thread Magnus Hagander
Hiroshi Inoue wrote:
> Magnus Hagander wrote:
>> Hiroshi Inoue wrote:
>>> Magnus Hagander wrote:
 Bruce Momjian wrote:
> Martin Pitt wrote:
>> I do see the benefit of failing to connect to an SSL-enabled server
>> *if* I have a root.crt which doesn't match. But why fail if I don't
>> have one?
> I have digested this thread, and have done two things:  improved the
> documentation and posted a patch to make the error message clearer.
>
> In terms of your suggestion about root.crt, I think sslverify != none
> should error if it can't verify the server's certificate, whether the
> root.crt file is there or not.  If you are asking for sslverify, it
> should do that or error, not ignore the setting if there is no
> root.crt
> file.  The only other approach would be to add an sslverify value of
> 'try' that tries only if root.crt exists.
 Doesn't "try" make the whole check pretty pointless, and you can just
 set it to "none" then?
>>> Yes the snapshot psqlodbc driver already set sslverify to none and can't
>>> change it though it may be differnet from the expected behavior. It was
>>> not so easy to implement because sslverify parameter is illegal for <=
>>> 8.3 libpq and the psqlodbc driver doesn't rely on environment variables
>>> at all.
>>
>> Whatever the default is, if you can't change the value I'd say that
>> makes the ODBC driver pretty darn broken. It would be equally broken if
>> it was set to the default and it wasn't possible to change it.
> 
> The psqlodbc driver has its own development cycle and doesn't use new
> core features at once usually. The problem is the default behavior about
> sslverify is incompatible with the 8.3 libpq behavior and the driver had
> to do something with it before 8.4 release. What the snapshot driver
> actualy does is to simply hide the *sslverify* functionality.

I thought you were talking about a release version. If it's just a
snapshot, that is of course Ok. My apologies.

Though it might be easier even in that case to use an environment
variable to override it - that way the user could still override the
ODBC driver if you just checked if the variable was present before you
set it.

//Magnus

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


Re: [BUGS] libpq 8.4 beta1: $PGHOST complains about missing root.crt

2009-04-13 Thread Hiroshi Inoue

Magnus Hagander wrote:

Hiroshi Inoue wrote:

Magnus Hagander wrote:

Hiroshi Inoue wrote:

Magnus Hagander wrote:

Bruce Momjian wrote:

Martin Pitt wrote:

I do see the benefit of failing to connect to an SSL-enabled server
*if* I have a root.crt which doesn't match. But why fail if I don't
have one?

I have digested this thread, and have done two things:  improved the
documentation and posted a patch to make the error message clearer.

In terms of your suggestion about root.crt, I think sslverify != none
should error if it can't verify the server's certificate, whether the
root.crt file is there or not.  If you are asking for sslverify, it
should do that or error, not ignore the setting if there is no
root.crt
file.  The only other approach would be to add an sslverify value of
'try' that tries only if root.crt exists.

Doesn't "try" make the whole check pretty pointless, and you can just
set it to "none" then?

Yes the snapshot psqlodbc driver already set sslverify to none and can't
change it though it may be differnet from the expected behavior. It was
not so easy to implement because sslverify parameter is illegal for <=
8.3 libpq and the psqlodbc driver doesn't rely on environment variables
at all.

Whatever the default is, if you can't change the value I'd say that
makes the ODBC driver pretty darn broken. It would be equally broken if
it was set to the default and it wasn't possible to change it.

The psqlodbc driver has its own development cycle and doesn't use new
core features at once usually. The problem is the default behavior about
sslverify is incompatible with the 8.3 libpq behavior and the driver had
to do something with it before 8.4 release. What the snapshot driver
actualy does is to simply hide the *sslverify* functionality.


I thought you were talking about a release version. If it's just a
snapshot, that is of course Ok. My apologies.

Though it might be easier even in that case to use an environment
variable to override it - that way the user could still override the
ODBC driver if you just checked if the variable was present before you
set it.


Unfortnately enviornment variables are application wide and aren't
appropriate at all for the driver which should set the properties
on the fly per e.g. connection according to the DSN or the connection
string.

regards,
Hiroshi Inoue


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


Re: [BUGS] libpq 8.4 beta1: $PGHOST complains about missing root.crt

2009-04-13 Thread Bruce Momjian
Magnus Hagander wrote:
> > I would actually call the two parameters 'verify-cert' and 'verify-cn',
> > and document that they also have "require" behavior.  Obviously you
> > can't verify certificates unless you require SSL.
> 
> I would prefer having "verify", "verify-no-cn" and "no-verify" or
> something like that. Making it the "default choice" to have verification
> enabled, and very clear that you're turning something off if you're not.
> And then just map require to verify. Or they could be "require-no-cn"
> and "require-no-cert" perhaps?
> 
> ("default choice" only for those using ssl of course - we'd still have
> "disable" as the default *value* of the parameter)

I think the "no" options are odd because they have _negative_
designations.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [BUGS] libpq 8.4 beta1: $PGHOST complains about missing root.crt

2009-04-13 Thread Magnus Hagander

On 14 apr 2009, at 04.33, Bruce Momjian  wrote:


Magnus Hagander wrote:
I would actually call the two parameters 'verify-cert' and 'verify- 
cn',

and document that they also have "require" behavior.  Obviously you
can't verify certificates unless you require SSL.


I would prefer having "verify", "verify-no-cn" and "no-verify" or
something like that. Making it the "default choice" to have  
verification
enabled, and very clear that you're turning something off if you're  
not.

And then just map require to verify. Or they could be "require-no-cn"
and "require-no-cert" perhaps?

("default choice" only for those using ssl of course - we'd still  
have

"disable" as the default *value* of the parameter)


I think the "no" options are odd because they have _negative_
designations.


That's the intention. When you're turning off something, I think it  
makes sense to use "no"


/Magnus


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