On 24/05/10 00:58, Tom Lane wrote:
Craig Ringer<cr...@postnewspapers.com.au> writes:
+ SSL_CTX_set_client_CA_list( SSL_context,
SSL_load_client_CA_file(ROOT_CERT_FILE) );
Hmm, what about failures? If we're loading the root cert file a second
time, it's possible that the user just changed it and the load now fails
for some reason.
True I guess. It's a pretty narrow window for failure given that we
*just* loaded it milliseconds ago, but not impossible.
As it is, SSL_ctx_set_client_CA_list handles null input gracefully, and
the client_CA member of the SSL_context starts out null, so there'll be
no harm in calling SSL_CTX_set_client_CA_list with a null arg when
SSL_load_client_CA_file fails. It just assigns null to the the
already-null client_CA member of the context. OpenSSL checks for null in
client_CA before doing anything with it.
If the user does somehow manage to trigger this case, the worst that'll
happen is that the list of trusted certificates will not be sent to the
client - exactly as currently happens. This may cause some clients to
fail to negotiate, but cannot grant access that would otherwise not be
granted. Triggering it requires access to the datadir (or where a
symlink from the datadir points) and has such a narrow time window I'm
not sure it's worth worrying about.
I *could* avoid the second file load by supplying the list of loaded
certificates from SSL_load_client_CA_file to OpenSSL as trusted
certificates for verifying clients. I'd really rather not mess with the
logic around verifying client certificates, though, especially given the
state of the OpenSSL documentation. What I've done is the approach
recommended in the documentation (see the man page for
SSL_CTX_set_client_CA_list) and seems safest.
Still, I'm happy to move some code around so that either load of
ROOT_CERT_FILE will report an error. The reason I did it this way is
that it's a small, clear change that only affects users of client
certificates. I've attached a revision that catches both failures, using
the existing error message. Hope that suits.
--
Craig Ringer
*** postgresql-8.4-8.4.3/src/backend/libpq/be-secure.c 2010-02-25 21:26:26.000000000 +0800
--- postgresql-8.4-8.4.3-patched/src/backend/libpq/be-secure.c 2010-05-24 07:46:32.000000000 +0800
***************
*** 715,720 ****
--- 715,721 ----
initialize_SSL(void)
{
struct stat buf;
+ STACK_OF(X509_NAME) * root_cert_list = NULL;
if (!SSL_context)
{
***************
*** 804,810 ****
ROOT_CERT_FILE)));
}
}
! else if (SSL_CTX_load_verify_locations(SSL_context, ROOT_CERT_FILE, NULL) != 1)
{
/*
* File was there, but we could not load it. This means the file is
--- 808,815 ----
ROOT_CERT_FILE)));
}
}
! else if ( (SSL_CTX_load_verify_locations(SSL_context, ROOT_CERT_FILE, NULL) != 1) ||
! (root_cert_list = SSL_load_client_CA_file(ROOT_CERT_FILE)) == NULL )
{
/*
* File was there, but we could not load it. This means the file is
***************
*** 818,823 ****
--- 823,835 ----
}
else
{
+ /*
+ * Tell OpenSSL to send the list of root certs we trust to the client
+ * with the CertificateRequest. This lets a client with a keystore
+ * select the appropriate client certificate to send to us.
+ */
+ SSL_CTX_set_client_CA_list(SSL_context, root_cert_list);
+
/*
* Check the Certificate Revocation List (CRL) if file exists.
* http://searchsecurity.techtarget.com/sDefinition/0,,sid14_gci803160,
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
