diff options
author | Damien Miller <djm@mindrot.org> | 2010-07-16 13:57:51 +1000 |
---|---|---|
committer | Damien Miller <djm@mindrot.org> | 2010-07-16 13:57:51 +1000 |
commit | 8a0268f1b3f62292d4124f8d158e0587c4f7c330 (patch) | |
tree | 43493a3202569a2939f5616127d9de8689613a7b | |
parent | d0244d498ba970b9d9348429eaf7a4a0ef2b903c (diff) |
- 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@
-rw-r--r-- | ChangeLog | 7 | ||||
-rw-r--r-- | auth-rsa.c | 4 | ||||
-rw-r--r-- | channels.c | 4 | ||||
-rw-r--r-- | jpake.c | 4 | ||||
-rw-r--r-- | key.c | 5 | ||||
-rw-r--r-- | misc.c | 14 | ||||
-rw-r--r-- | misc.h | 3 | ||||
-rw-r--r-- | monitor.c | 16 | ||||
-rw-r--r-- | packet.c | 4 | ||||
-rw-r--r-- | ssh-rsa.c | 7 |
10 files changed, 45 insertions, 23 deletions
@@ -22,6 +22,13 @@ | |||
22 | Hostname %h.example.org | 22 | Hostname %h.example.org |
23 | 23 | ||
24 | "I like it" markus@ | 24 | "I like it" markus@ |
25 | - djm@cvs.openbsd.org 2010/07/13 11:52:06 | ||
26 | [auth-rsa.c channels.c jpake.c key.c misc.c misc.h monitor.c] | ||
27 | [packet.c ssh-rsa.c] | ||
28 | implement a timing_safe_cmp() function to compare memory without leaking | ||
29 | timing information by short-circuiting like memcmp() and use it for | ||
30 | some of the more sensitive comparisons (though nothing high-value was | ||
31 | readily attackable anyway); "looks ok" markus@ | ||
25 | 32 | ||
26 | 20100714 | 33 | 20100714 |
27 | - (tim) [contrib/redhat/openssh.spec] Bug 1796: Test for skip_x11_askpass | 34 | - (tim) [contrib/redhat/openssh.spec] Bug 1796: Test for skip_x11_askpass |
diff --git a/auth-rsa.c b/auth-rsa.c index ef6767bfb..71abadf6c 100644 --- a/auth-rsa.c +++ b/auth-rsa.c | |||
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: auth-rsa.c,v 1.76 2010/05/11 02:58:04 djm Exp $ */ | 1 | /* $OpenBSD: auth-rsa.c,v 1.77 2010/07/13 11:52:06 djm Exp $ */ |
2 | /* | 2 | /* |
3 | * Author: Tatu Ylonen <ylo@cs.hut.fi> | 3 | * Author: Tatu Ylonen <ylo@cs.hut.fi> |
4 | * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland | 4 | * 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]) | |||
116 | MD5_Final(mdbuf, &md); | 116 | MD5_Final(mdbuf, &md); |
117 | 117 | ||
118 | /* Verify that the response is the original challenge. */ | 118 | /* Verify that the response is the original challenge. */ |
119 | if (memcmp(response, mdbuf, 16) != 0) { | 119 | if (timing_safe_cmp(response, mdbuf, 16) != 0) { |
120 | /* Wrong answer. */ | 120 | /* Wrong answer. */ |
121 | return (0); | 121 | return (0); |
122 | } | 122 | } |
diff --git a/channels.c b/channels.c index fe08257df..e52946bcd 100644 --- a/channels.c +++ b/channels.c | |||
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: channels.c,v 1.306 2010/06/25 07:20:04 djm Exp $ */ | 1 | /* $OpenBSD: channels.c,v 1.307 2010/07/13 11:52:06 djm Exp $ */ |
2 | /* | 2 | /* |
3 | * Author: Tatu Ylonen <ylo@cs.hut.fi> | 3 | * Author: Tatu Ylonen <ylo@cs.hut.fi> |
4 | * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland | 4 | * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland |
@@ -917,7 +917,7 @@ x11_open_helper(Buffer *b) | |||
917 | } | 917 | } |
918 | /* Check if authentication data matches our fake data. */ | 918 | /* Check if authentication data matches our fake data. */ |
919 | if (data_len != x11_fake_data_len || | 919 | if (data_len != x11_fake_data_len || |
920 | memcmp(ucp + 12 + ((proto_len + 3) & ~3), | 920 | timing_safe_cmp(ucp + 12 + ((proto_len + 3) & ~3), |
921 | x11_fake_data, x11_fake_data_len) != 0) { | 921 | x11_fake_data, x11_fake_data_len) != 0) { |
922 | debug2("X11 auth data does not match fake data."); | 922 | debug2("X11 auth data does not match fake data."); |
923 | return -1; | 923 | return -1; |
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: jpake.c,v 1.2 2009/03/05 07:18:19 djm Exp $ */ | 1 | /* $OpenBSD: jpake.c,v 1.3 2010/07/13 11:52:06 djm Exp $ */ |
2 | /* | 2 | /* |
3 | * Copyright (c) 2008 Damien Miller. All rights reserved. | 3 | * Copyright (c) 2008 Damien Miller. All rights reserved. |
4 | * | 4 | * |
@@ -434,7 +434,7 @@ jpake_check_confirm(const BIGNUM *k, | |||
434 | if (peer_confirm_hash_len != expected_confirm_hash_len) | 434 | if (peer_confirm_hash_len != expected_confirm_hash_len) |
435 | error("%s: confirmation length mismatch (my %u them %u)", | 435 | error("%s: confirmation length mismatch (my %u them %u)", |
436 | __func__, expected_confirm_hash_len, peer_confirm_hash_len); | 436 | __func__, expected_confirm_hash_len, peer_confirm_hash_len); |
437 | else if (memcmp(peer_confirm_hash, expected_confirm_hash, | 437 | else if (timing_safe_cmp(peer_confirm_hash, expected_confirm_hash, |
438 | expected_confirm_hash_len) == 0) | 438 | expected_confirm_hash_len) == 0) |
439 | success = 1; | 439 | success = 1; |
440 | bzero(expected_confirm_hash, expected_confirm_hash_len); | 440 | bzero(expected_confirm_hash, expected_confirm_hash_len); |
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: key.c,v 1.88 2010/05/07 11:30:29 djm Exp $ */ | 1 | /* $OpenBSD: key.c,v 1.89 2010/07/13 11:52:06 djm Exp $ */ |
2 | /* | 2 | /* |
3 | * read_bignum(): | 3 | * read_bignum(): |
4 | * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland | 4 | * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland |
@@ -52,6 +52,7 @@ | |||
52 | #include "uuencode.h" | 52 | #include "uuencode.h" |
53 | #include "buffer.h" | 53 | #include "buffer.h" |
54 | #include "log.h" | 54 | #include "log.h" |
55 | #include "misc.h" | ||
55 | #include "ssh2.h" | 56 | #include "ssh2.h" |
56 | 57 | ||
57 | static struct KeyCert * | 58 | static struct KeyCert * |
@@ -227,7 +228,7 @@ cert_compare(struct KeyCert *a, struct KeyCert *b) | |||
227 | return 0; | 228 | return 0; |
228 | if (buffer_len(&a->certblob) != buffer_len(&b->certblob)) | 229 | if (buffer_len(&a->certblob) != buffer_len(&b->certblob)) |
229 | return 0; | 230 | return 0; |
230 | if (memcmp(buffer_ptr(&a->certblob), buffer_ptr(&b->certblob), | 231 | if (timing_safe_cmp(buffer_ptr(&a->certblob), buffer_ptr(&b->certblob), |
231 | buffer_len(&a->certblob)) != 0) | 232 | buffer_len(&a->certblob)) != 0) |
232 | return 0; | 233 | return 0; |
233 | return 1; | 234 | return 1; |
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: misc.c,v 1.77 2010/07/02 04:32:44 djm Exp $ */ | 1 | /* $OpenBSD: misc.c,v 1.78 2010/07/13 11:52:06 djm Exp $ */ |
2 | /* | 2 | /* |
3 | * Copyright (c) 2000 Markus Friedl. All rights reserved. | 3 | * Copyright (c) 2000 Markus Friedl. All rights reserved. |
4 | * Copyright (c) 2005,2006 Damien Miller. All rights reserved. | 4 | * Copyright (c) 2005,2006 Damien Miller. All rights reserved. |
@@ -850,6 +850,18 @@ ms_to_timeval(struct timeval *tv, int ms) | |||
850 | tv->tv_usec = (ms % 1000) * 1000; | 850 | tv->tv_usec = (ms % 1000) * 1000; |
851 | } | 851 | } |
852 | 852 | ||
853 | int | ||
854 | timing_safe_cmp(const void *_s1, const void *_s2, size_t n) | ||
855 | { | ||
856 | u_char *s1 = (u_char *)_s1; | ||
857 | u_char *s2 = (u_char *)_s2; | ||
858 | int ret = 0; | ||
859 | |||
860 | for (; n > 0; n--, s1++, s2++) | ||
861 | ret |= *s1 ^ *s2; | ||
862 | return ret; | ||
863 | } | ||
864 | |||
853 | void | 865 | void |
854 | sock_set_v6only(int s) | 866 | sock_set_v6only(int s) |
855 | { | 867 | { |
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: misc.h,v 1.41 2010/01/09 23:04:13 dtucker Exp $ */ | 1 | /* $OpenBSD: misc.h,v 1.42 2010/07/13 11:52:06 djm Exp $ */ |
2 | 2 | ||
3 | /* | 3 | /* |
4 | * Author: Tatu Ylonen <ylo@cs.hut.fi> | 4 | * Author: Tatu Ylonen <ylo@cs.hut.fi> |
@@ -36,6 +36,7 @@ void sanitise_stdfd(void); | |||
36 | void ms_subtract_diff(struct timeval *, int *); | 36 | void ms_subtract_diff(struct timeval *, int *); |
37 | void ms_to_timeval(struct timeval *, int); | 37 | void ms_to_timeval(struct timeval *, int); |
38 | void sock_set_v6only(int); | 38 | void sock_set_v6only(int); |
39 | int timing_safe_cmp(const void *, const void *, size_t); | ||
39 | 40 | ||
40 | struct passwd *pwcopy(struct passwd *); | 41 | struct passwd *pwcopy(struct passwd *); |
41 | const char *ssh_gai_strerror(int); | 42 | const char *ssh_gai_strerror(int); |
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: monitor.c,v 1.106 2010/03/07 11:57:13 dtucker Exp $ */ | 1 | /* $OpenBSD: monitor.c,v 1.107 2010/07/13 11:52:06 djm Exp $ */ |
2 | /* | 2 | /* |
3 | * Copyright 2002 Niels Provos <provos@citi.umich.edu> | 3 | * Copyright 2002 Niels Provos <provos@citi.umich.edu> |
4 | * Copyright 2002 Markus Friedl <markus@openbsd.org> | 4 | * Copyright 2002 Markus Friedl <markus@openbsd.org> |
@@ -518,7 +518,7 @@ monitor_allowed_key(u_char *blob, u_int bloblen) | |||
518 | { | 518 | { |
519 | /* make sure key is allowed */ | 519 | /* make sure key is allowed */ |
520 | if (key_blob == NULL || key_bloblen != bloblen || | 520 | if (key_blob == NULL || key_bloblen != bloblen || |
521 | memcmp(key_blob, blob, key_bloblen)) | 521 | timing_safe_cmp(key_blob, blob, key_bloblen)) |
522 | return (0); | 522 | return (0); |
523 | return (1); | 523 | return (1); |
524 | } | 524 | } |
@@ -1103,14 +1103,14 @@ monitor_valid_userblob(u_char *data, u_int datalen) | |||
1103 | len = buffer_len(&b); | 1103 | len = buffer_len(&b); |
1104 | if ((session_id2 == NULL) || | 1104 | if ((session_id2 == NULL) || |
1105 | (len < session_id2_len) || | 1105 | (len < session_id2_len) || |
1106 | (memcmp(p, session_id2, session_id2_len) != 0)) | 1106 | (timing_safe_cmp(p, session_id2, session_id2_len) != 0)) |
1107 | fail++; | 1107 | fail++; |
1108 | buffer_consume(&b, session_id2_len); | 1108 | buffer_consume(&b, session_id2_len); |
1109 | } else { | 1109 | } else { |
1110 | p = buffer_get_string(&b, &len); | 1110 | p = buffer_get_string(&b, &len); |
1111 | if ((session_id2 == NULL) || | 1111 | if ((session_id2 == NULL) || |
1112 | (len != session_id2_len) || | 1112 | (len != session_id2_len) || |
1113 | (memcmp(p, session_id2, session_id2_len) != 0)) | 1113 | (timing_safe_cmp(p, session_id2, session_id2_len) != 0)) |
1114 | fail++; | 1114 | fail++; |
1115 | xfree(p); | 1115 | xfree(p); |
1116 | } | 1116 | } |
@@ -1158,7 +1158,7 @@ monitor_valid_hostbasedblob(u_char *data, u_int datalen, char *cuser, | |||
1158 | p = buffer_get_string(&b, &len); | 1158 | p = buffer_get_string(&b, &len); |
1159 | if ((session_id2 == NULL) || | 1159 | if ((session_id2 == NULL) || |
1160 | (len != session_id2_len) || | 1160 | (len != session_id2_len) || |
1161 | (memcmp(p, session_id2, session_id2_len) != 0)) | 1161 | (timing_safe_cmp(p, session_id2, session_id2_len) != 0)) |
1162 | fail++; | 1162 | fail++; |
1163 | xfree(p); | 1163 | xfree(p); |
1164 | 1164 | ||
@@ -1682,9 +1682,9 @@ mm_get_kex(Buffer *m) | |||
1682 | 1682 | ||
1683 | kex = xcalloc(1, sizeof(*kex)); | 1683 | kex = xcalloc(1, sizeof(*kex)); |
1684 | kex->session_id = buffer_get_string(m, &kex->session_id_len); | 1684 | kex->session_id = buffer_get_string(m, &kex->session_id_len); |
1685 | if ((session_id2 == NULL) || | 1685 | if (session_id2 == NULL || |
1686 | (kex->session_id_len != session_id2_len) || | 1686 | kex->session_id_len != session_id2_len || |
1687 | (memcmp(kex->session_id, session_id2, session_id2_len) != 0)) | 1687 | timing_safe_cmp(kex->session_id, session_id2, session_id2_len) != 0) |
1688 | fatal("mm_get_get: internal error: bad session id"); | 1688 | fatal("mm_get_get: internal error: bad session id"); |
1689 | kex->we_need = buffer_get_int(m); | 1689 | kex->we_need = buffer_get_int(m); |
1690 | kex->kex[KEX_DH_GRP1_SHA1] = kexdh_server; | 1690 | kex->kex[KEX_DH_GRP1_SHA1] = kexdh_server; |
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: packet.c,v 1.166 2009/06/27 09:29:06 andreas Exp $ */ | 1 | /* $OpenBSD: packet.c,v 1.167 2010/07/13 11:52:06 djm Exp $ */ |
2 | /* | 2 | /* |
3 | * Author: Tatu Ylonen <ylo@cs.hut.fi> | 3 | * Author: Tatu Ylonen <ylo@cs.hut.fi> |
4 | * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland | 4 | * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland |
@@ -1307,7 +1307,7 @@ packet_read_poll2(u_int32_t *seqnr_p) | |||
1307 | macbuf = mac_compute(mac, active_state->p_read.seqnr, | 1307 | macbuf = mac_compute(mac, active_state->p_read.seqnr, |
1308 | buffer_ptr(&active_state->incoming_packet), | 1308 | buffer_ptr(&active_state->incoming_packet), |
1309 | buffer_len(&active_state->incoming_packet)); | 1309 | buffer_len(&active_state->incoming_packet)); |
1310 | if (memcmp(macbuf, buffer_ptr(&active_state->input), | 1310 | if (timing_safe_cmp(macbuf, buffer_ptr(&active_state->input), |
1311 | mac->mac_len) != 0) { | 1311 | mac->mac_len) != 0) { |
1312 | logit("Corrupted MAC on input."); | 1312 | logit("Corrupted MAC on input."); |
1313 | if (need > PACKET_MAX_SIZE) | 1313 | if (need > PACKET_MAX_SIZE) |
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: ssh-rsa.c,v 1.41 2010/04/16 01:47:26 djm Exp $ */ | 1 | /* $OpenBSD: ssh-rsa.c,v 1.42 2010/07/13 11:52:06 djm Exp $ */ |
2 | /* | 2 | /* |
3 | * Copyright (c) 2000, 2003 Markus Friedl <markus@openbsd.org> | 3 | * Copyright (c) 2000, 2003 Markus Friedl <markus@openbsd.org> |
4 | * | 4 | * |
@@ -30,6 +30,7 @@ | |||
30 | #include "buffer.h" | 30 | #include "buffer.h" |
31 | #include "key.h" | 31 | #include "key.h" |
32 | #include "compat.h" | 32 | #include "compat.h" |
33 | #include "misc.h" | ||
33 | #include "ssh.h" | 34 | #include "ssh.h" |
34 | 35 | ||
35 | static int openssh_RSA_verify(int, u_char *, u_int, u_char *, u_int, RSA *); | 36 | 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, | |||
249 | error("bad decrypted len: %d != %d + %d", len, hlen, oidlen); | 250 | error("bad decrypted len: %d != %d + %d", len, hlen, oidlen); |
250 | goto done; | 251 | goto done; |
251 | } | 252 | } |
252 | if (memcmp(decrypted, oid, oidlen) != 0) { | 253 | if (timing_safe_cmp(decrypted, oid, oidlen) != 0) { |
253 | error("oid mismatch"); | 254 | error("oid mismatch"); |
254 | goto done; | 255 | goto done; |
255 | } | 256 | } |
256 | if (memcmp(decrypted + oidlen, hash, hlen) != 0) { | 257 | if (timing_safe_cmp(decrypted + oidlen, hash, hlen) != 0) { |
257 | error("hash mismatch"); | 258 | error("hash mismatch"); |
258 | goto done; | 259 | goto done; |
259 | } | 260 | } |