Always end BN_mod_exp_mont_consttime with normal Montgomery reduction.
authorTomas Mraz <tomas@openssl.org>
Thu, 9 Jun 2022 10:34:55 +0000 (12:34 +0200)
committerTomas Mraz <tomas@openssl.org>
Thu, 16 Jun 2022 13:22:35 +0000 (15:22 +0200)
commit0ae365e1f80648f4c52aa3ac9bbc279b6192b23e
tree3c64a03999de0210c8b171207d79a5d119c3022b
parentb2feb9f0e394da6570346598837f1b01eb58c028
Always end BN_mod_exp_mont_consttime with normal Montgomery reduction.

This partially fixes a bug where, on x86_64, BN_mod_exp_mont_consttime
would sometimes return m, the modulus, when it should have returned
zero. Thanks to Guido Vranken for reporting it. It is only a partial fix
because the same bug also exists in the "rsaz" codepath.

The bug only affects zero outputs (with non-zero inputs), so we believe
it has no security impact on our cryptographic functions.

The fx is to delete lowercase bn_from_montgomery altogether, and have the
mont5 path use the same BN_from_montgomery ending as the non-mont5 path.
This only impacts the final step of the whole exponentiation and has no
measurable perf impact.

See the original BoringSSL commit
https://boringssl.googlesource.com/boringssl/+/13c9d5c69d04485a7a8840c12185c832026c8315
for further analysis.

Original-author: David Benjamin <davidben@google.com>

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18510)
crypto/bn/asm/x86_64-mont5.pl
crypto/bn/bn_exp.c
test/recipes/10-test_bn_data/bnmod.txt