From 98ce4e901a55c074384c3728b50a46eb069041e0 Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Thu, 17 Jun 2021 13:23:06 +1000 Subject: [PATCH] TLS EtM: check all padding bytes are the same value Must be constant time so as not to provide an oracle. That is, don't leak length of data and padding. --- src/internal.c | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/src/internal.c b/src/internal.c index 18732349d..a4e1829dd 100644 --- a/src/internal.c +++ b/src/internal.c @@ -16110,9 +16110,33 @@ int ProcessReply(WOLFSSL* ssl) in->buffer + in->idx, ssl->curSize - (word16)digestSz); if (ret == 0) { - ssl->keys.padSz = - in->buffer[in->idx + ssl->curSize - - digestSz - 1]; + byte invalid = 0; + byte padding = (byte)-1; + word32 i; + word32 off = in->idx + ssl->curSize - digestSz - 1; + + /* Last of padding bytes - indicates length. */ + ssl->keys.padSz = in->buffer[off]; + /* Constant time checking of padding - don't leak + * the length of the data. + */ + /* Compare max pad bytes or at most data + pad. */ + for (i = 1; i < MAX_PAD_SIZE && off >= i; i++) { + /* Mask on indicates this is expected to be a + * padding byte. + */ + padding &= ctMaskLTE(i, ssl->keys.padSz); + /* When this is a padding byte and not equal + * to length then mask is set. + */ + invalid |= padding & + ctMaskNotEq(in->buffer[off - i], + ssl->keys.padSz); + } + /* If mask is set then there was an error. */ + if (invalid) { + ret = DECRYPT_ERROR; + } ssl->keys.padSz += 1; ssl->keys.decryptedCur = 1; }