File openssh-mitigate-lingering-secrets.patch of Package openssh-askpass-gnome

Overview Repositories Revisions Requests Users Attributes Meta

File openssh-mitigate-lingering-secrets.patch of Package openssh-askpass-gnome (Revision 2ac4759244768ea03d7fa8609a52ee10)

Currently displaying revision 2ac4759244768ea03d7fa8609a52ee10 , Show latest

From ebd24c765804ad0b70426f7298d7f6c8daa7038b Mon Sep 17 00:00:00 2001
From: Hans Petter Jansson <hpj@hpjansson.org>
Date: Mon, 7 Jun 2021 22:12:05 +0200
Subject: [PATCH] Add mitigations for secrets potentially lingering in memory

diff --git a/kex.c b/kex.c
index 7cd37d6..ae1a9ba 100644
--- a/kex.c
+++ b/kex.c
@@ -1394,16 +1394,16 @@ enc_destroy(struct sshenc *enc)
 		return;
 
 	if (enc->key) {
-		memset(enc->key, 0, enc->key_len);
+		explicit_bzero(enc->key, enc->key_len);
 		free(enc->key);
 	}
 
 	if (enc->iv) {
-		memset(enc->iv,  0, enc->iv_len);
+		explicit_bzero(enc->iv,  enc->iv_len);
 		free(enc->iv);
 	}
 
-	memset(enc, 0, sizeof(*enc));
+	explicit_bzero(enc, sizeof(*enc));
 }
 
 void
@@ -1414,7 +1414,7 @@ newkeys_destroy(struct newkeys *newkeys)
 
 	enc_destroy(&newkeys->enc);
 	mac_destroy(&newkeys->mac);
-	memset(&newkeys->comp, 0, sizeof(newkeys->comp));
+	explicit_bzero(&newkeys->comp, sizeof(newkeys->comp));
 }
 
 /*
diff --git a/mac.c b/mac.c
index 6d87a80..4f9d636 100644
--- a/mac.c
+++ b/mac.c
@@ -284,11 +284,11 @@ mac_destroy(struct sshmac *mac)
 		return;
 
 	if (mac->key) {
-		memset(mac->key, 0, mac->key_len);
+		explicit_bzero(mac->key, mac->key_len);
 		free(mac->key);
 	}
 
-	memset(mac, 0, sizeof(*mac));
+	explicit_bzero(mac, sizeof(*mac));
 }
 
 /* XXX copied from ciphers_valid */
diff --git a/monitor.c b/monitor.c
index 2e421cf..4845a67 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1740,8 +1740,12 @@ mm_answer_audit_end_command(struct ssh *ssh, int socket, struct sshbuf *m)
 void
 monitor_clear_keystate(struct ssh *ssh, struct monitor *pmonitor)
 {
-	ssh_clear_newkeys(ssh, MODE_IN);
-	ssh_clear_newkeys(ssh, MODE_OUT);
+	u_int mode;
+
+	for (mode = 0; mode < MODE_MAX; mode++) {
+		ssh_clear_curkeys(ssh, mode);  /* current keys */
+		ssh_clear_newkeys(ssh, mode);  /* next keys */
+	}
 	sshbuf_free(child_state);
 	child_state = NULL;
 }
diff --git a/packet.c b/packet.c
index 2cc3913..7a69675 100644
--- a/packet.c
+++ b/packet.c
@@ -656,6 +656,7 @@ ssh_packet_close_internal(struct ssh *ssh, int do_close, int do_audit)
 		ssh->local_ipaddr = NULL;
 		free(ssh->remote_ipaddr);
 		ssh->remote_ipaddr = NULL;
+		explicit_bzero(ssh->state, sizeof(*ssh->state));
 		free(ssh->state);
 		ssh->state = NULL;
 	}
@@ -781,8 +782,10 @@ compress_buffer(struct ssh *ssh, struct sshbuf *in, struct sshbuf *out)
 		case Z_OK:
 			/* Append compressed data to output_buffer. */
 			if ((r = sshbuf_put(out, buf, sizeof(buf) -
-			    ssh->state->compression_out_stream.avail_out)) != 0)
+			    ssh->state->compression_out_stream.avail_out)) != 0) {
+				explicit_bzero(buf, sizeof(buf));
 				return r;
+			}
 			break;
 		case Z_STREAM_ERROR:
 		default:
@@ -817,8 +820,10 @@ uncompress_buffer(struct ssh *ssh, struct sshbuf *in, struct sshbuf *out)
 		switch (status) {
 		case Z_OK:
 			if ((r = sshbuf_put(out, buf, sizeof(buf) -
-			    ssh->state->compression_in_stream.avail_out)) != 0)
+			    ssh->state->compression_in_stream.avail_out)) != 0) {
+				explicit_bzero(buf, sizeof(buf));
 				return r;
+			}
 			break;
 		case Z_BUF_ERROR:
 			/*
@@ -840,6 +845,17 @@ uncompress_buffer(struct ssh *ssh, struct sshbuf *in, struct sshbuf *out)
 	/* NOTREACHED */
 }
 
+void
+ssh_clear_curkeys(struct ssh *ssh, int mode)
+{
+	struct session_state *state = ssh->state;
+
+	if (state && state->newkeys[mode]) {
+		kex_free_newkeys(state->newkeys[mode]);
+		state->newkeys[mode] = NULL;
+	}
+}
+
 void
 ssh_clear_newkeys(struct ssh *ssh, int mode)
 {
@@ -1391,6 +1407,7 @@ ssh_packet_read_seqnr(struct ssh *ssh, u_char *typep, u_int32_t *seqnr_p)
 			goto out;
 	}
  out:
+	explicit_bzero(buf, sizeof (buf));
 	free(setp);
 	return r;
 }
@@ -2359,9 +2376,12 @@ ssh_packet_get_state(struct ssh *ssh, struct sshbuf *m)
 	    (r = sshbuf_put_u32(m, state->p_read.packets)) != 0 ||
 	    (r = sshbuf_put_u64(m, state->p_read.bytes)) != 0 ||
 	    (r = sshbuf_put_stringb(m, state->input)) != 0 ||
-	    (r = sshbuf_put_stringb(m, state->output)) != 0)
+	    (r = sshbuf_put_stringb(m, state->output)) != 0) {
+		sshbuf_obfuscate(m);
 		return r;
+	}
 
+	sshbuf_obfuscate(m);
 	return 0;
 }
 
@@ -2480,6 +2500,8 @@ ssh_packet_set_state(struct ssh *ssh, struct sshbuf *m)
 	size_t ilen, olen;
 	int r;
 
+	sshbuf_unobfuscate(m);
+
 	if ((r = kex_from_blob(m, &ssh->kex)) != 0 ||
 	    (r = newkeys_from_blob(m, ssh, MODE_OUT)) != 0 ||
 	    (r = newkeys_from_blob(m, ssh, MODE_IN)) != 0 ||
@@ -2493,7 +2515,7 @@ ssh_packet_set_state(struct ssh *ssh, struct sshbuf *m)
 	    (r = sshbuf_get_u64(m, &state->p_read.blocks)) != 0 ||
 	    (r = sshbuf_get_u32(m, &state->p_read.packets)) != 0 ||
 	    (r = sshbuf_get_u64(m, &state->p_read.bytes)) != 0)
-		return r;
+		goto out;
 	/*
 	 * We set the time here so that in post-auth privsep slave we
 	 * count from the completion of the authentication.
@@ -2502,10 +2524,10 @@ ssh_packet_set_state(struct ssh *ssh, struct sshbuf *m)
 	/* XXX ssh_set_newkeys overrides p_read.packets? XXX */
 	if ((r = ssh_set_newkeys(ssh, MODE_IN)) != 0 ||
 	    (r = ssh_set_newkeys(ssh, MODE_OUT)) != 0)
-		return r;
+		goto out;
 
 	if ((r = ssh_packet_set_postauth(ssh)) != 0)
-		return r;
+		goto out;
 
 	sshbuf_reset(state->input);
 	sshbuf_reset(state->output);
@@ -2513,12 +2535,19 @@ ssh_packet_set_state(struct ssh *ssh, struct sshbuf *m)
 	    (r = sshbuf_get_string_direct(m, &output, &olen)) != 0 ||
 	    (r = sshbuf_put(state->input, input, ilen)) != 0 ||
 	    (r = sshbuf_put(state->output, output, olen)) != 0)
-		return r;
+		goto out;
 
-	if (sshbuf_len(m))
-		return SSH_ERR_INVALID_FORMAT;
+	if (sshbuf_len(m)) {
+		r = SSH_ERR_INVALID_FORMAT;
+		goto out;
+	}
 	debug3("%s: done", __func__);
-	return 0;
+
+	r = 0;
+out:
+	if (r != 0)
+		sshbuf_obfuscate(m);
+	return r;
 }
 
 /* NEW API */
diff --git a/packet.h b/packet.h
index 9384734..17baf13 100644
--- a/packet.h
+++ b/packet.h
@@ -103,6 +103,7 @@ void     ssh_packet_close(struct ssh *);
 void	 ssh_packet_set_input_hook(struct ssh *, ssh_packet_hook_fn *, void *);
 void	 ssh_packet_clear_keys(struct ssh *);
 void     ssh_packet_clear_keys_noaudit(struct ssh *);
+void	 ssh_clear_curkeys(struct ssh *, int);
 void	 ssh_clear_newkeys(struct ssh *, int);
 
 int	 ssh_packet_is_rekeying(struct ssh *);
diff --git a/sshbuf.c b/sshbuf.c
index adfddf7..df88724 100644
--- a/sshbuf.c
+++ b/sshbuf.c
@@ -284,6 +284,31 @@ sshbuf_mutable_ptr(const struct sshbuf *buf)
 	return buf->d + buf->off;
 }
 
+/* Trivially obfuscate the buffer. This is used to make sensitive data
+ * (e.g. keystate) slightly less obvious if found lingering in kernel
+ * memory after being sent from the privsep child to its parent.
+ *
+ * Longer term we should consider using a one-time pad or a stream cipher
+ * here. */
+void
+sshbuf_obfuscate(struct sshbuf *buf)
+{
+	size_t i;
+
+	if (sshbuf_check_sanity(buf) != 0 || buf->readonly || buf->refcount > 1)
+		return;
+
+	for (i = buf->off; i < buf->size; i++) {
+		buf->d [i] ^= 0xaa;
+	}
+}
+
+void
+sshbuf_unobfuscate(struct sshbuf *buf)
+{
+	sshbuf_obfuscate(buf);
+}
+
 int
 sshbuf_check_reserve(const struct sshbuf *buf, size_t len)
 {
diff --git a/sshbuf.h b/sshbuf.h
index ebd64b1..54b16e3 100644
--- a/sshbuf.h
+++ b/sshbuf.h
@@ -291,6 +291,9 @@ sshbuf_find(const struct sshbuf *b, size_t start_offset,
  */
 char *sshbuf_dup_string(struct sshbuf *buf);
 
+void sshbuf_obfuscate(struct sshbuf *buf);
+void sshbuf_unobfuscate(struct sshbuf *buf);
+
 /* Macros for decoding/encoding integers */
 #define PEEK_U64(p) \
 	(((u_int64_t)(((const u_char *)(p))[0]) << 56) | \
diff --git a/sshd.c b/sshd.c
index dca7b1e..8a72e72 100644
--- a/sshd.c
+++ b/sshd.c
@@ -277,6 +277,19 @@ void destroy_sensitive_data(struct ssh *, int);
 void demote_sensitive_data(struct ssh *);
 static void do_ssh2_kex(struct ssh *);
 
+/*
+ * Clear some stack space. This is a bit naive, but hopefully helps mitigate
+ * information leaks due to registers and other data having been stored on
+ * the stack. Called after fork() and before exit().
+ */
+static void
+clobber_stack(void)
+{
+	char data [32768];
+
+	explicit_bzero(data, 32768);
+}
+
 /*
  * Close all listening sockets
  */
@@ -448,6 +461,8 @@ destroy_sensitive_data(struct ssh *ssh, int privsep)
 			sensitive_data.host_certificates[i] = NULL;
 		}
 	}
+
+	clobber_stack();
 }
 
 /* Demote private to public keys for network child */
@@ -620,6 +635,8 @@ privsep_preauth(struct ssh *ssh)
 static void
 privsep_postauth(struct ssh *ssh, Authctxt *authctxt)
 {
+	clobber_stack();
+
 #ifdef DISABLE_FD_PASSING
 	if (1) {
 #else
@@ -2215,6 +2232,7 @@ main(int ac, char **av)
 	if (use_privsep) {
 		mm_send_keystate(ssh, pmonitor);
 		ssh_packet_clear_keys(ssh);
+		clobber_stack();
 		exit(0);
 	}
 
@@ -2291,6 +2309,7 @@ main(int ac, char **av)
 	if (use_privsep)
 		mm_terminate();
 
+	clobber_stack();
 	exit(0);
 }
 
@@ -2458,8 +2477,10 @@ cleanup_exit(int i)
 	/* cleanup_exit can be called at the very least from the privsep
 	   wrappers used for auditing.  Make sure we don't recurse
 	   indefinitely. */
-	if (in_cleanup)
+	if (in_cleanup) {
+		clobber_stack();
 		_exit(i);
+	}
 	in_cleanup = 1;
 	if (the_active_state != NULL && the_authctxt != NULL) {
 		do_cleanup(the_active_state, the_authctxt);
@@ -2484,5 +2505,7 @@ cleanup_exit(int i)
 	    (!use_privsep || mm_is_monitor()))
 		audit_event(the_active_state, SSH_CONNECTION_ABANDON);
 #endif
+
+	clobber_stack();
 	_exit(i);
 }
-- 
2.29.2

Places

File openssh-mitigate-lingering-secrets.patch of Package openssh-askpass-gnome (Revision 2ac4759244768ea03d7fa8609a52ee10)

Places