File openssh-7.6p1-audit_race_condition.patch of Package openssh.38179
monitor_wrap.c | 46 ++++++++++++++++++++++++++++++++++++++++
monitor_wrap.h | 2 +
packet.c | 14 ++++++++----
packet.h | 1
session.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++--------
5 files changed, 116 insertions(+), 12 deletions(-)
Index: openssh-8.4p1/monitor_wrap.c
===================================================================
--- openssh-8.4p1.orig/monitor_wrap.c
+++ openssh-8.4p1/monitor_wrap.c
@@ -1187,4 +1187,50 @@ mm_audit_destroy_sensitive_data(struct s
mm_request_send(pmonitor->m_recvfd, MONITOR_REQ_AUDIT_SERVER_KEY_FREE, m);
sshbuf_free(m);
}
+
+int mm_forward_audit_messages(int fdin)
+{
+ u_char buf[4];
+ u_int blen, msg_len;
+ struct sshbuf *m;
+ int r, ret = 0;
+
+ debug3("%s: entering", __func__);
+ if ((m = sshbuf_new()) == NULL)
+ fatal("%s: sshbuf_new failed", __func__);
+ do {
+ blen = atomicio(read, fdin, buf, sizeof(buf));
+ if (blen == 0) /* closed pipe */
+ break;
+ if (blen != sizeof(buf)) {
+ error("%s: Failed to read the buffer from child", __func__);
+ ret = -1;
+ break;
+ }
+
+ msg_len = get_u32(buf);
+ if (msg_len > 256 * 1024)
+ fatal("%s: read: bad msg_len %d", __func__, msg_len);
+ sshbuf_reset(m);
+ if ((r = sshbuf_reserve(m, msg_len, NULL)) != 0)
+ fatal("%s: buffer error: %s", __func__, ssh_err(r));
+ if (atomicio(read, fdin, sshbuf_mutable_ptr(m), msg_len) != msg_len) {
+ error("%s: Failed to read the the buffer conent from the child", __func__);
+ ret = -1;
+ break;
+ }
+ if (atomicio(vwrite, pmonitor->m_recvfd, buf, blen) != blen ||
+ atomicio(vwrite, pmonitor->m_recvfd, sshbuf_mutable_ptr(m), msg_len) != msg_len) {
+ error("%s: Failed to write the messag to the monitor", __func__);
+ ret = -1;
+ break;
+ }
+ } while (1);
+ sshbuf_free(m);
+ return ret;
+}
+void mm_set_monitor_pipe(int fd)
+{
+ pmonitor->m_recvfd = fd;
+}
#endif /* SSH_AUDIT_EVENTS */
Index: openssh-8.4p1/monitor_wrap.h
===================================================================
--- openssh-8.4p1.orig/monitor_wrap.h
+++ openssh-8.4p1/monitor_wrap.h
@@ -91,6 +91,8 @@ void mm_audit_unsupported_body(struct ss
void mm_audit_kex_body(struct ssh *, int, char *, char *, char *, char *, pid_t, uid_t);
void mm_audit_session_key_free_body(struct ssh *, int, pid_t, uid_t);
void mm_audit_destroy_sensitive_data(struct ssh *, const char *, pid_t, uid_t);
+int mm_forward_audit_messages(int);
+void mm_set_monitor_pipe(int);
#endif
struct Session;
Index: openssh-8.4p1/session.c
===================================================================
--- openssh-8.4p1.orig/session.c
+++ openssh-8.4p1/session.c
@@ -158,6 +158,10 @@ static Session *sessions = NULL;
login_cap_t *lc;
#endif
+#ifdef SSH_AUDIT_EVENTS
+int paudit[2];
+#endif
+
static int is_child = 0;
static int in_chroot = 0;
@@ -273,7 +277,7 @@ prepare_auth_info_file(struct passwd *pw
goto out;
}
if (atomicio(vwrite, fd, sshbuf_mutable_ptr(info),
- sshbuf_len(info)) != sshbuf_len(info)) {
+ sshbuf_len(info)) != sshbuf_len(info)) {
error("%s: write: %s", __func__, strerror(errno));
goto out;
}
@@ -382,6 +386,8 @@ xauth_valid_string(const char *s)
return 1;
}
+void child_destroy_sensitive_data(struct ssh *ssh);
+
#define USE_PIPES 1
/*
* This is called to fork and execute a command when we have no tty. This
@@ -507,6 +513,8 @@ do_exec_no_pty(struct ssh *ssh, Session
close(err[0]);
#endif
+ child_destroy_sensitive_data(ssh);
+
/* Do processing for the child (exec command etc). */
do_child(ssh, s, command);
/* NOTREACHED */
@@ -624,6 +632,9 @@ do_exec_pty(struct ssh *ssh, Session *s,
/* Close the extra descriptor for the pseudo tty. */
close(ttyfd);
+ /* Do this early, so we will not block large MOTDs */
+ child_destroy_sensitive_data(ssh);
+
/* record login, etc. similar to login(1) */
#ifndef HAVE_OSF_SIA
do_login(ssh, s, command);
@@ -732,6 +743,8 @@ do_exec(struct ssh *ssh, Session *s, con
}
if (s->command != NULL && s->ptyfd == -1)
s->command_handle = PRIVSEP(audit_run_command(ssh, s->command));
+ if (pipe(paudit) < 0)
+ fatal("pipe: %s", strerror(errno));
#endif
if (s->ttyfd != -1)
ret = do_exec_pty(ssh, s, command);
@@ -747,6 +760,20 @@ do_exec(struct ssh *ssh, Session *s, con
*/
sshbuf_reset(loginmsg);
+#ifdef SSH_AUDIT_EVENTS
+ close(paudit[1]);
+ if (use_privsep && ret == 0) {
+ /*
+ * Read the audit messages from forked child and send them
+ * back to monitor. We don't want to communicate directly,
+ * because the messages might get mixed up.
+ * Continue after the pipe gets closed (all messages sent).
+ */
+ ret = mm_forward_audit_messages(paudit[0]);
+ }
+ close(paudit[0]);
+#endif /* SSH_AUDIT_EVENTS */
+
return ret;
}
@@ -1535,6 +1562,35 @@ child_close_fds(struct ssh *ssh)
closefrom(STDERR_FILENO + 1);
}
+void
+child_destroy_sensitive_data(struct ssh *ssh)
+{
+#ifdef SSH_AUDIT_EVENTS
+ int pparent = paudit[1];
+ close(paudit[0]);
+ /* Hack the monitor pipe to avoid race condition with parent */
+ if (use_privsep)
+ mm_set_monitor_pipe(pparent);
+#endif
+
+ /* remove hostkey from the child's memory */
+ destroy_sensitive_data(ssh, use_privsep);
+ ssh_packet_clear_keys_noaudit(ssh);
+ /*
+ * We can audit this, because we hacked the pipe to direct the
+ * messages over postauth child. But this message requires answer
+ * which we can't do using one-way pipe.
+ */
+ packet_destroy_all(ssh, 0, 1);
+ /* XXX this will clean the rest but should not audit anymore */
+ /* packet_clear_keys(ssh); */
+
+#ifdef SSH_AUDIT_EVENTS
+ /* Notify parent that we are done */
+ close(pparent);
+#endif
+}
+
/*
* Performs common processing for the child, such as setting up the
* environment, closing extra file descriptors, setting the user and group
@@ -1553,13 +1609,6 @@ do_child(struct ssh *ssh, Session *s, co
sshpkt_fmt_connection_id(ssh, remote_id, sizeof(remote_id));
- /* remove hostkey from the child's memory */
- destroy_sensitive_data(ssh, 1);
- ssh_packet_clear_keys(ssh);
- /* Don't audit this - both us and the parent would be talking to the
- monitor over a single socket, with no synchronization. */
- packet_destroy_all(ssh, 0, 1);
-
/* Force a password change */
if (s->authctxt->force_pwchange) {
do_setusercontext(pw);
Index: openssh-8.4p1/packet.c
===================================================================
--- openssh-8.4p1.orig/packet.c
+++ openssh-8.4p1/packet.c
@@ -599,7 +599,7 @@ ssh_packet_rdomain_in(struct ssh *ssh)
/* Closes the connection and clears and frees internal data structures. */
static void
-ssh_packet_close_internal(struct ssh *ssh, int do_close)
+ssh_packet_close_internal(struct ssh *ssh, int do_close, int do_audit)
{
struct session_state *state = ssh->state;
u_int mode;
@@ -651,7 +651,7 @@ ssh_packet_close_internal(struct ssh *ss
#endif /* WITH_ZLIB */
cipher_free(state->send_context);
cipher_free(state->receive_context);
- if (had_keys && state->server_side) {
+ if (do_audit && had_keys && state->server_side) {
/* Assuming this is called only from privsep child */
audit_session_key_free(ssh, MODE_MAX);
}
@@ -677,13 +677,19 @@ ssh_packet_close_internal(struct ssh *ss
void
ssh_packet_close(struct ssh *ssh)
{
- ssh_packet_close_internal(ssh, 1);
+ ssh_packet_close_internal(ssh, 1, 1);
}
void
ssh_packet_clear_keys(struct ssh *ssh)
{
- ssh_packet_close_internal(ssh, 0);
+ ssh_packet_close_internal(ssh, 0, 1);
+}
+
+void
+ssh_packet_clear_keys_noaudit(struct ssh *ssh)
+{
+ ssh_packet_close_internal(ssh, 0, 0);
}
/* Sets remote side protocol flags. */
Index: openssh-8.4p1/packet.h
===================================================================
--- openssh-8.4p1.orig/packet.h
+++ openssh-8.4p1/packet.h
@@ -102,6 +102,7 @@ int ssh_packet_get_connection_out(s
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_newkeys(struct ssh *, int);
int ssh_packet_is_rekeying(struct ssh *);
