From: Ken Hornstein <kenh@pobox.com>
Date: Wed, 31 May 2017 18:39:48 +0000 (-0400)
Subject: Normalize connection shutdown handling.
X-Git-Url: https://diplodocus.org/git/nmh/commitdiff_plain/5c8b15d4703e145128f51494ffc496dafd5fe718?ds=sidebyside;hp=06fff87f275c3577a962537d44c558649285d8e4

Normalize connection shutdown handling.

I originally tried to make things work so that the netsec client code gave
the client the option of closing the file descriptors if it wanted to, but
I was running into a problem where if TLS negotiation failed part of the
negotiation would be interpreted as connection data.  The code has been
changed to have the sockets close when the SSL BIO is released and to
have netsec_shutdown unconditionally close the file descriptors.
---

diff --git a/h/netsec.h b/h/netsec.h
index 6d3e1767..e381af86 100644
--- a/h/netsec.h
+++ b/h/netsec.h
@@ -15,15 +15,15 @@ netsec_context *netsec_init(void);
 
 /*
  * Shuts down the security context for a connection and frees all
- * associated resources.
+ * associated resources.  Will unconditionally close the network socket
+ * as well.
  *
  * Arguments:
  *
  * ns_context	- Network security context
- * closeflag	- If set to 1, close the socket descriptor as well.
  */
 
-void netsec_shutdown(netsec_context *ns_context, int closeflag);
+void netsec_shutdown(netsec_context *ns_context);
 
 /*
  * Sets the file descriptor for this connection.  This will be used by
diff --git a/mts/smtp/smtp.c b/mts/smtp/smtp.c
index b09fde33..69477b6c 100644
--- a/mts/smtp/smtp.c
+++ b/mts/smtp/smtp.c
@@ -649,7 +649,7 @@ sm_end (int type)
     }
 
     if (nsc != NULL) {
-	netsec_shutdown(nsc, 1);
+	netsec_shutdown(nsc);
 	nsc = NULL;
     }
 
diff --git a/sbr/netsec.c b/sbr/netsec.c
index 358b97df..dd75bfd5 100644
--- a/sbr/netsec.c
+++ b/sbr/netsec.c
@@ -54,6 +54,7 @@ static SSL_CTX *sslctx = NULL;		/* SSL Context */
 struct _netsec_context {
     int ns_readfd;		/* Read descriptor for network connection */
     int ns_writefd;		/* Write descriptor for network connection */
+    int ns_noclose;		/* Do not close file descriptors if set */
     int ns_snoop;		/* If true, display network data */
     int ns_snoop_noend;		/* If true, didn't get a CR/LF on last line */
     netsec_snoop_callback *ns_snoop_cb; /* Snoop output callback */
@@ -140,6 +141,7 @@ netsec_init(void)
     NEW(nsc);
     nsc->ns_readfd = -1;
     nsc->ns_writefd = -1;
+    nsc->ns_noclose = 0;
     nsc->ns_snoop = 0;
     nsc->ns_snoop_noend = 0;
     nsc->ns_snoop_cb = NULL;
@@ -180,11 +182,10 @@ netsec_init(void)
 
 /*
  * Shutdown the connection completely and free all resources.
- * The connection is only closed if the flag is given.
  */
 
 void
-netsec_shutdown(netsec_context *nsc, int closeflag)
+netsec_shutdown(netsec_context *nsc)
 {
     mh_xfree(nsc->ns_userid);
     mh_xfree(nsc->ns_hostname);
@@ -218,7 +219,7 @@ netsec_shutdown(netsec_context *nsc, int closeflag)
 	BIO_free_all(nsc->ssl_io);
 #endif /* TLS_SUPPORT */
 
-    if (closeflag) {
+    if (! nsc->ns_noclose) {
 	if (nsc->ns_readfd != -1)
 	    close(nsc->ns_readfd);
 	if (nsc->ns_writefd != -1 && nsc->ns_writefd != nsc->ns_readfd)
@@ -1507,7 +1508,7 @@ netsec_set_tls(netsec_context *nsc, int tls, int noverify, char **errstr)
 	 * SSL BIO -> socket BIO.
 	 */
 
-	rbio = BIO_new_socket(nsc->ns_readfd, BIO_NOCLOSE);
+	rbio = BIO_new_socket(nsc->ns_readfd, BIO_CLOSE);
 
 	if (! rbio) {
 	    netsec_err(errstr, "Unable to create a read socket BIO: %s",
@@ -1516,7 +1517,7 @@ netsec_set_tls(netsec_context *nsc, int tls, int noverify, char **errstr)
 	    return NOTOK;
 	}
 
-	wbio = BIO_new_socket(nsc->ns_writefd, BIO_NOCLOSE);
+	wbio = BIO_new_socket(nsc->ns_writefd, BIO_CLOSE);
 
 	if (! wbio) {
 	    netsec_err(errstr, "Unable to create a write socket BIO: %s",
@@ -1577,6 +1578,13 @@ netsec_set_tls(netsec_context *nsc, int tls, int noverify, char **errstr)
 	BIO_set_ssl(ssl_bio, ssl, BIO_CLOSE);
 	nsc->ssl_io = ssl_bio;
 
+	/*
+	 * Since SSL now owns these file descriptors, have it handle the
+	 * closing of them instead of netsec_shutdown().
+	 */
+
+	nsc->ns_noclose = 1;
+
 	return OK;
     }
     BIO_free_all(nsc->ssl_io);
diff --git a/uip/popsbr.c b/uip/popsbr.c
index 14bc9fc1..8d54e021 100644
--- a/uip/popsbr.c
+++ b/uip/popsbr.c
@@ -277,7 +277,7 @@ pop_init (char *host, char *port, char *user, char *proxy, int snoop,
 	case DONE: 
 	    if (poprint)	    
 		fprintf (stderr, "%s\n", response);
-	    netsec_shutdown(nsc, 1);
+	    netsec_shutdown(nsc);
 	    nsc = NULL;
 	    return NOTOK;
     }
@@ -546,7 +546,7 @@ int
 pop_done (void)
 {
     if (nsc)
-	netsec_shutdown(nsc, 1);
+	netsec_shutdown(nsc);
 
     return OK;
 }