Обсуждение: JDBC SSL hostname verification

Hello,

I have noticed that I was able to connect using SSL (with a trusted
certificate) to a server using a name that doesn't match that in the
certificate.

For example, if both "sql.example.com" and "other.example.net" point to
the same IP address, but the certificate is not issued to
"other.example.net" (only "sql.example.com"), the following works when
it shouldn't:

Properties props = new Properties();
props.setProperty("user", "username");
props.setProperty("password", "xxxxxxxx");
props.setProperty("ssl", "true");
Connection jdbcConnection =
DriverManager.getConnection("jdbc:postgresql://other.example.net/db", props)

Hostname verification (what 'verify-full' does with psql) is necessary
for ensuring the security of the connection. Verifying that the
certificate is trusted isn't sufficient.

Only this should work:

Connection jdbcConnection =
DriverManager.getConnection("jdbc:postgresql://sql.example.com/db", props)


Did I miss a property to set?


I hope I'm not duplicating an existing feature, but I couldn't find
anything that performed this verification in the existing code base, so
I've implemented a patch to support it. It seems to work well against
versions 8.4 and 9.0 at least.
If this of interest to anyone, I'd be happy to contribute it to the
PostgreSQL community. (Please let me know what the procedure to do so is.)


Best wishes,

Bruno.

On 6/08/2011 11:00 AM, Bruno Harbulot wrote:
> Hello,
>
> I have noticed that I was able to connect using SSL (with a trusted
> certificate) to a server using a name that doesn't match that in the
> certificate.
>
> For example, if both "sql.example.com" and "other.example.net" point
> to the same IP address, but the certificate is not issued to
> "other.example.net" (only "sql.example.com"), the following works when
> it shouldn't:
>
> Properties props = new Properties();
> props.setProperty("user", "username");
> props.setProperty("password", "xxxxxxxx");
> props.setProperty("ssl", "true");
> Connection jdbcConnection =
> DriverManager.getConnection("jdbc:postgresql://other.example.net/db",
> props)
>
> Hostname verification (what 'verify-full' does with psql) is necessary
> for ensuring the security of the connection. Verifying that the
> certificate is trusted isn't sufficient.

JSSE doesn't verify the hostname automatically. Quoting the JSSE
reference guide for Java 6:

"When using raw SSLSockets/SSLEngines you should always check the peer's
credentials before sending any data. The SSLSocket and SSLEngine classes
do not automatically verify that the hostname in a URL matches the
hostname in the peer's credentials. An application could be exploited
with URL spoofing if the hostname is not verified."

I was under the impression that PgJDBC verified the hostname its self
unless verification was disabled, but it seems not. Hmm. I guess you can
use a custom SSLSocketFactory to do the verification, but it really
should be something done by the stock JDBC driver. Patch?

--
Craig Ringer

On 06/08/2011 13:02, Craig Ringer wrote:
> On 6/08/2011 11:00 AM, Bruno Harbulot wrote:
>
> JSSE doesn't verify the hostname automatically. Quoting the JSSE
> reference guide for Java 6:
>
> "When using raw SSLSockets/SSLEngines you should always check the peer's
> credentials before sending any data. The SSLSocket and SSLEngine classes
> do not automatically verify that the hostname in a URL matches the
> hostname in the peer's credentials. An application could be exploited
> with URL spoofing if the hostname is not verified."
>
> I was under the impression that PgJDBC verified the hostname its self
> unless verification was disabled, but it seems not. Hmm. I guess you can
> use a custom SSLSocketFactory to do the verification, but it really
> should be something done by the stock JDBC driver. Patch?

Yes, you're absolutely right, there is no verification by default. In
fact, until the relatively recent RFC 6125 [1], the way host names
should be verified was specified differently in each protocol (e.g. RFC
2818 for HTTPS, see RFC 6125 appendix B).

This patch (attached), modifies org.postgresql.ssl.MakeSSL and changes this:
> Socket newConnection = factory.createSocket(stream.getSocket(), stream.getHost(), stream.getPort(), true);
> stream.changeSocket(newConnection);

Into this:
> SSLSocket newConnection = (SSLSocket)factory.createSocket(stream.getSocket(), stream.getHost(), stream.getPort(),
true);
> // Gets the session and do the verification...
> stream.changeSocket(newConnection);


In principle, this verification could indeed be done within the custom
SSLSocketFactory.createSocket(...) method. However, this might need to
be an SSLSocketFactory intended to be used only with this JDBC driver as
it would break the general API of SSLSocketFactory.

This is because the factory would return an SSLSocket for which the TLS
handshake has already taken place (since it needs to get the peer
certificate in order to verify them). From the SSLSocketFactory API
point of view, the factory creates SSLSockets, but doesn't initiate the
handshake. The handshake is only initiated (perhaps implicitly)
afterwards, in one of the 3 conditions documented in SSLSocket [2].
A number of settings can still be made before the handshake, and it's
normally not up to the SSLSocketFactory to make them.


I've tried to follow the rules in RFC 6125 to perform the verification
(although I haven't looked at what it says about certificates for IP
addresses, only host names).

Firstly, it looks for Subject Alternative Names and then for CN entries
in the Subject Distinguished Names (Subject DN). (It is preferrable to
use subjectAltNames rather than relying on CNs in the Subject DN, see
RFC 6125 section 1.5.)
If it finds a subjectAltName entry of type DNS that matches the
requested host name, it stops and accepts the certificate.
Otherwise, it looks within the Subject DN for and RDN with only one AVA
such as CN=the.host.name (section 2.3.1 of RFC 6125).
(I must admit I think the CN verification could be done more elegantly;
in addition, I haven't looked in details into the subtleties of
character encoding.)

I understand this may not be obvious if one is not used to certificates.
The Subject DN in the certificate is a sequence of relative
distinguished names (RDNs). Each RDN is an unordered set of Attribute
Value Assertions (AVAs), for example "CN=the.host.name".
Using the RFC 2253 string representation of a DN, RDNs are separated
with ',' and AVAs are separated with '+'. (OpenSSL tends to use a
different string representation, separated with "/" and sometimes in a
different order.) They can also be escaped with '\' (which means it's
not necessarily easy to implement with a StringTokenizer for example).
There are examples in RFC 2253, section 5 [3].



I guess one of the things that is missing is a property to turn this off.
 From a security perspective, my opinion tends to be to have this sort
of feature turned on by default, but I guess it may break some existing
configurations so some users may want to turn it off. (I would argue
that the motivation for using SSL/TLS is to secure the connection, so
making users aware that it's not secure because SSL/TLS wasn't fully
configured isn't necessarily a bad thing, but I also understand that
raising awareness by potentially breaking existing working
configurations can be a harsh way to do so.)


Anyway, the patch is attached, it's probably not perfect, but I'd be
happy to discuss it further if this feature is of interest to the
community. I hope this helps.


Best wishes,

Bruno.


[1] http://tools.ietf.org/html/rfc6125
[2]
http://download.oracle.com/javase/6/docs/api/javax/net/ssl/SSLSocket.html
[3] http://tools.ietf.org/html/rfc2253#section-5

   Dear Jdbc developers!

  Recently Bruno Harbulot sent in a patch, that handles
hostname verification for ssl connection. Let me send
a tentative patch, that may treat the ssl connections
close to that implemeted in libpg.

A few more connection properties are introduced these are:

-sslmode: similar to the libpg parameter, the allowed
values are disable, allow, prefer, require, verify-ca, verify-full
The parameter ssl should be made deprecated, as sslmode
can cover all the possibilities. In my patch ssl=true is still
necessary, to start ssl, then only require, verify-ca and verify-full
behave correctly. (In case of verify-full, the Common name
must match the hostname, or a leading * can match anything
except a dot.)

-sslcert,sslkey,sslrootcert: these are the locations of the client
certificate, client key, and server certificate. (CRLs ar not implemeted
yet.)
Surprisingly, java can read openssl certificates without any
modification, but the key must be converted to pkcs8 format:

openssl pkcs8 -topk8 -in client.key -out client.pk8 -outform DER -v1
PBE-MD5-DES

the ciphers, recognized by java are PBE-MD5-DES, PBE-SHA1-3DES,
PBE-SHA1-RC2-40,
or with the -nocrypt switch, it can be unencrypted. If any of these
parameters is missing, the default locations are looked up (in
$HOME/.postgresql).

-sslpassword: the password for the ssl key (different from the database
password)
-sslpasswordcallback: a class, implementing
javax.security.auth.callback.CallbackHandler
that can handle PassworCallback for the ssl password. If set,
sslpassword is ignored.
The supplied class must have either a constructor with a Properties
argument where
the connection info properties are given, or a zero argument constructor

If neither sslpassword nor sslpasswordcallback is set, and the key is
protected,
the user is prompted at the console for a password

-sslhostnameverifier: a class, implementing javax.net.ssl.HostnameVerifier
that can verify the server. The supplied class must have either a
constructor
with a Properties argument where the connection info properties are given,
or a zero argument constructor. If set the server hostname is verified
irrespective
of the value of sslmode. (This behaivor is to be discussed.)

-sslfactory: this parameter is modified slightly. The supplied class can
also have a constructor
with a Properties argument. In this case sslfactoryarg is ignored. If
set, then the supplied
factory class is wholly responsible for the SSL connection, but the
hostname verification is still
handled by the sslhostnameverifier class if supplied.

Attached are the patch to MakeSSL.java, and two new classes,
org.postgresql.ssl.LazyKeyManager and org.postgresql.ssl.LibPQFactory.

Sincerely Yours
            Andras

   Dear Jdbc developers!

  Here is a more comprehensive patch for SSL in the jdbc driver.
The following connection properties are introduced:

-sslmode: similar to the libpg parameter, the allowed
  values are disable, allow, prefer, require, verify-ca, verify-full
  The parameter ssl should be made deprecated, as sslmode
  can cover all the possibilities. (However, if sslmode is not
  set, the driver's behavior is backward compatible.)
  disable, require, verify-ca and verify-full behave correctly.
  At this point allow and prefer behave the same, and it is
  not possible, to fall back to nonssl, once ssl negotiation has
  begun.

-sslcert,sslkey,sslrootcert: these are the locations of the client
  certificate, client key, and server certificate. (CRLs ar not implemeted
  yet.)
  Surprisingly, java can read openssl certificates without any
  modification, but the key must be converted to pkcs8 format with
  the following comand:

openssl pkcs8 -topk8 -in client.key -out client.pk8 -outform DER -v1
PBE-MD5-DES

  the ciphers, recognized by java are PBE-MD5-DES, PBE-SHA1-3DES,
  PBE-SHA1-RC2-40,
  or with the -nocrypt switch, it can be unencrypted. If any of these
  parameters is missing, the default locations are looked up (in
  $HOME/.postgresql). The default filename for the key is postgresql.pk8
  instead of postgresql.key to allow simultaneous use of the jdbc driver
  and other libpq compatible applications. In some cases it is desirable
  to supress loading the default client certificate (and any other one),
  in this case specify sslcert with an empty argument.

-sslpassword: the password for the ssl key (different from the database
password)

-sslpasswordcallback: a class, implementing
  javax.security.auth.callback.CallbackHandler
  that can handle PassworCallback for the ssl password. If set,
  sslpassword is ignored.
  The supplied class must have either a constructor with a Properties
  argument where
  the connection info properties are given, or a zero argument constructor
  If neither sslpassword nor sslpasswordcallback is set, and the key is
  protected,
  the user is prompted at the console for a password

-sslhostnameverifier: a class, implementing javax.net.ssl.HostnameVerifier
  that can verify the server. The supplied class must have either a
  constructor
  with a Properties argument where the connection info properties are given,
  or a zero argument constructor. If set the server hostname is verified
  irrespective
  of the value of sslmode. (This behaivor is to be discussed.)

-sslfactory: this parameter is modified slightly. The supplied class can
  also have a constructor
  with a Properties argument. In this case sslfactoryarg is ignored. If
  set, then the supplied
  factory class is wholly responsible for the SSL connection, but the
  hostname verification is still
  handled by the sslhostnameverifier class if supplied.
  Warning! The sslfactory must not initiate a handshake in it's
  createSocket method, bacause a second startHandsake invocation
  in MakeSSL.convert() will break the connection.

  A few junit tests are also included. For them to run several databases
with different pg_hba.conf parameters must be set up. See the
certdir/README file for details. Right now some of the tests fail.
It is intentional, they correspond to the not yet libpq compatible
behaviour of allow and prefer.
  Patch and two binary files, certdir/goodclient.pk8 and
certdir/badclient.pk8 are attached. Any fedback is welcome!
    Sincerely Yours
            Andras

   Hi!

  Can You make any use of my SSL patch sent in on the 23th of August?
            Andras

Hi Bodor,

So do you have any test cases for this ?

Dave Cramer

dave.cramer(at)credativ(dot)ca
http://www.credativ.ca




2011/9/13 Bodor Andras <bodri.mh3@gmail.com>:
>
>  Hi!
>
>  Can You make any use of my SSL patch sent in on the 23th of August?
>           Andras
>
> --
> Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-jdbc
>

   Yes, it is also included in the patch
(package org.postgresql.test.ssl). It
tries to connect to a series of databases
with different ssl properties. The connection
strings are given in the ssltest.properties
file in the root of the distribution. Just
comment out the connstrings, that you don't
want to run. Also read the certdir/README
file. (build.xml is modified to run this test.)
            Andras


Dave Cramer wrote:
> Hi Bodor,
>
> So do you have any test cases for this ?
>
> Dave Cramer
>
> dave.cramer(at)credativ(dot)ca
> http://www.credativ.ca
>
>
>
>
> 2011/9/13 Bodor Andras<bodri.mh3@gmail.com>:
>>
>>   Hi!
>>
>>   Can You make any use of my SSL patch sent in on the 23th of August?
>>            Andras
>>
>> --
>> Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgsql-jdbc
>>
>

  Why don't you set prepareThreshold to 0 for your test cases?
In this case parameter passing starts immediately for the
prepared statement, and ForceBinaryTransfers can be dropped.
            Andras

 > Thank you for the thorough analysis. Now we need to decide what to do
 > with the ForceBinaryTransfers testing/debugging aid.
 >
 > It was originally added to ensure that while running the unit tests
 > as much as possible of the code paths would exercise the binary > > >
 > transfer
 > code. Normally binary transfers would only be activated after few
 > round-trips to the backend with the same prepared statement - which
 > basically never happens in unit tests.
 >
 > We have at least three options:
 > 1) drop the ForceBinaryTransfers
 >    - it has helpped debug the binary transfer to a working state
 >    - it is no longer that useful
 > 2) leave it as is
 >    - just add the hack to the one failing test to make it not fail
 >    - it is still useful to run the test suite to verify functionality
 >      stays correct with binary transfers
 > 3) fix it
 >    - less worries in the future and if some end user finds the
 >      undocumented feature they won't get bitten by it
 >
 > -Mikko
 >

On 09/19/2011 01:11 PM, Bodor Andras wrote:
> Why don't you set prepareThreshold to 0 for your test cases?
> In this case parameter passing starts immediately for the
> prepared statement, and ForceBinaryTransfers can be dropped.

AbstractJdbc2Statement:
     public boolean isUseServerPrepare() {
         return (preparedQuery != null && m_prepareThreshold != 0 && m_useCount + 1 >= m_prepareThreshold);
     }

If I read the above correctly then prepareThreshold of 0 disables prepared statements.

-Mikko

Mikko Tiihonen <mikko.tiihonen@nitorcreations.com> writes:
> On 09/19/2011 01:11 PM, Bodor Andras wrote:
>> Why don't you set prepareThreshold to 0 for your test cases?
>> In this case parameter passing starts immediately for the
>> prepared statement, and ForceBinaryTransfers can be dropped.

> AbstractJdbc2Statement:
>      public boolean isUseServerPrepare() {
>          return (preparedQuery != null && m_prepareThreshold != 0 && m_useCount + 1 >= m_prepareThreshold);
>      }

> If I read the above correctly then prepareThreshold of 0 disables prepared statements.

But "1" would do what you want, no?

FWIW, I'm hoping that this entire business of delaying the server-side
prepare will be obsolete as of 9.2.  There is already code committed in
HEAD that allows "prepared" statements to be re-planned for each new set
of parameter values, with a somewhat-informed choice of whether and when
to switch over to the traditional generic-plan approach.  It needs
tweaking for performance still, no doubt, but there is not going to be a
reason for the driver to do this anymore.

Obviously that's not much use for solving your immediate problem, but it
might lead you to conclude that putting a large amount of effort into
improving this mechanism would be a dead end.

            regards, tom lane

  You are right. But then it is a bug in either the code,
or the documentation. It says:

prepareThreshold = int
     Determine the number of PreparedStatement executions required
before switching over to use server side prepared statements.

I think, if zero executions are required, then the driver
should use prepared statement immediately. Both permanently
disabling and immediately enabling is a legitimate need of
the users, it should be clearly documented, how to do them.
  Anyway, setting prepareThreshold = -1 will do what I meant.
But it is more logical to me, that 0 means zero and -1 means
infinity (even better: Integer.MAX_VALUE).
  For the ForceBinaryTransfers I vote for dropping it, as
it adds unnecessary complications (and new bugs) to the
code.
            Andras

>> Why don't you set prepareThreshold to 0 for your test cases?
>> In this case parameter passing starts immediately for the
>> prepared statement, and ForceBinaryTransfers can be dropped.
>> Andras
>>
 >AbstractJdbc2Statement:
 >    public boolean isUseServerPrepare() {
 >        return (preparedQuery != null && m_prepareThreshold != 0 &&
 >m_useCount + 1 >= m_prepareThreshold);
 >    }
 >
 >If I read the above correctly then prepareThreshold of 0 disables
 >prepared statements.
 >
 >-Mikko

Self correction: I did not read the documentation
carefully enough. prepareThreshold = 1 should be used.
Negative values are changed to 0 by the driver. But
it should be mentioned somewhere, that 0 switches it
off completely.

Bodor Andras wrote:
> You are right. But then it is a bug in either the code,
> or the documentation. It says:
>
> prepareThreshold = int
> Determine the number of PreparedStatement executions required before
> switching over to use server side prepared statements.
>
> I think, if zero executions are required, then the driver
> should use prepared statement immediately. Both permanently
> disabling and immediately enabling is a legitimate need of
> the users, it should be clearly documented, how to do them.
> Anyway, setting prepareThreshold = -1 will do what I meant.
> But it is more logical to me, that 0 means zero and -1 means
> infinity (even better: Integer.MAX_VALUE).
> For the ForceBinaryTransfers I vote for dropping it, as
> it adds unnecessary complications (and new bugs) to the
> code.
> Andras
>
>>> Why don't you set prepareThreshold to 0 for your test cases?
>>> In this case parameter passing starts immediately for the
>>> prepared statement, and ForceBinaryTransfers can be dropped.
>>> Andras
>>>
>  >AbstractJdbc2Statement:
>  > public boolean isUseServerPrepare() {
>  > return (preparedQuery != null && m_prepareThreshold != 0 &&
>  >m_useCount + 1 >= m_prepareThreshold);
>  > }
>  >
>  >If I read the above correctly then prepareThreshold of 0 disables
>  >prepared statements.
>  >
>  >-Mikko

Tom Lane <tgl@sss.pgh.pa.us> Monday 19 of September 2011 16:47:12
> Mikko Tiihonen <mikko.tiihonen@nitorcreations.com> writes:
> > On 09/19/2011 01:11 PM, Bodor Andras wrote:
> >> Why don't you set prepareThreshold to 0 for your test cases?
> >> In this case parameter passing starts immediately for the
> >> prepared statement, and ForceBinaryTransfers can be dropped.
> >
> > AbstractJdbc2Statement:
> >      public boolean isUseServerPrepare() {
> >
> >          return (preparedQuery != null && m_prepareThreshold != 0 &&
> >          m_useCount + 1 >= m_prepareThreshold);
> >
> >      }
> >
> > If I read the above correctly then prepareThreshold of 0 disables
> > prepared statements.
>
> But "1" would do what you want, no?
>
> FWIW, I'm hoping that this entire business of delaying the server-side
> prepare will be obsolete as of 9.2.  There is already code committed in
> HEAD that allows "prepared" statements to be re-planned for each new set
> of parameter values, with a somewhat-informed choice of whether and when
> to switch over to the traditional generic-plan approach.  It needs
> tweaking for performance still, no doubt, but there is not going to be a
> reason for the driver to do this anymore.
>
> Obviously that's not much use for solving your immediate problem, but it
> might lead you to conclude that putting a large amount of effort into
> improving this mechanism would be a dead end.
>
>             regards, tom lane

The whole logic about this was, because PreparedStatement are not used in Java
to be prepared in means of database server. It's normal technique to write
even one shoot statements
ps = conn.prepareStatemn(SELECT * FROM foo where id = ?)
This simple techinique makes code more simple and saftly; it prevents SQL
injection, as well no one need to care about proper data escaping. So If
someone uses statements once, there may be no need to parsing plan for this
(prove why server should do this for you by invoking it 5 times).
As side effect of this popular concept many J2EE servers provides new pool,
with ecah connection (connection are pooled), you get statement pool, so web-
based and J2EE environments (which are mainly single-request based) may with
no effort form developer side use benefits of preparation.

The PG "one shot" statements will not make this obsolete
1) Driver will be backward compatible (it's still supports 7.x editions)
2) Above problem will be still present.
3) Java perpared statements uses ? inseted of positional parameters, so driver
need to parse query in any way.

Regards
Radek
http://www.softperience.eu/

> The PG "one shot" statements will not make this obsolete
> 1) Driver will be backward compatible (it's still supports 7.x editions)
> 2) Above problem will be still present.
> 3) Java perpared statements uses ? inseted of positional parameters, so driver
> need to parse query in any way.

I think what Tom is saying is that you'll be able to set
prepareThreshold to 1 in the driver (always use named server-side
prepared statements) and the server will automagically do the right
thing. Right now, the big problem is that only unnamed prepared
statements include the "feature" of being able to consider parameters
in the query plan (since unnamed statements are not reused, there's no
sense in planning them before you have parameters). Other aspects of
using prepared statements through the jdbc driver wouldn't change, but
we should be able to forget about prepareThreshold (if I understand
this correctly), which would be very nice.

---
Maciek Sakrejda | System Architect | Truviso

1065 E. Hillsdale Blvd., Suite 215
Foster City, CA 94404
(650) 242-3500 Main
www.truviso.com

On Mon, 19 Sep 2011, Bodor Andras wrote:

>  For the ForceBinaryTransfers I vote for dropping it, as
> it adds unnecessary complications (and new bugs) to the
> code.

ForceBinaryTransfers doesn't just make the driver use a named protocol
level statement, so it isn't an equivalent substitution to recommend
adjusting prepareTreshold.  The key to binary transfer is knowing what
types will be returned so you know if you can handle them as binary and
therefore whether to request them as binary or not.  So the
ForceBinaryTransfers hack is for testing and does a half execution to get
the returned data types and then does a real execution with the true data
types.

So we need this to make use of our existing test suite because it doesn't
normally do multiple execution which would let us naturally discover the
type information.  Whether it should be exposed to users or not is
debatable, but we need it internally.

Unless of course you are willing to add an extra network roundtrip to
every query execution to pickup the type information before execution.

Kris Jurka

Maciek Sakrejda <msakrejda@truviso.com> writes:
> I think what Tom is saying is that you'll be able to set
> prepareThreshold to 1 in the driver (always use named server-side
> prepared statements) and the server will automagically do the right
> thing. Right now, the big problem is that only unnamed prepared
> statements include the "feature" of being able to consider parameters
> in the query plan (since unnamed statements are not reused, there's no
> sense in planning them before you have parameters).

Right.  We've rejiggered the backend so that planning is delayed until
parameter values are available in all cases.  (If the code finds that
the plans it's getting with the parameters aren't materially better than
a generic plan would be, it'll eventually go back to using generic
plans.  But hopefully you will never need to club it over the head to
avoid doing that.)

Anyway, this isn't going to hit the street for another year, so there's
no reason to worry about it too much right now.  I'm just saying you
shouldn't invest a lot of work in related areas without being aware that
that change is coming.

            regards, tom lane

On 09/20/2011 08:39 AM, Tom Lane wrote:
> Maciek Sakrejda<msakrejda@truviso.com>  writes:
>> I think what Tom is saying is that you'll be able to set
>> prepareThreshold to 1 in the driver (always use named server-side
>> prepared statements) and the server will automagically do the right
>> thing. Right now, the big problem is that only unnamed prepared
>> statements include the "feature" of being able to consider parameters
>> in the query plan (since unnamed statements are not reused, there's no
>> sense in planning them before you have parameters).
> Right.  We've rejiggered the backend so that planning is delayed until
> parameter values are available in all cases.

Oooh, exciting. That'll cut down on a whole class of mailing list query
and get rid of a classic gotcha.

Time for me to build git and start playing.

--
Craig Ringer

You may try this:

INSERT INTO poi(geom,latitude,longitude,description,comment)
VALUES (ST_GeomFromText('POINT(' || ? || ' ' || ? || ')', 4326), ?, ?,
?, ?);

then you have 6 parameters to bind.
Andras

> Hi all,
>
> in my app I need to execute insert query one of the arguments of which
> is a PostGIS
> function call. Here is query
>
> INSERT INTO poi(geom,latitude,longitude,description,comment)
> VALUES (ST_GeomFromText('POINT(34.567 45.5621)', 4326), 34.567,
> 45.5621, 'some description', 'user comment');
>
> The problem is that I can't bind all necessary values, I can't write
> query as
>
> INSERT INTO poi(geom,latitude,longitude,description,comment)
> VALUES (ST_GeomFromText('POINT(? ?)', 4326), ?, ?, ?, ?);
>
> and bind all values because in this case I get exception
>
> org.postgresql.util.PSQLException: The column index is out of range:
> 6, number of columns: 5.
>
> How I can bind values for function arguments? I see two possible
> solutions:
> 1. create query string in runtime, concatenate all function arguments
> and then bind all other values
> 2. write stored fuction in database which will accept all arguments
> and the internally call ST_GeomFromText
>
> In first case I can't prepare statement for multiple inserts and need
> to create query string on every
> insert operation. The secon approach looks a more flexible.
>
> Any hints on this problem? Maybe I miss something in documentation and
> it is possible to bind values
> to the functions arguments inside insert query?
>
> Thanks,
> Alex
>

Andras,

I'm looking at your patch attached to this link
http://archives.postgresql.org/pgsql-jdbc/2011-08/msg00067.php right
now. Thanks by the way!

The only thing I'd like to pose to the list is the necessity for
sslinfo to be installed in any database. I can envision some
production environments which this may not be possible ?

Dave Cramer

dave.cramer(at)credativ(dot)ca
http://www.credativ.ca




On Thu, Sep 15, 2011 at 11:41 AM, Bodor Andras <bodri.mh3@gmail.com> wrote:
>
>  Yes, it is also included in the patch
> (package org.postgresql.test.ssl). It
> tries to connect to a series of databases
> with different ssl properties. The connection
> strings are given in the ssltest.properties
> file in the root of the distribution. Just
> comment out the connstrings, that you don't
> want to run. Also read the certdir/README
> file. (build.xml is modified to run this test.)
>           Andras
>
>
> Dave Cramer wrote:
>>
>> Hi Bodor,
>>
>> So do you have any test cases for this ?
>>
>> Dave Cramer
>>
>> dave.cramer(at)credativ(dot)ca
>> http://www.credativ.ca
>>
>>
>>
>>
>> 2011/9/13 Bodor Andras<bodri.mh3@gmail.com>:
>>>
>>>  Hi!
>>>
>>>  Can You make any use of my SSL patch sent in on the 23th of August?
>>>           Andras
>>>
>>> --
>>> Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org)
>>> To make changes to your subscription:
>>> http://www.postgresql.org/mailpref/pgsql-jdbc
>>>
>>
>
>
> --
> Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-jdbc
>

Dear Dave,

The installation of sslinfo is only necessary for the unit tests, it is
not used at all in the driver itself. Obviously I wanted to test weather
we were actually using ssl, but it is not essential. It can be removed,
or an additional option can be introduced to ssltest.properties.
The relevant lines are in
org.postgresql.test.ssl.SslTest.driver(String connstr, Object[]
expected)

There are a few things still to be done with this patch.
1. the jdbc datasource interface was not modified at all,
so it is unaware of the new options,
2. it should be decided, what is the expected behaviour of sslmode=allow
or prefer (they might be omitted completely),
3. I have not tested certificate chains yet,
4. when a client certificate is available, the v8 and v9 servers
behave differently (BUG #5468 is fixed in v9) so different unit test are
needed to check this,
5. there is a list of options somewhere in the code, this should
be updated as well,
6. documentation.

           Andras

On Thu, Nov 10, 2011 at 2:56 PM, Dave Cramer <pg@fastcrypt.com> wrote:
> Andras,
>
> I'm looking at your patch attached to this link
> http://archives.postgresql.org/pgsql-jdbc/2011-08/msg00067.php right
> now. Thanks by the way!
>
> The only thing I'd like to pose to the list is the necessity for
> sslinfo to be installed in any database. I can envision some
> production environments which this may not be possible ?
>
> Dave Cramer
>
> dave.cramer(at)credativ(dot)ca
> http://www.credativ.ca
>
>
>
>
> On Thu, Sep 15, 2011 at 11:41 AM, Bodor Andras <bodri.mh3@gmail.com> wrote:
>>
>>  Yes, it is also included in the patch
>> (package org.postgresql.test.ssl). It
>> tries to connect to a series of databases
>> with different ssl properties. The connection
>> strings are given in the ssltest.properties
>> file in the root of the distribution. Just
>> comment out the connstrings, that you don't
>> want to run. Also read the certdir/README
>> file. (build.xml is modified to run this test.)
>>           Andras
>>
>>
>> Dave Cramer wrote:
>>>
>>> Hi Bodor,
>>>
>>> So do you have any test cases for this ?
>>>
>>> Dave Cramer
>>>
>>> dave.cramer(at)credativ(dot)ca
>>> http://www.credativ.ca
>>>
>>>
>>>
>>>
>>> 2011/9/13 Bodor Andras<bodri.mh3@gmail.com>:
>>>>
>>>>  Hi!
>>>>
>>>>  Can You make any use of my SSL patch sent in on the 23th of August?
>>>>           Andras
>>>>
>>>> --
>>>> Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org)
>>>> To make changes to your subscription:
>>>> http://www.postgresql.org/mailpref/pgsql-jdbc
>>>>
>>>
>>
>>
>> --
>> Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgsql-jdbc
>>
>

Isn't sslinfo used only for testing the functionality?

BTW I haven't yet take the time to figure out whether smartcards through
pkcs11 interface can be used either with this, or the custom
certauthfactory solution.
It works with psql using sslkey=pkcs11:<certificate id>, and it would be
nice to have the same functionality with a similar config in jdbc as well.


On 2011-11-10 14:56, Dave Cramer wrote:
> Andras,
>
> I'm looking at your patch attached to this link
> http://archives.postgresql.org/pgsql-jdbc/2011-08/msg00067.php right
> now. Thanks by the way!
>
> The only thing I'd like to pose to the list is the necessity for
> sslinfo to be installed in any database. I can envision some
> production environments which this may not be possible ?
>
> Dave Cramer
>
> dave.cramer(at)credativ(dot)ca
> http://www.credativ.ca
>
>
>
>
> On Thu, Sep 15, 2011 at 11:41 AM, Bodor Andras<bodri.mh3@gmail.com>  wrote:
>>   Yes, it is also included in the patch
>> (package org.postgresql.test.ssl). It
>> tries to connect to a series of databases
>> with different ssl properties. The connection
>> strings are given in the ssltest.properties
>> file in the root of the distribution. Just
>> comment out the connstrings, that you don't
>> want to run. Also read the certdir/README
>> file. (build.xml is modified to run this test.)
>>            Andras
>>
>>
>> Dave Cramer wrote:
>>> Hi Bodor,
>>>
>>> So do you have any test cases for this ?
>>>
>>> Dave Cramer
>>>
>>> dave.cramer(at)credativ(dot)ca
>>> http://www.credativ.ca
>>>
>>>
>>>
>>>
>>> 2011/9/13 Bodor Andras<bodri.mh3@gmail.com>:
>>>>   Hi!
>>>>
>>>>   Can You make any use of my SSL patch sent in on the 23th of August?
>>>>            Andras
>>>>
>>>> --
>>>> Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org)
>>>> To make changes to your subscription:
>>>> http://www.postgresql.org/mailpref/pgsql-jdbc
>>>>
>>
>> --
>> Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgsql-jdbc
>>

Hi Bodor,

Understood.

So now all the tests are failing some due to unknown ca, others to
certificate expired ?

Dave Cramer

dave.cramer(at)credativ(dot)ca
http://www.credativ.ca




On Thu, Nov 10, 2011 at 9:30 AM, Bodor András <bodri.mh3@gmail.com> wrote:
> Dear Dave,
>
> The installation of sslinfo is only necessary for the unit tests, it is
> not used at all in the driver itself. Obviously I wanted to test weather
> we were actually using ssl, but it is not essential. It can be removed,
> or an additional option can be introduced to ssltest.properties.
> The relevant lines are in
> org.postgresql.test.ssl.SslTest.driver(String connstr, Object[]
> expected)
>
> There are a few things still to be done with this patch.
> 1. the jdbc datasource interface was not modified at all,
> so it is unaware of the new options,
> 2. it should be decided, what is the expected behaviour of sslmode=allow
> or prefer (they might be omitted completely),
> 3. I have not tested certificate chains yet,
> 4. when a client certificate is available, the v8 and v9 servers
> behave differently (BUG #5468 is fixed in v9) so different unit test are
> needed to check this,
> 5. there is a list of options somewhere in the code, this should
> be updated as well,
> 6. documentation.
>
>           Andras
>
> On Thu, Nov 10, 2011 at 2:56 PM, Dave Cramer <pg@fastcrypt.com> wrote:
>> Andras,
>>
>> I'm looking at your patch attached to this link
>> http://archives.postgresql.org/pgsql-jdbc/2011-08/msg00067.php right
>> now. Thanks by the way!
>>
>> The only thing I'd like to pose to the list is the necessity for
>> sslinfo to be installed in any database. I can envision some
>> production environments which this may not be possible ?
>>
>> Dave Cramer
>>
>> dave.cramer(at)credativ(dot)ca
>> http://www.credativ.ca
>>
>>
>>
>>
>> On Thu, Sep 15, 2011 at 11:41 AM, Bodor Andras <bodri.mh3@gmail.com> wrote:
>>>
>>>  Yes, it is also included in the patch
>>> (package org.postgresql.test.ssl). It
>>> tries to connect to a series of databases
>>> with different ssl properties. The connection
>>> strings are given in the ssltest.properties
>>> file in the root of the distribution. Just
>>> comment out the connstrings, that you don't
>>> want to run. Also read the certdir/README
>>> file. (build.xml is modified to run this test.)
>>>           Andras
>>>
>>>
>>> Dave Cramer wrote:
>>>>
>>>> Hi Bodor,
>>>>
>>>> So do you have any test cases for this ?
>>>>
>>>> Dave Cramer
>>>>
>>>> dave.cramer(at)credativ(dot)ca
>>>> http://www.credativ.ca
>>>>
>>>>
>>>>
>>>>
>>>> 2011/9/13 Bodor Andras<bodri.mh3@gmail.com>:
>>>>>
>>>>>  Hi!
>>>>>
>>>>>  Can You make any use of my SSL patch sent in on the 23th of August?
>>>>>           Andras
>>>>>
>>>>> --
>>>>> Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org)
>>>>> To make changes to your subscription:
>>>>> http://www.postgresql.org/mailpref/pgsql-jdbc
>>>>>
>>>>
>>>
>>>
>>> --
>>> Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org)
>>> To make changes to your subscription:
>>> http://www.postgresql.org/mailpref/pgsql-jdbc
>>>
>>
>

Andras,

I noticed that the server.crt in the patch is only good for 1 month
and expires in Sept of this year.

Dave Cramer

dave.cramer(at)credativ(dot)ca
http://www.credativ.ca




On Thu, Nov 10, 2011 at 10:45 AM, Bodor András <bodri.mh3@gmail.com> wrote:
> Can you send me some error log, and your database setup?
>
> On Thu, Nov 10, 2011 at 4:19 PM, Dave Cramer <pg@fastcrypt.com> wrote:
>> Hi Bodor,
>>
>> Understood.
>>
>> So now all the tests are failing some due to unknown ca, others to
>> certificate expired ?
>>
>> Dave Cramer
>>
>> dave.cramer(at)credativ(dot)ca
>> http://www.credativ.ca
>>
>>
>>
>>
>> On Thu, Nov 10, 2011 at 9:30 AM, Bodor András <bodri.mh3@gmail.com> wrote:
>>> Dear Dave,
>>>
>>> The installation of sslinfo is only necessary for the unit tests, it is
>>> not used at all in the driver itself. Obviously I wanted to test weather
>>> we were actually using ssl, but it is not essential. It can be removed,
>>> or an additional option can be introduced to ssltest.properties.
>>> The relevant lines are in
>>> org.postgresql.test.ssl.SslTest.driver(String connstr, Object[]
>>> expected)
>>>
>>> There are a few things still to be done with this patch.
>>> 1. the jdbc datasource interface was not modified at all,
>>> so it is unaware of the new options,
>>> 2. it should be decided, what is the expected behaviour of sslmode=allow
>>> or prefer (they might be omitted completely),
>>> 3. I have not tested certificate chains yet,
>>> 4. when a client certificate is available, the v8 and v9 servers
>>> behave differently (BUG #5468 is fixed in v9) so different unit test are
>>> needed to check this,
>>> 5. there is a list of options somewhere in the code, this should
>>> be updated as well,
>>> 6. documentation.
>>>
>>>           Andras
>>>
>>> On Thu, Nov 10, 2011 at 2:56 PM, Dave Cramer <pg@fastcrypt.com> wrote:
>>>> Andras,
>>>>
>>>> I'm looking at your patch attached to this link
>>>> http://archives.postgresql.org/pgsql-jdbc/2011-08/msg00067.php right
>>>> now. Thanks by the way!
>>>>
>>>> The only thing I'd like to pose to the list is the necessity for
>>>> sslinfo to be installed in any database. I can envision some
>>>> production environments which this may not be possible ?
>>>>
>>>> Dave Cramer
>>>>
>>>> dave.cramer(at)credativ(dot)ca
>>>> http://www.credativ.ca
>>>>
>>>>
>>>>
>>>>
>>>> On Thu, Sep 15, 2011 at 11:41 AM, Bodor Andras <bodri.mh3@gmail.com> wrote:
>>>>>
>>>>>  Yes, it is also included in the patch
>>>>> (package org.postgresql.test.ssl). It
>>>>> tries to connect to a series of databases
>>>>> with different ssl properties. The connection
>>>>> strings are given in the ssltest.properties
>>>>> file in the root of the distribution. Just
>>>>> comment out the connstrings, that you don't
>>>>> want to run. Also read the certdir/README
>>>>> file. (build.xml is modified to run this test.)
>>>>>           Andras
>>>>>
>>>>>
>>>>> Dave Cramer wrote:
>>>>>>
>>>>>> Hi Bodor,
>>>>>>
>>>>>> So do you have any test cases for this ?
>>>>>>
>>>>>> Dave Cramer
>>>>>>
>>>>>> dave.cramer(at)credativ(dot)ca
>>>>>> http://www.credativ.ca
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> 2011/9/13 Bodor Andras<bodri.mh3@gmail.com>:
>>>>>>>
>>>>>>>  Hi!
>>>>>>>
>>>>>>>  Can You make any use of my SSL patch sent in on the 23th of August?
>>>>>>>           Andras
>>>>>>>
>>>>>>> --
>>>>>>> Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org)
>>>>>>> To make changes to your subscription:
>>>>>>> http://www.postgresql.org/mailpref/pgsql-jdbc
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org)
>>>>> To make changes to your subscription:
>>>>> http://www.postgresql.org/mailpref/pgsql-jdbc
>>>>>
>>>>
>>>
>>
>

Hi all,


Sorry I haven't been able to find the time to try your patch Andras, but
I've been able to read some of the code.
It looks good and it's a good idea to have a more complete set of
parameters that matches the behaviour of libpq as closely as possible.
(I guess one of the questions will be which one should be used by
default, since the Java defaults and the libpq defaults for TLS are
quite different approaches.)

Just a few quick points, if I may:

- I would suggest keeping the use of Subject Alternative Names (as I had
suggested in my patch [1]), to move towards a hostname verification in
line with RFC 6125, keeping the CN check as a fallback solution. A lot
of CAs now issue certificate with a SAN too, and it tends to be a
cleaner way to represent the hostname in general.
This is something that could probably be implemented in libpq too, but I
haven't looked at its code at all unfortunately.

- To avoid the unlikely event of a problem with using "JKS" on line 131
of LibPQFactory.java, this could be used:
     KeyStore.getInstance(KeyStore.getDefaultType())
(Just in case this runs in a JRE with a security provider that doesn't
support JKS...)
Perhaps it might be worth considering using 'getDefaultAlgorithm()' when
creating the TrustManager as well.

- I've just noticed that the FileInputStream opened at line 130 of the
LazyKeyManager is never closed.

- I wonder if it might not be simpler to load the user cert and private
key into an in-memory KeyStore and create a KeyManager using it (as it's
done for the TrustManager), via a KeyManagerFactory. This would probably
remove the need to handle the client alias choice manually.


Best wishes,

Bruno.



[1] http://archives.postgresql.org/pgsql-jdbc/2011-08/msg00023.php

On 10/11/2011 14:30, Bodor András wrote:
> Dear Dave,
>
> The installation of sslinfo is only necessary for the unit tests, it is
> not used at all in the driver itself. Obviously I wanted to test weather
> we were actually using ssl, but it is not essential. It can be removed,
> or an additional option can be introduced to ssltest.properties.
> The relevant lines are in
> org.postgresql.test.ssl.SslTest.driver(String connstr, Object[]
> expected)
>
> There are a few things still to be done with this patch.
> 1. the jdbc datasource interface was not modified at all,
> so it is unaware of the new options,
> 2. it should be decided, what is the expected behaviour of sslmode=allow
> or prefer (they might be omitted completely),
> 3. I have not tested certificate chains yet,
> 4. when a client certificate is available, the v8 and v9 servers
> behave differently (BUG #5468 is fixed in v9) so different unit test are
> needed to check this,
> 5. there is a list of options somewhere in the code, this should
> be updated as well,
> 6. documentation.
>
>             Andras

Can you send me some error log, and your database setup?

On Thu, Nov 10, 2011 at 4:19 PM, Dave Cramer <pg@fastcrypt.com> wrote:
> Hi Bodor,
>
> Understood.
>
> So now all the tests are failing some due to unknown ca, others to
> certificate expired ?
>
> Dave Cramer
>
> dave.cramer(at)credativ(dot)ca
> http://www.credativ.ca
>
>
>
>
> On Thu, Nov 10, 2011 at 9:30 AM, Bodor András <bodri.mh3@gmail.com> wrote:
>> Dear Dave,
>>
>> The installation of sslinfo is only necessary for the unit tests, it is
>> not used at all in the driver itself. Obviously I wanted to test weather
>> we were actually using ssl, but it is not essential. It can be removed,
>> or an additional option can be introduced to ssltest.properties.
>> The relevant lines are in
>> org.postgresql.test.ssl.SslTest.driver(String connstr, Object[]
>> expected)
>>
>> There are a few things still to be done with this patch.
>> 1. the jdbc datasource interface was not modified at all,
>> so it is unaware of the new options,
>> 2. it should be decided, what is the expected behaviour of sslmode=allow
>> or prefer (they might be omitted completely),
>> 3. I have not tested certificate chains yet,
>> 4. when a client certificate is available, the v8 and v9 servers
>> behave differently (BUG #5468 is fixed in v9) so different unit test are
>> needed to check this,
>> 5. there is a list of options somewhere in the code, this should
>> be updated as well,
>> 6. documentation.
>>
>>           Andras
>>
>> On Thu, Nov 10, 2011 at 2:56 PM, Dave Cramer <pg@fastcrypt.com> wrote:
>>> Andras,
>>>
>>> I'm looking at your patch attached to this link
>>> http://archives.postgresql.org/pgsql-jdbc/2011-08/msg00067.php right
>>> now. Thanks by the way!
>>>
>>> The only thing I'd like to pose to the list is the necessity for
>>> sslinfo to be installed in any database. I can envision some
>>> production environments which this may not be possible ?
>>>
>>> Dave Cramer
>>>
>>> dave.cramer(at)credativ(dot)ca
>>> http://www.credativ.ca
>>>
>>>
>>>
>>>
>>> On Thu, Sep 15, 2011 at 11:41 AM, Bodor Andras <bodri.mh3@gmail.com> wrote:
>>>>
>>>>  Yes, it is also included in the patch
>>>> (package org.postgresql.test.ssl). It
>>>> tries to connect to a series of databases
>>>> with different ssl properties. The connection
>>>> strings are given in the ssltest.properties
>>>> file in the root of the distribution. Just
>>>> comment out the connstrings, that you don't
>>>> want to run. Also read the certdir/README
>>>> file. (build.xml is modified to run this test.)
>>>>           Andras
>>>>
>>>>
>>>> Dave Cramer wrote:
>>>>>
>>>>> Hi Bodor,
>>>>>
>>>>> So do you have any test cases for this ?
>>>>>
>>>>> Dave Cramer
>>>>>
>>>>> dave.cramer(at)credativ(dot)ca
>>>>> http://www.credativ.ca
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> 2011/9/13 Bodor Andras<bodri.mh3@gmail.com>:
>>>>>>
>>>>>>  Hi!
>>>>>>
>>>>>>  Can You make any use of my SSL patch sent in on the 23th of August?
>>>>>>           Andras
>>>>>>
>>>>>> --
>>>>>> Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org)
>>>>>> To make changes to your subscription:
>>>>>> http://www.postgresql.org/mailpref/pgsql-jdbc
>>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org)
>>>> To make changes to your subscription:
>>>> http://www.postgresql.org/mailpref/pgsql-jdbc
>>>>
>>>
>>
>

For the time beeing, you may create new certificates by issuing

openssl req -x509 -newkey -nodes -days 36500 -nodes -keyout server.key
-out server.crt

they will be good for 100 years. Or shall I send a new patch?

For the question of Magosányi Árpád, rigth now pkcs11 is not supported,
but it is not a complicated thing. We can return to it, when this patch works.
           Andras

On Thu, Nov 10, 2011 at 4:55 PM, Dave Cramer <pg@fastcrypt.com> wrote:
> Andras,
>
> I noticed that the server.crt in the patch is only good for 1 month
> and expires in Sept of this year.
>
> Dave Cramer
>
> dave.cramer(at)credativ(dot)ca
> http://www.credativ.ca
>
>
>
>
> On Thu, Nov 10, 2011 at 10:45 AM, Bodor András <bodri.mh3@gmail.com> wrote:
>> Can you send me some error log, and your database setup?
>>
>> On Thu, Nov 10, 2011 at 4:19 PM, Dave Cramer <pg@fastcrypt.com> wrote:
>>> Hi Bodor,
>>>
>>> Understood.
>>>
>>> So now all the tests are failing some due to unknown ca, others to
>>> certificate expired ?
>>>
>>> Dave Cramer
>>>
>>> dave.cramer(at)credativ(dot)ca
>>> http://www.credativ.ca
>>>
>>>
>>>
>>>
>>> On Thu, Nov 10, 2011 at 9:30 AM, Bodor András <bodri.mh3@gmail.com> wrote:
>>>> Dear Dave,
>>>>
>>>> The installation of sslinfo is only necessary for the unit tests, it is
>>>> not used at all in the driver itself. Obviously I wanted to test weather
>>>> we were actually using ssl, but it is not essential. It can be removed,
>>>> or an additional option can be introduced to ssltest.properties.
>>>> The relevant lines are in
>>>> org.postgresql.test.ssl.SslTest.driver(String connstr, Object[]
>>>> expected)
>>>>
>>>> There are a few things still to be done with this patch.
>>>> 1. the jdbc datasource interface was not modified at all,
>>>> so it is unaware of the new options,
>>>> 2. it should be decided, what is the expected behaviour of sslmode=allow
>>>> or prefer (they might be omitted completely),
>>>> 3. I have not tested certificate chains yet,
>>>> 4. when a client certificate is available, the v8 and v9 servers
>>>> behave differently (BUG #5468 is fixed in v9) so different unit test are
>>>> needed to check this,
>>>> 5. there is a list of options somewhere in the code, this should
>>>> be updated as well,
>>>> 6. documentation.
>>>>
>>>>           Andras
>>>>
>>>> On Thu, Nov 10, 2011 at 2:56 PM, Dave Cramer <pg@fastcrypt.com> wrote:
>>>>> Andras,
>>>>>
>>>>> I'm looking at your patch attached to this link
>>>>> http://archives.postgresql.org/pgsql-jdbc/2011-08/msg00067.php right
>>>>> now. Thanks by the way!
>>>>>
>>>>> The only thing I'd like to pose to the list is the necessity for
>>>>> sslinfo to be installed in any database. I can envision some
>>>>> production environments which this may not be possible ?
>>>>>
>>>>> Dave Cramer
>>>>>
>>>>> dave.cramer(at)credativ(dot)ca
>>>>> http://www.credativ.ca
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On Thu, Sep 15, 2011 at 11:41 AM, Bodor Andras <bodri.mh3@gmail.com> wrote:
>>>>>>
>>>>>>  Yes, it is also included in the patch
>>>>>> (package org.postgresql.test.ssl). It
>>>>>> tries to connect to a series of databases
>>>>>> with different ssl properties. The connection
>>>>>> strings are given in the ssltest.properties
>>>>>> file in the root of the distribution. Just
>>>>>> comment out the connstrings, that you don't
>>>>>> want to run. Also read the certdir/README
>>>>>> file. (build.xml is modified to run this test.)
>>>>>>           Andras
>>>>>>
>>>>>>
>>>>>> Dave Cramer wrote:
>>>>>>>
>>>>>>> Hi Bodor,
>>>>>>>
>>>>>>> So do you have any test cases for this ?
>>>>>>>
>>>>>>> Dave Cramer
>>>>>>>
>>>>>>> dave.cramer(at)credativ(dot)ca
>>>>>>> http://www.credativ.ca
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> 2011/9/13 Bodor Andras<bodri.mh3@gmail.com>:
>>>>>>>>
>>>>>>>>  Hi!
>>>>>>>>
>>>>>>>>  Can You make any use of my SSL patch sent in on the 23th of August?
>>>>>>>>           Andras
>>>>>>>>
>>>>>>>> --
>>>>>>>> Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org)
>>>>>>>> To make changes to your subscription:
>>>>>>>> http://www.postgresql.org/mailpref/pgsql-jdbc
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org)
>>>>>> To make changes to your subscription:
>>>>>> http://www.postgresql.org/mailpref/pgsql-jdbc
>>>>>>
>>>>>
>>>>
>>>
>>
>

If you could create the certs that would be good. What do we do about
the CA errors ?



Dave Cramer

dave.cramer(at)credativ(dot)ca
http://www.credativ.ca




On Thu, Nov 10, 2011 at 11:13 AM, Bodor András <bodri.mh3@gmail.com> wrote:
> For the time beeing, you may create new certificates by issuing
>
> openssl req -x509 -newkey -nodes -days 36500 -nodes -keyout server.key
> -out server.crt
>
> they will be good for 100 years. Or shall I send a new patch?
>
> For the question of Magosányi Árpád, rigth now pkcs11 is not supported,
> but it is not a complicated thing. We can return to it, when this patch works.
>           Andras
>
> On Thu, Nov 10, 2011 at 4:55 PM, Dave Cramer <pg@fastcrypt.com> wrote:
>> Andras,
>>
>> I noticed that the server.crt in the patch is only good for 1 month
>> and expires in Sept of this year.
>>
>> Dave Cramer
>>
>> dave.cramer(at)credativ(dot)ca
>> http://www.credativ.ca
>>
>>
>>
>>
>> On Thu, Nov 10, 2011 at 10:45 AM, Bodor András <bodri.mh3@gmail.com> wrote:
>>> Can you send me some error log, and your database setup?
>>>
>>> On Thu, Nov 10, 2011 at 4:19 PM, Dave Cramer <pg@fastcrypt.com> wrote:
>>>> Hi Bodor,
>>>>
>>>> Understood.
>>>>
>>>> So now all the tests are failing some due to unknown ca, others to
>>>> certificate expired ?
>>>>
>>>> Dave Cramer
>>>>
>>>> dave.cramer(at)credativ(dot)ca
>>>> http://www.credativ.ca
>>>>
>>>>
>>>>
>>>>
>>>> On Thu, Nov 10, 2011 at 9:30 AM, Bodor András <bodri.mh3@gmail.com> wrote:
>>>>> Dear Dave,
>>>>>
>>>>> The installation of sslinfo is only necessary for the unit tests, it is
>>>>> not used at all in the driver itself. Obviously I wanted to test weather
>>>>> we were actually using ssl, but it is not essential. It can be removed,
>>>>> or an additional option can be introduced to ssltest.properties.
>>>>> The relevant lines are in
>>>>> org.postgresql.test.ssl.SslTest.driver(String connstr, Object[]
>>>>> expected)
>>>>>
>>>>> There are a few things still to be done with this patch.
>>>>> 1. the jdbc datasource interface was not modified at all,
>>>>> so it is unaware of the new options,
>>>>> 2. it should be decided, what is the expected behaviour of sslmode=allow
>>>>> or prefer (they might be omitted completely),
>>>>> 3. I have not tested certificate chains yet,
>>>>> 4. when a client certificate is available, the v8 and v9 servers
>>>>> behave differently (BUG #5468 is fixed in v9) so different unit test are
>>>>> needed to check this,
>>>>> 5. there is a list of options somewhere in the code, this should
>>>>> be updated as well,
>>>>> 6. documentation.
>>>>>
>>>>>           Andras
>>>>>
>>>>> On Thu, Nov 10, 2011 at 2:56 PM, Dave Cramer <pg@fastcrypt.com> wrote:
>>>>>> Andras,
>>>>>>
>>>>>> I'm looking at your patch attached to this link
>>>>>> http://archives.postgresql.org/pgsql-jdbc/2011-08/msg00067.php right
>>>>>> now. Thanks by the way!
>>>>>>
>>>>>> The only thing I'd like to pose to the list is the necessity for
>>>>>> sslinfo to be installed in any database. I can envision some
>>>>>> production environments which this may not be possible ?
>>>>>>
>>>>>> Dave Cramer
>>>>>>
>>>>>> dave.cramer(at)credativ(dot)ca
>>>>>> http://www.credativ.ca
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Thu, Sep 15, 2011 at 11:41 AM, Bodor Andras <bodri.mh3@gmail.com> wrote:
>>>>>>>
>>>>>>>  Yes, it is also included in the patch
>>>>>>> (package org.postgresql.test.ssl). It
>>>>>>> tries to connect to a series of databases
>>>>>>> with different ssl properties. The connection
>>>>>>> strings are given in the ssltest.properties
>>>>>>> file in the root of the distribution. Just
>>>>>>> comment out the connstrings, that you don't
>>>>>>> want to run. Also read the certdir/README
>>>>>>> file. (build.xml is modified to run this test.)
>>>>>>>           Andras
>>>>>>>
>>>>>>>
>>>>>>> Dave Cramer wrote:
>>>>>>>>
>>>>>>>> Hi Bodor,
>>>>>>>>
>>>>>>>> So do you have any test cases for this ?
>>>>>>>>
>>>>>>>> Dave Cramer
>>>>>>>>
>>>>>>>> dave.cramer(at)credativ(dot)ca
>>>>>>>> http://www.credativ.ca
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> 2011/9/13 Bodor Andras<bodri.mh3@gmail.com>:
>>>>>>>>>
>>>>>>>>>  Hi!
>>>>>>>>>
>>>>>>>>>  Can You make any use of my SSL patch sent in on the 23th of August?
>>>>>>>>>           Andras
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org)
>>>>>>>>> To make changes to your subscription:
>>>>>>>>> http://www.postgresql.org/mailpref/pgsql-jdbc
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org)
>>>>>>> To make changes to your subscription:
>>>>>>> http://www.postgresql.org/mailpref/pgsql-jdbc
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

Hello,


I'm attaching a tentative patch merging the concepts discussed in this
thread (and based on András Bodor's patch) and on the discussion in this
older thread:
http://archives.postgresql.org/pgsql-jdbc/2011-05/msg00069.php


I've tried to resolve the differences between the two approaches: Libpq
and Java JSSE defaults. I guess there are two categories of potential
users for this: users coming from psql and those coming from the Java
world. They'd probably tend to expect different default settings and
options.


This relies on an SslContextFactory and extending classes.

In this patch, SslContextFactory itself isn't actually used (it could be
with a given option). It would use the default SSLContext (which may be
set via SSLContext.setDefault(...) in Java 6 and above, and thus may not
necessarily be the one initialised via the usual javax.* properties).


There's a BasicSslContextFactory (extending SslContextFactory) which
provide methods to create trust managers, key managers and secure
randoms. These are meant to be overridden, otherwise, it will use the
default (null) to initialise the SSLContext (which is the same as using
the JSSE defaults).
It includes convenience methods to build key and trust managers from
KeyStores.


LibPQSslContextFactory extends BasicSslContextFactory and re-uses most
of András's code, initialised from a set of Properties.
This is the one used when 'sslmode' is set. Note that this is where
there would be a conflict regarding the default values (if not testing
for 'sslmode'), since the default CA file is different from what Java
would use as its default.


PropertiesSslContextFactory extends BasicSslContextFactory but is based
on the work by Marc-André Laverdière and Craig Ringer from the previous
thread.
I've also tweaked it to fall back on the default values from the JSSE,
also allowing for non-file-based keystores (e.g. PKCS#11), as discussed
here:
http://archives.postgresql.org/pgsql-jdbc/2011-11/msg00024.php

This one could certainly be modified to use Properties that are not the
System properties, but info passed in the JDBC URL.



Regarding the hostname verification, I've moved András's verifier into a
separate class (DefaultHostnameVerifier). It's used by default unless a
hostname verifier class name is passed or sslmode is set and set to a
value different from 'verify-full' (in which case
PassthroughHostnameVerifier is used). I've also added the SAN verification.


In addition to the 'sslfactory' parameter (which was there, for an
SSLSocketFactory), I've considered adding a parameter for an
SslContextFactory, but haven't. Any opinions? It might make it easier to
pass more complex SSLContexts directly (e.g. via the of the
SSLContextFactories I've done in http://www.jsslutils.org/), but this
could also be done by using a custom SSLSocketFactory.


I must admit it hasn't been extensively tested, but I'd welcome comments
about this approach. I hope this may help unify this work with the
earlier work (sorry, I wasn't aware of it when I first replied to this
thread a few days ago).


Best wishes,

Bruno.