Skip to content
Permalink
Browse files

Remove support for pre-authentication compression. Doing compression

early in the protocol probably seemed reasonable in the 1990s, but
today it's clearly a bad idea in terms of both cryptography (cf.
multiple compression oracle attacks in TLS) and attack surface.

Moreover, to support it across privilege-separation zlib needed
the assistance of a complex shared-memory manager that made the
required attack surface considerably larger.

Prompted by Guido Vranken pointing out a compiler-elided security
check in the shared memory manager found by Stack
(http://css.csail.mit.edu/stack/); ok deraadt@ markus@

NB. pre-auth authentication has been disabled by default in sshd
for >10 years.
  • Loading branch information...
djmdjm committed Sep 28, 2016
1 parent f9b0f55 commit 3095060f479b86288e31c79ecbc5131a66bcd2f9
@@ -1,4 +1,4 @@
/* $OpenBSD: monitor.c,v 1.165 2016/09/05 13:57:31 djm Exp $ */
/* $OpenBSD: monitor.c,v 1.166 2016/09/28 16:33:06 djm Exp $ */
/*
* Copyright 2002 Niels Provos <provos@citi.umich.edu>
* Copyright 2002 Markus Friedl <markus@openbsd.org>
@@ -70,7 +70,6 @@
#include "misc.h"
#include "servconf.h"
#include "monitor.h"
#include "monitor_mm.h"
#ifdef GSSAPI
#include "ssh-gss.h"
#endif
@@ -335,31 +334,6 @@ monitor_child_postauth(struct monitor *pmonitor)
monitor_read(pmonitor, mon_dispatch, NULL);
}

void
monitor_sync(struct monitor *pmonitor)
{
if (options.compression) {
/* The member allocation is not visible, so sync it */
mm_share_sync(&pmonitor->m_zlib, &pmonitor->m_zback);
}
}

/* Allocation functions for zlib */
static void *
mm_zalloc(struct mm_master *mm, u_int ncount, u_int size)
{
if (size == 0 || ncount == 0 || ncount > SIZE_MAX / size)
fatal("%s: mm_zalloc(%u, %u)", __func__, ncount, size);

return mm_malloc(mm, size * ncount);
}

static void
mm_zfree(struct mm_master *mm, void *address)
{
mm_free(mm, address);
}

static int
monitor_read_log(struct monitor *pmonitor)
{
@@ -1292,13 +1266,6 @@ monitor_apply_keystate(struct monitor *pmonitor)
kex->host_key_index=&get_hostkey_index;
kex->sign = sshd_hostkey_sign;
}

/* Update with new address */
if (options.compression) {
ssh_packet_set_compress_hooks(ssh, pmonitor->m_zlib,
(ssh_packet_comp_alloc_func *)mm_zalloc,
(ssh_packet_comp_free_func *)mm_zfree);
}
}

/* This function requries careful sanity checking */
@@ -1351,24 +1318,11 @@ monitor_openfds(struct monitor *mon, int do_logfds)
struct monitor *
monitor_init(void)
{
struct ssh *ssh = active_state; /* XXX */
struct monitor *mon;

mon = xcalloc(1, sizeof(*mon));

monitor_openfds(mon, 1);

/* Used to share zlib space across processes */
if (options.compression) {
mon->m_zback = mm_create(NULL, MM_MEMSIZE);
mon->m_zlib = mm_create(mon->m_zback, 20 * MM_MEMSIZE);

/* Compression needs to share state across borders */
ssh_packet_set_compress_hooks(ssh, mon->m_zlib,
(ssh_packet_comp_alloc_func *)mm_zalloc,
(ssh_packet_comp_free_func *)mm_zfree);
}

return mon;
}

@@ -1,4 +1,4 @@
/* $OpenBSD: monitor.h,v 1.19 2015/01/19 19:52:16 markus Exp $ */
/* $OpenBSD: monitor.h,v 1.20 2016/09/28 16:33:07 djm Exp $ */

/*
* Copyright 2002 Niels Provos <provos@citi.umich.edu>
@@ -58,21 +58,17 @@ enum monitor_reqtype {
MONITOR_REQ_TERM = 50,
};

struct mm_master;
struct monitor {
int m_recvfd;
int m_sendfd;
int m_log_recvfd;
int m_log_sendfd;
struct mm_master *m_zback;
struct mm_master *m_zlib;
struct kex **m_pkex;
pid_t m_pid;
};

struct monitor *monitor_init(void);
void monitor_reinit(struct monitor *);
void monitor_sync(struct monitor *);

struct Authctxt;
void monitor_child_preauth(struct Authctxt *, struct monitor *);

0 comments on commit 3095060

Please sign in to comment.
You can’t perform that action at this time.