diff options
author | wtc%netscape.com <devnull@localhost> | 2003-02-25 00:41:29 +0000 |
---|---|---|
committer | wtc%netscape.com <devnull@localhost> | 2003-02-25 00:41:29 +0000 |
commit | b6f60142adb08c8866f54c68211a46c1a8d17720 (patch) | |
tree | 62cae9daee536e94b2979e259fa583b7bc486d4c | |
parent | 6deebffb838c0db1f5245465d8d120cfa646b997 (diff) | |
download | nss-hg-b6f60142adb08c8866f54c68211a46c1a8d17720.tar.gz |
Fix bug 160207. Make TLS implementation resistant to timing attacks on
CBC block mode cipher suites in TLS. See bug for details.
Tag: MOZILLA_1_3_BRANCH
-rw-r--r-- | security/nss/lib/ssl/ssl3con.c | 59 |
1 files changed, 29 insertions, 30 deletions
diff --git a/security/nss/lib/ssl/ssl3con.c b/security/nss/lib/ssl/ssl3con.c index 4bb873b03..10c30c528 100644 --- a/security/nss/lib/ssl/ssl3con.c +++ b/security/nss/lib/ssl/ssl3con.c @@ -7393,9 +7393,10 @@ const ssl3BulkCipherDef *cipher_def; ssl3State * ssl3 = ss->ssl3; ssl3CipherSpec * crSpec; SECStatus rv; - unsigned int hashBytes; + unsigned int hashBytes = MAX_MAC_LENGTH + 1; unsigned int padding_length; PRBool isTLS; + PRBool padIsBad = PR_FALSE; SSL3ContentType rType; SSL3Opaque hash[MAX_MAC_LENGTH]; @@ -7430,6 +7431,7 @@ const ssl3BulkCipherDef *cipher_def; SSL_DBG(("%d: SSL3[%d]: HandleRecord, tried to get %d bytes", SSL_GETPID(), ss->fd, MAX_FRAGMENT_LENGTH + 2048)); /* sslBuffer_Grow has set a memory error code. */ + /* Perhaps we should send an alert. (but we have no memory!) */ return SECFailure; } } @@ -7455,11 +7457,11 @@ const ssl3BulkCipherDef *cipher_def; PRINT_BUF(80, (ss, "cleartext:", databuf->buf, databuf->len)); if (rv != SECSuccess) { + int err = ssl_MapLowLevelError(SSL_ERROR_DECRYPTION_FAILURE); ssl_ReleaseSpecReadLock(ss); - ssl_MapLowLevelError(SSL_ERROR_DECRYPTION_FAILURE); - SSL3_SendAlert(ss, alert_fatal, - isTLS ? decryption_failed : bad_record_mac); - ssl_MapLowLevelError(SSL_ERROR_DECRYPTION_FAILURE); + SSL3_SendAlert(ss, alert_fatal, + isTLS ? decryption_failed : bad_record_mac); + PORT_SetError(err); return SECFailure; } @@ -7467,48 +7469,45 @@ const ssl3BulkCipherDef *cipher_def; if (cipher_def->type == type_block) { padding_length = *(databuf->buf + databuf->len - 1); /* TLS permits padding to exceed the block size, up to 255 bytes. */ - if (padding_length + crSpec->mac_size >= databuf->len) - goto bad_pad; + if (padding_length + 1 + crSpec->mac_size > databuf->len) + padIsBad = PR_TRUE; /* if TLS, check value of first padding byte. */ - if (padding_length && isTLS && padding_length != - *(databuf->buf + databuf->len - 1 - padding_length)) - goto bad_pad; - databuf->len -= padding_length + 1; - if (databuf->len <= 0) { -bad_pad: - /* must not hold spec lock when calling SSL3_SendAlert. */ - ssl_ReleaseSpecReadLock(ss); - /* SSL3 & TLS must send bad_record_mac if padding check fails. */ - SSL3_SendAlert(ss, alert_fatal, bad_record_mac); - PORT_SetError(SSL_ERROR_BAD_BLOCK_PADDING); - return SECFailure; - } + else if (padding_length && isTLS && + padding_length != + *(databuf->buf + databuf->len - (padding_length + 1))) + padIsBad = PR_TRUE; + else + databuf->len -= padding_length + 1; } - /* Check the MAC. */ - if (databuf->len < crSpec->mac_size) { - /* record is too short to have a valid mac. */ - goto bad_mac; - } - databuf->len -= crSpec->mac_size; + /* Remove the MAC. */ + if (databuf->len >= crSpec->mac_size) + databuf->len -= crSpec->mac_size; + else + padIsBad = PR_TRUE; /* really macIsBad */ + + /* compute the MAC */ rType = cText->type; rv = ssl3_ComputeRecordMAC( - crSpec, (ss->sec.isServer) ? crSpec->client.write_mac_context + crSpec, (ss->sec.isServer) ? crSpec->client.write_mac_context : crSpec->server.write_mac_context, rType, cText->version, crSpec->read_seq_num, databuf->buf, databuf->len, hash, &hashBytes); if (rv != SECSuccess) { + int err = ssl_MapLowLevelError(SSL_ERROR_MAC_COMPUTATION_FAILURE); ssl_ReleaseSpecReadLock(ss); - ssl_MapLowLevelError(SSL_ERROR_MAC_COMPUTATION_FAILURE); + SSL3_SendAlert(ss, alert_fatal, bad_record_mac); + PORT_SetError(err); return rv; } - if (hashBytes != (unsigned)crSpec->mac_size || + /* Check the MAC */ + if (hashBytes != (unsigned)crSpec->mac_size || padIsBad || PORT_Memcmp(databuf->buf + databuf->len, hash, crSpec->mac_size) != 0) { -bad_mac: /* must not hold spec lock when calling SSL3_SendAlert. */ ssl_ReleaseSpecReadLock(ss); SSL3_SendAlert(ss, alert_fatal, bad_record_mac); + /* always log mac error, in case attacker can read server logs. */ PORT_SetError(SSL_ERROR_BAD_MAC_READ); SSL_DBG(("%d: SSL3[%d]: mac check failed", SSL_GETPID(), ss->fd)); |