- djm@cvs.openbsd.org 2010/07/13 11:52:06

[auth-rsa.c channels.c jpake.c key.c misc.c misc.h monitor.c]
     [packet.c ssh-rsa.c]
     implement a timing_safe_cmp() function to compare memory without leaking
     timing information by short-circuiting like memcmp() and use it for
     some of the more sensitive comparisons (though nothing high-value was
     readily attackable anyway); "looks ok" markus@
This commit is contained in:
Damien Miller 2010-07-16 13:57:51 +10:00
parent d0244d498b
commit 8a0268f1b3
10 changed files with 45 additions and 23 deletions

View File

@ -22,6 +22,13 @@
Hostname %h.example.org Hostname %h.example.org
"I like it" markus@ "I like it" markus@
- djm@cvs.openbsd.org 2010/07/13 11:52:06
[auth-rsa.c channels.c jpake.c key.c misc.c misc.h monitor.c]
[packet.c ssh-rsa.c]
implement a timing_safe_cmp() function to compare memory without leaking
timing information by short-circuiting like memcmp() and use it for
some of the more sensitive comparisons (though nothing high-value was
readily attackable anyway); "looks ok" markus@
20100714 20100714
- (tim) [contrib/redhat/openssh.spec] Bug 1796: Test for skip_x11_askpass - (tim) [contrib/redhat/openssh.spec] Bug 1796: Test for skip_x11_askpass

View File

@ -1,4 +1,4 @@
/* $OpenBSD: auth-rsa.c,v 1.76 2010/05/11 02:58:04 djm Exp $ */ /* $OpenBSD: auth-rsa.c,v 1.77 2010/07/13 11:52:06 djm Exp $ */
/* /*
* Author: Tatu Ylonen <ylo@cs.hut.fi> * Author: Tatu Ylonen <ylo@cs.hut.fi>
* Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@ -116,7 +116,7 @@ auth_rsa_verify_response(Key *key, BIGNUM *challenge, u_char response[16])
MD5_Final(mdbuf, &md); MD5_Final(mdbuf, &md);
/* Verify that the response is the original challenge. */ /* Verify that the response is the original challenge. */
if (memcmp(response, mdbuf, 16) != 0) { if (timing_safe_cmp(response, mdbuf, 16) != 0) {
/* Wrong answer. */ /* Wrong answer. */
return (0); return (0);
} }

View File

@ -1,4 +1,4 @@
/* $OpenBSD: channels.c,v 1.306 2010/06/25 07:20:04 djm Exp $ */ /* $OpenBSD: channels.c,v 1.307 2010/07/13 11:52:06 djm Exp $ */
/* /*
* Author: Tatu Ylonen <ylo@cs.hut.fi> * Author: Tatu Ylonen <ylo@cs.hut.fi>
* Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@ -917,7 +917,7 @@ x11_open_helper(Buffer *b)
} }
/* Check if authentication data matches our fake data. */ /* Check if authentication data matches our fake data. */
if (data_len != x11_fake_data_len || if (data_len != x11_fake_data_len ||
memcmp(ucp + 12 + ((proto_len + 3) & ~3), timing_safe_cmp(ucp + 12 + ((proto_len + 3) & ~3),
x11_fake_data, x11_fake_data_len) != 0) { x11_fake_data, x11_fake_data_len) != 0) {
debug2("X11 auth data does not match fake data."); debug2("X11 auth data does not match fake data.");
return -1; return -1;

View File

@ -1,4 +1,4 @@
/* $OpenBSD: jpake.c,v 1.2 2009/03/05 07:18:19 djm Exp $ */ /* $OpenBSD: jpake.c,v 1.3 2010/07/13 11:52:06 djm Exp $ */
/* /*
* Copyright (c) 2008 Damien Miller. All rights reserved. * Copyright (c) 2008 Damien Miller. All rights reserved.
* *
@ -434,7 +434,7 @@ jpake_check_confirm(const BIGNUM *k,
if (peer_confirm_hash_len != expected_confirm_hash_len) if (peer_confirm_hash_len != expected_confirm_hash_len)
error("%s: confirmation length mismatch (my %u them %u)", error("%s: confirmation length mismatch (my %u them %u)",
__func__, expected_confirm_hash_len, peer_confirm_hash_len); __func__, expected_confirm_hash_len, peer_confirm_hash_len);
else if (memcmp(peer_confirm_hash, expected_confirm_hash, else if (timing_safe_cmp(peer_confirm_hash, expected_confirm_hash,
expected_confirm_hash_len) == 0) expected_confirm_hash_len) == 0)
success = 1; success = 1;
bzero(expected_confirm_hash, expected_confirm_hash_len); bzero(expected_confirm_hash, expected_confirm_hash_len);

5
key.c
View File

@ -1,4 +1,4 @@
/* $OpenBSD: key.c,v 1.88 2010/05/07 11:30:29 djm Exp $ */ /* $OpenBSD: key.c,v 1.89 2010/07/13 11:52:06 djm Exp $ */
/* /*
* read_bignum(): * read_bignum():
* Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@ -52,6 +52,7 @@
#include "uuencode.h" #include "uuencode.h"
#include "buffer.h" #include "buffer.h"
#include "log.h" #include "log.h"
#include "misc.h"
#include "ssh2.h" #include "ssh2.h"
static struct KeyCert * static struct KeyCert *
@ -227,7 +228,7 @@ cert_compare(struct KeyCert *a, struct KeyCert *b)
return 0; return 0;
if (buffer_len(&a->certblob) != buffer_len(&b->certblob)) if (buffer_len(&a->certblob) != buffer_len(&b->certblob))
return 0; return 0;
if (memcmp(buffer_ptr(&a->certblob), buffer_ptr(&b->certblob), if (timing_safe_cmp(buffer_ptr(&a->certblob), buffer_ptr(&b->certblob),
buffer_len(&a->certblob)) != 0) buffer_len(&a->certblob)) != 0)
return 0; return 0;
return 1; return 1;

14
misc.c
View File

@ -1,4 +1,4 @@
/* $OpenBSD: misc.c,v 1.77 2010/07/02 04:32:44 djm Exp $ */ /* $OpenBSD: misc.c,v 1.78 2010/07/13 11:52:06 djm Exp $ */
/* /*
* Copyright (c) 2000 Markus Friedl. All rights reserved. * Copyright (c) 2000 Markus Friedl. All rights reserved.
* Copyright (c) 2005,2006 Damien Miller. All rights reserved. * Copyright (c) 2005,2006 Damien Miller. All rights reserved.
@ -850,6 +850,18 @@ ms_to_timeval(struct timeval *tv, int ms)
tv->tv_usec = (ms % 1000) * 1000; tv->tv_usec = (ms % 1000) * 1000;
} }
int
timing_safe_cmp(const void *_s1, const void *_s2, size_t n)
{
u_char *s1 = (u_char *)_s1;
u_char *s2 = (u_char *)_s2;
int ret = 0;
for (; n > 0; n--, s1++, s2++)
ret |= *s1 ^ *s2;
return ret;
}
void void
sock_set_v6only(int s) sock_set_v6only(int s)
{ {

3
misc.h
View File

@ -1,4 +1,4 @@
/* $OpenBSD: misc.h,v 1.41 2010/01/09 23:04:13 dtucker Exp $ */ /* $OpenBSD: misc.h,v 1.42 2010/07/13 11:52:06 djm Exp $ */
/* /*
* Author: Tatu Ylonen <ylo@cs.hut.fi> * Author: Tatu Ylonen <ylo@cs.hut.fi>
@ -36,6 +36,7 @@ void sanitise_stdfd(void);
void ms_subtract_diff(struct timeval *, int *); void ms_subtract_diff(struct timeval *, int *);
void ms_to_timeval(struct timeval *, int); void ms_to_timeval(struct timeval *, int);
void sock_set_v6only(int); void sock_set_v6only(int);
int timing_safe_cmp(const void *, const void *, size_t);
struct passwd *pwcopy(struct passwd *); struct passwd *pwcopy(struct passwd *);
const char *ssh_gai_strerror(int); const char *ssh_gai_strerror(int);

View File

@ -1,4 +1,4 @@
/* $OpenBSD: monitor.c,v 1.106 2010/03/07 11:57:13 dtucker Exp $ */ /* $OpenBSD: monitor.c,v 1.107 2010/07/13 11:52:06 djm Exp $ */
/* /*
* Copyright 2002 Niels Provos <provos@citi.umich.edu> * Copyright 2002 Niels Provos <provos@citi.umich.edu>
* Copyright 2002 Markus Friedl <markus@openbsd.org> * Copyright 2002 Markus Friedl <markus@openbsd.org>
@ -518,7 +518,7 @@ monitor_allowed_key(u_char *blob, u_int bloblen)
{ {
/* make sure key is allowed */ /* make sure key is allowed */
if (key_blob == NULL || key_bloblen != bloblen || if (key_blob == NULL || key_bloblen != bloblen ||
memcmp(key_blob, blob, key_bloblen)) timing_safe_cmp(key_blob, blob, key_bloblen))
return (0); return (0);
return (1); return (1);
} }
@ -1103,14 +1103,14 @@ monitor_valid_userblob(u_char *data, u_int datalen)
len = buffer_len(&b); len = buffer_len(&b);
if ((session_id2 == NULL) || if ((session_id2 == NULL) ||
(len < session_id2_len) || (len < session_id2_len) ||
(memcmp(p, session_id2, session_id2_len) != 0)) (timing_safe_cmp(p, session_id2, session_id2_len) != 0))
fail++; fail++;
buffer_consume(&b, session_id2_len); buffer_consume(&b, session_id2_len);
} else { } else {
p = buffer_get_string(&b, &len); p = buffer_get_string(&b, &len);
if ((session_id2 == NULL) || if ((session_id2 == NULL) ||
(len != session_id2_len) || (len != session_id2_len) ||
(memcmp(p, session_id2, session_id2_len) != 0)) (timing_safe_cmp(p, session_id2, session_id2_len) != 0))
fail++; fail++;
xfree(p); xfree(p);
} }
@ -1158,7 +1158,7 @@ monitor_valid_hostbasedblob(u_char *data, u_int datalen, char *cuser,
p = buffer_get_string(&b, &len); p = buffer_get_string(&b, &len);
if ((session_id2 == NULL) || if ((session_id2 == NULL) ||
(len != session_id2_len) || (len != session_id2_len) ||
(memcmp(p, session_id2, session_id2_len) != 0)) (timing_safe_cmp(p, session_id2, session_id2_len) != 0))
fail++; fail++;
xfree(p); xfree(p);
@ -1682,9 +1682,9 @@ mm_get_kex(Buffer *m)
kex = xcalloc(1, sizeof(*kex)); kex = xcalloc(1, sizeof(*kex));
kex->session_id = buffer_get_string(m, &kex->session_id_len); kex->session_id = buffer_get_string(m, &kex->session_id_len);
if ((session_id2 == NULL) || if (session_id2 == NULL ||
(kex->session_id_len != session_id2_len) || kex->session_id_len != session_id2_len ||
(memcmp(kex->session_id, session_id2, session_id2_len) != 0)) timing_safe_cmp(kex->session_id, session_id2, session_id2_len) != 0)
fatal("mm_get_get: internal error: bad session id"); fatal("mm_get_get: internal error: bad session id");
kex->we_need = buffer_get_int(m); kex->we_need = buffer_get_int(m);
kex->kex[KEX_DH_GRP1_SHA1] = kexdh_server; kex->kex[KEX_DH_GRP1_SHA1] = kexdh_server;

View File

@ -1,4 +1,4 @@
/* $OpenBSD: packet.c,v 1.166 2009/06/27 09:29:06 andreas Exp $ */ /* $OpenBSD: packet.c,v 1.167 2010/07/13 11:52:06 djm Exp $ */
/* /*
* Author: Tatu Ylonen <ylo@cs.hut.fi> * Author: Tatu Ylonen <ylo@cs.hut.fi>
* Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@ -1307,7 +1307,7 @@ packet_read_poll2(u_int32_t *seqnr_p)
macbuf = mac_compute(mac, active_state->p_read.seqnr, macbuf = mac_compute(mac, active_state->p_read.seqnr,
buffer_ptr(&active_state->incoming_packet), buffer_ptr(&active_state->incoming_packet),
buffer_len(&active_state->incoming_packet)); buffer_len(&active_state->incoming_packet));
if (memcmp(macbuf, buffer_ptr(&active_state->input), if (timing_safe_cmp(macbuf, buffer_ptr(&active_state->input),
mac->mac_len) != 0) { mac->mac_len) != 0) {
logit("Corrupted MAC on input."); logit("Corrupted MAC on input.");
if (need > PACKET_MAX_SIZE) if (need > PACKET_MAX_SIZE)

View File

@ -1,4 +1,4 @@
/* $OpenBSD: ssh-rsa.c,v 1.41 2010/04/16 01:47:26 djm Exp $ */ /* $OpenBSD: ssh-rsa.c,v 1.42 2010/07/13 11:52:06 djm Exp $ */
/* /*
* Copyright (c) 2000, 2003 Markus Friedl <markus@openbsd.org> * Copyright (c) 2000, 2003 Markus Friedl <markus@openbsd.org>
* *
@ -30,6 +30,7 @@
#include "buffer.h" #include "buffer.h"
#include "key.h" #include "key.h"
#include "compat.h" #include "compat.h"
#include "misc.h"
#include "ssh.h" #include "ssh.h"
static int openssh_RSA_verify(int, u_char *, u_int, u_char *, u_int, RSA *); static int openssh_RSA_verify(int, u_char *, u_int, u_char *, u_int, RSA *);
@ -249,11 +250,11 @@ openssh_RSA_verify(int type, u_char *hash, u_int hashlen,
error("bad decrypted len: %d != %d + %d", len, hlen, oidlen); error("bad decrypted len: %d != %d + %d", len, hlen, oidlen);
goto done; goto done;
} }
if (memcmp(decrypted, oid, oidlen) != 0) { if (timing_safe_cmp(decrypted, oid, oidlen) != 0) {
error("oid mismatch"); error("oid mismatch");
goto done; goto done;
} }
if (memcmp(decrypted + oidlen, hash, hlen) != 0) { if (timing_safe_cmp(decrypted + oidlen, hash, hlen) != 0) {
error("hash mismatch"); error("hash mismatch");
goto done; goto done;
} }