From b643efb71d833e986bbd311963a276719be21755 Mon Sep 17 00:00:00 2001 From: tb Date: Fri, 31 Mar 2023 19:39:15 +0000 Subject: [PATCH] Copy BN_FLG flags in BN_copy() BN_copy() forgot to copy the flags from the source to the target. Fix this by copying the flags. In fact, only copy BN_FLG_CONSTTIME since propagating BN_FLG_MALLOCED and BN_FLG_STATIC_DATA is wrong. Ignore the BN_FLG_FREE flag "used for debugging" which of course means "unused" like a lot of other debug code that somehow ended up in public headers. Also: make BN_FLG_CONSTTIME sticky on the target, i.e., don't clear the flag when copying from a non-constant time BIGNUM to a constant time one for the following reason: if a is constant time, BN_sqr(a, a, ctx) would use a BIGNUM without the flag internally, then copy the result to a in which process a would lose its constant time flag. Fixing this would be a lot of pointless work since someone had the good sense of not relying on a fragile flag for something this important. Rather, libcrypto always uses the constant time paths instead of the faster, cryptographically inadequate paths. Before this was changed, this was a pretty bad bug. The RSA code uses the horrible BN_with_flags() function to create local versions of the private moduli and set BN_FLG_CONSTTIME on them. If the RSA_FLAG_CACHE_PRIVATE for caching moduli is set on the RSA, which it is by default, it attempts to set these constant time versions on the RSA's internal Montgomery contexts. Since it is called BN_MONT_CTX_set(), the setter doesn't set a BIGNUM on the BN_MONT_CTX, rather it copies it over, losing the BN_FLG_CONSTTIME flag in the process and make all the horrible leaky RSA code leak some more. Good job. This is all harmless and is mostly a cosmetic fix. BN_FLG_CONSTTIME should be removed internally. It will be kept since various language bindings of course picked it up and expose it. ok beck jsing --- lib/libcrypto/bn/bn_lib.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/libcrypto/bn/bn_lib.c b/lib/libcrypto/bn/bn_lib.c index 49cc6662db3..980c9b54570 100644 --- a/lib/libcrypto/bn/bn_lib.c +++ b/lib/libcrypto/bn/bn_lib.c @@ -1,4 +1,4 @@ -/* $OpenBSD: bn_lib.c,v 1.78 2023/03/27 10:25:02 tb Exp $ */ +/* $OpenBSD: bn_lib.c,v 1.79 2023/03/31 19:39:15 tb Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -379,6 +379,9 @@ BN_copy(BIGNUM *a, const BIGNUM *b) memcpy(a->d, b->d, sizeof(b->d[0]) * b->top); #endif + /* Copy constant time flag from b, but make it sticky on a. */ + a->flags |= b->flags & BN_FLG_CONSTTIME; + a->top = b->top; a->neg = b->neg; return (a); -- 2.20.1