We still need BN_mul and, in particular, bn_mul_recursive will either
require bn_abs_sub_words be generalized or that we add a parallel
bn_abs_sub_part_words, but start with the easy one.
While I'm here, simplify the i and j mess in here. It's patterned after
the multiplication one, but can be much simpler.
Bug: 234
Change-Id: If936099d53304f2512262a1cbffb6c28ae30ccee
Reviewed-on: https://boringssl-review.googlesource.com/25325
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
The fallback functions still themselves leak, but I've left TODOs there.
This only affects BN_mod_mul_montgomery on platforms where we don't use
the bn_mul_mont assembly, but BN_mul additionally affects the final
multiplication in RSA CRT.
Bug: 232
Change-Id: Ia1ae16162c38e10c056b76d6b2afbed67f1a5e16
Reviewed-on: https://boringssl-review.googlesource.com/25260
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This has no behavior change, but it has a semantic one. This CL is an
assertion that all BIGNUM functions tolerate non-minimal BIGNUMs now.
Specifically:
- Functions that do not touch top/width are assumed to not care.
- Functions that do touch top/width will be changed by this CL. These
should be checked in review that they tolerate non-minimal BIGNUMs.
Subsequent CLs will start adjusting the widths that BIGNUM functions
output, to fix timing leaks.
Bug: 232
Change-Id: I3a2b41b071f2174452f8d3801bce5c78947bb8f7
Reviewed-on: https://boringssl-review.googlesource.com/25257
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Test this by re-running bn_tests.txt tests a lot. For the most part,
this was done by scattering bn_minimal_width or bn_correct_top calls as
needed. We'll incrementally tease apart the functions that need to act
on non-minimal BIGNUMs in constant-time.
BN_sqr was switched to call bn_correct_top at the end, rather than
sample bn_minimal_width, in anticipation of later splitting it into
BN_sqr (for calculators) and BN_sqr_fixed (for BN_mod_mul_montgomery).
BN_div_word also uses bn_correct_top because it calls BN_lshift so
officially shouldn't rely on BN_lshift returning something
minimal-width, though I expect we'd want to split off a BN_lshift_fixed
than change that anyway?
The shifts sample bn_minimal_width rather than bn_correct_top because
they all seem to try to be very clever around the bit width. If we need
constant-time versions of them, we can adjust them later.
Bug: 232
Change-Id: Ie17b39034a713542dbe906cf8954c0c5483c7db7
Reviewed-on: https://boringssl-review.googlesource.com/25255
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
mem.h for |OPENSSL_cleanse| and bn/internal.h for things like
|bn_less_than_words| and |bn_correct_top|.
Change-Id: I3c447a565dd9e4f18fb2ff5d59f80564b4df8cea
Reviewed-on: https://boringssl-review.googlesource.com/23164
Reviewed-by: Adam Langley <agl@google.com>
As part of excising BIGNUM from EC scalars, we will need a "words"
version of BN_mod_mul_montgomery. That, in turn, requires BN_sqr and
BN_mul for cases where we don't have bn_mul_mont.
BN_sqr and BN_mul have a lot of logic in there, with the most complex
cases being not even remotely constant time. Fortunately, those only
apply to RSA-sized numbers, not EC-sized numbers. (With the exception, I
believe, of 32-bit P-521 which just barely exceeds the cutoff.) Imposing
a limit also makes it easier to stack-allocate temporaries (BN_CTX
serves a similar purpose in BIGNUM).
Extract bn_mul_small and bn_sqr_small and test them as part of
bn_tests.txt. Later changes will build on these.
If we end up reusing these functions for RSA in the future (though that
would require tending to the egregiously non-constant-time code in the
no-asm build), we probably want to extract a version where there is an
explicit tmp parameter as in bn_sqr_normal rather than the stack bits.
Change-Id: If414981eefe12d6664ab2f5e991a359534aa7532
Reviewed-on: https://boringssl-review.googlesource.com/23068
Reviewed-by: Adam Langley <agl@google.com>
Also replace a pointless call to bn_mul_words with a memset.
Change-Id: Ief30ddab0e84864561b73fe2776bd0477931cf7f
Reviewed-on: https://boringssl-review.googlesource.com/23066
Reviewed-by: Adam Langley <agl@google.com>
This is an OpenSSL thing to support platforms where BN_ULONG is not
actually the size it claims to be. We define BN_ULONG to uint32_t and
uint64_t which are guaranteed by C to implement arithemetic modulo 2^32
and 2^64, respectively. Thus there is no need for any of this.
Change-Id: I098cd4cc050a136b9f2c091dfbc28dd83e01f531
Reviewed-on: https://boringssl-review.googlesource.com/21784
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Fixes failed compile with [-Werror=implicit-fallthrough=], which is
default on gcc-7.x on distributions like fedora.
Enabling no implicit fallthrough for more than just clang as well to
catch this going forward.
Change-Id: I6cd880dac70ec126bd7812e2d9e5ff804d32cadd
Signed-off-by: Vincent Batts <vbatts@redhat.com>
Reviewed-on: https://boringssl-review.googlesource.com/20564
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
crypto/{asn1,x509,x509v3,pem} were skipped as they are still OpenSSL
style.
Change-Id: I3cd9a60e1cb483a981aca325041f3fbce294247c
Reviewed-on: https://boringssl-review.googlesource.com/19504
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
