Copy BN_FLG flags in BN_copy()
authortb <tb@openbsd.org>
Fri, 31 Mar 2023 19:39:15 +0000 (19:39 +0000)
committertb <tb@openbsd.org>
Fri, 31 Mar 2023 19:39:15 +0000 (19:39 +0000)
commitb643efb71d833e986bbd311963a276719be21755
treec62cdc16ed202feed4a638a51945cc97ac92b33f
parent7d91647159257b31a2d233274299e309bd81844e
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