]> git.ipfire.org Git - thirdparty/openssh-portable.git/commitdiff
upstream commit
authormarkus@openbsd.org <markus@openbsd.org>
Wed, 31 May 2017 08:09:45 +0000 (08:09 +0000)
committerDamien Miller <djm@mindrot.org>
Thu, 1 Jun 2017 04:54:46 +0000 (14:54 +1000)
clear session keys from memory; ok djm@

Upstream-ID: ecd178819868975affd5fd6637458b7c712b6a0f

monitor.c
monitor_wrap.h
opacket.h
packet.c
packet.h
session.c
sshd.c
umac.c

index 2dee9721de380a42dd5cc574b07989e6878d85ac..ef107a2e815afa5958ad31371ee2e7a185b275df 100644 (file)
--- a/monitor.c
+++ b/monitor.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: monitor.c,v 1.169 2017/05/30 14:10:53 markus Exp $ */
+/* $OpenBSD: monitor.c,v 1.170 2017/05/31 08:09:45 markus Exp $ */
 /*
  * Copyright 2002 Niels Provos <provos@citi.umich.edu>
  * Copyright 2002 Markus Friedl <markus@openbsd.org>
@@ -1583,6 +1583,17 @@ mm_answer_audit_command(int socket, Buffer *m)
 }
 #endif /* SSH_AUDIT_EVENTS */
 
+void
+monitor_clear_keystate(struct monitor *pmonitor)
+{
+       struct ssh *ssh = active_state; /* XXX */
+
+       ssh_clear_newkeys(ssh, MODE_IN);
+       ssh_clear_newkeys(ssh, MODE_OUT);
+       sshbuf_free(child_state);
+       child_state = NULL;
+}
+
 void
 monitor_apply_keystate(struct monitor *pmonitor)
 {
index 958cdbc9235425fef9a335a73f62fce433b09271..9e032d204b40201856a5bf178fb3087e5b20b3f6 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: monitor_wrap.h,v 1.34 2017/05/30 14:10:53 markus Exp $ */
+/* $OpenBSD: monitor_wrap.h,v 1.35 2017/05/31 08:09:45 markus Exp $ */
 
 /*
  * Copyright 2002 Niels Provos <provos@citi.umich.edu>
@@ -34,7 +34,6 @@ extern int use_privsep;
 enum mm_keytype { MM_NOKEY, MM_HOSTKEY, MM_USERKEY };
 
 struct monitor;
-struct mm_master;
 struct Authctxt;
 
 void mm_log_handler(LogLevel, const char *, void *);
@@ -86,6 +85,7 @@ void mm_session_pty_cleanup2(struct Session *);
 struct newkeys *mm_newkeys_from_blob(u_char *, int);
 int mm_newkeys_to_blob(int, u_char **, u_int *);
 
+void monitor_clear_keystate(struct monitor *);
 void monitor_apply_keystate(struct monitor *);
 void mm_get_keystate(struct monitor *);
 void mm_send_keystate(struct monitor*);
index 46d31f805f7d89496be1ff4a0597b7441ad50686..c49d0c04a7904ae2ac849fc005c6ef50e55ca739 100644 (file)
--- a/opacket.h
+++ b/opacket.h
@@ -149,5 +149,7 @@ void        packet_disconnect(const char *, ...)
        ssh_packet_set_mux(active_state)
 #define packet_get_mux() \
        ssh_packet_get_mux(active_state)
+#define packet_clear_keys() \
+       ssh_packet_clear_keys(active_state)
 
 #endif /* _OPACKET_H */
index 46dcc5b5f125bbea4400290d2f4013e0a6a53aaa..862aeb052d596bcb5f328fe8f888153851239ce5 100644 (file)
--- a/packet.c
+++ b/packet.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: packet.c,v 1.256 2017/05/08 06:03:39 djm Exp $ */
+/* $OpenBSD: packet.c,v 1.257 2017/05/31 08:09:45 markus Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -559,8 +559,8 @@ ssh_local_port(struct ssh *ssh)
 
 /* Closes the connection and clears and frees internal data structures. */
 
-void
-ssh_packet_close(struct ssh *ssh)
+static void
+ssh_packet_close_internal(struct ssh *ssh, int do_close)
 {
        struct session_state *state = ssh->state;
        u_int mode;
@@ -568,20 +568,26 @@ ssh_packet_close(struct ssh *ssh)
        if (!state->initialized)
                return;
        state->initialized = 0;
-       if (state->connection_in == state->connection_out) {
-               shutdown(state->connection_out, SHUT_RDWR);
-               close(state->connection_out);
-       } else {
-               close(state->connection_in);
-               close(state->connection_out);
+       if (do_close) {
+               if (state->connection_in == state->connection_out) {
+                       shutdown(state->connection_out, SHUT_RDWR);
+                       close(state->connection_out);
+               } else {
+                       close(state->connection_in);
+                       close(state->connection_out);
+               }
        }
        sshbuf_free(state->input);
        sshbuf_free(state->output);
        sshbuf_free(state->outgoing_packet);
        sshbuf_free(state->incoming_packet);
-       for (mode = 0; mode < MODE_MAX; mode++)
-               kex_free_newkeys(state->newkeys[mode]);
-       if (state->compression_buffer) {
+       for (mode = 0; mode < MODE_MAX; mode++) {
+               kex_free_newkeys(state->newkeys[mode]); /* current keys */
+               state->newkeys[mode] = NULL;
+               ssh_clear_newkeys(ssh, mode);           /* next keys */
+       }
+       /* comression state is in shared mem, so we can only release it once */
+       if (do_close && state->compression_buffer) {
                sshbuf_free(state->compression_buffer);
                if (state->compression_out_started) {
                        z_streamp stream = &state->compression_out_stream;
@@ -609,10 +615,24 @@ ssh_packet_close(struct ssh *ssh)
        cipher_free(state->send_context);
        cipher_free(state->receive_context);
        state->send_context = state->receive_context = NULL;
-       free(ssh->remote_ipaddr);
-       ssh->remote_ipaddr = NULL;
-       free(ssh->state);
-       ssh->state = NULL;
+       if (do_close) {
+               free(ssh->remote_ipaddr);
+               ssh->remote_ipaddr = NULL;
+               free(ssh->state);
+               ssh->state = NULL;
+       }
+}
+
+void
+ssh_packet_close(struct ssh *ssh)
+{
+       ssh_packet_close_internal(ssh, 1);
+}
+
+void
+ssh_packet_clear_keys(struct ssh *ssh)
+{
+       ssh_packet_close_internal(ssh, 0);
 }
 
 /* Sets remote side protocol flags. */
@@ -791,6 +811,15 @@ uncompress_buffer(struct ssh *ssh, struct sshbuf *in, struct sshbuf *out)
        /* NOTREACHED */
 }
 
+void
+ssh_clear_newkeys(struct ssh *ssh, int mode)
+{
+       if (ssh->kex && ssh->kex->newkeys) {
+               kex_free_newkeys(ssh->kex->newkeys[mode]);
+               ssh->kex->newkeys[mode] = NULL;
+       }
+}
+
 int
 ssh_set_newkeys(struct ssh *ssh, int mode)
 {
@@ -820,26 +849,16 @@ ssh_set_newkeys(struct ssh *ssh, int mode)
                max_blocks = &state->max_blocks_in;
        }
        if (state->newkeys[mode] != NULL) {
-               debug("%s: rekeying after %llu %s blocks"
-                   " (%llu bytes total)", __func__,
-                   (unsigned long long)ps->blocks, dir,
-                   (unsigned long long)ps->bytes);
+               debug("set_newkeys: rekeying, input %llu bytes %llu blocks, "
+                  "output %llu bytes %llu blocks",
+                  (unsigned long long)state->p_read.bytes,
+                  (unsigned long long)state->p_read.blocks,
+                  (unsigned long long)state->p_send.bytes,
+                  (unsigned long long)state->p_send.blocks);
                cipher_free(*ccp);
                *ccp = NULL;
-               enc  = &state->newkeys[mode]->enc;
-               mac  = &state->newkeys[mode]->mac;
-               comp = &state->newkeys[mode]->comp;
-               mac_clear(mac);
-               explicit_bzero(enc->iv,  enc->iv_len);
-               explicit_bzero(enc->key, enc->key_len);
-               explicit_bzero(mac->key, mac->key_len);
-               free(enc->name);
-               free(enc->iv);
-               free(enc->key);
-               free(mac->name);
-               free(mac->key);
-               free(comp->name);
-               free(state->newkeys[mode]);
+               kex_free_newkeys(state->newkeys[mode]);
+               state->newkeys[mode] = NULL;
        }
        /* note that both bytes and the seqnr are not reset */
        ps->packets = ps->blocks = 0;
@@ -1784,15 +1803,20 @@ sshpkt_fatal(struct ssh *ssh, const char *tag, int r)
 
        switch (r) {
        case SSH_ERR_CONN_CLOSED:
+               ssh_packet_clear_keys(ssh);
                logdie("Connection closed by %s", remote_id);
        case SSH_ERR_CONN_TIMEOUT:
+               ssh_packet_clear_keys(ssh);
                logdie("Connection %s %s timed out",
                    ssh->state->server_side ? "from" : "to", remote_id);
        case SSH_ERR_DISCONNECTED:
+               ssh_packet_clear_keys(ssh);
                logdie("Disconnected from %s", remote_id);
        case SSH_ERR_SYSTEM_ERROR:
-               if (errno == ECONNRESET)
+               if (errno == ECONNRESET) {
+                       ssh_packet_clear_keys(ssh);
                        logdie("Connection reset by %s", remote_id);
+               }
                /* FALLTHROUGH */
        case SSH_ERR_NO_CIPHER_ALG_MATCH:
        case SSH_ERR_NO_MAC_ALG_MATCH:
@@ -1800,12 +1824,14 @@ sshpkt_fatal(struct ssh *ssh, const char *tag, int r)
        case SSH_ERR_NO_KEX_ALG_MATCH:
        case SSH_ERR_NO_HOSTKEY_ALG_MATCH:
                if (ssh && ssh->kex && ssh->kex->failed_choice) {
+                       ssh_packet_clear_keys(ssh);
                        logdie("Unable to negotiate with %s: %s. "
                            "Their offer: %s", remote_id, ssh_err(r),
                            ssh->kex->failed_choice);
                }
                /* FALLTHROUGH */
        default:
+               ssh_packet_clear_keys(ssh);
                logdie("%s%sConnection %s %s: %s",
                    tag != NULL ? tag : "", tag != NULL ? ": " : "",
                    ssh->state->server_side ? "from" : "to",
index 2b8069cd6b4de8c95d7216561fe4dedb6775470d..6ce6dd56078b10e8dae76594ca91e9461ee49a82 100644 (file)
--- a/packet.h
+++ b/packet.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: packet.h,v 1.80 2017/05/30 14:18:15 markus Exp $ */
+/* $OpenBSD: packet.h,v 1.81 2017/05/31 08:09:45 markus Exp $ */
 
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
@@ -97,6 +97,8 @@ int      ssh_packet_get_connection_in(struct ssh *);
 int      ssh_packet_get_connection_out(struct ssh *);
 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_clear_newkeys(struct ssh *, int);
 
 int     ssh_packet_is_rekeying(struct ssh *);
 void     ssh_packet_set_protocol_flags(struct ssh *, u_int);
index a08aa69d1679bf5b8278a055ac870da8897fb888..cbd27c6896b4a4ede7c1d22575b9959a61ddb086 100644 (file)
--- a/session.c
+++ b/session.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: session.c,v 1.286 2016/11/30 03:00:05 djm Exp $ */
+/* $OpenBSD: session.c,v 1.287 2017/05/31 08:09:45 markus Exp $ */
 /*
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
  *                    All rights reserved
@@ -1486,6 +1486,7 @@ do_child(Session *s, const char *command)
 
        /* remove hostkey from the child's memory */
        destroy_sensitive_data();
+       packet_clear_keys();
 
        /* Force a password change */
        if (s->authctxt->force_pwchange) {
diff --git a/sshd.c b/sshd.c
index aa3729e7d365847f0fb9b8d006355fc769330df9..06cb81f277a7c2769078b3f8fa54464ae8e80d78 100644 (file)
--- a/sshd.c
+++ b/sshd.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshd.c,v 1.489 2017/05/31 07:00:13 markus Exp $ */
+/* $OpenBSD: sshd.c,v 1.490 2017/05/31 08:09:45 markus Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -649,6 +649,7 @@ privsep_postauth(Authctxt *authctxt)
        else if (pmonitor->m_pid != 0) {
                verbose("User child is on pid %ld", (long)pmonitor->m_pid);
                buffer_clear(&loginmsg);
+               monitor_clear_keystate(pmonitor);
                monitor_child_postauth(pmonitor);
 
                /* NEVERREACHED */
@@ -2032,6 +2033,7 @@ main(int ac, char **av)
         */
        if (use_privsep) {
                mm_send_keystate(pmonitor);
+               packet_clear_keys();
                exit(0);
        }
 
diff --git a/umac.c b/umac.c
index 6eb55b26ef7f7d040782180818119377954a392b..9f2187c9af9811958c1c8f2d8fa86e96d2e52d24 100644 (file)
--- a/umac.c
+++ b/umac.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: umac.c,v 1.11 2014/07/22 07:13:42 guenther Exp $ */
+/* $OpenBSD: umac.c,v 1.12 2017/05/31 08:09:45 markus Exp $ */
 /* -----------------------------------------------------------------------
  * 
  * umac.c -- C Implementation UMAC Message Authentication
@@ -203,6 +203,8 @@ static void kdf(void *bufp, aes_int_key key, UINT8 ndx, int nbytes)
         aes_encryption(in_buf, out_buf, key);
         memcpy(dst_buf,out_buf,nbytes);
     }
+    explicit_bzero(in_buf, sizeof(in_buf));
+    explicit_bzero(out_buf, sizeof(out_buf));
 }
 
 /* The final UHASH result is XOR'd with the output of a pseudorandom
@@ -227,6 +229,7 @@ static void pdf_init(pdf_ctx *pc, aes_int_key prf_key)
     /* Initialize pdf and cache */
     memset(pc->nonce, 0, sizeof(pc->nonce));
     aes_encryption(pc->nonce, pc->cache, pc->prf_key);
+    explicit_bzero(buf, sizeof(buf));
 }
 
 static void pdf_gen_xor(pdf_ctx *pc, const UINT8 nonce[8], UINT8 buf[8])
@@ -991,6 +994,7 @@ static void uhash_init(uhash_ctx_t ahc, aes_int_key prf_key)
     kdf(ahc->ip_trans, prf_key, 4, STREAMS * sizeof(UINT32));
     endian_convert_if_le(ahc->ip_trans, sizeof(UINT32),
                          STREAMS * sizeof(UINT32));
+    explicit_bzero(buf, sizeof(buf));
 }
 
 /* ---------------------------------------------------------------------- */
@@ -1200,6 +1204,7 @@ int umac_delete(struct umac_ctx *ctx)
     if (ctx) {
         if (ALLOC_BOUNDARY)
             ctx = (struct umac_ctx *)ctx->free_ptr;
+        explicit_bzero(ctx, sizeof(*ctx) + ALLOC_BOUNDARY);
         free(ctx);
     }
     return (1);
@@ -1227,6 +1232,7 @@ struct umac_ctx *umac_new(const u_char key[])
         aes_key_setup(key, prf_key);
         pdf_init(&ctx->pdf, prf_key);
         uhash_init(&ctx->hash, prf_key);
+        explicit_bzero(prf_key, sizeof(prf_key));
     }
         
     return (ctx);