Skip to content

Fix false positives in WOLFSSL_CIPHER_TEXT_CHECK for small records #10288

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Frauschi wants to merge 1 commit into from
Closed

Fix false positives in WOLFSSL_CIPHER_TEXT_CHECK for small records #10288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20673,9 +20673,16 @@ static WC_INLINE int Encrypt(WOLFSSL* ssl, byte* out, const byte* input,
}

#ifdef WOLFSSL_CIPHER_TEXT_CHECK
if (ssl->specs.bulk_cipher_algorithm != wolfssl_cipher_null) {
/* The compare in CIPHER_STATE_END compares sz bytes of ciphertext
* against the saved plaintext. For very small records that makes
* the glitch check statistically unreliable (e.g. a 1-byte
* compare legitimately collides roughly 1/256 of the time). Only
* dothe check when there is a full sanityCheck buffer of
* plaintext to compare against. */
if (ssl->specs.bulk_cipher_algorithm != wolfssl_cipher_null &&
sz >= sizeof(ssl->encrypt.sanityCheck)) {
XMEMCPY(ssl->encrypt.sanityCheck, input,
min(sz, sizeof(ssl->encrypt.sanityCheck)));
sizeof(ssl->encrypt.sanityCheck));
}
#endif

Expand Down Expand Up @@ -20761,9 +20768,12 @@ static WC_INLINE int Encrypt(WOLFSSL* ssl, byte* out, const byte* input,
case CIPHER_STATE_END:
{
#ifdef WOLFSSL_CIPHER_TEXT_CHECK
/* Only compare when CIPHER_STATE_BEGIN prepared the check with a
* full sanityCheck buffer of plaintext (see rationale there). */
if (ssl->specs.bulk_cipher_algorithm != wolfssl_cipher_null &&
sz >= sizeof(ssl->encrypt.sanityCheck) &&
XMEMCMP(out, ssl->encrypt.sanityCheck,
min(sz, sizeof(ssl->encrypt.sanityCheck))) == 0) {
sizeof(ssl->encrypt.sanityCheck)) == 0) {

WOLFSSL_MSG("Encrypt sanity check failed! Glitch?");
WOLFSSL_ERROR_VERBOSE(ENCRYPT_ERROR);
Expand Down
18 changes: 14 additions & 4 deletions src/tls13.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2638,10 +2638,18 @@ static int EncryptTls13(WOLFSSL* ssl, byte* output, const byte* input,
#endif

#ifdef WOLFSSL_CIPHER_TEXT_CHECK
/* The compare in CIPHER_STATE_END compares dataSz bytes of
* ciphertext against the saved plaintext. For very small AEAD
* records (e.g. a 1-byte empty TLS 1.3 app-data record, whose
* plaintext is just the content-type byte) that makes the glitch
* check statistically unreliable. With only 1 byte compared,
* legitimate encryption collides roughly 1/256 of the time. Only
* do the check when there is a full sanityCheck buffer of
* plaintext to compare against. */
if (ssl->specs.bulk_cipher_algorithm != wolfssl_cipher_null &&
dataSz > 0) {
dataSz >= sizeof(ssl->encrypt.sanityCheck)) {
XMEMCPY(ssl->encrypt.sanityCheck, input,
min(dataSz, sizeof(ssl->encrypt.sanityCheck)));
sizeof(ssl->encrypt.sanityCheck));
}
#endif

Expand Down Expand Up @@ -2824,10 +2832,12 @@ static int EncryptTls13(WOLFSSL* ssl, byte* output, const byte* input,
#endif

#ifdef WOLFSSL_CIPHER_TEXT_CHECK
/* Only compare when CIPHER_STATE_BEGIN prepared the check with a
* full sanityCheck buffer of plaintext (see rationale there). */
if (ssl->specs.bulk_cipher_algorithm != wolfssl_cipher_null &&
dataSz > 0 &&
dataSz >= sizeof(ssl->encrypt.sanityCheck) &&
XMEMCMP(output, ssl->encrypt.sanityCheck,
min(dataSz, sizeof(ssl->encrypt.sanityCheck))) == 0) {
sizeof(ssl->encrypt.sanityCheck)) == 0) {

WOLFSSL_MSG("EncryptTls13 sanity check failed! Glitch?");
return ENCRYPT_ERROR;
Expand Down
Loading