From 99d11a3ed2eb13e2f3ba13280d416369c45a30a6 Mon Sep 17 00:00:00 2001 From: Darren Tucker Date: Mon, 1 Dec 2008 21:40:48 +1100 Subject: - markus@cvs.openbsd.org 2008/11/21 15:47:38 [packet.c] packet_disconnect() on padding error, too. should reduce the success probability for the CPNI-957037 Plaintext Recovery Attack to 2^-18 ok djm@ --- packet.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'packet.c') diff --git a/packet.c b/packet.c index 8abd43eb4..4ded17fac 100644 --- a/packet.c +++ b/packet.c @@ -1,4 +1,4 @@ -/* $OpenBSD: packet.c,v 1.157 2008/07/10 18:08:11 markus Exp $ */ +/* $OpenBSD: packet.c,v 1.158 2008/11/21 15:47:38 markus Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -1152,7 +1152,8 @@ packet_read_poll2(u_int32_t *seqnr_p) #ifdef PACKET_DEBUG buffer_dump(&incoming_packet); #endif - packet_disconnect("Bad packet length %u.", packet_length); + packet_disconnect("Bad packet length %-10u", + packet_length); } DBG(debug("input: packet len %u", packet_length+4)); buffer_consume(&input, block_size); @@ -1161,9 +1162,11 @@ packet_read_poll2(u_int32_t *seqnr_p) need = 4 + packet_length - block_size; DBG(debug("partial packet %d, need %d, maclen %d", block_size, need, maclen)); - if (need % block_size != 0) - fatal("padding error: need %d block %d mod %d", + if (need % block_size != 0) { + logit("padding error: need %d block %d mod %d", need, block_size, need % block_size); + packet_disconnect("Bad packet length %-10u", packet_length); + } /* * check if the entire packet has been received and * decrypt into incoming_packet -- cgit v1.2.3 From 13ae44ce5865b720708aae9cb1d2e2f08a0d90cb Mon Sep 17 00:00:00 2001 From: Damien Miller Date: Wed, 28 Jan 2009 16:38:41 +1100 Subject: - markus@cvs.openbsd.org 2009/01/26 09:58:15 [cipher.c cipher.h packet.c] Work around the CPNI-957037 Plaintext Recovery Attack by always reading 256K of data on packet size or HMAC errors (in CBC mode only). Help, feedback and ok djm@ Feedback from Martin Albrecht and Paterson Kenny --- ChangeLog | 8 ++++++- cipher.c | 49 ++++++++++++++++++++++++------------------- cipher.h | 3 ++- packet.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------- 4 files changed, 102 insertions(+), 30 deletions(-) (limited to 'packet.c') diff --git a/ChangeLog b/ChangeLog index 1ce3a84b8..8584ff316 100644 --- a/ChangeLog +++ b/ChangeLog @@ -80,6 +80,12 @@ - naddy@cvs.openbsd.org 2009/01/24 17:10:22 [ssh_config.5 sshd_config.5] sync list of preferred ciphers; ok djm@ + - markus@cvs.openbsd.org 2009/01/26 09:58:15 + [cipher.c cipher.h packet.c] + Work around the CPNI-957037 Plaintext Recovery Attack by always + reading 256K of data on packet size or HMAC errors (in CBC mode only). + Help, feedback and ok djm@ + Feedback from Martin Albrecht and Paterson Kenny 20090107 - (djm) [uidswap.c] bz#1412: Support >16 supplemental groups in OS X. @@ -5089,5 +5095,5 @@ OpenServer 6 and add osr5bigcrypt support so when someone migrates passwords between UnixWare and OpenServer they will still work. OK dtucker@ -$Id: ChangeLog,v 1.5178 2009/01/28 05:34:00 djm Exp $ +$Id: ChangeLog,v 1.5179 2009/01/28 05:38:41 djm Exp $ diff --git a/cipher.c b/cipher.c index b264063c4..bb5c0ac3a 100644 --- a/cipher.c +++ b/cipher.c @@ -1,4 +1,4 @@ -/* $OpenBSD: cipher.c,v 1.81 2006/08/03 03:34:42 deraadt Exp $ */ +/* $OpenBSD: cipher.c,v 1.82 2009/01/26 09:58:15 markus Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -63,31 +63,32 @@ struct Cipher { u_int block_size; u_int key_len; u_int discard_len; + u_int cbc_mode; const EVP_CIPHER *(*evptype)(void); } ciphers[] = { - { "none", SSH_CIPHER_NONE, 8, 0, 0, EVP_enc_null }, - { "des", SSH_CIPHER_DES, 8, 8, 0, EVP_des_cbc }, - { "3des", SSH_CIPHER_3DES, 8, 16, 0, evp_ssh1_3des }, - { "blowfish", SSH_CIPHER_BLOWFISH, 8, 32, 0, evp_ssh1_bf }, - - { "3des-cbc", SSH_CIPHER_SSH2, 8, 24, 0, EVP_des_ede3_cbc }, - { "blowfish-cbc", SSH_CIPHER_SSH2, 8, 16, 0, EVP_bf_cbc }, - { "cast128-cbc", SSH_CIPHER_SSH2, 8, 16, 0, EVP_cast5_cbc }, - { "arcfour", SSH_CIPHER_SSH2, 8, 16, 0, EVP_rc4 }, - { "arcfour128", SSH_CIPHER_SSH2, 8, 16, 1536, EVP_rc4 }, - { "arcfour256", SSH_CIPHER_SSH2, 8, 32, 1536, EVP_rc4 }, - { "aes128-cbc", SSH_CIPHER_SSH2, 16, 16, 0, EVP_aes_128_cbc }, - { "aes192-cbc", SSH_CIPHER_SSH2, 16, 24, 0, EVP_aes_192_cbc }, - { "aes256-cbc", SSH_CIPHER_SSH2, 16, 32, 0, EVP_aes_256_cbc }, + { "none", SSH_CIPHER_NONE, 8, 0, 0, 0, EVP_enc_null }, + { "des", SSH_CIPHER_DES, 8, 8, 0, 1, EVP_des_cbc }, + { "3des", SSH_CIPHER_3DES, 8, 16, 0, 1, evp_ssh1_3des }, + { "blowfish", SSH_CIPHER_BLOWFISH, 8, 32, 0, 1, evp_ssh1_bf }, + + { "3des-cbc", SSH_CIPHER_SSH2, 8, 24, 0, 1, EVP_des_ede3_cbc }, + { "blowfish-cbc", SSH_CIPHER_SSH2, 8, 16, 0, 1, EVP_bf_cbc }, + { "cast128-cbc", SSH_CIPHER_SSH2, 8, 16, 0, 1, EVP_cast5_cbc }, + { "arcfour", SSH_CIPHER_SSH2, 8, 16, 0, 0, EVP_rc4 }, + { "arcfour128", SSH_CIPHER_SSH2, 8, 16, 1536, 0, EVP_rc4 }, + { "arcfour256", SSH_CIPHER_SSH2, 8, 32, 1536, 0, EVP_rc4 }, + { "aes128-cbc", SSH_CIPHER_SSH2, 16, 16, 0, 1, EVP_aes_128_cbc }, + { "aes192-cbc", SSH_CIPHER_SSH2, 16, 24, 0, 1, EVP_aes_192_cbc }, + { "aes256-cbc", SSH_CIPHER_SSH2, 16, 32, 0, 1, EVP_aes_256_cbc }, { "rijndael-cbc@lysator.liu.se", - SSH_CIPHER_SSH2, 16, 32, 0, EVP_aes_256_cbc }, - { "aes128-ctr", SSH_CIPHER_SSH2, 16, 16, 0, evp_aes_128_ctr }, - { "aes192-ctr", SSH_CIPHER_SSH2, 16, 24, 0, evp_aes_128_ctr }, - { "aes256-ctr", SSH_CIPHER_SSH2, 16, 32, 0, evp_aes_128_ctr }, + SSH_CIPHER_SSH2, 16, 32, 0, 1, EVP_aes_256_cbc }, + { "aes128-ctr", SSH_CIPHER_SSH2, 16, 16, 0, 0, evp_aes_128_ctr }, + { "aes192-ctr", SSH_CIPHER_SSH2, 16, 24, 0, 0, evp_aes_128_ctr }, + { "aes256-ctr", SSH_CIPHER_SSH2, 16, 32, 0, 0, evp_aes_128_ctr }, #ifdef USE_CIPHER_ACSS - { "acss@openssh.org", SSH_CIPHER_SSH2, 16, 5, 0, EVP_acss }, + { "acss@openssh.org", SSH_CIPHER_SSH2, 16, 5, 0, 0, EVP_acss }, #endif - { NULL, SSH_CIPHER_INVALID, 0, 0, 0, NULL } + { NULL, SSH_CIPHER_INVALID, 0, 0, 0, 0, NULL } }; /*--*/ @@ -110,6 +111,12 @@ cipher_get_number(const Cipher *c) return (c->number); } +u_int +cipher_is_cbc(const Cipher *c) +{ + return (c->cbc_mode); +} + u_int cipher_mask_ssh1(int client) { diff --git a/cipher.h b/cipher.h index 49bbc1682..3dd2270bb 100644 --- a/cipher.h +++ b/cipher.h @@ -1,4 +1,4 @@ -/* $OpenBSD: cipher.h,v 1.36 2006/03/25 22:22:42 djm Exp $ */ +/* $OpenBSD: cipher.h,v 1.37 2009/01/26 09:58:15 markus Exp $ */ /* * Author: Tatu Ylonen @@ -81,6 +81,7 @@ void cipher_cleanup(CipherContext *); void cipher_set_key_string(CipherContext *, Cipher *, const char *, int); u_int cipher_blocksize(const Cipher *); u_int cipher_keylen(const Cipher *); +u_int cipher_is_cbc(const Cipher *); u_int cipher_get_number(const Cipher *); void cipher_get_keyiv(CipherContext *, u_char *, u_int); diff --git a/packet.c b/packet.c index 4ded17fac..33559cd4f 100644 --- a/packet.c +++ b/packet.c @@ -1,4 +1,4 @@ -/* $OpenBSD: packet.c,v 1.158 2008/11/21 15:47:38 markus Exp $ */ +/* $OpenBSD: packet.c,v 1.159 2009/01/26 09:58:15 markus Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -84,6 +84,8 @@ #define DBG(x) #endif +#define PACKET_MAX_SIZE (256 * 1024) + /* * This variable contains the file descriptors used for communicating with * the other side. connection_in is used for reading; connection_out for @@ -160,6 +162,10 @@ static u_int ssh1_keylen; /* roundup current message to extra_pad bytes */ static u_char extra_pad = 0; +/* XXX discard incoming data after MAC error */ +static u_int packet_discard = 0; +static Mac *packet_discard_mac = NULL; + struct packet { TAILQ_ENTRY(packet) next; u_char type; @@ -209,6 +215,36 @@ packet_set_timeout(int timeout, int count) packet_timeout_ms = timeout * count * 1000; } +static void +packet_stop_discard(void) +{ + if (packet_discard_mac) { + char buf[1024]; + + memset(buf, 'a', sizeof(buf)); + while (buffer_len(&incoming_packet) < PACKET_MAX_SIZE) + buffer_append(&incoming_packet, buf, sizeof(buf)); + (void) mac_compute(packet_discard_mac, + p_read.seqnr, + buffer_ptr(&incoming_packet), + PACKET_MAX_SIZE); + } + logit("Finished discarding for %.200s", get_remote_ipaddr()); + cleanup_exit(255); +} + +static void +packet_start_discard(Enc *enc, Mac *mac, u_int packet_length, u_int discard) +{ + if (!cipher_is_cbc(enc->cipher)) + packet_disconnect("Packet corrupt"); + if (packet_length != PACKET_MAX_SIZE && mac && mac->enabled) + packet_discard_mac = mac; + if (buffer_len(&input) >= discard) + packet_stop_discard(); + packet_discard = discard - buffer_len(&input); +} + /* Returns 1 if remote host is connected via socket, 0 if not. */ int @@ -1127,6 +1163,9 @@ packet_read_poll2(u_int32_t *seqnr_p) Mac *mac = NULL; Comp *comp = NULL; + if (packet_discard) + return SSH_MSG_NONE; + if (newkeys[MODE_IN] != NULL) { enc = &newkeys[MODE_IN]->enc; mac = &newkeys[MODE_IN]->mac; @@ -1148,12 +1187,14 @@ packet_read_poll2(u_int32_t *seqnr_p) block_size); cp = buffer_ptr(&incoming_packet); packet_length = get_u32(cp); - if (packet_length < 1 + 4 || packet_length > 256 * 1024) { + if (packet_length < 1 + 4 || packet_length > PACKET_MAX_SIZE) { #ifdef PACKET_DEBUG buffer_dump(&incoming_packet); #endif - packet_disconnect("Bad packet length %-10u", - packet_length); + logit("Bad packet length %u.", packet_length); + packet_start_discard(enc, mac, packet_length, + PACKET_MAX_SIZE); + return SSH_MSG_NONE; } DBG(debug("input: packet len %u", packet_length+4)); buffer_consume(&input, block_size); @@ -1165,7 +1206,9 @@ packet_read_poll2(u_int32_t *seqnr_p) if (need % block_size != 0) { logit("padding error: need %d block %d mod %d", need, block_size, need % block_size); - packet_disconnect("Bad packet length %-10u", packet_length); + packet_start_discard(enc, mac, packet_length, + PACKET_MAX_SIZE - block_size); + return SSH_MSG_NONE; } /* * check if the entire packet has been received and @@ -1188,11 +1231,19 @@ packet_read_poll2(u_int32_t *seqnr_p) macbuf = mac_compute(mac, p_read.seqnr, buffer_ptr(&incoming_packet), buffer_len(&incoming_packet)); - if (memcmp(macbuf, buffer_ptr(&input), mac->mac_len) != 0) - packet_disconnect("Corrupted MAC on input."); + if (memcmp(macbuf, buffer_ptr(&input), mac->mac_len) != 0) { + logit("Corrupted MAC on input."); + if (need > PACKET_MAX_SIZE) + fatal("internal error need %d", need); + packet_start_discard(enc, mac, packet_length, + PACKET_MAX_SIZE - need); + return SSH_MSG_NONE; + } + DBG(debug("MAC #%d ok", p_read.seqnr)); buffer_consume(&input, mac->mac_len); } + /* XXX now it's safe to use fatal/packet_disconnect */ if (seqnr_p != NULL) *seqnr_p = p_read.seqnr; if (++p_read.seqnr == 0) @@ -1325,6 +1376,13 @@ packet_read_poll(void) void packet_process_incoming(const char *buf, u_int len) { + if (packet_discard) { + keep_alive_timeouts = 0; /* ?? */ + if (len >= packet_discard) + packet_stop_discard(); + packet_discard -= len; + return; + } buffer_append(&input, buf, len); } -- cgit v1.2.3 From 61433bec808fc90de066902e793147fd5015a2cc Mon Sep 17 00:00:00 2001 From: Damien Miller Date: Sat, 14 Feb 2009 16:35:01 +1100 Subject: - markus@cvs.openbsd.org 2009/02/13 11:50:21 [packet.c] check for enc !=NULL in packet_start_discard --- ChangeLog | 5 ++++- packet.c | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) (limited to 'packet.c') diff --git a/ChangeLog b/ChangeLog index 8e0c681ba..a2d2045af 100644 --- a/ChangeLog +++ b/ChangeLog @@ -30,6 +30,9 @@ - jmc@cvs.openbsd.org 2009/02/12 07:34:20 [ssh_config.5] kill trailing whitespace; + - markus@cvs.openbsd.org 2009/02/13 11:50:21 + [packet.c] + check for enc !=NULL in packet_start_discard 20090212 - (djm) [sshpty.c] bz#1419: OSX uses cloning ptys that automagically @@ -5156,5 +5159,5 @@ OpenServer 6 and add osr5bigcrypt support so when someone migrates passwords between UnixWare and OpenServer they will still work. OK dtucker@ -$Id: ChangeLog,v 1.5193 2009/02/14 05:34:39 djm Exp $ +$Id: ChangeLog,v 1.5194 2009/02/14 05:35:01 djm Exp $ diff --git a/packet.c b/packet.c index 33559cd4f..5afc84ce0 100644 --- a/packet.c +++ b/packet.c @@ -1,4 +1,4 @@ -/* $OpenBSD: packet.c,v 1.159 2009/01/26 09:58:15 markus Exp $ */ +/* $OpenBSD: packet.c,v 1.160 2009/02/13 11:50:21 markus Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -236,7 +236,7 @@ packet_stop_discard(void) static void packet_start_discard(Enc *enc, Mac *mac, u_int packet_length, u_int discard) { - if (!cipher_is_cbc(enc->cipher)) + if (enc == NULL || !cipher_is_cbc(enc->cipher)) packet_disconnect("Packet corrupt"); if (packet_length != PACKET_MAX_SIZE && mac && mac->enabled) packet_discard_mac = mac; -- cgit v1.2.3