From: tb <tb@openbsd.org>
Date: Fri, 31 Mar 2023 19:39:15 +0000 (+0000)
Subject: Copy BN_FLG flags in BN_copy()
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=b643efb71d833e986bbd311963a276719be21755;p=openbsd

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
---

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);