From d284d277707f9985e69bdba1511ecfbb1e53ac46 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Wed, 20 Mar 2019 20:01:12 +0100 Subject: Make err_clear_constant_time really constant time [extended tests] Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/8558) --- crypto/err/err.c | 47 +++++++++++++++++++++++------------------------ crypto/err/err.h | 1 + crypto/rsa/rsa_eay.c | 2 +- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/crypto/err/err.c b/crypto/err/err.c index 5ce774a3f5..d02e8ffae0 100644 --- a/crypto/err/err.c +++ b/crypto/err/err.c @@ -827,8 +827,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 @@ -1158,23 +1174,6 @@ int ERR_pop_to_mark(void) return 1; } -#ifdef UINTPTR_T -# undef UINTPTR_T -#endif -/* - * uintptr_t is the answer, but unformtunately we can't assume that all - * compilers supported by 1.0.2 have it :-( - */ -#if defined(OPENSSL_SYS_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; @@ -1186,11 +1185,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/err/err.h b/crypto/err/err.h index f42365620d..c12524dd4c 100644 --- a/crypto/err/err.h +++ b/crypto/err/err.h @@ -143,6 +143,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 { diff --git a/crypto/rsa/rsa_eay.c b/crypto/rsa/rsa_eay.c index 7f20fd6738..1c798a09e7 100644 --- a/crypto/rsa/rsa_eay.c +++ b/crypto/rsa/rsa_eay.c @@ -589,7 +589,7 @@ static int RSA_eay_private_decrypt(int flen, const unsigned char *from, goto err; } RSAerr(RSA_F_RSA_EAY_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) { -- cgit v1.2.3