Обсуждение: SSL support for javax.sql.DataSource

Wouldn't it be a good thing to have SSL support for DataSource
connections?

It shouldn't be too hard; attached is a patch (against 8.3dev-600)
that shows what I mean. It is probably unclean to hard code the
NonValidatingFactory...

What do you think?

Yours,
Laurenz Albe

On Mon, 18 Jun 2007, Albe Laurenz wrote:

> Wouldn't it be a good thing to have SSL support for DataSource
> connections?

Sure would, we'd like to support all the options available in a URL in the
DataSource.

> It shouldn't be too hard; attached is a patch (against 8.3dev-600)
> that shows what I mean. It is probably unclean to hard code the
> NonValidatingFactory...
>

Especially since it changes the default behavior of the driver to not
validate.  If you'd like to change the default, that discussion should
happen elsewhere and should affect all the connection methods.  Why not
just export the sslfactory option to the DataSource as well?

Kris Jurka

Kris Jurka wrote:
>> Wouldn't it be a good thing to have SSL support for DataSource
>> connections?
>>
>> It shouldn't be too hard; attached is a patch (against 8.3dev-600)
>> that shows what I mean. It is probably unclean to hard code the
>> NonValidatingFactory...
>
> Especially since it changes the default behavior of the driver to not
> validate.  If you'd like to change the default, that discussion should

> happen elsewhere and should affect all the connection
> methods.  Why not
> just export the sslfactory option to the DataSource as well?

Sure, that would be easy.

I wanted to know if this was welcome at all, so I started with a simple
patch to explain my idea.

Should I go ahead and write a patch against CVS HEAD, including
sslfactory?
I guess I should write a patch or the documentation too then.

As you say, that discussion should happen elsewhere, but I believe that
SSL without certificate validation would be a good default
because this is the way it is done everywhere else in PostgreSQL.

Yours,
Laurenz Albe

On Wed, 20 Jun 2007, Albe Laurenz wrote:

> Should I go ahead and write a patch against CVS HEAD, including
> sslfactory? I guess I should write a patch or the documentation too
> then.

Yes, please.

> As you say, that discussion should happen elsewhere, but I believe that
> SSL without certificate validation would be a good default
> because this is the way it is done everywhere else in PostgreSQL.
>

One of the ideas that Oliver had was to make the ssl parameter take a
String value so you could say things like ssl=try or ssl=require or
ssl=none.  See the brief code around
org.postgresql.core.v3.ConnectionFactoryImpl#openConnectionImpl.  We could
do that and add ssl=validate or ssl=novalidate.  That would make it easier
for people to change the validation setting without getting into the
details of sslfactory.  I didn't think ssl=try was a real useful setting
so resisted the idea at the time, but now that there are more interesting
options perhaps we should give the idea another look.

Kris Jurka

Kris Jurka wrote:
>> Should I go ahead and write a patch against CVS HEAD, including
>> sslfactory? I guess I should write a patch or the documentation too
>> then.
>
> Yes, please.

I have attached the patch, the documentation change is included.

I could not test if the documentation change is ok because I
cannot build the documentation
(I get a javax.xml.transform.TransformerException:
  java.lang.ArrayIndexOutOfBoundsException: -1
from xslt).

>> I believe that
>> SSL without certificate validation would be a good default
>> because this is the way it is done everywhere else in PostgreSQL.
>
> One of the ideas that Oliver had was to make the ssl parameter take a
> String value so you could say things like ssl=try or ssl=require or
> ssl=none.
> [...] We could do that and add ssl=validate or ssl=novalidate.
> That would make it easier for people to change the validation
> setting without getting into the details of sslfactory.
> I didn't think ssl=try was a real useful setting
> so resisted the idea at the time, but now that there are more
> interesting options perhaps we should give the idea another look.

The most intuitive setting for people used to libpq would be
ssl=disable/allow/prefer/require, with "prefer" or maybe "allow"
as default.

I think that certificate validation is orthogonal to that, it
can go with either of the above, so it had better be another property,
which means one might as well use "sslfactory" as it is now...

I will be on vacations from July 28 to August 10, so I cannot respond
during that time if there is a problem with my patch.

Yours,
Laurenz Albe

On Wed, 25 Jul 2007, Albe Laurenz wrote:

> I have attached the patch, the documentation change is included.
>
> I could not test if the documentation change is ok because I
> cannot build the documentation

I tried it on a couple different machines and got a couple different
errors.  It passes "ant validate-doc", so I've applied it for now and will
deal with the doc build failures later.

Kris Jurka

Kris Jurka wrote:
> [...] I've applied it for now and will
> deal with the doc build failures later.

Thanks.

One more thing that occurred to me after I had sent off the patch:

Maybe the getter should be "isSsl()" and not "getSsl()" since the value
is boolean.

Yours,
Laurenz Albe