diff options
| -rw-r--r-- | src/lib/libssl/s3_pkt.c | 47 | ||||
| -rw-r--r-- | src/lib/libssl/src/ssl/s3_pkt.c | 47 |
2 files changed, 62 insertions, 32 deletions
diff --git a/src/lib/libssl/s3_pkt.c b/src/lib/libssl/s3_pkt.c index 6ccea9aee5..3f88429e79 100644 --- a/src/lib/libssl/s3_pkt.c +++ b/src/lib/libssl/s3_pkt.c | |||
| @@ -238,6 +238,8 @@ static int ssl3_get_record(SSL *s) | |||
| 238 | unsigned int mac_size; | 238 | unsigned int mac_size; |
| 239 | int clear=0; | 239 | int clear=0; |
| 240 | size_t extra; | 240 | size_t extra; |
| 241 | int decryption_failed_or_bad_record_mac = 0; | ||
| 242 | unsigned char *mac = NULL; | ||
| 241 | 243 | ||
| 242 | rr= &(s->s3->rrec); | 244 | rr= &(s->s3->rrec); |
| 243 | sess=s->session; | 245 | sess=s->session; |
| @@ -353,8 +355,11 @@ again: | |||
| 353 | /* SSLerr() and ssl3_send_alert() have been called */ | 355 | /* SSLerr() and ssl3_send_alert() have been called */ |
| 354 | goto err; | 356 | goto err; |
| 355 | 357 | ||
| 356 | /* otherwise enc_err == -1 */ | 358 | /* Otherwise enc_err == -1, which indicates bad padding |
| 357 | goto decryption_failed_or_bad_record_mac; | 359 | * (rec->length has not been changed in this case). |
| 360 | * To minimize information leaked via timing, we will perform | ||
| 361 | * the MAC computation anyway. */ | ||
| 362 | decryption_failed_or_bad_record_mac = 1; | ||
| 358 | } | 363 | } |
| 359 | 364 | ||
| 360 | #ifdef TLS_DEBUG | 365 | #ifdef TLS_DEBUG |
| @@ -380,28 +385,46 @@ printf("\n"); | |||
| 380 | SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_PRE_MAC_LENGTH_TOO_LONG); | 385 | SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_PRE_MAC_LENGTH_TOO_LONG); |
| 381 | goto f_err; | 386 | goto f_err; |
| 382 | #else | 387 | #else |
| 383 | goto decryption_failed_or_bad_record_mac; | 388 | decryption_failed_or_bad_record_mac = 1; |
| 384 | #endif | 389 | #endif |
| 385 | } | 390 | } |
| 386 | /* check the MAC for rr->input (it's in mac_size bytes at the tail) */ | 391 | /* check the MAC for rr->input (it's in mac_size bytes at the tail) */ |
| 387 | if (rr->length < mac_size) | 392 | if (rr->length >= mac_size) |
| 388 | { | 393 | { |
| 394 | rr->length -= mac_size; | ||
| 395 | mac = &rr->data[rr->length]; | ||
| 396 | } | ||
| 397 | else | ||
| 398 | { | ||
| 399 | /* record (minus padding) is too short to contain a MAC */ | ||
| 389 | #if 0 /* OK only for stream ciphers */ | 400 | #if 0 /* OK only for stream ciphers */ |
| 390 | al=SSL_AD_DECODE_ERROR; | 401 | al=SSL_AD_DECODE_ERROR; |
| 391 | SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_LENGTH_TOO_SHORT); | 402 | SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_LENGTH_TOO_SHORT); |
| 392 | goto f_err; | 403 | goto f_err; |
| 393 | #else | 404 | #else |
| 394 | goto decryption_failed_or_bad_record_mac; | 405 | decryption_failed_or_bad_record_mac = 1; |
| 406 | rr->length = 0; | ||
| 395 | #endif | 407 | #endif |
| 396 | } | 408 | } |
| 397 | rr->length-=mac_size; | ||
| 398 | i=s->method->ssl3_enc->mac(s,md,0); | 409 | i=s->method->ssl3_enc->mac(s,md,0); |
| 399 | if (memcmp(md,&(rr->data[rr->length]),mac_size) != 0) | 410 | if (mac == NULL || memcmp(md, mac, mac_size) != 0) |
| 400 | { | 411 | { |
| 401 | goto decryption_failed_or_bad_record_mac; | 412 | decryption_failed_or_bad_record_mac = 1; |
| 402 | } | 413 | } |
| 403 | } | 414 | } |
| 404 | 415 | ||
| 416 | if (decryption_failed_or_bad_record_mac) | ||
| 417 | { | ||
| 418 | /* A separate 'decryption_failed' alert was introduced with TLS 1.0, | ||
| 419 | * SSL 3.0 only has 'bad_record_mac'. But unless a decryption | ||
| 420 | * failure is directly visible from the ciphertext anyway, | ||
| 421 | * we should not reveal which kind of error occured -- this | ||
| 422 | * might become visible to an attacker (e.g. via a logfile) */ | ||
| 423 | al=SSL_AD_BAD_RECORD_MAC; | ||
| 424 | SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC); | ||
| 425 | goto f_err; | ||
| 426 | } | ||
| 427 | |||
| 405 | /* r->length is now just compressed */ | 428 | /* r->length is now just compressed */ |
| 406 | if (s->expand != NULL) | 429 | if (s->expand != NULL) |
| 407 | { | 430 | { |
| @@ -443,14 +466,6 @@ printf("\n"); | |||
| 443 | 466 | ||
| 444 | return(1); | 467 | return(1); |
| 445 | 468 | ||
| 446 | decryption_failed_or_bad_record_mac: | ||
| 447 | /* Separate 'decryption_failed' alert was introduced with TLS 1.0, | ||
| 448 | * SSL 3.0 only has 'bad_record_mac'. But unless a decryption | ||
| 449 | * failure is directly visible from the ciphertext anyway, | ||
| 450 | * we should not reveal which kind of error occured -- this | ||
| 451 | * might become visible to an attacker (e.g. via logfile) */ | ||
| 452 | al=SSL_AD_BAD_RECORD_MAC; | ||
| 453 | SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC); | ||
| 454 | f_err: | 469 | f_err: |
| 455 | ssl3_send_alert(s,SSL3_AL_FATAL,al); | 470 | ssl3_send_alert(s,SSL3_AL_FATAL,al); |
| 456 | err: | 471 | err: |
diff --git a/src/lib/libssl/src/ssl/s3_pkt.c b/src/lib/libssl/src/ssl/s3_pkt.c index 6ccea9aee5..3f88429e79 100644 --- a/src/lib/libssl/src/ssl/s3_pkt.c +++ b/src/lib/libssl/src/ssl/s3_pkt.c | |||
| @@ -238,6 +238,8 @@ static int ssl3_get_record(SSL *s) | |||
| 238 | unsigned int mac_size; | 238 | unsigned int mac_size; |
| 239 | int clear=0; | 239 | int clear=0; |
| 240 | size_t extra; | 240 | size_t extra; |
| 241 | int decryption_failed_or_bad_record_mac = 0; | ||
| 242 | unsigned char *mac = NULL; | ||
| 241 | 243 | ||
| 242 | rr= &(s->s3->rrec); | 244 | rr= &(s->s3->rrec); |
| 243 | sess=s->session; | 245 | sess=s->session; |
| @@ -353,8 +355,11 @@ again: | |||
| 353 | /* SSLerr() and ssl3_send_alert() have been called */ | 355 | /* SSLerr() and ssl3_send_alert() have been called */ |
| 354 | goto err; | 356 | goto err; |
| 355 | 357 | ||
| 356 | /* otherwise enc_err == -1 */ | 358 | /* Otherwise enc_err == -1, which indicates bad padding |
| 357 | goto decryption_failed_or_bad_record_mac; | 359 | * (rec->length has not been changed in this case). |
| 360 | * To minimize information leaked via timing, we will perform | ||
| 361 | * the MAC computation anyway. */ | ||
| 362 | decryption_failed_or_bad_record_mac = 1; | ||
| 358 | } | 363 | } |
| 359 | 364 | ||
| 360 | #ifdef TLS_DEBUG | 365 | #ifdef TLS_DEBUG |
| @@ -380,28 +385,46 @@ printf("\n"); | |||
| 380 | SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_PRE_MAC_LENGTH_TOO_LONG); | 385 | SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_PRE_MAC_LENGTH_TOO_LONG); |
| 381 | goto f_err; | 386 | goto f_err; |
| 382 | #else | 387 | #else |
| 383 | goto decryption_failed_or_bad_record_mac; | 388 | decryption_failed_or_bad_record_mac = 1; |
| 384 | #endif | 389 | #endif |
| 385 | } | 390 | } |
| 386 | /* check the MAC for rr->input (it's in mac_size bytes at the tail) */ | 391 | /* check the MAC for rr->input (it's in mac_size bytes at the tail) */ |
| 387 | if (rr->length < mac_size) | 392 | if (rr->length >= mac_size) |
| 388 | { | 393 | { |
| 394 | rr->length -= mac_size; | ||
| 395 | mac = &rr->data[rr->length]; | ||
| 396 | } | ||
| 397 | else | ||
| 398 | { | ||
| 399 | /* record (minus padding) is too short to contain a MAC */ | ||
| 389 | #if 0 /* OK only for stream ciphers */ | 400 | #if 0 /* OK only for stream ciphers */ |
| 390 | al=SSL_AD_DECODE_ERROR; | 401 | al=SSL_AD_DECODE_ERROR; |
| 391 | SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_LENGTH_TOO_SHORT); | 402 | SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_LENGTH_TOO_SHORT); |
| 392 | goto f_err; | 403 | goto f_err; |
| 393 | #else | 404 | #else |
| 394 | goto decryption_failed_or_bad_record_mac; | 405 | decryption_failed_or_bad_record_mac = 1; |
| 406 | rr->length = 0; | ||
| 395 | #endif | 407 | #endif |
| 396 | } | 408 | } |
| 397 | rr->length-=mac_size; | ||
| 398 | i=s->method->ssl3_enc->mac(s,md,0); | 409 | i=s->method->ssl3_enc->mac(s,md,0); |
| 399 | if (memcmp(md,&(rr->data[rr->length]),mac_size) != 0) | 410 | if (mac == NULL || memcmp(md, mac, mac_size) != 0) |
| 400 | { | 411 | { |
| 401 | goto decryption_failed_or_bad_record_mac; | 412 | decryption_failed_or_bad_record_mac = 1; |
| 402 | } | 413 | } |
| 403 | } | 414 | } |
| 404 | 415 | ||
| 416 | if (decryption_failed_or_bad_record_mac) | ||
| 417 | { | ||
| 418 | /* A separate 'decryption_failed' alert was introduced with TLS 1.0, | ||
| 419 | * SSL 3.0 only has 'bad_record_mac'. But unless a decryption | ||
| 420 | * failure is directly visible from the ciphertext anyway, | ||
| 421 | * we should not reveal which kind of error occured -- this | ||
| 422 | * might become visible to an attacker (e.g. via a logfile) */ | ||
| 423 | al=SSL_AD_BAD_RECORD_MAC; | ||
| 424 | SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC); | ||
| 425 | goto f_err; | ||
| 426 | } | ||
| 427 | |||
| 405 | /* r->length is now just compressed */ | 428 | /* r->length is now just compressed */ |
| 406 | if (s->expand != NULL) | 429 | if (s->expand != NULL) |
| 407 | { | 430 | { |
| @@ -443,14 +466,6 @@ printf("\n"); | |||
| 443 | 466 | ||
| 444 | return(1); | 467 | return(1); |
| 445 | 468 | ||
| 446 | decryption_failed_or_bad_record_mac: | ||
| 447 | /* Separate 'decryption_failed' alert was introduced with TLS 1.0, | ||
| 448 | * SSL 3.0 only has 'bad_record_mac'. But unless a decryption | ||
| 449 | * failure is directly visible from the ciphertext anyway, | ||
| 450 | * we should not reveal which kind of error occured -- this | ||
| 451 | * might become visible to an attacker (e.g. via logfile) */ | ||
| 452 | al=SSL_AD_BAD_RECORD_MAC; | ||
| 453 | SSLerr(SSL_F_SSL3_GET_RECORD,SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC); | ||
| 454 | f_err: | 469 | f_err: |
| 455 | ssl3_send_alert(s,SSL3_AL_FATAL,al); | 470 | ssl3_send_alert(s,SSL3_AL_FATAL,al); |
| 456 | err: | 471 | err: |
