Andrew Chernow wrote:
> Magnus Hagander wrote:
>> Alex Hunsaker wrote:
>>> On Sat, Dec 27, 2008 at 11:50, Andrew Chernow <a...@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);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
