From 1e627aa5ad95c6dc0518d94e9a009997b081a1ab Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Wed, 1 Apr 2026 18:57:21 +0900 Subject: gnutls_cipher_decrypt3: make PKCS#7 unpadding branch free This tries to make the logic of PKCS#7 padding removal constant-time, by removing potential branching operations. Reported-by: Doria Tang of Stony Brook University Fixes: #1815 Fixes: CVE-2026-5419 Fixes: GNUTLS-SA-2026-04-29-13 CVSS: 3.7 Low CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:L/I:N/A:N Signed-off-by: Daiki Ueno --- lib/crypto-api.c | 54 +++++++++++++++++------ lib/libgnutls.map | 2 + tests/Makefile.am | 2 +- tests/pkcs7-pad.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 153 insertions(+), 14 deletions(-) create mode 100644 tests/pkcs7-pad.c Index: gnutls-3.8.10/lib/crypto-api.c =================================================================== --- gnutls-3.8.10.orig/lib/crypto-api.c +++ gnutls-3.8.10/lib/crypto-api.c @@ -499,6 +499,39 @@ error: return ret; } +/* If succeeds, returns the number of padding bytes to be removed; + * zero otherwise. + */ +unsigned int _gnutls_pkcs7_unpad(const uint8_t *block, unsigned int block_size) +{ + uint8_t padding = block[block_size - 1]; + volatile unsigned int mask = ~0; + volatile unsigned int count = 0; + + /* Count consecutive PADDING bytes from the end, in a + * constant-time manner. + */ + for (size_t i = block_size; i > 0; i--) { + volatile unsigned int mask2; + + mask2 = -(unsigned int)(block[i - 1] == padding); + mask2 &= -(unsigned int)(count < padding); + + /* MASK is initially ~0 and will be flipped to 0 upon first + * non-padding bytes. + */ + mask &= mask2; + count += 1 & mask; + } + + /* PADDING == 0 is effectively excluded here, given COUNT + * will never be 0. + */ + mask = -(unsigned int)(count <= block_size); + mask &= -(unsigned int)(count == padding); + return count & mask; +} + /** * gnutls_cipher_decrypt3: * @handle: is a #gnutls_cipher_hd_t type @@ -533,22 +566,17 @@ int gnutls_cipher_decrypt3(gnutls_cipher if (_gnutls_cipher_type(h->ctx_enc.e) == CIPHER_BLOCK && (flags & GNUTLS_CIPHER_PADDING_PKCS7)) { uint8_t *p = ptext; - uint8_t padding = p[*ptext_len - 1]; - if (!padding || - padding > _gnutls_cipher_get_block_size(h->ctx_enc.e)) { - return gnutls_assert_val(GNUTLS_E_DECRYPTION_FAILED); - } - /* Check that the prior bytes are all PADDING */ - for (size_t i = *ptext_len - padding; i < *ptext_len; i++) { - if (padding != p[*ptext_len - 1]) { - return gnutls_assert_val( - GNUTLS_E_DECRYPTION_FAILED); - } - } + size_t block_size = _gnutls_cipher_get_block_size(h->ctx_enc.e); + uint8_t *block = &p[*ptext_len - block_size]; + unsigned int padding = _gnutls_pkcs7_unpad(block, block_size); + volatile unsigned int mask; + + mask = -(unsigned int)(padding == 0); + ret = GNUTLS_E_DECRYPTION_FAILED & mask; *ptext_len -= padding; } - return 0; + return ret; } /** Index: gnutls-3.8.10/lib/libgnutls.map =================================================================== --- gnutls-3.8.10.orig/lib/libgnutls.map +++ gnutls-3.8.10/lib/libgnutls.map @@ -1569,4 +1569,6 @@ GNUTLS_PRIVATE_3_4 { _gnutls_pathbuf_append; _gnutls_pathbuf_truncate; _gnutls_pathbuf_deinit; + # needed by tests/pkcs7-pad + _gnutls_pkcs7_unpad; } GNUTLS_3_4; Index: gnutls-3.8.10/tests/Makefile.am =================================================================== --- gnutls-3.8.10.orig/tests/Makefile.am +++ gnutls-3.8.10/tests/Makefile.am @@ -243,7 +243,7 @@ ctests += mini-record-2 simple gnutls_hm x509cert-dntypes id-on-xmppAddr tls13-compat-mode ciphersuite-name \ x509-upnconstraint xts-key-check cipher-padding pkcs7-verify-double-free \ fips-rsa-sizes tls12-rehandshake-ticket pathbuf tls-force-ems \ - psk-importer privkey-derive dh-compute2 ecdh-compute2 \ + psk-importer privkey-derive dh-compute2 ecdh-compute2 pkcs7-pad \ mini-dtls-fragments ctests += tls-channel-binding Index: gnutls-3.8.10/tests/pkcs7-pad.c =================================================================== --- /dev/null +++ gnutls-3.8.10/tests/pkcs7-pad.c @@ -0,0 +1,109 @@ +/* + * Copyright (C) 2026 Red Hat, Inc. + * + * This file is part of GnuTLS. + * + * GnuTLS is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * GnuTLS is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with GnuTLS. If not, see . + */ + +/* Test that _gnutls_pkcs7_unpad is branch-free, using valgrind */ + +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#include +#include + +#ifdef HAVE_VALGRIND_MEMCHECK_H +#include +#endif + +#include "utils.h" + +static inline void _gnutls_memory_mark_undefined(void *addr, size_t size) +{ +#ifdef HAVE_VALGRIND_MEMCHECK_H + if (RUNNING_ON_VALGRIND) + VALGRIND_MAKE_MEM_UNDEFINED(addr, size); +#endif +} + +static inline void _gnutls_memory_mark_defined(void *addr, size_t size) +{ +#ifdef HAVE_VALGRIND_MEMCHECK_H + if (RUNNING_ON_VALGRIND) + VALGRIND_MAKE_MEM_DEFINED(addr, size); +#endif +} + +extern unsigned int _gnutls_pkcs7_unpad(const uint8_t *block, + unsigned int block_size); + +static unsigned int wrap_pkcs7_unpad(uint8_t *block, unsigned int block_size) +{ + unsigned int padding; + + _gnutls_memory_mark_undefined(block, block_size); + + padding = _gnutls_pkcs7_unpad(block, block_size); + + _gnutls_memory_mark_defined(block, block_size); + _gnutls_memory_mark_defined(&padding, sizeof(padding)); + + return padding; +} + +#define PAD 5 + +void doit(void) +{ + uint8_t block[16]; + unsigned int padding; + + memset(block, 0xFF, sizeof(block)); + memset(&block[sizeof(block) - PAD], PAD, PAD); + + padding = wrap_pkcs7_unpad(block, sizeof(block)); + if (padding != PAD) + fail("padding should be %d\n", PAD); + + /* The last padding byte exceeds the block size */ + block[sizeof(block) - 1] = sizeof(block) + 1; + padding = wrap_pkcs7_unpad(block, sizeof(block)); + if (padding != 0) + fail("padding should be 0\n"); + block[sizeof(block) - 1] = PAD; + + /* The last padding byte is zero */ + block[sizeof(block) - 1] = 0; + padding = wrap_pkcs7_unpad(block, sizeof(block)); + if (padding != 0) + fail("padding should be 0\n"); + block[sizeof(block) - 1] = PAD; + + /* The first padding byte is invalid */ + block[sizeof(block) - PAD] = PAD + 1; + padding = wrap_pkcs7_unpad(block, sizeof(block)); + if (padding != 0) + fail("padding should be 0\n"); + block[sizeof(block) - PAD] = PAD; + + /* The byte before the first padding equals to PAD */ + block[sizeof(block) - PAD - 1] = PAD; + padding = wrap_pkcs7_unpad(block, sizeof(block)); + if (padding != PAD) + fail("padding should be %d\n", PAD); + block[sizeof(block) - PAD - 1] = 0xFF; +} Index: gnutls-3.8.10/tests/cipher-padding.c =================================================================== --- gnutls-3.8.10.orig/tests/cipher-padding.c +++ gnutls-3.8.10/tests/cipher-padding.c @@ -43,9 +43,11 @@ static void start(gnutls_cipher_algorith uint8_t key16[64]; uint8_t iv16[32]; uint8_t plaintext[128]; + uint8_t plaintext2[128]; uint8_t ciphertext[128]; size_t block_size; size_t size; + size_t ciphertext_size; gnutls_datum_t key, iv; success("%s %zu %u\n", gnutls_cipher_get_name(algo), plaintext_size, @@ -80,39 +82,41 @@ static void start(gnutls_cipher_algorith } /* Get the ciphertext size */ - ret = gnutls_cipher_encrypt3(ch, plaintext, plaintext_size, NULL, &size, - flags); + ret = gnutls_cipher_encrypt3(ch, plaintext, plaintext_size, NULL, + &ciphertext_size, flags); if (ret < 0) { fail("gnutls_cipher_encrypt3 failed\n"); } if (flags & GNUTLS_CIPHER_PADDING_PKCS7) { - if (size <= plaintext_size) { + if (ciphertext_size <= plaintext_size) { fail("no padding appended\n"); } - if (size != CLAMP(plaintext_size, block_size)) { - fail("size does not match: %zu (expected %zu)\n", size, + if (ciphertext_size != CLAMP(plaintext_size, block_size)) { + fail("size does not match: %zu (expected %zu)\n", + ciphertext_size, CLAMP(plaintext_size, block_size)); } } else { - if (size != plaintext_size) { - fail("size does not match: %zu (expected %zu)\n", size, - plaintext_size); + if (ciphertext_size != plaintext_size) { + fail("size does not match: %zu (expected %zu)\n", + ciphertext_size, plaintext_size); } } /* Encrypt with padding */ ret = gnutls_cipher_encrypt3(ch, plaintext, plaintext_size, ciphertext, - &size, flags); + &ciphertext_size, flags); if (ret < 0) { fail("gnutls_cipher_encrypt3 failed\n"); } /* Decrypt with padding */ - ret = gnutls_cipher_decrypt3(ch, ciphertext, size, ciphertext, &size, - flags); + size = ciphertext_size; + ret = gnutls_cipher_decrypt3(ch, ciphertext, ciphertext_size, + plaintext2, &size, flags); if (ret < 0) { - fail("gnutls_cipher_encrypt3 failed\n"); + fail("gnutls_cipher_decrypt3 failed\n"); } if (size != plaintext_size) { @@ -120,10 +124,33 @@ static void start(gnutls_cipher_algorith plaintext_size); } - if (memcmp(ciphertext, plaintext, size) != 0) { + if (memcmp(plaintext2, plaintext, size) != 0) { fail("plaintext does not match\n"); } + if ((flags & GNUTLS_CIPHER_PADDING_PKCS7) && + plaintext_size % block_size != 0) { + /* Encrypt with manual padding */ + memset(&plaintext[plaintext_size], + ciphertext_size - plaintext_size, + ciphertext_size - plaintext_size); + /* Insert a wrong padding byte */ + plaintext[plaintext_size] = block_size; + ret = gnutls_cipher_encrypt3(ch, plaintext, ciphertext_size, + ciphertext, &ciphertext_size, 0); + if (ret < 0) { + fail("gnutls_cipher_encrypt3 failed\n"); + } + + /* Decrypt with padding */ + size = ciphertext_size; + ret = gnutls_cipher_decrypt3(ch, ciphertext, ciphertext_size, + plaintext, &size, flags); + if (ret != GNUTLS_E_DECRYPTION_FAILED) { + fail("gnutls_cipher_decrypt3 succeeded\n"); + } + } + gnutls_cipher_deinit(ch); }