Skip site navigation (1) Skip section navigation (2)

Re: patch: Client certificate requirements

From: "Alex Hunsaker" <badalex(at)gmail(dot)com>
To: "Magnus Hagander" <magnus(at)hagander(dot)net>
Cc: "PG Hackers" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: patch: Client certificate requirements
Date: 2008-11-15 22:20:39
Message-ID: 34d269d40811151420p24f80a5i206febbb2ecd0831@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-hackers
On Thu, Oct 23, 2008 at 08:51, Magnus Hagander <magnus(at)hagander(dot)net> 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.

In response to

Responses

pgsql-hackers by date

Next:From: Tom LaneDate: 2008-11-15 22:23:47
Subject: Re: pgsql: Enable script to generate preproc.y in build process.
Previous:From: Andrew DunstanDate: 2008-11-15 22:18:38
Subject: Re: pgsql: Enable script to generate preproc.y in build process.

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group