Improve error reporting for SSL connection failures. Remove redundant

free operations in client_cert_cb --- openssl will also attempt to free
these structures, resulting in core dumps.

diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index 44218c1..f0d1a52 100644 (file)
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/libpq/be-secure.c,v 1.53 2004/11/17 04:05:42 neilc Exp $
+ *       $PostgreSQL: pgsql/src/backend/libpq/be-secure.c,v 1.54 2004/11/20 00:18:13 tgl Exp $
  *
  *       Since the server static private key ($DataDir/server.key)
  *       will normally be stored unencrypted so that the database
@@ -727,37 +727,6 @@ initialize_SSL(void)
        return 0;
 }
 
-#ifdef WIN32
-/*
- *     Win32 socket code uses nonblocking sockets. We ned to deal with that
- *     by waiting on the socket if the SSL accept operation didn't complete
- *     right away.
- */
-static int pgwin32_SSL_accept(SSL *ssl)
-{
-       int r;
-
-       while (1)
-       {
-               int rc;
-               int waitfor;
-
-               r = SSL_accept(ssl);
-               if (r == 1)
-                       return 1;
-
-               rc = SSL_get_error(ssl, r);
-               if (rc != SSL_ERROR_WANT_READ && rc != SSL_ERROR_WANT_WRITE)
-                       return r;
-
-               waitfor = (rc == SSL_ERROR_WANT_READ)?FD_READ|FD_CLOSE|FD_ACCEPT:FD_WRITE|FD_CLOSE;
-               if (pgwin32_waitforsinglesocket(SSL_get_fd(ssl), waitfor) == 0)
-                       return -1;
-       }
-}
-#define SSL_accept(ssl) pgwin32_SSL_accept(ssl)
-#endif
-
 /*
  *     Destroy global SSL context.
  */
@@ -777,7 +746,9 @@ destroy_SSL(void)
 static int
 open_server_SSL(Port *port)
 {
-       int r;
+       int                     r;
+       int                     err;
+
        Assert(!port->ssl);
        Assert(!port->peer);
 
@@ -799,12 +770,50 @@ open_server_SSL(Port *port)
                close_SSL(port);
                return -1;
        }
-       if ((r=SSL_accept(port->ssl)) <= 0)
+
+aloop:
+       r = SSL_accept(port->ssl);
+       if (r <= 0)
        {
-               ereport(COMMERROR,
-                               (errcode(ERRCODE_PROTOCOL_VIOLATION),
-                                errmsg("could not accept SSL connection: %i",
-                                               SSL_get_error(port->ssl,r))));
+               err = SSL_get_error(port->ssl, r);
+               switch (err)
+               {
+                       case SSL_ERROR_WANT_READ:
+                       case SSL_ERROR_WANT_WRITE:
+#ifdef WIN32
+                               pgwin32_waitforsinglesocket(SSL_get_fd(port->ssl),
+                                       (err==SSL_ERROR_WANT_READ) ?
+                                               FD_READ|FD_CLOSE|FD_ACCEPT : FD_WRITE|FD_CLOSE);
+#endif
+                               goto aloop;
+                       case SSL_ERROR_SYSCALL:
+                               if (r < 0)
+                                       ereport(COMMERROR,
+                                                       (errcode_for_socket_access(),
+                                                        errmsg("could not accept SSL connection: %m")));
+                               else
+                                       ereport(COMMERROR,
+                                                       (errcode(ERRCODE_PROTOCOL_VIOLATION),
+                                                        errmsg("could not accept SSL connection: EOF detected")));
+                               break;
+                       case SSL_ERROR_SSL:
+                               ereport(COMMERROR,
+                                               (errcode(ERRCODE_PROTOCOL_VIOLATION),
+                                                errmsg("could not accept SSL connection: %s",
+                                                               SSLerrmessage())));
+                               break;
+                       case SSL_ERROR_ZERO_RETURN:
+                               ereport(COMMERROR,
+                                               (errcode(ERRCODE_PROTOCOL_VIOLATION),
+                                                errmsg("could not accept SSL connection: EOF detected")));
+                               break;
+                       default:
+                               ereport(COMMERROR,
+                                               (errcode(ERRCODE_PROTOCOL_VIOLATION),
+                                                errmsg("unrecognized SSL error code: %d",
+                                                               err)));
+                               break;
+               }
                close_SSL(port);
                return -1;
        }

diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index 637d93e..0e09a99 100644 (file)
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.55 2004/10/16 03:26:43 momjian Exp $
+ *       $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.56 2004/11/20 00:18:18 tgl Exp $
  *
  * NOTES
  *       [ Most of these notes are wrong/obsolete, but perhaps not all ]
@@ -267,6 +267,11 @@ pqsecure_open_client(PGconn *conn)
                        close_SSL(conn);
                        return PGRES_POLLING_FAILED;
                }
+               /*
+                * Initialize errorMessage to empty.  This allows open_client_SSL()
+                * to detect whether client_cert_cb() has stored a message.
+                */
+               resetPQExpBuffer(&conn->errorMessage);
        }
        /* Begin or continue the actual handshake */
        return open_client_SSL(conn);
@@ -797,7 +802,6 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
                printfPQExpBuffer(&conn->errorMessage,
                libpq_gettext("certificate present, but not private key file \"%s\"\n"),
                                                  fnbuf);
-               X509_free(*x509);
                return 0;
        }
        if (!S_ISREG(buf.st_mode) || (buf.st_mode & 0077) ||
@@ -806,7 +810,6 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
                printfPQExpBuffer(&conn->errorMessage,
                libpq_gettext("private key file \"%s\" has wrong permissions\n"),
                                                  fnbuf);
-               X509_free(*x509);
                return 0;
        }
        if ((fp = fopen(fnbuf, "r")) == NULL)
@@ -814,7 +817,6 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
                printfPQExpBuffer(&conn->errorMessage,
                         libpq_gettext("could not open private key file \"%s\": %s\n"),
                                                  fnbuf, pqStrerror(errno, sebuf, sizeof(sebuf)));
-               X509_free(*x509);
                return 0;
        }
        if (fstat(fileno(fp), &buf2) == -1 ||
@@ -822,7 +824,6 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
        {
                printfPQExpBuffer(&conn->errorMessage,
                                                  libpq_gettext("private key file \"%s\" changed during execution\n"), fnbuf);
-               X509_free(*x509);
                return 0;
        }
        if (PEM_read_PrivateKey(fp, pkey, cb, NULL) == NULL)
@@ -833,7 +834,6 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
                                  libpq_gettext("could not read private key file \"%s\": %s\n"),
                                                  fnbuf, err);
                SSLerrfree(err);
-               X509_free(*x509);
                fclose(fp);
                return 0;
        }
@@ -848,8 +848,6 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
                        libpq_gettext("certificate does not match private key file \"%s\": %s\n"),
                                                  fnbuf, err);
                SSLerrfree(err);
-               X509_free(*x509);
-               EVP_PKEY_free(*pkey);
                return 0;
        }
 
@@ -1045,11 +1043,23 @@ open_client_SSL(PGconn *conn)
                                }
                        case SSL_ERROR_SSL:
                                {
-                                       char       *err = SSLerrmessage();
+                                       /*
+                                        * If there are problems with the local certificate files,
+                                        * these will be detected by client_cert_cb() which is
+                                        * called from SSL_connect().  We want to return that
+                                        * error message and not the rather unhelpful error that
+                                        * OpenSSL itself returns.  So check to see if an error
+                                        * message was already stored.
+                                        */
+                                       if (conn->errorMessage.len == 0)
+                                       {
+                                               char       *err = SSLerrmessage();
 
-                                       printfPQExpBuffer(&conn->errorMessage,
-                                                                 libpq_gettext("SSL error: %s\n"), err);
-                                       SSLerrfree(err);
+                                               printfPQExpBuffer(&conn->errorMessage,
+                                                                                 libpq_gettext("SSL error: %s\n"),
+                                                                                 err);
+                                               SSLerrfree(err);
+                                       }
                                        close_SSL(conn);
                                        return PGRES_POLLING_FAILED;
                                }