On Fri, Aug 1, 2008 at 13:31, Alvaro Herrera <[EMAIL PROTECTED]> wrote:
> Something that's bothering me is that PGSSLKEY is inconsistent with the
> sslkey conninfo parameter.  PGSSLKEY specifies an engine (basically a
> driver for specialized hardware AFAICT) from which the key is to be
> loaded, but sslkey is a simple filename.  This means that there's no way
> to load a key from hardware if you want to specify it per connection.
> Not that I have any such hardware, but it looks bogus.
>
> Obviously one still wants to be able to specify a different file name
> from the default; I tried to see if there's any way to load an engine
> that would load the key from a file, but could not extract any sense
> from the man page:
> http://www.openssl.org/docs/crypto/engine.html
>
> Maybe this means that we should provide separate parameters, say
> "sslkey" and "sslkeyfile", and a new env var PGSSLKEYFILE.
>
> Thoughts?  Am I overengineering this stuff?

I think the patch as it stands is ok for now (mainly because I don't
have any hardware either so I can't test or attest to how it should be
done i.e. if those params would be enough)

Should PGROOTCERT be PGSSLROOTCERT to be more congruent with all the
other ssl params?

Find attached an updated patch against HEAD (no other changes).   I
also gave it a look over and tested it to make sure it worked as
described.
*** a/doc/src/sgml/libpq.sgml
--- b/doc/src/sgml/libpq.sgml
***************
*** 309,314 ****
--- 309,358 ----
          </varlistentry>
  
          <varlistentry>
+          <term><literal>sslcert</literal></term>
+          <listitem>
+           <para>
+            This parameter specifies the file name of the client SSL
+            certificate.  This option is only available if
+            <productname>PostgreSQL</> is compiled with SSL support.
+           </para>
+          </listitem>
+         </varlistentry>
+ 
+         <varlistentry>
+          <term><literal>sslkey</literal></term>
+          <listitem>
+           <para>
+            This parameter specifies the file name of the client SSL key.
+            This option is only available if <productname>PostgreSQL</> is
+            compiled with SSL support.
+           </para>
+          </listitem>
+         </varlistentry>
+ 
+         <varlistentry>
+          <term><literal>sslrootcert</literal></term>
+          <listitem>
+           <para>
+            This parameter specifies the file name of the root SSL certificate.
+            This option is only available if <productname>PostgreSQL</> is
+            compiled with SSL support.
+           </para>
+          </listitem>
+         </varlistentry>
+ 
+         <varlistentry>
+          <term><literal>sslcrl</literal></term>
+          <listitem>
+           <para>
+            This parameter specifies the file name of the SSL certificate
+            revocation list (CRL).  This option is only available if
+            <productname>PostgreSQL</> is compiled with SSL support.
+           </para>
+          </listitem>
+         </varlistentry>
+ 
+         <varlistentry>
           <term><literal>krbsrvname</literal></term>
           <listitem>
            <para>
***************
*** 5767,5772 **** myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
--- 5811,5838 ----
      <listitem>
       <para>
        <indexterm>
+        <primary><envar>PGROOTCERT</envar></primary>
+       </indexterm>
+       <envar>PGROOTCERT</envar> specifies the file name where the SSL
+       root certificate is stored.  This can be overridden by the
+       <literal>sslrootcert</literal> connection parameter.
+      </para>
+     </listitem>
+ 
+     <listitem>
+      <para>
+       <indexterm>
+        <primary><envar>PGSSLCRL</envar></primary>
+       </indexterm>
+       <envar>PGSSLCRL</envar> specifies the file name where the SSL certificate
+       revocation list is stored.  This can be overridden by the
+       <literal>sslcrl</literal> connection parameter.
+      </para>
+     </listitem>
+ 
+     <listitem>
+      <para>
+       <indexterm>
         <primary><envar>PGKRBSRVNAME</envar></primary>
        </indexterm>
        <envar>PGKRBSRVNAME</envar> sets the Kerberos service name to use
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
***************
*** 186,191 **** static const PQconninfoOption PQconninfoOptions[] = {
--- 186,206 ----
  	{"sslverify", "PGSSLVERIFY", DefaultSSLVerify, NULL,
  	"SSL-Verify", "", 5},		/* sizeof("chain") == 5 */
  
+ 	/* These parameters are only present in an SSL-enabled build. */
+ #ifdef USE_SSL
+ 	{"sslcert", "PGSSLCERT", NULL, NULL,
+ 	"SSL-Client-Cert", "", 64},
+ 
+ 	{"sslkey", "PGSSLKEY", NULL, NULL,
+ 	"SSL-Client-Key", "", 64},
+ 
+ 	{"sslrootcert", "PGROOTCERT", NULL, NULL,
+ 	"SSL-Root-Certificate", "", 64},
+ 
+ 	{"sslcrl", "PGSSLCRL", NULL, NULL,
+ 	"SSL-Revocation-List", "", 64},
+ #endif
+ 
  #if defined(KRB5) || defined(ENABLE_GSS) || defined(ENABLE_SSPI)
  	/* Kerberos and GSSAPI authentication support specifying the service name */
  	{"krbsrvname", "PGKRBSRVNAME", PG_KRB_SRVNAM, NULL,
***************
*** 423,428 **** connectOptions1(PGconn *conn, const char *conninfo)
--- 438,451 ----
  	tmp = conninfo_getval(connOptions, "sslverify");
  	conn->sslverify = tmp ? strdup(tmp) : NULL;
  #ifdef USE_SSL
+ 	tmp = conninfo_getval(connOptions, "sslkey");
+ 	conn->sslkey = tmp ? strdup(tmp) : NULL;
+ 	tmp = conninfo_getval(connOptions, "sslcert");
+ 	conn->sslcert = tmp ? strdup(tmp) : NULL;
+ 	tmp = conninfo_getval(connOptions, "sslrootcert");
+ 	conn->sslrootcert = tmp ? strdup(tmp) : NULL;
+ 	tmp = conninfo_getval(connOptions, "sslcrl");
+ 	conn->sslcrl = tmp ? strdup(tmp) : NULL;
  	tmp = conninfo_getval(connOptions, "requiressl");
  	if (tmp && tmp[0] == '1')
  	{
***************
*** 2035,2040 **** freePGconn(PGconn *conn)
--- 2058,2073 ----
  		free(conn->sslmode);
  	if (conn->sslverify)
  		free(conn->sslverify);
+ #ifdef USE_SSL
+ 	if (conn->sslcert)
+ 		free(conn->sslcert);
+ 	if (conn->sslkey)
+ 		free(conn->sslkey);
+ 	if (conn->sslrootcert)
+ 		free(conn->sslrootcert);
+ 	if (conn->sslcrl)
+ 		free(conn->sslcrl);
+ #endif
  #if defined(KRB5) || defined(ENABLE_GSS) || defined(ENABLE_SSPI)
  	if (conn->krbsrvname)
  		free(conn->krbsrvname);
*** a/src/interfaces/libpq/fe-secure.c
--- b/src/interfaces/libpq/fe-secure.c
***************
*** 509,515 **** client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
  	}
  
  	/* read the user certificate */
! 	snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, USER_CERT_FILE);
  
  	/*
  	 * OpenSSL <= 0.9.8 lacks error stack handling, which means it's likely to
--- 509,518 ----
  	}
  
  	/* read the user certificate */
! 	if (conn->sslcert)
! 		strncpy(fnbuf, conn->sslcert, sizeof(fnbuf));
! 	else
! 		snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, USER_CERT_FILE);
  
  	/*
  	 * OpenSSL <= 0.9.8 lacks error stack handling, which means it's likely to
***************
*** 560,566 **** client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
  	BIO_free(bio);
  
  #if (SSLEAY_VERSION_NUMBER >= 0x00907000L) && !defined(OPENSSL_NO_ENGINE)
! 	if (getenv("PGSSLKEY"))
  	{
  		/* read the user key from engine */
  		char	   *engine_env = getenv("PGSSLKEY");
--- 563,569 ----
  	BIO_free(bio);
  
  #if (SSLEAY_VERSION_NUMBER >= 0x00907000L) && !defined(OPENSSL_NO_ENGINE)
! 	if (getenv("PGSSLKEY") && !conn->sslkey)
  	{
  		/* read the user key from engine */
  		char	   *engine_env = getenv("PGSSLKEY");
***************
*** 612,618 **** client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
  #endif   /* use PGSSLKEY */
  	{
  		/* read the user key from file */
! 		snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, USER_KEY_FILE);
  		if (stat(fnbuf, &buf) != 0)
  		{
  			printfPQExpBuffer(&conn->errorMessage,
--- 615,625 ----
  #endif   /* use PGSSLKEY */
  	{
  		/* read the user key from file */
! 		if (conn->sslkey)
! 			strncpy(fnbuf, conn->sslkey, sizeof(fnbuf));
! 		else
! 			snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, USER_KEY_FILE);
! 
  		if (stat(fnbuf, &buf) != 0)
  		{
  			printfPQExpBuffer(&conn->errorMessage,
***************
*** 820,826 **** initialize_SSL(PGconn *conn)
  	/* Set up to verify server cert, if root.crt is present */
  	if (pqGetHomeDirectory(homedir, sizeof(homedir)))
  	{
! 		snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, ROOT_CERT_FILE);
  		if (stat(fnbuf, &buf) == 0)
  		{
  			X509_STORE *cvstore;
--- 827,837 ----
  	/* Set up to verify server cert, if root.crt is present */
  	if (pqGetHomeDirectory(homedir, sizeof(homedir)))
  	{
! 		if (conn->sslrootcert)
! 			strncpy(fnbuf, conn->sslrootcert, sizeof(fnbuf));
! 		else
! 			snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, ROOT_CERT_FILE);
! 
  		if (stat(fnbuf, &buf) == 0)
  		{
  			X509_STORE *cvstore;
***************
*** 838,845 **** initialize_SSL(PGconn *conn)
  
  			if ((cvstore = SSL_CTX_get_cert_store(SSL_context)) != NULL)
  			{
  				/* setting the flags to check against the complete CRL chain */
! 				if (X509_STORE_load_locations(cvstore, ROOT_CRL_FILE, NULL) != 0)
  /* OpenSSL 0.96 does not support X509_V_FLAG_CRL_CHECK */
  #ifdef X509_V_FLAG_CRL_CHECK
  					X509_STORE_set_flags(cvstore,
--- 849,861 ----
  
  			if ((cvstore = SSL_CTX_get_cert_store(SSL_context)) != NULL)
  			{
+ 				if (conn->sslcrl)
+ 					strncpy(fnbuf, conn->sslcrl, sizeof(fnbuf));
+ 				else
+ 					snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, ROOT_CRL_FILE);
+ 
  				/* setting the flags to check against the complete CRL chain */
! 				if (X509_STORE_load_locations(cvstore, fnbuf, NULL) != 0)
  /* OpenSSL 0.96 does not support X509_V_FLAG_CRL_CHECK */
  #ifdef X509_V_FLAG_CRL_CHECK
  					X509_STORE_set_flags(cvstore,
*** a/src/interfaces/libpq/libpq-int.h
--- b/src/interfaces/libpq/libpq-int.h
***************
*** 292,297 **** struct pg_conn
--- 292,304 ----
  	char	   *pgpass;
  	char	   *sslmode;		/* SSL mode (require,prefer,allow,disable) */
  	char	   *sslverify;		/* Verify server SSL certificate (none,chain,cn) */
+ #ifdef USE_SSL
+ 	char	   *sslkey;			/* client key filename */
+ 	char	   *sslcert;		/* client certificate filename */
+ 	char	   *sslrootcert;	/* root certificate filename */
+ 	char	   *sslcrl;			/* certificate revocation list filename */
+ #endif
+ 
  #if defined(KRB5) || defined(ENABLE_GSS) || defined(ENABLE_SSPI)
  	char	   *krbsrvname;		/* Kerberos service name */
  #endif
-- 
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