diff options
author | ian.mcgreer%sun.com <devnull@localhost> | 2003-02-28 22:40:06 +0000 |
---|---|---|
committer | ian.mcgreer%sun.com <devnull@localhost> | 2003-02-28 22:40:06 +0000 |
commit | 6121622a1ac9992afb7462af6e2573a5617b59b3 (patch) | |
tree | bab86d2aa5553d6d13d5b5540e98990fe0e662f6 | |
parent | b6dae4d0db88092603cb3d40c5caf40ce99b29f5 (diff) | |
parent | 382d44444561e263b8b27d1b8e79f0f749204c2e (diff) | |
download | nss-hg-6121622a1ac9992afb7462af6e2573a5617b59b3.tar.gz |
Fix bug 160207. Make TLS implementation resistant to timing attacks on
CBC block mode cipher suites in TLS. See bug for details.
-rw-r--r-- | security/nss/lib/ssl/ssl3con.c | 60 |
1 files changed, 29 insertions, 31 deletions
diff --git a/security/nss/lib/ssl/ssl3con.c b/security/nss/lib/ssl/ssl3con.c index 858e02d39..792ef5372 100644 --- a/security/nss/lib/ssl/ssl3con.c +++ b/security/nss/lib/ssl/ssl3con.c @@ -7208,9 +7208,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]; @@ -7245,6 +7246,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; } } @@ -7270,11 +7272,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); - if (isTLS) - (void)SSL3_SendAlert(ss, alert_fatal, decryption_failed); - ssl_MapLowLevelError(SSL_ERROR_DECRYPTION_FAILURE); + SSL3_SendAlert(ss, alert_fatal, + isTLS ? decryption_failed : bad_record_mac); + PORT_SetError(err); return SECFailure; } @@ -7282,49 +7284,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 doesn't have an alert for bad padding, so use bad mac. */ - SSL3_SendAlert(ss, alert_fatal, - isTLS ? decryption_failed : 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)); |