Code review for SSLKEY patch.

src/backend/libpq/be-secure.c

@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/libpq/be-secure.c,v 1.78 2007/02/16 02:59:40 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/libpq/be-secure.c,v 1.79 2007/02/16 17:06:59 tgl Exp $
  *
  *	  Since the server static private key ($DataDir/server.key)
  *	  will normally be stored unencrypted so that the database
@@ -95,8 +95,7 @@
 #if SSLEAY_VERSION_NUMBER >= 0x0907000L
 #include <openssl/conf.h>
 #endif
-
-#endif
+#endif /* USE_SSL */
 
 #include "libpq/libpq.h"
 #include "tcop/tcopprot.h"
@@ -130,8 +129,8 @@ static const char *SSLerrmessage(void);
 
 static SSL_CTX *SSL_context = NULL;
 
-/* GUC variable controlling SSL cipher list*/
-extern char *SSLCipherSuites;
+/* GUC variable controlling SSL cipher list */
+char *SSLCipherSuites = NULL;
 
 #endif
 

src/backend/postmaster/postmaster.c

@@ -37,7 +37,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/postmaster/postmaster.c,v 1.524 2007/02/16 02:59:41 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/postmaster/postmaster.c,v 1.525 2007/02/16 17:06:59 tgl Exp $
  *
  * NOTES
  *
@@ -187,7 +187,6 @@ static int	SendStop = false;
 
 /* still more option variables */
 bool		EnableSSL = false;
-char	   *SSLCipherSuites;
 bool		SilentMode = false; /* silent mode (-S) */
 
 int			PreAuthDelay = 0;

src/backend/utils/misc/guc.c

@@ -10,7 +10,7 @@
  * Written by Peter Eisentraut <peter_e@gmx.net>.
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.375 2007/02/16 02:59:41 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.376 2007/02/16 17:07:00 tgl Exp $
  *
  *--------------------------------------------------------------------
  */
@@ -106,6 +106,11 @@ extern bool fullPageWrites;
 extern bool trace_sort;
 #endif
 
+#ifdef USE_SSL
+extern char *SSLCipherSuites;
+#endif
+
+
 static const char *assign_log_destination(const char *value,
 					   bool doit, GucSource source);
 
@@ -2314,6 +2319,7 @@ static struct config_string ConfigureNamesString[] =
 		NULL, assign_temp_tablespaces, NULL
 	},
 
+#ifdef USE_SSL
 	{
 		{"ssl_ciphers", PGC_POSTMASTER, CONN_AUTH_SECURITY,
 			gettext_noop("Sets the list of allowed SSL ciphers."),
@@ -2323,7 +2329,8 @@ static struct config_string ConfigureNamesString[] =
 		&SSLCipherSuites,
 		"ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH", NULL, NULL
 	},
-			
+#endif /* USE_SSL */
+
 	/* End-of-list marker */
 	{
 		{NULL, 0, 0, NULL, NULL}, NULL, NULL, NULL, NULL

src/backend/utils/misc/postgresql.conf.sample

@@ -74,7 +74,8 @@
 
 #authentication_timeout = 1min		# 1s-600s
 #ssl = off				# (change requires restart)
-#ssl_ciphers = 'ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH' # List of ciphers to use
+#ssl_ciphers = 'ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH'	# Allowed SSL ciphers
+					# (change requires restart)
 #password_encryption = on
 #db_user_namespace = off
 

src/include/postmaster/postmaster.h

@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/postmaster/postmaster.h,v 1.16 2007/02/16 02:59:41 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/postmaster/postmaster.h,v 1.17 2007/02/16 17:07:00 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -15,7 +15,6 @@
 
 /* GUC options */
 extern bool EnableSSL;
-extern char *SSLCipherSuites;
 extern bool SilentMode;
 extern int	ReservedBackends;
 extern int	PostPortNumber;

src/interfaces/libpq/fe-secure.c

@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.93 2007/02/16 02:59:41 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.94 2007/02/16 17:07:00 tgl Exp $
  *
  * NOTES
  *	  [ Most of these notes are wrong/obsolete, but perhaps not all ]
@@ -619,7 +619,7 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
 		char	*engine_env = getenv("PGSSLKEY");
 		char	*engine_colon = strchr(engine_env, ':');
 		char	*engine_str;
-		ENGINE	*engine_ptr = NULL;
+		ENGINE	*engine_ptr;
 
 		if (!engine_colon)
 		{
@@ -630,24 +630,28 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
 
 		engine_str = malloc(engine_colon - engine_env + 1);
 		strlcpy(engine_str, engine_env, engine_colon - engine_env + 1);
-		if ((engine_ptr = ENGINE_by_id(engine_str)) == NULL)
+		engine_ptr = ENGINE_by_id(engine_str);
+		if (engine_ptr == NULL)
 		{
 			char	  *err = SSLerrmessage();
 
 			printfPQExpBuffer(&conn->errorMessage,
-				libpq_gettext("could not load SSL engine \"%s\":%s\n"), engine_str, err);
-			free(engine_str);
+				libpq_gettext("could not load SSL engine \"%s\": %s\n"),
+							  engine_str, err);
 			SSLerrfree(err);
+			free(engine_str);
 			return 0;
 		}	
-		if ((*pkey = ENGINE_load_private_key(engine_ptr,
-						engine_colon + 1, NULL, NULL)) == NULL)
+
+		*pkey = ENGINE_load_private_key(engine_ptr, engine_colon + 1,
+										NULL, NULL);
+		if (*pkey == NULL)
 		{
 			char	  *err = SSLerrmessage();
 
 			printfPQExpBuffer(&conn->errorMessage,
-				libpq_gettext("could not read private SSL key %s from engine \"%s\": %s\n"),
-							engine_colon + 1, engine_str, err);
+				libpq_gettext("could not read private SSL key \"%s\" from engine \"%s\": %s\n"),
+							  engine_colon + 1, engine_str, err);
 			SSLerrfree(err);
 			free(engine_str);
 			return 0;
@@ -655,9 +659,9 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
 		free(engine_str);
 	}
 	else
-#endif
+#endif /* use PGSSLKEY */
 	{
-		/* read the user key from file*/
+		/* read the user key from file */
 		snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, USER_KEY_FILE);
 		if (stat(fnbuf, &buf) == -1)
 		{
@@ -666,7 +670,7 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
 							fnbuf);
 			return 0;
 		}
-	#ifndef WIN32
+#ifndef WIN32
 		if (!S_ISREG(buf.st_mode) || (buf.st_mode & 0077) ||
 			buf.st_uid != geteuid())
 		{
@@ -675,7 +679,7 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
 							fnbuf);
 			return 0;
 		}
-	#endif
+#endif
 		if ((fp = fopen(fnbuf, "r")) == NULL)
 		{
 			printfPQExpBuffer(&conn->errorMessage,
@@ -683,7 +687,7 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
 							fnbuf, pqStrerror(errno, sebuf, sizeof(sebuf)));
 			return 0;
 		}
-	#ifndef WIN32
+#ifndef WIN32
 		if (fstat(fileno(fp), &buf2) == -1 ||
 			buf.st_dev != buf2.st_dev || buf.st_ino != buf2.st_ino)
 		{
@@ -691,7 +695,7 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
 							libpq_gettext("private key file \"%s\" changed during execution\n"), fnbuf);
 			return 0;
 		}
-	#endif
+#endif
 		if (PEM_read_PrivateKey(fp, pkey, NULL, NULL) == NULL)
 		{
 			char	   *err = SSLerrmessage();
@@ -705,6 +709,7 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
 		}
 		fclose(fp);
 	}
+
 	/* verify that the cert and key go together */
 	if (!X509_check_private_key(*x509, *pkey))
 	{
@@ -788,7 +793,7 @@ init_ssl_system(PGconn *conn)
 	{
 		if (pq_initssllib)
 		{
-#if (SSLEAY_VERSION_NUMBER >= 0x00907000L) 
+#if SSLEAY_VERSION_NUMBER >= 0x00907000L
 			OPENSSL_config(NULL);
 #endif			
 			SSL_library_init();