From 2f3732538d7d8e1ae255ca68c20efc61a1d5b3e2 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Fri, 4 Sep 2026 09:17:32 +0000 Subject: [PATCH] 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. CVE: CVE-2026-5419 Upstream-Status: Backport [https://gitlab.com/gnutls/gnutls/-/commit/1e627aa5ad95c6dc0518d94e9a009997b081a1ab] Backport Changes: - Adjusted the upstream hunk to match the GnuTLS 3.8.12 code layout. - Drop .gitignore from the backport. 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 Signed-off-by: Jakub Szczudlo --- 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 diff --git a/lib/crypto-api.c b/lib/crypto-api.c index 01539d5b52..32143e9de0 100644 --- a/lib/crypto-api.c +++ b/lib/crypto-api.c @@ -498,6 +498,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 @@ -532,22 +565,17 @@ int gnutls_cipher_decrypt3(gnutls_cipher_hd_t handle, const void *ctext, 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; } /** diff --git a/lib/libgnutls.map b/lib/libgnutls.map index 955704e..5cc12c8 100644 --- a/lib/libgnutls.map +++ b/lib/libgnutls.map @@ -1574,4 +1574,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; diff --git a/tests/Makefile.am b/tests/Makefile.am index ab2685c..1304d2f 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -241,7 +241,7 @@ ctests += mini-record-2 simple gnutls_hmac_fast set_pkcs12_cred cert certuniquei 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 diff --git a/tests/pkcs7-pad.c b/tests/pkcs7-pad.c new file mode 100644 index 0000000..d4c3798 --- /dev/null +++ b/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; +} -- 2.43.0