[openssl/openssl] 037264: Revert "Fix Timing Oracle in RSA decryption"

[Bernd Edlinger]

  Branch: refs/heads/OpenSSL_1_1_1-stable
  Home:   https://github.com/openssl/openssl
  Commit: 0372649a943fb23f7f08c7acdbc01464b9df03f0
      https://github.com/openssl/openssl/commit/0372649a943fb23f7f08c7acdbc01464b9df03f0
  Author: Bernd Edlinger <bernd.edlinger at hotmail.de>
  Date:   2023-03-31 (Fri, 31 Mar 2023)

  Changed paths:
    M crypto/bn/bn_blind.c
    M crypto/bn/bn_err.c
    M crypto/bn/bn_local.h
    M crypto/bn/build.info
    R crypto/bn/rsa_sup_mul.c
    M crypto/err/openssl.txt
    M crypto/rsa/rsa_ossl.c
    M include/crypto/bn.h
    M include/openssl/bnerr.h

  Log Message:
  -----------
  Revert "Fix Timing Oracle in RSA decryption"

This reverts commit 43d8f88511991533f53680a751e9326999a6a31f.

Reviewed-by: Paul Dale <pauli at openssl.org>
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20284)


  Commit: 3f499b24f3bcd66db022074f7e8b4f6ee266a3ae
      https://github.com/openssl/openssl/commit/3f499b24f3bcd66db022074f7e8b4f6ee266a3ae
  Author: Bernd Edlinger <bernd.edlinger at hotmail.de>
  Date:   2023-03-31 (Fri, 31 Mar 2023)

  Changed paths:
    M CHANGES
    M crypto/bn/bn_asm.c
    M crypto/bn/bn_blind.c
    M crypto/bn/bn_lib.c
    M crypto/bn/bn_local.h
    M crypto/rsa/rsa_ossl.c

  Log Message:
  -----------
  Alternative fix for CVE-2022-4304

This is about a timing leak in the topmost limb
of the internal result of RSA_private_decrypt,
before the padding check.

There are in fact at least three bugs together that
caused the timing leak:

First and probably most important is the fact that
the blinding did not use the constant time code path
at all when the RSA object was used for a private
decrypt, due to the fact that the Montgomery context
rsa->_method_mod_n was not set up early enough in
rsa_ossl_private_decrypt, when BN_BLINDING_create_param
needed it, and that was persisted as blinding->m_ctx,
although the RSA object creates the Montgomery context
just a bit later.

Then the infamous bn_correct_top was used on the
secret value right after the blinding was removed.

And finally the function BN_bn2binpad did not use
the constant-time code path since the BN_FLG_CONSTTIME
was not set on the secret value.

In order to address the first problem, this patch
makes sure that the rsa->_method_mod_n is initialized
right before the blinding context.

And to fix the second problem, we add a new utility
function bn_correct_top_consttime, a const-time
variant of bn_correct_top.

Together with the fact, that BN_bn2binpad is already
constant time if the flag BN_FLG_CONSTTIME is set,
this should eliminate the timing oracle completely.

In addition the no-asm variant may also have
branches that depend on secret values, because the last
invocation of bn_sub_words in bn_from_montgomery_word
had branches when the function is compiled by certain
gcc compiler versions, due to the clumsy coding style.

So additionally this patch stream-lined the no-asm
C-code in order to avoid branches where possible and
improve the resulting code quality.

Reviewed-by: Paul Dale <pauli at openssl.org>
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20284)


Compare: https://github.com/openssl/openssl/compare/0d16b7e99aaf...3f499b24f3bc