Audit race condition resolved (#1308295)
This commit is contained in:
Jakub Jelen
parent
7b15444065
commit
44fc97266b
143
openssh-7.1p2-audit-race-condition.patch
Normal file
143
openssh-7.1p2-audit-race-condition.patch
Normal file
@ -0,0 +1,143 @@
|
||||
diff --git a/monitor_wrap.c b/monitor_wrap.c
|
||||
index 89a1762..fe98e08 100644
|
||||
--- a/monitor_wrap.c
|
||||
+++ b/monitor_wrap.c
|
||||
@@ -1251,4 +1251,48 @@ mm_audit_destroy_sensitive_data(const char *fp, pid_t pid, uid_t uid)
|
||||
mm_request_send(pmonitor->m_recvfd, MONITOR_REQ_AUDIT_SERVER_KEY_FREE, &m);
|
||||
buffer_free(&m);
|
||||
}
|
||||
+
|
||||
+int mm_forward_audit_messages(int fdin)
|
||||
+{
|
||||
+ u_char buf[4];
|
||||
+ u_int blen, msg_len;
|
||||
+ Buffer m;
|
||||
+ int ret = 0;
|
||||
+
|
||||
+ debug3("%s: entering", __func__);
|
||||
+ buffer_init(&m);
|
||||
+ 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);
|
||||
+ buffer_clear(&m);
|
||||
+ buffer_append_space(&m, msg_len);
|
||||
+ if (atomicio(read, fdin, buffer_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, buffer_ptr(&m), msg_len) != msg_len) {
|
||||
+ error("%s: Failed to write the messag to the monitor", __func__);
|
||||
+ ret = -1;
|
||||
+ break;
|
||||
+ }
|
||||
+ } while (1);
|
||||
+ buffer_free(&m);
|
||||
+ return ret;
|
||||
+}
|
||||
+void mm_set_monitor_pipe(int fd)
|
||||
+{
|
||||
+ pmonitor->m_recvfd = fd;
|
||||
+}
|
||||
#endif /* SSH_AUDIT_EVENTS */
|
||||
diff --git a/monitor_wrap.h b/monitor_wrap.h
|
||||
index e73134e..fbfe395 100644
|
||||
--- a/monitor_wrap.h
|
||||
+++ b/monitor_wrap.h
|
||||
@@ -86,6 +86,8 @@ void mm_audit_unsupported_body(int);
|
||||
void mm_audit_kex_body(int, char *, char *, char *, char *, pid_t, uid_t);
|
||||
void mm_audit_session_key_free_body(int, pid_t, uid_t);
|
||||
void mm_audit_destroy_sensitive_data(const char *, pid_t, uid_t);
|
||||
+int mm_forward_audit_messages(int);
|
||||
+void mm_set_monitor_pipe(int);
|
||||
#endif
|
||||
|
||||
struct Session;
|
||||
diff --git a/session.c b/session.c
|
||||
index 8949fd1..9afb764 100644
|
||||
--- a/session.c
|
||||
+++ b/session.c
|
||||
@@ -159,6 +159,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 have_dev_log = 1;
|
||||
@@ -875,6 +879,8 @@ do_exec(Session *s, const char *command)
|
||||
}
|
||||
if (s->command != NULL && s->ptyfd == -1)
|
||||
s->command_handle = PRIVSEP(audit_run_command(s->command));
|
||||
+ if (pipe(paudit) < 0)
|
||||
+ fatal("pipe: %s", strerror(errno));
|
||||
#endif
|
||||
if (s->ttyfd != -1)
|
||||
ret = do_exec_pty(s, command);
|
||||
@@ -890,6 +896,20 @@ do_exec(Session *s, const char *command)
|
||||
*/
|
||||
buffer_clear(&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;
|
||||
}
|
||||
|
||||
@@ -1707,12 +1727,28 @@ do_child(Session *s, const char *command)
|
||||
struct passwd *pw = s->pw;
|
||||
int r = 0;
|
||||
|
||||
+#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(1);
|
||||
- /* Don't audit this - both us and the parent would be talking to the
|
||||
- monitor over a single socket, with no synchronization. */
|
||||
+ destroy_sensitive_data(use_privsep);
|
||||
+ /*
|
||||
+ * We can audit this, because wer 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(0, 1);
|
||||
|
||||
+#ifdef SSH_AUDIT_EVENTS
|
||||
+ /* Notify parent that we are done */
|
||||
+ close(pparent);
|
||||
+#endif
|
||||
+
|
||||
/* Force a password change */
|
||||
if (s->authctxt->force_pwchange) {
|
||||
do_setusercontext(pw);
|
||||
@ -106,6 +106,8 @@ Patch103: openssh-5.8p1-packet.patch
|
||||
# https://bugzilla.redhat.com/show_bug.cgi?id=1171248
|
||||
# record pfs= field in CRYPTO_SESSION audit event
|
||||
Patch200: openssh-6.7p1-audit.patch
|
||||
# Audit race condition in forked child (#1310684)
|
||||
Patch201: openssh-7.1p2-audit-race-condition.patch
|
||||
|
||||
# --- pam_ssh-agent ---
|
||||
# make it build reusing the openssh sources
|
||||
@ -473,6 +475,7 @@ popd
|
||||
%patch937 -p1 -b .x11-fallback
|
||||
|
||||
%patch200 -p1 -b .audit
|
||||
%patch201 -p1 -b .audit-race
|
||||
%patch700 -p1 -b .fips
|
||||
|
||||
%patch100 -p1 -b .coverity
|
||||
|
||||
Loading…
Reference in New Issue
Block a user