Re: Support for cert auth in JDBC

On 05/18/2011 03:48 PM, Marc-André Laverdière wrote:
> It is true that anyone who knows the right Java APIs can put that
> factory together with little pain. I just think that the average user
> shouldn't have to bother about it.

In that, I agree. Canned factories for simple cases like loading a
PKCS#12 format cert file, loading an app-specified keystore with a
callback for a password prompt, etc would make a lot of sense.

> So we end up with 3 different keystores (and maybe trust stores too),
> all of which may have different passwords. It is actually not that much
> of a big deal to use, but the 'defaults' won't help us here too much I
> think.

Quite right, if your different keystores must have different passwords
then you'd need to implement your own factory.

> P.S. Undocumented detail: we need to put the user's name in the
> connection settings anyway, as it won't work without it.

Interesting. I've only ever used setups where X.509 certs are
supplementary to traditional password authentication. I'm surprised that
the username isn't just deduced from the X.509 DN.

A few comments on your code: Overall, it looks pretty sensible at first
reading. There are a few things I noticed:

- The code doesn't accept arguments for keystore and truststore type, so
it won't be possible to load (eg) a PKCS#12 certificate file directly.
Only stores of the default types will work.

- It's not very subclassing-friendly, which could be an issue if it's to
be shipped in PgJDBC.

Specific subclassing use cases I see are allowing different
configuration mechanisms (Properties instances in thread-local storage,
JNDI, BeanManager contexts, blah blah) and accepting pre-configured
KeyStore & TrustStore instances. I needed to use the latter in my app,
and the former could become an issue whenever you need to create
CertAuthFactory instances with different configurations in the same app
- for example, for different users.

Personally, I dislike the use of system properties for configuration of
the class. It's totally thread-unsafe and is a real pain in any
situation where you don't necessarily want exactly the same
configuration for every instance. There's nothing wrong with having it
as a convenient default, but it'd be nice to be able to override it with
another configuration mechanism if this code is to ship with PgJDBC.

Alas the SSLSocketFactory interface doesn't offer any really nice
options like a way to pass a Properties instance to the ctor, and
accepting URL-encoded query-string like params as a String isn't
attractive given their length and the potential password security
issues. What I'd want to do is allow subclasses to override the
configuration behaviour by:

- Lazily initializing _internalFactory on first access rather than in
the ctor, so that:

- a subclass's ctor can do something useful and application-defined with
the String parameter passed to the one-arg form, like reading a config
file, looking up a Properties instance from a config service or JNDI,
etc. This would require that:

- Configuration values are acessed via protected methods rather than
direct System.getProperty() calls so those can be overridden by a
subclass to get the information from elsewhere.

Ideally (IMO) also:

- the KeyStore instantiation and loading for ks and trustKs is done in
protected methods too, so these can be completely overridden to obtain
instances another way.

--
Craig Ringer