Commit Graph

16 Commits

Author SHA1 Message Date
David Benjamin
5148345282 BN_mod_exp_mont_consttime: check for zero modulus.
Don't dereference |d| when |top| is zero. Also test that various BIGNUM
methods behave correctly on zero/even inputs.

(Imported from upstream's cf633fa00244e39eea2f2c0b623f7d5bbefa904e.)

We already had the BN_div and BN_MONT_CTX_set tests, but align them with
upstream's for consistency.

Change-Id: Ice5d04f559b4d5672e23c400637c07d8ee401727
Reviewed-on: https://boringssl-review.googlesource.com/5783
Reviewed-by: Adam Langley <agl@google.com>
2015-09-01 19:12:28 +00:00
David Benjamin
fe71f1d4a5 Fix spurious bn_test failures.
BN_rand generates a single-word zero BIGNUM with quite a large
probability.

A zero BIGNUM in turn will end up having a NULL |d|-buffer, which we
shouldn't dereference without checking.

(Imported from upstream's 9c989aaa749d88b63bef5d5beeb3046eae62d836.)

Change-Id: Ic4d113e4fcf4ea4c0a4e905a1c4ba3fb758d9fc6
Reviewed-on: https://boringssl-review.googlesource.com/5782
Reviewed-by: Adam Langley <agl@google.com>
2015-09-01 19:10:01 +00:00
Matt Braithwaite
6488725e5e Re-add |BN_bn2mpi| and |BN_mpi2bn| from OpenSSL at fd682e4c.
This benefits mainly M2Crypto.

Change-Id: I29bd0fa31b218760055ba467673f3882e46010c7
Reviewed-on: https://boringssl-review.googlesource.com/5722
Reviewed-by: Adam Langley <agl@google.com>
2015-08-18 23:49:31 +00:00
David Benjamin
9e45d6e42f Check for 0 modulus in BN_MONT_CTX_set.
The function BN_MONT_CTX_set was assuming that the modulus was non-zero
and therefore that |mod->top| > 0. In an error situation that may not be
the case and could cause a seg fault.

This is a follow on from CVE-2015-1794.

(Imported from upstream's 512368c9ed4d53fb230000e83071eb81bf628b22.)

The CVE itself doesn't affect us as the bit strength check in the DHE logic
excludes zero.

Also add tests to bn_test for a couple of division by zero cases. (This and
BN_div.)

Change-Id: Ibd8ef98d6be48eb95110021c23cd8e278656764d
Reviewed-on: https://boringssl-review.googlesource.com/5690
Reviewed-by: Adam Langley <agl@google.com>
2015-08-17 20:32:38 +00:00
Brian Smith
62138c5d5a Check the return value of |BN_sub| in bn_test.cc.
Coverity complains about this.

Change-Id: I5140fd82bdbe7c2e3b36a8bc9006770a431741ea
Reviewed-on: https://boringssl-review.googlesource.com/5619
Reviewed-by: Adam Langley <agl@google.com>
2015-08-07 00:52:58 +00:00
David Benjamin
b9c579db6d Add crypto/bytestring-based BIGNUM DER functions.
RSA and ECDSA will both require being able to convert ASN.1 INTEGERs to
and from DER. Don't bother handling negative BIGNUMs for now. It doesn't
seem necessary and saves bothering with two's-complement vs
sign-and-magnitude.

BUG=499653

Change-Id: I1e80052067ed528809493af73b04f82539d564ff
Reviewed-on: https://boringssl-review.googlesource.com/5268
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 00:47:39 +00:00
David Benjamin
15eaafb18a Fix bn_test's bc output and shut it up a little.
bn_test's output is meant to be piped to bc, but this got broken somewhat:

- OpenSSL uses uppercase hex rather than BoringSSL's lowercase. bc only accepts
  uppercase. Document that this needs some shell pipeline until we replace
  them with better tests because this is all ridiculous.

- Some stderr outputs moved to stdout to avoid cluttering stdout. Just remove
  them. The operations are fast enough to not need progress.

- To cut down on noise, only write the bc transcript given a command-line flag.
  Also remove the -results flag since it's pointless. (It writes only the
  results and not the inputs.)

Change-Id: I08f87cac1e03fab461f0dc40b9d4285bd877807d
Reviewed-on: https://boringssl-review.googlesource.com/4896
Reviewed-by: Adam Langley <agl@google.com>
2015-05-28 17:25:41 +00:00
David Benjamin
eb930b8703 Fix signed/unsigned warning in bn_test.cc.
BN_num_bits returns unsigned.

Change-Id: Ie346dbe0a12d3597d2b12e56b57dfc147e65dcc3
Reviewed-on: https://boringssl-review.googlesource.com/4895
Reviewed-by: Adam Langley <agl@google.com>
2015-05-27 22:04:16 +00:00
David Benjamin
b3a7b51f18 Fix off-by-one in BN_rand
If BN_rand is called with |bits| set to 1 and |top| set to 1 then a 1 byte
buffer overflow can occur.

See also upstream's efee575ad464bfb60bf72dcb73f9b51768f4b1a1. But rather than
making |BN_rand| fail, be consistent with the |bits| = 0 case and just don't
set the bits that don't exist. Add tests to ensure the degenerate cases behave.

Change-Id: I5e9fbe6fd8f7f7b2e011a680f2fbe6d7ed4dab65
Reviewed-on: https://boringssl-review.googlesource.com/4893
Reviewed-by: Adam Langley <agl@google.com>
2015-05-27 22:03:05 +00:00
David Benjamin
c561aa64b6 Require source files define __STDC_FORMAT_MACROS to use BN FMT macros.
inttypes.h kindly requires a feature macro in C++ on some platforms, due
to a bizarre footnote in C99 (see footnote 191 in section 7.8.1). As
bn.h is a public header, we must leak this wart to the consumer. On
platforms with unfriendly inttypes.h headers, using BN_DEC_FMT1 and
friends now require the feature macro be defined externally.

This broke the Chromium Android Clang builder:
http://build.chromium.org/p/chromium.linux/builders/Android%20Clang%20Builder%20%28dbg%29/builds/59288

Change-Id: I88275a6788c7babd0eae32cae86f115bfa93a591
Reviewed-on: https://boringssl-review.googlesource.com/4688
Reviewed-by: Adam Langley <agl@google.com>
2015-05-11 18:38:08 +00:00
David Benjamin
160f4ef14c Test BN_mul for negative zero.
Change-Id: I235c59c14ec08c3338c22d080f304bdf2d7adef0
Reviewed-on: https://boringssl-review.googlesource.com/4486
Reviewed-by: Adam Langley <agl@google.com>
2015-05-04 22:41:25 +00:00
David Benjamin
8bac8c48ec Test negatives for BN_div.
Change-Id: I8ebe58724e8b81a7f21762eff51f0ffd141ab08b
Reviewed-on: https://boringssl-review.googlesource.com/4485
Reviewed-by: Adam Langley <agl@google.com>
2015-05-04 22:41:10 +00:00
David Benjamin
c85573ccd8 Ensure BN_asc2bn, BN_dec2bn, and BN_hex2bn never give -0.
See upstream's a0eed48d37a4b7beea0c966caf09ad46f4a92a44. Rather than import
that, we should just ensure neg + zero isn't a possible state.

Add some tests for asc2bn and dec2bn while we're here. Also fix a bug with
dec2bn where it doesn't actually ignore trailing data as it's supposed to.

Change-Id: I2385b67b740e57020c75a247bee254085ab7ce15
Reviewed-on: https://boringssl-review.googlesource.com/4484
Reviewed-by: Adam Langley <agl@google.com>
2015-04-23 20:53:24 +00:00
Brian Smith
9da82c1ccc Remove superfluous crypto/bio dependencies from tests.
Limiting uses of crypto/bio to code that really need to it by avoiding
the use of BIO just to write to stdout/stderr.

Change-Id: I34e0f773161aeec073691e439ac353fb7b1785f3
Reviewed-on: https://boringssl-review.googlesource.com/3930
Reviewed-by: Adam Langley <agl@google.com>
2015-04-13 20:24:18 +00:00
Brian Smith
83a82981dc Rename BIO_print_errors_fp back to ERR_print_errors_fp & refactor it.
A previous change in BoringSSL renamed ERR_print_errors_fp to
BIO_print_errors_fp as part of refactoring the code to improve the
layering of modules within BoringSSL. Rename it back for better
compatibility with code that was using the function under the original
name. Move its definition back to crypto/err using an implementation
that avoids depending on crypto/bio.

Change-Id: Iee7703bb1eb4a3d640aff6485712bea71d7c1052
Reviewed-on: https://boringssl-review.googlesource.com/4310
Reviewed-by: Adam Langley <agl@google.com>
2015-04-13 20:23:29 +00:00
David Benjamin
a5974bfae7 Convert bn_test to C++.
Along the way, fix a host of missing failure checks. This will save some
headache when it comes time to run these under the malloc failure tests.

Change-Id: I3fd589bd094178723398e793d6bc578884e99b67
Reviewed-on: https://boringssl-review.googlesource.com/4126
Reviewed-by: Adam Langley <agl@google.com>
2015-04-01 19:53:51 +00:00