diff options
author | miod <> | 2003-02-22 17:46:50 +0000 |
---|---|---|
committer | miod <> | 2003-02-22 17:46:50 +0000 |
commit | 87d7c154b9268b39059a87e45807e00030655fbf (patch) | |
tree | 0a4af9690c4d9d6a18c0ed0b29df199e43dc2171 | |
parent | 8e56586eb490b614a4b8859efc62af858f14f81c (diff) | |
download | openbsd-87d7c154b9268b39059a87e45807e00030655fbf.tar.gz openbsd-87d7c154b9268b39059a87e45807e00030655fbf.tar.bz2 openbsd-87d7c154b9268b39059a87e45807e00030655fbf.zip |
Errata 021:
security fix from openssl 0.9.7a:
In ssl3_get_record (ssl/s3_pkt.c), minimize information leaked
via timing by performing a MAC computation even if incorrrect
block cipher padding has been found. This is a countermeasure
against active attacks where the attacker has to distinguish
between bad padding and a MAC verification error. (CAN-2003-0078)
adapted from a patch from Ryan W. Maple, via markus@
-rw-r--r-- | src/lib/libssl/src/ssl/s3_pkt.c | 57 | ||||
-rw-r--r-- | src/lib/libssl/src/ssl/ssl.h | 1 | ||||
-rw-r--r-- | src/lib/libssl/src/ssl/ssl_err.c | 1 |
3 files changed, 48 insertions, 11 deletions
diff --git a/src/lib/libssl/src/ssl/s3_pkt.c b/src/lib/libssl/src/ssl/s3_pkt.c index 9ab76604a6..4abd52326e 100644 --- a/src/lib/libssl/src/ssl/s3_pkt.c +++ b/src/lib/libssl/src/ssl/s3_pkt.c | |||
@@ -231,7 +231,7 @@ static int ssl3_read_n(SSL *s, int n, int max, int extend) | |||
231 | static int ssl3_get_record(SSL *s) | 231 | static int ssl3_get_record(SSL *s) |
232 | { | 232 | { |
233 | int ssl_major,ssl_minor,al; | 233 | int ssl_major,ssl_minor,al; |
234 | int n,i,ret= -1; | 234 | int enc_err,n,i,ret= -1; |
235 | SSL3_RECORD *rr; | 235 | SSL3_RECORD *rr; |
236 | SSL_SESSION *sess; | 236 | SSL_SESSION *sess; |
237 | unsigned char *p; | 237 | unsigned char *p; |
@@ -239,6 +239,8 @@ static int ssl3_get_record(SSL *s) | |||
239 | short version; | 239 | short version; |
240 | unsigned int mac_size; | 240 | unsigned int mac_size; |
241 | int clear=0,extra; | 241 | int clear=0,extra; |
242 | int decryption_failed_or_bad_record_mac = 0; | ||
243 | unsigned char *mac = NULL; | ||
242 | 244 | ||
243 | rr= &(s->s3->rrec); | 245 | rr= &(s->s3->rrec); |
244 | sess=s->session; | 246 | sess=s->session; |
@@ -342,16 +344,26 @@ again: | |||
342 | /* decrypt in place in 'rr->input' */ | 344 | /* decrypt in place in 'rr->input' */ |
343 | rr->data=rr->input; | 345 | rr->data=rr->input; |
344 | 346 | ||
345 | if (!s->method->ssl3_enc->enc(s,0)) | 347 | enc_err = s->method->ssl3_enc->enc(s,0); |
348 | if (enc_err <= 0) | ||
346 | { | 349 | { |
347 | al=SSL_AD_DECRYPT_ERROR; | 350 | if (enc_err == 0) |
348 | goto f_err; | 351 | /* SSLerr() and ssl3_send_alert() have been called */ |
352 | goto err; | ||
353 | |||
354 | /* Otherwise enc_err == -1, which indicates bad padding | ||
355 | * (rec->length has not been changed in this case). | ||
356 | * To minimize information leaked via timing, we will perform | ||
357 | * the MAC computation anyway. */ | ||
358 | decryption_failed_or_bad_record_mac = 1; | ||
349 | } | 359 | } |
360 | |||
350 | #ifdef TLS_DEBUG | 361 | #ifdef TLS_DEBUG |
351 | printf("dec %d\n",rr->length); | 362 | printf("dec %d\n",rr->length); |
352 | { unsigned int z; for (z=0; z<rr->length; z++) printf("%02X%c",rr->data[z],((z+1)%16)?' ':'\n'); } | 363 | { unsigned int z; for (z=0; z<rr->length; z++) printf("%02X%c",rr->data[z],((z+1)%16)?' ':'\n'); } |
353 | printf("\n"); | 364 | printf("\n"); |
354 | #endif | 365 | #endif |
366 | |||
355 | /* r->length is now the compressed data plus mac */ | 367 | /* r->length is now the compressed data plus mac */ |
356 | if ( (sess == NULL) || | 368 | if ( (sess == NULL) || |
357 | (s->enc_read_ctx == NULL) || | 369 | (s->enc_read_ctx == NULL) || |
@@ -364,28 +376,51 @@ printf("\n"); | |||
364 | 376 | ||
365 | if (rr->length > SSL3_RT_MAX_COMPRESSED_LENGTH+extra+mac_size) | 377 | if (rr->length > SSL3_RT_MAX_COMPRESSED_LENGTH+extra+mac_size) |
366 | { | 378 | { |
379 | #if 0 /* OK only for stream ciphers (then rr->length is visible from ciphertext anyway) */ | ||
367 | al=SSL_AD_RECORD_OVERFLOW; | 380 | al=SSL_AD_RECORD_OVERFLOW; |
368 | SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_PRE_MAC_LENGTH_TOO_LONG); | 381 | SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_PRE_MAC_LENGTH_TOO_LONG); |
369 | goto f_err; | 382 | goto f_err; |
383 | #else | ||
384 | decryption_failed_or_bad_record_mac = 1; | ||
385 | #endif | ||
370 | } | 386 | } |
371 | /* check the MAC for rr->input (it's in mac_size bytes at the tail) */ | 387 | /* check the MAC for rr->input (it's in mac_size bytes at the tail) */ |
372 | if (rr->length < mac_size) | 388 | if (rr->length >= mac_size) |
373 | { | 389 | { |
390 | rr->length -= mac_size; | ||
391 | mac = &rr->data[rr->length]; | ||
392 | } | ||
393 | else | ||
394 | { | ||
395 | /* record (minus padding) is too short to contain a MAC */ | ||
396 | #if 0 /* OK only for stream ciphers */ | ||
374 | al=SSL_AD_DECODE_ERROR; | 397 | al=SSL_AD_DECODE_ERROR; |
375 | SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_LENGTH_TOO_SHORT); | 398 | SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_LENGTH_TOO_SHORT); |
376 | goto f_err; | 399 | goto f_err; |
400 | #else | ||
401 | decryption_failed_or_bad_record_mac = 1; | ||
402 | rr->length = 0; | ||
403 | #endif | ||
377 | } | 404 | } |
378 | rr->length-=mac_size; | ||
379 | i=s->method->ssl3_enc->mac(s,md,0); | 405 | i=s->method->ssl3_enc->mac(s,md,0); |
380 | if (memcmp(md,&(rr->data[rr->length]),mac_size) != 0) | 406 | if (mac == NULL || memcmp(md, mac, mac_size) != 0) |
381 | { | 407 | { |
382 | al=SSL_AD_BAD_RECORD_MAC; | 408 | decryption_failed_or_bad_record_mac = 1; |
383 | SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_BAD_MAC_DECODE); | ||
384 | ret= -1; | ||
385 | goto f_err; | ||
386 | } | 409 | } |
387 | } | 410 | } |
388 | 411 | ||
412 | if (decryption_failed_or_bad_record_mac) | ||
413 | { | ||
414 | /* A separate 'decryption_failed' alert was introduced with TLS 1.0, | ||
415 | * SSL 3.0 only has 'bad_record_mac'. But unless a decryption | ||
416 | * failure is directly visible from the ciphertext anyway, | ||
417 | * we should not reveal which kind of error occured -- this | ||
418 | * might become visible to an attacker (e.g. via a logfile) */ | ||
419 | al=SSL_AD_BAD_RECORD_MAC; | ||
420 | SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC); | ||
421 | goto f_err; | ||
422 | } | ||
423 | |||
389 | /* r->length is now just compressed */ | 424 | /* r->length is now just compressed */ |
390 | if (s->expand != NULL) | 425 | if (s->expand != NULL) |
391 | { | 426 | { |
diff --git a/src/lib/libssl/src/ssl/ssl.h b/src/lib/libssl/src/ssl/ssl.h index a673c0d97d..6995173709 100644 --- a/src/lib/libssl/src/ssl/ssl.h +++ b/src/lib/libssl/src/ssl/ssl.h | |||
@@ -1403,6 +1403,7 @@ void ERR_load_SSL_strings(void); | |||
1403 | #define SSL_R_DATA_BETWEEN_CCS_AND_FINISHED 145 | 1403 | #define SSL_R_DATA_BETWEEN_CCS_AND_FINISHED 145 |
1404 | #define SSL_R_DATA_LENGTH_TOO_LONG 146 | 1404 | #define SSL_R_DATA_LENGTH_TOO_LONG 146 |
1405 | #define SSL_R_DECRYPTION_FAILED 147 | 1405 | #define SSL_R_DECRYPTION_FAILED 147 |
1406 | #define SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC 1109 | ||
1406 | #define SSL_R_DH_PUBLIC_VALUE_LENGTH_IS_WRONG 148 | 1407 | #define SSL_R_DH_PUBLIC_VALUE_LENGTH_IS_WRONG 148 |
1407 | #define SSL_R_DIGEST_CHECK_FAILED 149 | 1408 | #define SSL_R_DIGEST_CHECK_FAILED 149 |
1408 | #define SSL_R_ENCRYPTED_LENGTH_TOO_LONG 150 | 1409 | #define SSL_R_ENCRYPTED_LENGTH_TOO_LONG 150 |
diff --git a/src/lib/libssl/src/ssl/ssl_err.c b/src/lib/libssl/src/ssl/ssl_err.c index 8c7c571a77..a75499004e 100644 --- a/src/lib/libssl/src/ssl/ssl_err.c +++ b/src/lib/libssl/src/ssl/ssl_err.c | |||
@@ -258,6 +258,7 @@ static ERR_STRING_DATA SSL_str_reasons[]= | |||
258 | {SSL_R_DATA_BETWEEN_CCS_AND_FINISHED ,"data between ccs and finished"}, | 258 | {SSL_R_DATA_BETWEEN_CCS_AND_FINISHED ,"data between ccs and finished"}, |
259 | {SSL_R_DATA_LENGTH_TOO_LONG ,"data length too long"}, | 259 | {SSL_R_DATA_LENGTH_TOO_LONG ,"data length too long"}, |
260 | {SSL_R_DECRYPTION_FAILED ,"decryption failed"}, | 260 | {SSL_R_DECRYPTION_FAILED ,"decryption failed"}, |
261 | {SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC,"decryption failed or bad record mac"}, | ||
261 | {SSL_R_DH_PUBLIC_VALUE_LENGTH_IS_WRONG ,"dh public value length is wrong"}, | 262 | {SSL_R_DH_PUBLIC_VALUE_LENGTH_IS_WRONG ,"dh public value length is wrong"}, |
262 | {SSL_R_DIGEST_CHECK_FAILED ,"digest check failed"}, | 263 | {SSL_R_DIGEST_CHECK_FAILED ,"digest check failed"}, |
263 | {SSL_R_ENCRYPTED_LENGTH_TOO_LONG ,"encrypted length too long"}, | 264 | {SSL_R_ENCRYPTED_LENGTH_TOO_LONG ,"encrypted length too long"}, |