Make err_clear_constant_time really constant time

[extended tests]

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8542)

(cherry picked from commit 94dc53a3f7549040dd9e61a25485070c14b41c49)

diff --git a/crypto/err/err.c b/crypto/err/err.c
index ba7577b7dfb2c9a9c2183f323b062089a0ff1f13..8cbf21f696962d6230fe35031d10759085dc41f3 100644 (file)
--- 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 ed77fadcba58617aaa853f61fb2b13cef6a8053f..b48d70825d0b57ac253e187b15b534bac6c8d372 100644 (file)
--- 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 29a261ceb7d55b79de0a46cc1f060289bbd1c206..779ed24bbaa53312a6d1a0b78e12f28a5c67218c 100644 (file)
--- 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 {