On Thu, Oct 23, 2008 at 08:51, Magnus Hagander <[EMAIL PROTECTED]> wrote:
> Magnus Hagander wrote:
>> This patch adds a configuration option to pg_hba.conf for "clientcert".
>> This makes it possible to have different client certificate requirements
>> on different connections. It also makes sure that if you specify that
>> you want client cert verification and the root store isn't there, we
>> give an error instead of silently allowing the user in (like we do now).
>>
>> This still does not implement actual client certificate validation -
>> that's for a later step. It just cleans up the handling we have now.
>
> Uh, with docs.
>
> //Magnus

Hi in getting ready to view the other clientcert patch, I thought I
should give this a quick look over.

this hunk will break non ssl builds (due to port->peer):

*** a/src/backend/libpq/auth.c
--- b/src/backend/libpq/auth.c
***************
*** 272,277 **** ClientAuthentication(Port *port)
--- 272,303 ----
                                 errmsg("missing or erroneous pg_hba.conf 
file"),
                                 errhint("See server log for details.")));

+       /*
+        * This is the first point where we have access to the hba record for
+        * the current connection, so perform any verifications based on the
+        * hba options field that should be done *before* the authentication
+        * here.
+        */
+       if (port->hba->clientcert)
+       {
+               /*
+                * When we parse pg_hba.conf, we have already made sure that we 
have
+                * been able to load a certificate store. Thus, if a 
certificate is
+                * present on the client, it has been verified against our root
+                * certificate store, and the connection would have been aborted
+                * already if it didn't verify ok.
+                */
+               if (!port->peer)
+               {
+                       ereport(FATAL,
+                                       
(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+                                        errmsg("connection requires a valid 
client certificate")));
+               }
+       }
+
+       /*
+        * Now proceed to do the actual authentication check
+        */
        switch (port->hba->auth_method)
        {


and how about instead of:

***************
*** 754,768 **** initialize_SSL(void)
                elog(FATAL, "could not set the cipher list (no valid ciphers 
available)");

        /*
!        * Require and check client certificates only if we have a root.crt 
file.
         */
!       if (!SSL_CTX_load_verify_locations(SSL_context, ROOT_CERT_FILE, NULL))
        {
!               /* Not fatal - we do not require client certificates */
                ereport(LOG,
                                (errmsg("could not load root certificate file 
\"%s\": %s",
                                                ROOT_CERT_FILE, 
SSLerrmessage()),
!                                errdetail("Will not verify client 
certificates.")));
        }
        else
        {
--- 768,795 ----
                elog(FATAL, "could not set the cipher list (no valid ciphers 
available)");

        /*
!        * Attempt to load CA store, so we can verify client certificates if 
needed.
         */
!       if (access(ROOT_CERT_FILE, R_OK))
        {
!               /*
!                * Root certificate file simply not found. Don't log an error 
here, because
!                * it's quite likely the user isn't planning on using client 
certificates.
!                */
!               ssl_loaded_verify_locations = false;
!       }
!       else if (!SSL_CTX_load_verify_locations(SSL_context, ROOT_CERT_FILE, 
NULL))
!       {
!               /*
!                * File was there, but we could not load it. This means the 
file is somehow
!                * broken, and we should log this. Don't log it as a fatal 
error, because
!                * there is still a chance that the user isn't going to use 
client certs.
!                */
!               ssl_loaded_verify_locations = false;
                ereport(LOG,
                                (errmsg("could not load root certificate file 
\"%s\": %s",
                                                ROOT_CERT_FILE, 
SSLerrmessage()),
!                                errdetail("Will not be able to verify client 
certificates.")));
        }
        else
        {
***************

we do something like:

+       if (access(ROOT_CERT_FILE, R_OK))
+       {
+               ssl_loaded_verify_locations = false;
+
+               /*
+               * If root certificate file simply not found. Don't log
an error here, because
+               * it's quite likely the user isn't planning on using
client certificates.
+               *
+               * Anything else gets logged (permission errors etc)
+               */
+               if (errno != ENOENT)
+                       ereport(LOG,
+                               (errmsg("could not load root
certificate file \"%s\": %s",
+                                               ROOT_CERT_FILE,
strerror(errno)),
+                                errdetail("Will not be able to verify
client certificates.")));
+       }
+       else if (!SSL_CTX_load_verify_locations(SSL_context,
ROOT_CERT_FILE, NULL))

??

Other than that it looks good.

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

Reply via email to