The warning is:
C4210: nonstandard extension used : function given file scope.
It is caused by function declarations that aren't at the top level in a
file.
Change-Id: Ib1c2ae64e15e66eb0a7255a29c0e560fbf55c2b2
Reviewed-on: https://boringssl-review.googlesource.com/6210
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
The goto always jumps into the loop so the for's initialisation
expression can never be executed. Clang warns about this.
Change-Id: I3c3d4b8430754099e9ca6fd20101868c40165245
2ab24a2d40 added sections to ARM assembly
files. However, in cases where .align directives were not next to the
labels that they were intended to apply to, the section directives would
cause them to be ignored.
Change-Id: I32117f6747ff8545b80c70dd3b8effdc6e6f67e0
Reviewed-on: https://boringssl-review.googlesource.com/6050
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This change causes each global arm or aarch64 asm function to be put
into its own section by default. This matches the behaviour of the
-ffunction-sections option to GCC and allows the --gc-sections option to
the linker to discard unused asm functions on a function-by-function
basis.
Sometimes several asm functions will share the same data an, in that
situation, the data is put into the section of one of the functions and
the section of the other function is merged with the added
“.global_with_section” directive.
Change-Id: I12c9b844d48d104d28beb816764358551eac4456
Reviewed-on: https://boringssl-review.googlesource.com/6003
Reviewed-by: Adam Langley <agl@google.com>
Not content with signing negative RSA moduli, still other Estonian IDs have too
many leading zeros. Work around those too.
This workaround will be removed in six months.
BUG=534766
Change-Id: Ica23b1b1499f9dbe39e94cf7b540900860e8e135
Reviewed-on: https://boringssl-review.googlesource.com/5980
Reviewed-by: Adam Langley <agl@google.com>
Target date for removal of the workaround is 6 months.
BUG=532048
Change-Id: I402f75e46736936725575559cd8eb194115ab0df
Reviewed-on: https://boringssl-review.googlesource.com/5910
Reviewed-by: Adam Langley <agl@google.com>
Estonian IDs issued between September 2014 to September 2015 are broken and use
negative moduli. They last five years and are common enough that we need to
work around this bug.
Add parallel "buggy" versions of BN_cbs2unsigned and RSA_parse_public_key which
tolerate this mistake, to align with OpenSSL's previous behavior. This code is
currently hooked up to rsa_pub_decode in RSA_ASN1_METHOD so that d2i_X509 is
tolerant. (This isn't a huge deal as the rest of that stack still uses the
legacy ASN.1 code which is overly lenient in many other ways.)
In future, when Chromium isn't using crypto/x509 and has more unified
certificate handling code, we can put client certificates under a slightly
different codepath, so this needn't hold for all certificates forever. Then in
September 2019, when the broken Estonian certificates all expire, we can purge
this codepath altogether.
BUG=532048
Change-Id: Iadb245048c71dba2eec45dd066c4a6e077140751
Reviewed-on: https://boringssl-review.googlesource.com/5894
Reviewed-by: Adam Langley <agl@google.com>
History has shown there are bugs in not setting the error code
appropriately, which makes any decision making based on
|ERR_peek_last_error|, etc. suspect. Also, this call was interfering
with the link-time optimizer's ability to discard the implementations of
many functions in crypto/err during dead code elimination.
Change-Id: Iba9e553bf0a72a1370ceb17ff275f5a20fca31ec
Reviewed-on: https://boringssl-review.googlesource.com/5748
Reviewed-by: Adam Langley <agl@google.com>
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>
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>
arm_arch.h is included from ARM asm files, but lives in crypto/, not
openssl/include/. Since the asm files are often built from a different
location than their position in the source tree, relative include paths
are unlikely to work so, rather than having crypto/ be a de-facto,
second global include path, this change moves arm_arch.h to
include/openssl/.
It also removes entries from many include paths because they should be
needed as relative includes are always based on the locations of the
source file.
Change-Id: I638ff43d641ca043a4fc06c0d901b11c6ff73542
Reviewed-on: https://boringssl-review.googlesource.com/5746
Reviewed-by: Adam Langley <agl@google.com>
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>
BN_bin2bn takes a size_t as it should, but it passes that into bn_wexpand which
takes unsigned. Switch bn_wexpand and bn_expand to take size_t before they
check bounds against INT_MAX.
BIGNUM itself still uses int everywhere and we may want to audit all the
arithmetic at some point. Although I suspect having bn_expand require that the
number of bits fit in an int is sufficient to make everything happy, unless
we're doing interesting arithmetic on the number of bits somewhere.
Change-Id: Id191a4a095adb7c938cde6f5a28bee56644720c6
Reviewed-on: https://boringssl-review.googlesource.com/5680
Reviewed-by: Adam Langley <agl@google.com>
Move the bn_expand call inside decode_hex; it's an implementation detail of
hex-decoding. decode_dec instead works with BN_mul_word and BN_add_word so it
can just rely on BN internally expanding things and check the return value.
Also clean up the decode_hex loop so it's somewhat more readable and check for
INT_MAX in bn_x2bn. It uses int over size_t rather pervasively, but while I'm
here at least make that function check overflow.
BUG=517474
Change-Id: I4f043973ee43071a02ea5d4313a8fdaf12404e84
Reviewed-on: https://boringssl-review.googlesource.com/5679
Reviewed-by: Adam Langley <agl@google.com>
Probably a remnant of ifdef soup somewhere.
Change-Id: I472f236a2db54a97490b22b0bbcc1701a2dba3b3
Reviewed-on: https://boringssl-review.googlesource.com/5623
Reviewed-by: Adam Langley <agl@google.com>
gyp doesn't allow two files to share the same name to avoid bugs in OS X
libtool, so asn1.c's need to all get a prefix.
Change-Id: I3593597912c49dd02655cae329fb253ed4f6f56d
Reviewed-on: https://boringssl-review.googlesource.com/5431
Reviewed-by: Adam Langley <agl@google.com>
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>
BN_copy can fail on malloc failure. The case in crypto/rsa was causing the
malloc tests in all_tests.go to infinite loop.
Change-Id: Id5900512013fba9960444d78a8c056aa4314fb2d
Reviewed-on: https://boringssl-review.googlesource.com/5110
Reviewed-by: Adam Langley <agl@google.com>
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>
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>
The functions BN_rshift and BN_lshift shift their arguments to the right or
left by a specified number of bits. Unpredicatable results (including
crashes) can occur if a negative number is supplied for the shift value.
Thanks to Mateusz Kocielski (LogicalTrust), Marek Kroemeke and Filip Palian
for discovering and reporting this issue.
(Imported from upstream's 7cc18d8158b5fc2676393d99b51c30c135502107.)
Change-Id: Ib9f5e410a46df3d7f02a61374807fba209612bd3
Reviewed-on: https://boringssl-review.googlesource.com/4892
Reviewed-by: Adam Langley <agl@google.com>
Currently far from passing and I haven't even tried with a leak checker yet.
Also bn_test is slow.
Change-Id: I4fe2783aa5f7897839ca846062ae7e4a367d2469
Reviewed-on: https://boringssl-review.googlesource.com/4794
Reviewed-by: Adam Langley <agl@google.com>
Unlike the standalone build, builds generated from util/generate_build_files.py
do not exclude x86_64-gcc.c. Match the consumer builds by making the standalone
build unconditionally include it. (This would have noticed the missing
preprocessor checks in the file.)
Change-Id: I8d20f269dea63776320ae636ee1e5339cb85fa30
Reviewed-on: https://boringssl-review.googlesource.com/4761
Reviewed-by: Adam Langley <agl@google.com>
Android (on OS X) builds with NO_ASM and was getting both generic.c and
x86_64-gcc.c. This change updates the latter so that it's excluded in
NO_ASM builds.
Change-Id: I1f0e1c5e551eed9c575ce632ec3016fce7ec9d2e
Reviewed-on: https://boringssl-review.googlesource.com/4741
Reviewed-by: Adam Langley <agl@google.com>
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>
First batch of the alphabet.
Change-Id: If4e60f4fbb69e04eb4b70aa1b2240e329251bfa5
Reviewed-on: https://boringssl-review.googlesource.com/4514
Reviewed-by: Adam Langley <agl@google.com>
This facilitates "universal" builds, ones that target multiple
architectures, e.g. ARMv5 through ARMv7.
(Imported from upstream's c1669e1c205dc8e695fb0c10a655f434e758b9f7)
This is a change from a while ago which was a source of divergence between our
perlasm and upstream's. This change in upstream came with the following comment
in Configure:
Note that -march is not among compiler options in below linux-armv4
target line. Not specifying one is intentional to give you choice to:
a) rely on your compiler default by not specifying one;
b) specify your target platform explicitly for optimal performance,
e.g. -march=armv6 or -march=armv7-a;
c) build "universal" binary that targets *range* of platforms by
specifying minimum and maximum supported architecture;
As for c) option. It actually makes no sense to specify maximum to be
less than ARMv7, because it's the least requirement for run-time
switch between platform-specific code paths. And without run-time
switch performance would be equivalent to one for minimum. Secondly,
there are some natural limitations that you'd have to accept and
respect. Most notably you can *not* build "universal" binary for
big-endian platform. This is because ARMv7 processor always picks
instructions in little-endian order. Another similar limitation is
that -mthumb can't "cross" -march=armv6t2 boundary, because that's
where it became Thumb-2. Well, this limitation is a bit artificial,
because it's not really impossible, but it's deemed too tricky to
support. And of course you have to be sure that your binutils are
actually up to the task of handling maximum target platform.
Change-Id: Ie5f674d603393f0a1354a0d0973987484a4a650c
Reviewed-on: https://boringssl-review.googlesource.com/4488
Reviewed-by: Adam Langley <agl@google.com>
Pointer out and suggested by: Ard Biesheuvel.
(Imported from upstream's 5dcf70a1c57c2019bfad640fe14fd4a73212860a)
This is from a while ago, but it's one source of divergence between our copy of
these files and master's.
Change-Id: I6525a27f25eb86a92420c32996af47ecc42ee020
Reviewed-on: https://boringssl-review.googlesource.com/4487
Reviewed-by: Adam Langley <agl@google.com>
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>
This introduces a per-RSA/DSA/DH lock. This is good for lock contention,
although pthread locks are depressingly bloated.
Change-Id: I07c4d1606fc35135fc141ebe6ba904a28c8f8a0c
Reviewed-on: https://boringssl-review.googlesource.com/4324
Reviewed-by: Adam Langley <agl@google.com>
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>
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>
It seems Android's inttypes.h refuses to define those macros on C++ unless
__STDC_FORMAT_MACROS is set. This unbreaks the roll on Android.
Change-Id: Iad6c971b4789f0302534d9e5022534c6124e0ff0
Reviewed-on: https://boringssl-review.googlesource.com/4202
Reviewed-by: Adam Langley <agl@google.com>
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>
In the event of an error |rr| could be NULL. Therefore don't assume you can
use |rr| in the error handling code.
(Imported from upstream's 8c5a7b33c6269c3bd6bc0df6b4c22e4fba03b485.)
Change-Id: I0b392991ce8170dc418e93003af256d535d1e2e8
Reviewed-on: https://boringssl-review.googlesource.com/4005
Reviewed-by: Adam Langley <agl@google.com>
Fix a few remnants of them being unsigned long. Also rename extremely unhelpful
variable names in SSL_get_error. i is now ret_code to match the header.
Change-Id: Ic31d6626bfe09c9e21c03691dfc716c5573833ea
Reviewed-on: https://boringssl-review.googlesource.com/3881
Reviewed-by: Adam Langley <agl@google.com>
This reverts the non-ARM portions of 97999919bb.
x86_64 perlasm already makes .globl imply .hidden. (Confusingly, ARM does not.)
Since we don't need it, revert those to minimize divergence with upstream.
Change-Id: I2d205cfb1183e65d4f18a62bde187d206b1a96de
Reviewed-on: https://boringssl-review.googlesource.com/3610
Reviewed-by: Adam Langley <agl@google.com>
We are leaking asm symbols in Android builds because the asm code isn't
affected by -fvisibility=hidden. This change hides all asm symbols.
This assumes that no asm symbols are public API and that should be true.
Some points to note:
In crypto/rc4/asm/rc4-md5-x86_64.pl there are |RC4_set_key| and
|RC4_options| functions which aren't getting marked as hidden. That's
because those functions aren't actually ever generated. (I'm just trying
to minimise drift with upstream here.)
In crypto/rc4/asm/rc4-x86_64.pl there's |RC4_options| which is "public"
API, except that we've never had it in the header files. So I've just
deleted it. Since we have an internal caller, we'll probably have to put
it back in the future, but it can just be done in rc4.c to save
problems.
BUG=448386
Change-Id: I3846617a0e3d73ec9e5ec3638a53364adbbc6260
Reviewed-on: https://boringssl-review.googlesource.com/3520
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
