Code review for SSLKEY patch.

diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index 620454311f72c20cbc27a33e37c14800c6eb63ba..7c46ddd6a4786d467bef6d54a556395fde00121e 100644 (file)
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -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
 

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index dea8843dce24cd187ebf3afffe0f32c53d40f756..868ed3eb254743fe7ecb497b9fc9da2a5e3cd2cf 100644 (file)
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -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;

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 7412e09979facf3c20165259c9b1a91b527ed84b..10423efe336eae5ccd4dc6780ba7d1551603a64a 100644 (file)
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -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

diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index ca5b2aafb1100401baeb895508af73a276c521a0..703f5cbaa3324b01fae40dd6d8ffa35cf712fc62 100644 (file)
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/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
 

diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h
index ed261bdb75a26b912b724c4d2685223c19a9f203..6df40fa869c846ceac0ada02d2d0545d13e0d68b 100644 (file)
--- a/src/include/postmaster/postmaster.h
+++ b/src/include/postmaster/postmaster.h
@@ -15,7 +15,6 @@
 
 /* GUC options */
 extern bool EnableSSL;
-extern char *SSLCipherSuites;
 extern bool SilentMode;
 extern int     ReservedBackends;
 extern int     PostPortNumber;

diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index 6940887e02d23df7e62b4589fe9522855df6e088..8d9119b84ec8c91ec6bb3bfed1b1062554ed576e 100644 (file)
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -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();