postgres.git / summary / log / commit / refs

commit    36a3be6540b90c6a5d307c0ed9de2076ce5a821c
Author:   Tom Lane <tgl@sss.pgh.pa.us>
Date:     Sun Nov 24 18:09:38 2013 +0000

    Fix new and latent bugs with errno handling in secure_read/secure_write.
    
    These functions must be careful that they return the intended value of
    errno to their callers.  There were several scenarios where this might
    not happen:
    
    1. The recent SSL renegotiation patch added a hunk of code that would
    execute after setting errno.  In the first place, it's doubtful that we
    should consider renegotiation to be successfully completed after a failure,
    and in the second, there's no real guarantee that the called OpenSSL
    routines wouldn't clobber errno.  Fix by not executing that hunk except
    during success exit.
    
    2. errno was left in an unknown state in case of an unrecognized return
    code from SSL_get_error().  While this is a "can't happen" case, it seems
    like a good idea to be sure we know what would happen, so reset errno to
    ECONNRESET in such cases.  (The corresponding code in libpq's fe-secure.c
    already did this.)
    
    3. There was an (undocumented) assumption that client_read_ended() wouldn't
    change errno.  While true in the current state of the code, this seems less
    than future-proof.  Add explicit saving/restoring of errno to make sure
    that changes in the called functions won't break things.
    
    I see no need to back-patch, since #1 is new code and the other two issues
    are mostly hypothetical.
    
    Per discussion with Amit Kapila.


 src/backend/libpq/be-secure.c | 37 +++++++++++++++++++++----------------
 src/backend/tcop/postgres.c   |  6 ++++++
 2 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index 7f01a78e79e..fb468fec43f 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -295,6 +295,7 @@ rloop:
 						(errcode(ERRCODE_PROTOCOL_VIOLATION),
 						 errmsg("unrecognized SSL error code: %d",
 								err)));
+				errno = ECONNRESET;
 				n = -1;
 				break;
 		}
@@ -416,28 +417,32 @@ wloop:
 						(errcode(ERRCODE_PROTOCOL_VIOLATION),
 						 errmsg("unrecognized SSL error code: %d",
 								err)));
+				errno = ECONNRESET;
 				n = -1;
 				break;
 		}
 
-		/* is renegotiation complete? */
-		if (in_ssl_renegotiation &&
-			SSL_num_renegotiations(port->ssl) >= 1)
+		if (n >= 0)
 		{
-			in_ssl_renegotiation = false;
-			port->count = 0;
-		}
+			/* is renegotiation complete? */
+			if (in_ssl_renegotiation &&
+				SSL_num_renegotiations(port->ssl) >= 1)
+			{
+				in_ssl_renegotiation = false;
+				port->count = 0;
+			}
 
-		/*
-		 * if renegotiation is still ongoing, and we've gone beyond the limit,
-		 * kill the connection now -- continuing to use it can be considered a
-		 * security problem.
-		 */
-		if (in_ssl_renegotiation &&
-			port->count > ssl_renegotiation_limit * 1024L)
-			ereport(FATAL,
-					(errcode(ERRCODE_PROTOCOL_VIOLATION),
-					 errmsg("SSL failed to renegotiate connection before limit expired")));
+			/*
+			 * if renegotiation is still ongoing, and we've gone beyond the
+			 * limit, kill the connection now -- continuing to use it can be
+			 * considered a security problem.
+			 */
+			if (in_ssl_renegotiation &&
+				port->count > ssl_renegotiation_limit * 1024L)
+				ereport(FATAL,
+						(errcode(ERRCODE_PROTOCOL_VIOLATION),
+						 errmsg("SSL failed to renegotiate connection before limit expired")));
+		}
 	}
 	else
 #endif
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 181e3fe1f6c..3d74654c827 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -526,16 +526,22 @@ prepare_for_client_read(void)
 
 /*
  * client_read_ended -- get out of the client-input state
+ *
+ * This is called just after low-level reads.  It must preserve errno!
  */
 void
 client_read_ended(void)
 {
 	if (DoingCommandRead)
 	{
+		int			save_errno = errno;
+
 		ImmediateInterruptOK = false;
 
 		DisableNotifyInterrupt();
 		DisableCatchupInterrupt();
+
+		errno = save_errno;
 	}
 }
 


[parent: 08d1b22b3be2]