Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
openSUSE
openssh-askpass-gnome.28809
openssh-mitigate-lingering-secrets.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File openssh-mitigate-lingering-secrets.patch of Package openssh-askpass-gnome.28809
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 --- kex.c | 8 ++++---- mac.c | 4 ++-- monitor.c | 8 ++++++-- packet.c | 49 +++++++++++++++++++++++++++++++++++++++---------- packet.h | 1 + sshbuf.c | 25 +++++++++++++++++++++++++ sshbuf.h | 3 +++ sshd.c | 25 ++++++++++++++++++++++++- 8 files changed, 104 insertions(+), 19 deletions(-) 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
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
