From c4e901dbdb217a78fcca75478dd8cf3720f6219c 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. (manually cherry picked from commit fe16ae5f95fa86ddb049a8d1e2caee0b80b32282) Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/8294) --- test/bntest.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 108 insertions(+), 3 deletions(-) diff --git a/test/bntest.c b/test/bntest.c index 686eab8af8..606cc11c0c 100644 --- a/test/bntest.c +++ b/test/bntest.c @@ -1,5 +1,5 @@ /* - * Copyright 1995-2016 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 @@ -85,6 +85,7 @@ int test_sqrt(BIO *bp, BN_CTX *ctx); int test_small_prime(BIO *bp, BN_CTX *ctx); int test_bn2dec(BIO *bp); int rand_neg(void); +static int test_ctx_consttime_flag(void); static int results = 0; static unsigned char lst[] = @@ -312,11 +313,18 @@ int main(int argc, char *argv[]) goto err; (void)BIO_flush(out); #endif + + /* silently flush any pre-existing error on the stack */ + ERR_clear_error(); + + message(out, "BN_CTX_get BN_FLG_CONSTTIME"); + if (!test_ctx_consttime_flag()) + goto err; + (void)BIO_flush(out); + BN_CTX_free(ctx); BIO_free(out); - ERR_print_errors_fp(stderr); - #ifndef OPENSSL_NO_CRYPTO_MDEBUG if (CRYPTO_mem_leaks_fp(stderr) <= 0) EXIT(1); @@ -2092,3 +2100,100 @@ int rand_neg(void) return (sign[(neg++) % 8]); } + +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 (NULL == (b[i] = BN_CTX_get(c))) { + fprintf(stderr, "ERROR: BN_CTX_get() failed.\n"); + 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 (NULL == (b[i] = BN_CTX_get(c))) { + fprintf(stderr, "ERROR: BN_CTX_get() failed.\n"); + goto err; + } + if (BN_get_flags(b[i], BN_FLG_CONSTTIME) != 0) { + fprintf(stderr, "ERROR: BN_FLG_CONSTTIME should not be set.\n"); + 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 (NULL == (nctx = BN_CTX_new())) { + fprintf(stderr, "ERROR: BN_CTX_new() failed.\n"); + goto err; + } + if (NULL == (sctx = BN_CTX_secure_new())) { + fprintf(stderr, "ERROR: BN_CTX_secure_new() failed.\n"); + goto err; + } + + for (i = 0; i < 2; i++) { + BN_CTX *c = i == 0 ? nctx : sctx; + if (!test_ctx_set_ct_flag(c) + || !test_ctx_check_ct_flag(c)) + goto err; + } + + st = 1; + err: + BN_CTX_free(nctx); + BN_CTX_free(sctx); + return st; +} -- 2.25.1