Re: new libpq SSL connection option - Mailing list pgsql-hackers

Andrew Chernow wrote:
> Magnus Hagander wrote:
>> Alex Hunsaker wrote:
>>> On Sat, Dec 27, 2008 at 11:50, Andrew Chernow <ac@esilo.com> wrote:
>>>> Why does pqGetHomeDirectory have to succeed to use
>>>> conn->sslrootcert. Maybe
>>>> this should be an OR of the two since sslrootcert is not dependent on
>>>> homedir?
>>>>
>>>> around line 970 src/interfaces/libpq/fe-secure.c
>>>>
>>>> if (conn->sslrootcert || pqGetHomeDirectory(homedir, sizeof(homedir)))
>>>
>>> Certainly, did we miss anywhere else?
>>>
>
> Yes, the homedir variable is used again later in the function.  homedir
> could be invalid since pqGetHomeDirectory might not get called.  Maybe
> something like below would do the trick:
>
> /* when used, it can't be an empty string. */
> *homedir = 0;
>
> /* If either are NULL, homedir is needed */
> if (!conn->sslrootcert || !conn->sslcrl)
>   pqGetHomeDirectory(homedir, sizeof(homedir));
>
> /* one of them must be valid */
> if (conn->sslrootcert || *homedir)

How about this patch?

There's a lot of whitespace change due to indentation change, so I've
included a version without it for reference.


Also, it looks like we have the same problem with the private key, in
client_cert_cb(), agreed?


//Magnus
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index ca2d33e..6957542 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -964,76 +964,85 @@ initialize_SSL(PGconn *conn)
      * If sslverify is set to anything other than "none", perform certificate
      * verification. If set to "cn" we will also do further verifications after
      * the connection has been completed.
+     *
+     * If we are going to look for either root certificate or CRL in the home directory,
+     * we need pqGetHomeDirectory() to succeed. In other cases, we don't need to
+     * get the home directory explicitly.
      */
-
-    /* Set up to verify server cert, if root.crt is present */
-    if (pqGetHomeDirectory(homedir, sizeof(homedir)))
+    if (!conn->sslrootcert || !conn->sslcrl)
     {
-        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)
+        if (!pqGetHomeDirectory(homedir, sizeof(homedir)))
         {
-            X509_STORE *cvstore;
-
-            if (!SSL_CTX_load_verify_locations(SSL_context, fnbuf, NULL))
+            if (strcmp(conn->sslverify, "none") != 0)
             {
-                char       *err = SSLerrmessage();
-
                 printfPQExpBuffer(&conn->errorMessage,
-                                  libpq_gettext("could not read root certificate file \"%s\": %s\n"),
-                                  fnbuf, err);
-                SSLerrfree(err);
+                                  libpq_gettext("cannot find home directory to locate root certificate file"));
                 return -1;
             }
+        }
+    }
+    else
+    {
+        homedir[0] = '\0';
+    }

-            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,
-                          X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL);
-                /* if not found, silently ignore;  we do not require CRL */
-#else
-                {
-                    char       *err = SSLerrmessage();

-                    printfPQExpBuffer(&conn->errorMessage,
-                                      libpq_gettext("SSL library does not support CRL certificates (file \"%s\")\n"),
-                                      fnbuf);
-                    SSLerrfree(err);
-                    return -1;
-                }
-#endif
-            }
+    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;
+
+        if (!SSL_CTX_load_verify_locations(SSL_context, fnbuf, NULL))
+        {
+            char       *err = SSLerrmessage();

-            SSL_CTX_set_verify(SSL_context, SSL_VERIFY_PEER, verify_cb);
+            printfPQExpBuffer(&conn->errorMessage,
+                              libpq_gettext("could not read root certificate file \"%s\": %s\n"),
+                              fnbuf, err);
+            SSLerrfree(err);
+            return -1;
         }
-        else
+
+        if ((cvstore = SSL_CTX_get_cert_store(SSL_context)) != NULL)
         {
-            if (strcmp(conn->sslverify, "none") != 0)
+            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,
+                      X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL);
+            /* if not found, silently ignore;  we do not require CRL */
+#else
             {
+                char       *err = SSLerrmessage();
+
                 printfPQExpBuffer(&conn->errorMessage,
-                                  libpq_gettext("root certificate file (%s) not found"), fnbuf);
+                                  libpq_gettext("SSL library does not support CRL certificates (file \"%s\")\n"),
+                                  fnbuf);
+                SSLerrfree(err);
                 return -1;
             }
+#endif
         }
-    }
+
+        SSL_CTX_set_verify(SSL_context, SSL_VERIFY_PEER, verify_cb);
+    } /* root certificate exists */
     else
     {
         if (strcmp(conn->sslverify, "none") != 0)
         {
             printfPQExpBuffer(&conn->errorMessage,
-                              libpq_gettext("cannot find home directory to locate root certificate file"));
+                              libpq_gettext("root certificate file (%s) not found"), fnbuf);
             return -1;
         }
     }
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index ca2d33e..6957542 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -964,11 +964,30 @@ initialize_SSL(PGconn *conn)
      * If sslverify is set to anything other than "none", perform certificate
      * verification. If set to "cn" we will also do further verifications after
      * the connection has been completed.
+     *
+     * If we are going to look for either root certificate or CRL in the home directory,
+     * we need pqGetHomeDirectory() to succeed. In other cases, we don't need to
+     * get the home directory explicitly.
      */
-
-    /* Set up to verify server cert, if root.crt is present */
-    if (pqGetHomeDirectory(homedir, sizeof(homedir)))
+    if (!conn->sslrootcert || !conn->sslcrl)
+    {
+        if (!pqGetHomeDirectory(homedir, sizeof(homedir)))
     {
+            if (strcmp(conn->sslverify, "none") != 0)
+            {
+                printfPQExpBuffer(&conn->errorMessage,
+                                  libpq_gettext("cannot find home directory to locate root certificate file"));
+                return -1;
+            }
+        }
+    }
+    else
+    {
+        homedir[0] = '\0';
+    }
+
+
+
         if (conn->sslrootcert)
             strncpy(fnbuf, conn->sslrootcert, sizeof(fnbuf));
         else
@@ -1017,7 +1036,7 @@ initialize_SSL(PGconn *conn)
             }

             SSL_CTX_set_verify(SSL_context, SSL_VERIFY_PEER, verify_cb);
-        }
+    } /* root certificate exists */
         else
         {
             if (strcmp(conn->sslverify, "none") != 0)
@@ -1027,16 +1046,6 @@ initialize_SSL(PGconn *conn)
                 return -1;
             }
         }
-    }
-    else
-    {
-        if (strcmp(conn->sslverify, "none") != 0)
-        {
-            printfPQExpBuffer(&conn->errorMessage,
-                              libpq_gettext("cannot find home directory to locate root certificate file"));
-            return -1;
-        }
-    }

     /* set up mechanism to provide client certificate, if available */
     SSL_CTX_set_client_cert_cb(SSL_context, client_cert_cb);