From 2024f92be0ecb952df02e07befae17a7f90d98d0 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Thu, 30 Nov 2023 15:31:19 +0100 Subject: [PATCH] Fix CVE-2023-5992: Marvin Resolves: RHEL-17087 --- opensc-0.23.0-constant-time-pkcs1.patch | 383 ++++++++++++++++++++++++ opensc.spec | 3 + 2 files changed, 386 insertions(+) create mode 100644 opensc-0.23.0-constant-time-pkcs1.patch diff --git a/opensc-0.23.0-constant-time-pkcs1.patch b/opensc-0.23.0-constant-time-pkcs1.patch new file mode 100644 index 0000000..339356a --- /dev/null +++ b/opensc-0.23.0-constant-time-pkcs1.patch @@ -0,0 +1,383 @@ +diff --git a/src/common/Makefile.am b/src/common/Makefile.am +index 5153428dc..9ecbffe8f 100644 +--- a/src/common/Makefile.am ++++ b/src/common/Makefile.am +@@ -8,7 +8,8 @@ dist_noinst_DATA = \ + LICENSE.compat_getopt compat_getopt.txt \ + compat_getopt_main.c \ + README.compat_strlcpy compat_strlcpy.3 +-noinst_HEADERS = compat_strlcat.h compat_strlcpy.h compat_strnlen.h compat_getpass.h compat_getopt.h simclist.h libpkcs11.h libscdl.h ++noinst_HEADERS = compat_strlcat.h compat_strlcpy.h compat_strnlen.h compat_getpass.h \ ++ compat_getopt.h simclist.h libpkcs11.h libscdl.h constant-time.h + + AM_CPPFLAGS = -I$(top_srcdir)/src + +@@ -43,7 +44,8 @@ TIDY_FILES = \ + compat_report_rangecheckfailure.c \ + compat___iob_func.c \ + simclist.c simclist.h \ +- libpkcs11.c libscdl.c ++ libpkcs11.c libscdl.c \ ++ constant-time.h + + check-local: + if [ -x "$(CLANGTIDY)" ]; then clang-tidy -config='' -header-filter=.* $(TIDY_FILES) -- $(TIDY_FLAGS); fi +diff --git a/src/common/constant-time.h b/src/common/constant-time.h +new file mode 100644 +index 000000000..f70251f5d +--- /dev/null ++++ b/src/common/constant-time.h +@@ -0,0 +1,108 @@ ++/* Original source: https://github.com/openssl/openssl/blob/9890cc42daff5e2d0cad01ac4bf78c391f599a6e/include/internal/constant_time.h */ ++ ++#ifndef CONSTANT_TIME_H ++# define CONSTANT_TIME_H ++ ++# include ++# include ++ ++#if !defined(inline) ++# if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L ++# define constant_inline inline ++# elif defined(__GNUC__) && __GNUC__>=2 ++# define constant_inline __inline__ ++# elif defined(_MSC_VER) ++# define constant_inline __inline ++# else ++# define constant_inline ++# endif ++#else /* use what caller wants as inline may be from config.h */ ++# define constant_inline inline /* inline */ ++#endif ++ ++static constant_inline unsigned int value_barrier(unsigned int a) ++{ ++ volatile unsigned int r = a; ++ return r; ++} ++ ++static constant_inline size_t value_barrier_s(size_t a) ++{ ++ volatile size_t r = a; ++ return r; ++} ++ ++/* MSB */ ++static constant_inline size_t constant_time_msb_s(size_t a) ++{ ++ return 0 - (a >> (sizeof(a) * 8 - 1)); ++} ++ ++static constant_inline unsigned int ++constant_time_msb(unsigned int a) ++{ ++ return 0 - (a >> (sizeof(a) * 8 - 1)); ++} ++ ++/* Select */ ++static constant_inline unsigned int ++constant_time_select(unsigned int mask, unsigned int a, unsigned int b) ++{ ++ return (value_barrier(mask) & a) | (value_barrier(~mask) & b); ++} ++ ++static constant_inline unsigned char ++constant_time_select_8(unsigned char mask, unsigned char a, unsigned char b) ++{ ++ return (unsigned char)constant_time_select(mask, a, b); ++} ++ ++static constant_inline size_t constant_time_select_s(size_t mask, size_t a, size_t b) ++{ ++ return (value_barrier_s(mask) & a) | (value_barrier_s(~mask) & b); ++} ++ ++/* Zero */ ++static constant_inline unsigned int ++constant_time_is_zero(unsigned int a) ++{ ++ return constant_time_msb(~a & (a - 1)); ++} ++ ++static constant_inline size_t constant_time_is_zero_s(size_t a) ++{ ++ return constant_time_msb_s(~a & (a - 1)); ++} ++ ++/* Comparison*/ ++static constant_inline size_t constant_time_lt_s(size_t a, size_t b) ++{ ++ return constant_time_msb_s(a ^ ((a ^ b) | ((a - b) ^ b))); ++} ++ ++static constant_inline unsigned int ++constant_time_lt(unsigned int a, unsigned int b) ++{ ++ return constant_time_msb(a ^ ((a ^ b) | ((a - b) ^ b))); ++} ++ ++static constant_inline unsigned int ++constant_time_ge(unsigned int a, unsigned int b) ++{ ++ return ~constant_time_lt(a, b); ++} ++ ++/* Equality*/ ++ ++static constant_inline unsigned int ++constant_time_eq(unsigned int a, unsigned int b) ++{ ++ return constant_time_is_zero(a ^ b); ++} ++ ++static constant_inline size_t constant_time_eq_s(size_t a, size_t b) ++{ ++ return constant_time_is_zero_s(a ^ b); ++} ++ ++#endif /* CONSTANT_TIME_H */ +diff --git a/src/libopensc/internal.h b/src/libopensc/internal.h +index 74014235a..13eccfa1a 100644 +--- a/src/libopensc/internal.h ++++ b/src/libopensc/internal.h +@@ -168,6 +168,8 @@ int sc_pkcs1_strip_01_padding(struct sc_context *ctx, const u8 *in_dat, size_t i + u8 *out_dat, size_t *out_len); + int sc_pkcs1_strip_02_padding(struct sc_context *ctx, const u8 *data, size_t len, + u8 *out_dat, size_t *out_len); ++int sc_pkcs1_strip_02_padding_constant_time(sc_context_t *ctx, unsigned int n, const u8 *data, ++ unsigned int data_len, u8 *out, unsigned int *out_len); + int sc_pkcs1_strip_digest_info_prefix(unsigned int *algorithm, + const u8 *in_dat, size_t in_len, u8 *out_dat, size_t *out_len); + +diff --git a/src/libopensc/padding.c b/src/libopensc/padding.c +index 283746699..ceb2a1e21 100644 +--- a/src/libopensc/padding.c ++++ b/src/libopensc/padding.c +@@ -33,9 +33,12 @@ + #include + + #include "internal.h" ++#include "common/constant-time.h" + + /* TODO doxygen comments */ + ++#define SC_PKCS1_PADDING_MIN_SIZE 11 ++ + /* + * Prefixes for pkcs-v1 signatures + */ +@@ -184,6 +187,84 @@ sc_pkcs1_strip_02_padding(sc_context_t *ctx, const u8 *data, size_t len, u8 *out + LOG_FUNC_RETURN(ctx, len - n); + } + ++/* Original source: https://github.com/openssl/openssl/blob/9890cc42daff5e2d0cad01ac4bf78c391f599a6e/crypto/rsa/rsa_pk1.c#L171 */ ++int ++sc_pkcs1_strip_02_padding_constant_time(sc_context_t *ctx, unsigned int n, const u8 *data, unsigned int data_len, u8 *out, unsigned int *out_len) ++{ ++ unsigned int i = 0; ++ u8 *msg = NULL; ++ unsigned int good, found_zero_byte, mask; ++ unsigned int zero_index = 0, msg_index, mlen = -1, len = 0; ++ LOG_FUNC_CALLED(ctx); ++ ++ if (data == NULL || data_len <= 0 || data_len > n || n < SC_PKCS1_PADDING_MIN_SIZE) ++ LOG_FUNC_RETURN(ctx, SC_ERROR_INTERNAL); ++ ++ msg = calloc(n, sizeof(u8)); ++ if (msg == NULL) ++ LOG_FUNC_RETURN(ctx, SC_ERROR_INTERNAL); ++ ++ /* ++ * We can not check length of input data straight away and still we need to read ++ * from input even when the input is not as long as needed to keep the time constant. ++ * If data has wrong size, it is padded by zeroes from left and the following checks ++ * do not pass. ++ */ ++ len = data_len; ++ for (data += len, msg += n, i = 0; i < n; i++) { ++ mask = ~constant_time_is_zero(len); ++ len -= 1 & mask; ++ data -= 1 & mask; ++ *--msg = *data & mask; ++ } ++ // check first byte to be 0x00 ++ good = constant_time_is_zero(msg[0]); ++ // check second byte to be 0x02 ++ good &= constant_time_eq(msg[1], 2); ++ ++ // find zero byte after random data in padding ++ found_zero_byte = 0; ++ for (i = 2; i < n; i++) { ++ unsigned int equals0 = constant_time_is_zero(msg[i]); ++ zero_index = constant_time_select(~found_zero_byte & equals0, i, zero_index); ++ found_zero_byte |= equals0; ++ } ++ ++ // zero_index stands for index of last found zero ++ good &= constant_time_ge(zero_index, 2 + 8); ++ ++ // start of the actual message in data ++ msg_index = zero_index + 1; ++ ++ // length of message ++ mlen = data_len - msg_index; ++ ++ // check that there is a message after padding ++ good &= constant_time_ge(mlen, 1); ++ // check that message fits into out buffer ++ good &= constant_time_ge(*out_len, mlen); ++ ++ // move the result in-place by |num|-SC_PKCS1_PADDING_MIN_SIZE-|mlen| bytes to the left. ++ *out_len = constant_time_select(constant_time_lt(n - SC_PKCS1_PADDING_MIN_SIZE, *out_len), ++ n - SC_PKCS1_PADDING_MIN_SIZE, *out_len); ++ for (msg_index = 1; msg_index < n - SC_PKCS1_PADDING_MIN_SIZE; msg_index <<= 1) { ++ mask = ~constant_time_eq(msg_index & (n - SC_PKCS1_PADDING_MIN_SIZE - mlen), 0); ++ for (i = SC_PKCS1_PADDING_MIN_SIZE; i < n - msg_index; i++) ++ msg[i] = constant_time_select_8(mask, msg[i + msg_index], msg[i]); ++ } ++ // move message into out buffer, if good ++ for (i = 0; i < *out_len; i++) { ++ unsigned int msg_index; ++ // when out is longer than message in data, use some bogus index in msg ++ mask = good & constant_time_lt(i, mlen); ++ msg_index = constant_time_select(mask, i + SC_PKCS1_PADDING_MIN_SIZE, 0); // to now overflow msg buffer ++ out[i] = constant_time_select_8(mask, msg[msg_index], out[i]); ++ } ++ ++ free(msg); ++ return constant_time_select(good, mlen, SC_ERROR_WRONG_PADDING); ++} ++ + /* add/remove DigestInfo prefix */ + static int sc_pkcs1_add_digest_info_prefix(unsigned int algorithm, + const u8 *in, size_t in_len, u8 *out, size_t *out_len) +diff --git a/src/libopensc/pkcs15-sec.c b/src/libopensc/pkcs15-sec.c +index 7c3a39432..b5e492fe2 100644 +--- a/src/libopensc/pkcs15-sec.c ++++ b/src/libopensc/pkcs15-sec.c +@@ -308,12 +308,13 @@ int sc_pkcs15_decipher(struct sc_pkcs15_card *p15card, + + /* Strip any padding */ + if (pad_flags & SC_ALGORITHM_RSA_PAD_PKCS1) { +- size_t s = r; +- r = sc_pkcs1_strip_02_padding(ctx, out, s, out, &s); +- LOG_TEST_RET(ctx, r, "Invalid PKCS#1 padding"); ++ unsigned int s = r; ++ unsigned int key_size = alg_info->key_length; ++ r = sc_pkcs1_strip_02_padding_constant_time(ctx, key_size / 8, out, s, out, &s); ++ /* for keeping PKCS#1 v1.5 depadding constant-time, do not log error here */ + } + +- LOG_FUNC_RETURN(ctx, r); ++ return r; + } + + /* derive one key from another. RSA can use decipher, so this is for only ECDH +diff --git a/src/pkcs11/framework-pkcs15.c b/src/pkcs11/framework-pkcs15.c +index 809cd72d9..9c75759a0 100644 +--- a/src/pkcs11/framework-pkcs15.c ++++ b/src/pkcs11/framework-pkcs15.c +@@ -28,6 +28,7 @@ + #include "libopensc/cardctl.h" + #include "ui/notify.h" + #include "common/compat_strnlen.h" ++#include "common/constant-time.h" + #ifdef ENABLE_OPENSSL + #include + #else +@@ -4603,15 +4604,51 @@ pkcs15_prkey_decrypt(struct sc_pkcs11_session *session, void *obj, + rv = sc_pkcs15_decipher(fw_data->p15_card, prkey->prv_p15obj, flags, + pEncryptedData, ulEncryptedDataLen, decrypted, sizeof(decrypted)); + +- if (rv < 0 && !sc_pkcs11_conf.lock_login && !prkey_has_path) ++ if (!((flags & SC_ALGORITHM_RSA_PAD_PKCS1) && constant_time_eq_s(rv, SC_ERROR_WRONG_PADDING)) && ++ rv < 0 && !sc_pkcs11_conf.lock_login && !prkey_has_path) + if (reselect_app_df(fw_data->p15_card) == SC_SUCCESS) + rv = sc_pkcs15_decipher(fw_data->p15_card, prkey->prv_p15obj, flags, + pEncryptedData, ulEncryptedDataLen, decrypted, sizeof(decrypted)); + + sc_unlock(p11card->card); + +- sc_log(context, "Decryption complete. Result %d.", rv); ++ /* Handle buffer after PKCS#1 v1.5 depadding constant-time */ ++ if (flags & SC_ALGORITHM_RSA_PAD_PKCS1) { ++ CK_ULONG mask, good, rv_pkcs11; ++ ++ sc_log(context, "Decryption complete."); ++ /* only padding error must be handled in constant-time way */ ++ if ((~constant_time_eq_s(rv, SC_ERROR_WRONG_PADDING) & constant_time_lt_s(sizeof(decrypted), rv))) ++ return sc_to_cryptoki_error(rv, "C_Decrypt"); ++ ++ /* check rv for error */ ++ good = ~constant_time_eq_s(rv, SC_ERROR_WRONG_PADDING); ++ rv_pkcs11 = constant_time_select_s(good, CKR_OK, SC_ERROR_WRONG_PADDING); ++ if (pData == NULL_PTR) { ++ /* set length only if rv good */ ++ *pulDataLen = constant_time_select_s(good, rv, *pulDataLen); ++ /* return error only if original rv < 0 */ ++ return rv_pkcs11; ++ } ++ ++ /* check whether *pulDataLen < rv and set return value accordingly */ ++ mask = good & constant_time_lt_s(*pulDataLen, rv); ++ rv_pkcs11 = constant_time_select_s(mask, CKR_BUFFER_TOO_SMALL, rv_pkcs11); ++ good &= ~mask; + ++ /* move everything from decrypted into out buffer, if rv is ok */ ++ for (CK_ULONG i = 0; i < *pulDataLen; i++) { /* iterate over whole pData to not disclose real depadded length */ ++ CK_ULONG msg_index; ++ mask = good & constant_time_lt_s(i, sizeof(decrypted)); /* i should be in the bounds of decrypted */ ++ mask &= constant_time_lt_s(i, constant_time_select_s(good, rv, 0)); /* check that is in bounds of depadded message */ ++ msg_index = constant_time_select_s(mask, i, 0); ++ pData[i] = constant_time_select_8(mask, decrypted[msg_index], pData[i]); ++ } ++ *pulDataLen = constant_time_select_s(good, rv, *pulDataLen); ++ return rv_pkcs11; ++ } ++ ++ sc_log(context, "Decryption complete. Result %d.", rv); + if (rv < 0) + return sc_to_cryptoki_error(rv, "C_Decrypt"); + +@@ -4622,7 +4659,6 @@ pkcs15_prkey_decrypt(struct sc_pkcs11_session *session, void *obj, + if (buff_too_small) + return CKR_BUFFER_TOO_SMALL; + memcpy(pData, decrypted, *pulDataLen); +- + return CKR_OK; + } + +diff --git a/src/pkcs11/misc.c b/src/pkcs11/misc.c +index 5ca1176b1..2893b2bf3 100644 +--- a/src/pkcs11/misc.c ++++ b/src/pkcs11/misc.c +@@ -24,6 +24,7 @@ + #include + + #include "sc-pkcs11.h" ++#include "common/constant-time.h" + + #define DUMP_TEMPLATE_MAX 32 + +@@ -174,7 +175,7 @@ CK_RV reset_login_state(struct sc_pkcs11_slot *slot, CK_RV rv) + slot->p11card->framework->logout(slot); + } + +- if (rv == CKR_USER_NOT_LOGGED_IN) { ++ if (constant_time_eq_s(rv, CKR_USER_NOT_LOGGED_IN)) { + slot->login_user = -1; + pop_all_login_states(slot); + } +diff --git a/src/pkcs11/pkcs11-object.c b/src/pkcs11/pkcs11-object.c +index f04c0b4c5..93cc319c2 100644 +--- a/src/pkcs11/pkcs11-object.c ++++ b/src/pkcs11/pkcs11-object.c +@@ -1034,7 +1034,7 @@ C_Decrypt(CK_SESSION_HANDLE hSession, /* the session's handle */ + rv = reset_login_state(session->slot, rv); + } + +- sc_log(context, "C_Decrypt() = %s", lookup_enum ( RV_T, rv )); ++ sc_log(context, "C_Decrypt()"); + sc_pkcs11_unlock(); + return rv; + } diff --git a/opensc.spec b/opensc.spec index e112ce7..2edba6f 100644 --- a/opensc.spec +++ b/opensc.spec @@ -78,6 +78,8 @@ Patch21: %{name}-0.23.0-pin-bypass.patch # https://github.com/OpenSC/OpenSC/commit/085994384a7171c5c68f6718d9db10ed77c5af1 # https://github.com/OpenSC/OpenSC/commit/0f0985f6343eeac4044661d56807ee9286db42c Patch22: %{name}-0.23.0-pkcs15init.patch +# https://github.com/OpenSC/OpenSC/pull/2948 +Patch23: %{name}-0.23.0-constant-time-pkcs1.patch BuildRequires: pcsc-lite-devel BuildRequires: readline-devel @@ -122,6 +124,7 @@ every software/card that does so, too. %patch20 -p1 -b .cache-offsets %patch21 -p1 -b .pin-bypass %patch22 -p1 -b .pkcs15init +%patch23 -p1 -b .constant-time-pkcs1.5 cp -p src/pkcs15init/README ./README.pkcs15init cp -p src/scconf/README.scconf .