changeset 12696:3dfea19a9dc1

6961070 kssl should send close_notify on socket shutdown 6949092 create_instance_name() leaks instance_name
author Vladimir Kotal <Vladimir.Kotal@Sun.COM>
date Fri, 25 Jun 2010 21:50:45 +0200
parents b259c8945841
children 794116a9929f
files usr/src/cmd/cmd-inet/usr.sbin/kssl/ksslcfg/ksslcfg.c usr/src/uts/common/inet/kssl/ksslapi.c usr/src/uts/common/inet/kssl/ksslfilter.c usr/src/uts/common/inet/kssl/ksslproto.h usr/src/uts/common/inet/kssl/ksslrec.c
diffstat 5 files changed, 101 insertions(+), 18 deletions(-) [+]
line wrap: on
line diff
--- a/usr/src/cmd/cmd-inet/usr.sbin/kssl/ksslcfg/ksslcfg.c	Fri Jun 25 12:47:24 2010 -0700
+++ b/usr/src/cmd/cmd-inet/usr.sbin/kssl/ksslcfg/ksslcfg.c	Fri Jun 25 21:50:45 2010 +0200
@@ -202,6 +202,7 @@
 		if ((*inaddr_any_name = malloc(len)) == NULL) {
 			(void) fprintf(stderr,
 			    gettext("Error: memory allocation failure.\n"));
+			free(instance_name);
 			free(cname);
 			return (NULL);
 		}
--- a/usr/src/uts/common/inet/kssl/ksslapi.c	Fri Jun 25 12:47:24 2010 -0700
+++ b/usr/src/uts/common/inet/kssl/ksslapi.c	Fri Jun 25 21:50:45 2010 +0200
@@ -345,7 +345,7 @@
 
 	mutex_enter(&ssl->kssl_lock);
 
-	if (ssl->close_notify == B_TRUE) {
+	if (ssl->close_notify_clnt == B_TRUE) {
 		DTRACE_PROBE(kssl_err__close_notify);
 		goto sendnewalert;
 	}
@@ -1052,7 +1052,7 @@
 				error = EBADMSG;
 				goto error;
 			} else {
-				ssl->close_notify = B_TRUE;
+				ssl->close_notify_clnt = B_TRUE;
 				ssl->activeinput = B_FALSE;
 				freeb(mp);
 				return (KSSL_CMD_NONE);
@@ -1187,7 +1187,31 @@
 	mblk_t *retmp = mp, *bp = mp, *prevbp = mp, *copybp;
 
 	ASSERT(ssl != NULL);
+
+	/*
+	 * Produce new close_notify message. This is necessary to perform
+	 * proper cleanup w.r.t. SSL protocol spec by sending close_notify SSL
+	 * alert record if running with KSSL proxy.
+	 * This should be done prior to sending the FIN so the client side can
+	 * attempt to do graceful cleanup. Ideally, we should wait for client's
+	 * close_notify but not all clients send it which would hang the
+	 * connection. This way of closing the SSL session (Incomplete Close)
+	 * prevents truncation attacks for protocols without end-of-data
+	 * markers (as opposed to the Premature Close).
+	 * Checking the close_notify_srvr flag will prevent from sending the
+	 * close_notify message twice in case of duplicate shutdown() calls.
+	 */
+	if (mp == NULL && !ssl->close_notify_srvr) {
+		kssl_send_alert(ssl, alert_warning, close_notify);
+		if (ssl->alert_sendbuf == NULL)
+			return (NULL);
+		mp = bp = retmp = prevbp = ssl->alert_sendbuf;
+		ssl->alert_sendbuf = NULL;
+		ssl->close_notify_srvr = B_TRUE;
+	}
+
 	ASSERT(mp != NULL);
+	ASSERT(bp != NULL);
 
 	do {
 		if (DB_REF(bp) > 1) {
@@ -1220,8 +1244,14 @@
 }
 
 /*
- * Builds a single SSL record.
- * In-line encryption of the record.
+ * Builds a single SSL record by prepending SSL header (optional) and performing
+ * encryption and MAC. The encryption of the record is done in-line.
+ * Expects an mblk with associated dblk's base to have space for the SSL header
+ * or an mblk which already has the header present. In both cases it presumes
+ * that the mblk's dblk limit has space for the MAC + padding.
+ * If the close_notify_srvr flag is set it is presumed that the mblk already
+ * contains SSL header in which case only the record length field will be
+ * adjusted with the MAC/padding size.
  */
 static kssl_status_t
 kssl_build_single_record(ssl_t *ssl, mblk_t *mp)
@@ -1237,19 +1267,28 @@
 	mac_sz = spec->mac_hashsz;
 
 	ASSERT(DB_REF(mp) == 1);
-	ASSERT((mp->b_rptr - mp->b_datap->db_base >= SSL3_HDR_LEN) &&
-	    (mp->b_datap->db_lim - mp->b_wptr >= mac_sz + spec->cipher_bsize));
+	/* The dblk must always have space for the padding and MAC suffix. */
+	ASSERT(mp->b_datap->db_lim - mp->b_wptr >= mac_sz + spec->cipher_bsize);
 
-	len = MBLKL(mp);
+	/* kssl_send_alert() constructs the SSL header by itself. */
+	if (!ssl->close_notify_srvr)
+		len = MBLKL(mp) - SSL3_HDR_LEN;
+	else
+		len = MBLKL(mp);
 
 	ASSERT(len > 0);
 
 	mutex_enter(&ssl->kssl_lock);
 
-	recstart = mp->b_rptr = mp->b_rptr - SSL3_HDR_LEN;
-	recstart[0] = content_application_data;
-	recstart[1] = ssl->major_version;
-	recstart[2] = ssl->minor_version;
+	recstart = mp->b_rptr;
+	if (!ssl->close_notify_srvr) {
+		/* The dblk must have space for the SSL header prefix. */
+		ASSERT(mp->b_rptr - mp->b_datap->db_base >= SSL3_HDR_LEN);
+		recstart = mp->b_rptr = mp->b_rptr - SSL3_HDR_LEN;
+		recstart[0] = content_application_data;
+		recstart[1] = ssl->major_version;
+		recstart[2] = ssl->minor_version;
+	}
 	versionp = &recstart[1];
 
 	reclen = len + mac_sz;
@@ -1263,14 +1302,16 @@
 	recstart[3] = (reclen >> 8) & 0xff;
 	recstart[4] = reclen & 0xff;
 
-	if (kssl_mac_encrypt_record(ssl, content_application_data, versionp,
+	if (kssl_mac_encrypt_record(ssl, recstart[0], versionp,
 	    recstart, mp) != 0) {
 		/* Do we need an internal_error Alert here? */
 		mutex_exit(&ssl->kssl_lock);
 		return (KSSL_STS_ERR);
 	}
 
-	KSSL_COUNTER(appdata_record_outs, 1);
+	/* Alert messages are accounted in kssl_send_alert(). */
+	if (recstart[0] == content_application_data)
+		KSSL_COUNTER(appdata_record_outs, 1);
 	mutex_exit(&ssl->kssl_lock);
 	return (KSSL_STS_OK);
 }
--- a/usr/src/uts/common/inet/kssl/ksslfilter.c	Fri Jun 25 12:47:24 2010 -0700
+++ b/usr/src/uts/common/inet/kssl/ksslfilter.c	Fri Jun 25 21:50:45 2010 +0200
@@ -306,6 +306,42 @@
 }
 
 /*
+ * Called from shutdown() processing. This will produce close_notify message
+ * to indicate the end of data to the client.
+ */
+sof_rval_t
+kssl_shutdown_cb(sof_handle_t handle, void *cookie, int *howp, cred_t *cr)
+{
+	ksslf_t *kssl = (ksslf_t *)cookie;
+	kssl_ctx_t kssl_ctx = kssl->ksslf_ctx;
+	mblk_t *outmp;
+	boolean_t flowctrld;
+	struct nmsghdr msg;
+
+	_NOTE(ARGUNUSED(cr));
+
+	if (kssl_ctx == NULL)
+		return (SOF_RVAL_EINVAL);
+
+	/*
+	 * We only want to send close_notify when doing SHUT_WR/SHUT_RDWR
+	 * because it signals that server is done writing data.
+	 */
+	if (*howp == SHUT_RD)
+		return (SOF_RVAL_CONTINUE);
+
+	/* Go on if we fail to build the record. */
+	if ((outmp = kssl_build_record(kssl_ctx, NULL)) == NULL)
+		return (SOF_RVAL_CONTINUE);
+
+	bzero(&msg, sizeof (msg));
+	(void) sof_inject_data_out(handle, outmp, &msg,
+	    &flowctrld);
+
+	return (SOF_RVAL_CONTINUE);
+}
+
+/*
  * Called for each incoming segment.
  *
  * A packet may carry multiple SSL records, so the function calls
@@ -544,6 +580,7 @@
 	.sofop_data_out = kssl_data_out_cb,
 	.sofop_mblk_prop = kssl_mblk_prop_cb,
 	.sofop_getsockname = kssl_getsockname_cb,
+	.sofop_shutdown = kssl_shutdown_cb,
 };
 
 int
--- a/usr/src/uts/common/inet/kssl/ksslproto.h	Fri Jun 25 12:47:24 2010 -0700
+++ b/usr/src/uts/common/inet/kssl/ksslproto.h	Fri Jun 25 21:50:45 2010 +0200
@@ -42,6 +42,7 @@
 #define	SSL3_RANDOM_LENGTH		32
 #define	SSL3_SESSIONID_BYTES		32
 #define	SSL3_HDR_LEN			5
+#define	SSL3_ALERT_LEN			2
 #define	SSL3_MAX_RECORD_LENGTH		16384
 #define	SSL3_PRE_MASTER_SECRET_LEN	48
 #define	SSL3_MASTER_SECRET_LEN		48
@@ -286,7 +287,8 @@
 	uint32_t		tcp_mss;
 	SSL3WaitState		hs_waitstate;
 	boolean_t		resumed;
-	boolean_t		close_notify;
+	boolean_t		close_notify_clnt;
+	boolean_t		close_notify_srvr;
 	boolean_t		fatal_alert;
 	boolean_t		fatal_error;
 	boolean_t		alert_sent;
--- a/usr/src/uts/common/inet/kssl/ksslrec.c	Fri Jun 25 12:47:24 2010 -0700
+++ b/usr/src/uts/common/inet/kssl/ksslrec.c	Fri Jun 25 21:50:45 2010 +0200
@@ -1796,10 +1796,12 @@
 	spec = &ssl->spec[KSSL_WRITE];
 
 	ASSERT(ssl->alert_sendbuf == NULL);
-	if (ssl->major_version == 0x03)
-		len = 7;
-	else
+	if (ssl->major_version == 0x03) {
+		len = SSL3_HDR_LEN + SSL3_ALERT_LEN;
+	} else {
+		/* KSSL generates 5 byte SSLv2 alert messages only. */
 		len = 5;
+	}
 	ssl->alert_sendbuf = mp = allocb(len + spec->mac_hashsz +
 	    spec->cipher_bsize, BPRI_HI);
 	if (mp == NULL) {
@@ -1821,7 +1823,7 @@
 		/* alert contents */
 		buf[0] = (uchar_t)level;
 		buf[1] = (uchar_t)desc;
-		buf += 2;
+		buf += SSL3_ALERT_LEN;
 	} else {
 	/* SSLv2 has different encoding. */
 		/* 2-byte encoding of the length */