From 3c97136e82ecd61f7fcc9032c3159070aeb43475 Mon Sep 17 00:00:00 2001 From: Nicola Tuveri Date: Tue, 12 Feb 2019 00:37:25 +0200 Subject: [PATCH] Test for constant-time flag leakage in BN_CTX This commit adds a simple unit test to make sure that the constant-time flag does not "leak" among BN_CTX frames: - test_ctx_consttime_flag() initializes (and later frees before returning) a BN_CTX object, then it calls in sequence test_ctx_set_ct_flag() and test_ctx_check_ct_flag() using the same BN_CTX object. The process is run twice, once with a "normal" BN_CTX_new() object, then with a BN_CTX_secure_new() one. - test_ctx_set_ct_flag() starts a frame in the given BN_CTX and sets the BN_FLG_CONSTTIME flag on some of the BIGNUMs obtained from the frame before ending it. - test_ctx_check_ct_flag() then starts a new frame and gets a number of BIGNUMs from it. In absence of leaks, none of the BIGNUMs in the new frame should have BN_FLG_CONSTTIME set. In actual BN_CTX usage inside libcrypto the leak could happen at any depth level in the BN_CTX stack, with varying results depending on the patterns of sibling trees of nested function calls sharing the same BN_CTX object, and the effect of unintended BN_FLG_CONSTTIME on the called BN_* functions. This simple unit test abstracts away this complexity and verifies that the leak does not happen between two sibling functions sharing the same BN_CTX object at the same level of nesting. (cherry picked from commit fe16ae5f95fa86ddb049a8d1e2caee0b80b32282) Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/8253) --- test/bntest.c | 161 ++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 122 insertions(+), 39 deletions(-) diff --git a/test/bntest.c b/test/bntest.c index 720fd62bf9..077f5e8d85 100644 --- a/test/bntest.c +++ b/test/bntest.c @@ -1,5 +1,5 @@ /* - * Copyright 1995-2018 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 1995-2019 The OpenSSL Project Authors. All Rights Reserved. * * Licensed under the OpenSSL license (the "License"). You may not use * this file except in compliance with the License. You can obtain a copy @@ -123,7 +123,7 @@ static int getint(STANZA *s, int *out, const char *attribute) *out = (int)word; st = 1; -err: + err: BN_free(ret); return st; } @@ -138,7 +138,6 @@ static int equalBN(const char *op, const BIGNUM *expected, const BIGNUM *actual) return 0; } - /* * Return a "random" flag for if a BN should be negated. */ @@ -150,7 +149,6 @@ static int rand_neg(void) return sign[(neg++) % 8]; } - static int test_swap(void) { BIGNUM *a = NULL, *b = NULL, *c = NULL, *d = NULL; @@ -166,7 +164,7 @@ static int test_swap(void) BN_bntest_rand(b, 1024, 1, 0); BN_copy(c, a); BN_copy(d, b); - top = BN_num_bits(a)/BN_BITS2; + top = BN_num_bits(a) / BN_BITS2; /* regular swap */ BN_swap(a, b); @@ -252,14 +250,13 @@ static int test_sub(void) goto err; } st = 1; -err: + err: BN_free(a); BN_free(b); BN_free(c); return st; } - static int test_div_recip(void) { BIGNUM *a = NULL, *b = NULL, *c = NULL, *d = NULL, *e = NULL; @@ -293,7 +290,7 @@ static int test_div_recip(void) goto err; } st = 1; -err: + err: BN_free(a); BN_free(b); BN_free(c); @@ -303,7 +300,6 @@ err: return st; } - static int test_mod(void) { BIGNUM *a = NULL, *b = NULL, *c = NULL, *d = NULL, *e = NULL; @@ -328,7 +324,7 @@ static int test_mod(void) goto err; } st = 1; -err: + err: BN_free(a); BN_free(b); BN_free(c); @@ -573,7 +569,7 @@ static int test_modexp_mont5(void) st = 1; -err: + err: BN_MONT_CTX_free(mont); BN_free(a); BN_free(p); @@ -1141,7 +1137,7 @@ static int file_sum(STANZA *s) } st = 1; -err: + err: BN_free(a); BN_free(b); BN_free(sum); @@ -1190,7 +1186,7 @@ static int file_lshift1(STANZA *s) goto err; st = 1; -err: + err: BN_free(a); BN_free(lshift1); BN_free(zero); @@ -1219,7 +1215,7 @@ static int file_lshift(STANZA *s) goto err; st = 1; -err: + err: BN_free(a); BN_free(lshift); BN_free(ret); @@ -1249,7 +1245,7 @@ static int file_rshift(STANZA *s) } st = 1; -err: + err: BN_free(a); BN_free(rshift); BN_free(ret); @@ -1306,7 +1302,7 @@ static int file_square(STANZA *s) #endif st = 1; -err: + err: BN_free(a); BN_free(square); BN_free(zero); @@ -1343,7 +1339,7 @@ static int file_product(STANZA *s) goto err; st = 1; -err: + err: BN_free(a); BN_free(b); BN_free(product); @@ -1426,7 +1422,7 @@ static int file_quotient(STANZA *s) } st = 1; -err: + err: BN_free(a); BN_free(b); BN_free(quotient); @@ -1480,7 +1476,7 @@ static int file_modmul(STANZA *s) } st = 1; -err: + err: BN_free(a); BN_free(b); BN_free(m); @@ -1532,7 +1528,7 @@ static int file_modexp(STANZA *s) goto err; st = 1; -err: + err: BN_free(a); BN_free(b); BN_free(c); @@ -1560,7 +1556,7 @@ static int file_exp(STANZA *s) goto err; st = 1; -err: + err: BN_free(a); BN_free(e); BN_free(exp); @@ -1591,7 +1587,7 @@ static int file_modsqrt(STANZA *s) goto err; st = 1; -err: + err: BN_free(a); BN_free(p); BN_free(mod_sqrt); @@ -1621,8 +1617,8 @@ static int test_bn2padded(void) /* Test a random numbers at various byte lengths. */ for (size_t bytes = 128 - 7; bytes <= 128; bytes++) { -#define TOP_BIT_ON 0 -#define BOTTOM_BIT_NOTOUCH 0 +# define TOP_BIT_ON 0 +# define BOTTOM_BIT_NOTOUCH 0 if (!TEST_true(BN_rand(n, bytes * 8, TOP_BIT_ON, BOTTOM_BIT_NOTOUCH))) goto err; if (!TEST_int_eq(BN_num_bytes(n),A) bytes @@ -1653,7 +1649,7 @@ static int test_bn2padded(void) } st = 1; -err: + err: BN_free(n); return st; #else @@ -1725,7 +1721,7 @@ static int test_dec2bn(void) goto err; st = 1; -err: + err: BN_free(bn); return st; } @@ -1791,7 +1787,7 @@ static int test_hex2bn(void) goto err; st = 1; -err: + err: BN_free(bn); return st; } @@ -1845,7 +1841,7 @@ static int test_asc2bn(void) goto err; st = 1; -err: + err: BN_free(bn); return st; } @@ -1889,7 +1885,7 @@ static int test_mpi(int i) BN_free(bn2); st = 1; -err: + err: BN_free(bn); return st; } @@ -1915,7 +1911,7 @@ static int test_rand(void) goto err; st = 1; -err: + err: BN_free(bn); return st; } @@ -1979,7 +1975,7 @@ static int test_negzero(void) goto err; st = 1; -err: + err: BN_free(a); BN_free(b); BN_free(c); @@ -2020,7 +2016,7 @@ static int test_badmod(void) ERR_clear_error(); if (!TEST_false(BN_mod_exp_mont_consttime(a, BN_value_one(), BN_value_one(), - zero, ctx, NULL))) + zero, ctx, NULL))) goto err; ERR_clear_error(); @@ -2042,12 +2038,12 @@ static int test_badmod(void) ERR_clear_error(); if (!TEST_false(BN_mod_exp_mont_consttime(a, BN_value_one(), BN_value_one(), - b, ctx, NULL))) + b, ctx, NULL))) goto err; ERR_clear_error(); st = 1; -err: + err: BN_free(a); BN_free(b); BN_free(zero); @@ -2081,7 +2077,7 @@ static int test_expmodzero(void) goto err; st = 1; -err: + err: BN_free(zero); BN_free(a); BN_free(r); @@ -2127,7 +2123,7 @@ static int test_expmodone(void) } ret = 1; -err: + err: BN_free(r); BN_free(a); BN_free(p); @@ -2148,7 +2144,7 @@ static int test_smallprime(void) goto err; st = 1; -err: + err: BN_free(r); return st; } @@ -2172,7 +2168,7 @@ static int test_is_prime(int i) } ret = 1; -err: + err: BN_free(r); return ret; } @@ -2195,11 +2191,97 @@ static int test_not_prime(int i) } ret = 1; -err: + err: BN_free(r); return ret; } +static int test_ctx_set_ct_flag(BN_CTX *c) +{ + int st = 0; + size_t i; + BIGNUM *b[15]; + + BN_CTX_start(c); + for (i = 0; i < OSSL_NELEM(b); i++) { + if (!TEST_ptr(b[i] = BN_CTX_get(c))) + goto err; + if (i % 2 == 1) + BN_set_flags(b[i], BN_FLG_CONSTTIME); + } + + st = 1; + err: + BN_CTX_end(c); + return st; +} + +static int test_ctx_check_ct_flag(BN_CTX *c) +{ + int st = 0; + size_t i; + BIGNUM *b[30]; + + BN_CTX_start(c); + for (i = 0; i < OSSL_NELEM(b); i++) { + if (!TEST_ptr(b[i] = BN_CTX_get(c))) + goto err; + if (!TEST_false(BN_get_flags(b[i], BN_FLG_CONSTTIME))) + goto err; + } + + st = 1; + err: + BN_CTX_end(c); + return st; +} + +static int test_ctx_consttime_flag(void) +{ + /*- + * The constant-time flag should not "leak" among BN_CTX frames: + * + * - test_ctx_set_ct_flag() starts a frame in the given BN_CTX and + * sets the BN_FLG_CONSTTIME flag on some of the BIGNUMs obtained + * from the frame before ending it. + * - test_ctx_check_ct_flag() then starts a new frame and gets a + * number of BIGNUMs from it. In absence of leaks, none of the + * BIGNUMs in the new frame should have BN_FLG_CONSTTIME set. + * + * In actual BN_CTX usage inside libcrypto the leak could happen at + * any depth level in the BN_CTX stack, with varying results + * depending on the patterns of sibling trees of nested function + * calls sharing the same BN_CTX object, and the effect of + * unintended BN_FLG_CONSTTIME on the called BN_* functions. + * + * This simple unit test abstracts away this complexity and verifies + * that the leak does not happen between two sibling functions + * sharing the same BN_CTX object at the same level of nesting. + * + */ + BN_CTX *nctx = NULL; + BN_CTX *sctx = NULL; + size_t i = 0; + int st = 0; + + if (!TEST_ptr(nctx = BN_CTX_new()) + || !TEST_ptr(sctx = BN_CTX_secure_new())) + goto err; + + for (i = 0; i < 2; i++) { + BN_CTX *c = i == 0 ? nctx : sctx; + if (!TEST_true(test_ctx_set_ct_flag(c)) + || !TEST_true(test_ctx_check_ct_flag(c))) + goto err; + } + + st = 1; + err: + BN_CTX_free(nctx); + BN_CTX_free(sctx); + return st; +} + static int file_test_run(STANZA *s) { static const FILETEST filetests[] = { @@ -2287,6 +2369,7 @@ int setup_tests(void) ADD_TEST(test_expmodone); ADD_TEST(test_smallprime); ADD_TEST(test_swap); + ADD_TEST(test_ctx_consttime_flag); #ifndef OPENSSL_NO_EC2M ADD_TEST(test_gf2m_add); ADD_TEST(test_gf2m_mod); -- 2.25.1