From 2a483a87e74ec33bfcd904dbaeaa017c658f0389 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Wed, 20 Mar 2019 20:01:12 +0100 Subject: [PATCH] Make err_clear_constant_time really constant time [extended tests] Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/8542) (cherry picked from commit 94dc53a3f7549040dd9e61a25485070c14b41c49) --- crypto/err/err.c | 49 ++++++++++++++++++++----------------------- crypto/rsa/rsa_ossl.c | 2 +- include/openssl/err.h | 1 + 3 files changed, 25 insertions(+), 27 deletions(-) diff --git a/crypto/err/err.c b/crypto/err/err.c index ba7577b7df..8cbf21f696 100644 --- a/crypto/err/err.c +++ b/crypto/err/err.c @@ -464,8 +464,24 @@ static unsigned long get_error_values(int inc, int top, const char **file, return ERR_R_INTERNAL_ERROR; } + while (es->bottom != es->top) { + if (es->err_flags[es->top] & ERR_FLAG_CLEAR) { + err_clear(es, es->top); + es->top = es->top > 0 ? es->top - 1 : ERR_NUM_ERRORS - 1; + continue; + } + i = (es->bottom + 1) % ERR_NUM_ERRORS; + if (es->err_flags[i] & ERR_FLAG_CLEAR) { + es->bottom = i; + err_clear(es, es->bottom); + continue; + } + break; + } + if (es->bottom == es->top) return 0; + if (top) i = es->top; /* last error */ else @@ -824,25 +840,6 @@ int ERR_pop_to_mark(void) return 1; } -#ifdef UINTPTR_T -# undef UINTPTR_T -#endif -/* - * uintptr_t is the answer, but unfortunately C89, current "least common - * denominator" doesn't define it. Most legacy platforms typedef it anyway, - * so that attempt to fill the gaps means that one would have to identify - * that track these gaps, which would be undesirable. Macro it is... - */ -#if defined(__VMS) && __INITIAL_POINTER_SIZE==64 -/* - * But we can't use size_t on VMS, because it adheres to sizeof(size_t)==4 - * even in 64-bit builds, which means that it won't work as mask. - */ -# define UINTPTR_T unsigned long long -#else -# define UINTPTR_T size_t -#endif - void err_clear_last_constant_time(int clear) { ERR_STATE *es; @@ -854,11 +851,11 @@ void err_clear_last_constant_time(int clear) top = es->top; - es->err_flags[top] &= ~(0 - clear); - es->err_buffer[top] &= ~(0UL - clear); - es->err_file[top] = (const char *)((UINTPTR_T)es->err_file[top] & - ~((UINTPTR_T)0 - clear)); - es->err_line[top] |= 0 - clear; - - es->top = (top + ERR_NUM_ERRORS - clear) % ERR_NUM_ERRORS; + /* + * Flag error as cleared but remove it elsewhere to avoid two errors + * accessing the same error stack location, revealing timing information. + */ + clear = constant_time_select_int(constant_time_eq_int(clear, 0), + 0, ERR_FLAG_CLEAR); + es->err_flags[top] |= clear; } diff --git a/crypto/rsa/rsa_ossl.c b/crypto/rsa/rsa_ossl.c index ed77fadcba..b48d70825d 100644 --- a/crypto/rsa/rsa_ossl.c +++ b/crypto/rsa/rsa_ossl.c @@ -472,7 +472,7 @@ static int rsa_ossl_private_decrypt(int flen, const unsigned char *from, goto err; } RSAerr(RSA_F_RSA_OSSL_PRIVATE_DECRYPT, RSA_R_PADDING_CHECK_FAILED); - err_clear_last_constant_time(r >= 0); + err_clear_last_constant_time(1 & ~constant_time_msb(r)); err: if (ctx != NULL) diff --git a/include/openssl/err.h b/include/openssl/err.h index 29a261ceb7..779ed24bba 100644 --- a/include/openssl/err.h +++ b/include/openssl/err.h @@ -37,6 +37,7 @@ extern "C" { # define ERR_TXT_STRING 0x02 # define ERR_FLAG_MARK 0x01 +# define ERR_FLAG_CLEAR 0x02 # define ERR_NUM_ERRORS 16 typedef struct err_state_st { -- 2.25.1