Some Android devices in our builder pool are so old that they lack
getrandom. This change attempts to make them happy.
Change-Id: I5eea04f1b1dc599852e3b8448ad829bea05b9fe9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57527
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: Adam Langley <agl@google.com>
When the parameters are incorrect, all assumptions of (EC)DSA fly out
the window, including whether the retry loop actually terminates.
While ECDSA is broadly used with fixed, named groups, DSA was
catastrophically mis-specified with arbitrary parameters being the
default and only mode. Cap the number of retries in DSA_do_sign so
invalid DSA groups cannot infinite loop, e.g. if the "generator" is
really nilpotent.
This also caps the iteration count for ECDSA. We do, sadly, support
arbitrary curves via EC_GROUP_new_curve_GFp, to help Conscrypt remain
compatible with a badly-designed Java API. After
https://boringssl-review.googlesource.com/c/boringssl/+/51925, we
documented that untrusted parameters are not supported and may produce
garbage outputs, but we did not document that infinite loops are
possible. I don't have an example where an invalid curve breaks ECDSA,
but as it breaks all preconditions, I cannot be confident it doesn't
exist, so just cap the iterations.
Thanks to Hanno Böck who originally reported an infinite loop on
invalid DSA groups. While that variation did not affect BoringSSL, it
inspired us to find other invalid groups which did.
Thanks also to Guido Vranken who found, in
https://github.com/openssl/openssl/issues/20268, an infinite loop when
the private key is zero. That was fixed in the preceding CL, as it
impacts valid groups too, but the infinite loop is ultimately in the
same place, so this change also would have mitigated the loop.
Update-Note: If signing starts failing with ECDSA_R_INVALID_ITERATIONS,
something went horribly wrong because it should not be possible with
real curves. (Needing even one retry has probability 2^-256 or so.)
Change-Id: If8fb0157055d3d8cb180fe4f27ea7eb349ec2738
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57228
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
DSA private keys cannot be zero. If they are, trying to sign an all
zeros digest loops forever. Thanks to Guido Vranken who reported this in
https://github.com/openssl/openssl/issues/20268
Along the way, because OpenSSL's bad API design made constructing DSA
objects such a mess, just move all the consistency checks to
dsa_check_parameters (now dsa_check_key) so it is consistently checked
everywhere.
Ideally we'd get a better handle on DSA state, like we hope to do for
RSA state (though not there yet), so checks only happen once. But we
consider DSA deprecated so it's not worth putting much effort into it.
Update-Note: Some invalid DSA keys will be rejected by the parser and at
use. Nothing should be using DSA anymore.
Change-Id: I25d3faf145a85389c47abdd9db8e9b0056b37d8a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57227
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This reverts commit 8ce0e1c14e48109773f1e94e5f8b020aa1e24dc5.
The original commit didn't work on Android because:
a) urandom_test didn't handle the fact that Android requires
getrandom() and will never fall back to /dev/urandom.
b) Android may open files in /dev/__properties__ which
confused urandom_test.
The original change is patchset 1 so the differences build on that.
Change-Id: Ib840ec20d60cb28d126d3d09271b18fbd9ec1371
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53705
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: Adam Langley <agl@google.com>
We already reject values that are out of bounds. Also we were using the
wrong error, so fix that. Zero should additionally be rejected,
otherwise, when signing an all-zero digest (impossible unless your
system signs untrusted, pre-hashed inputs), ECDSA can infinite loop.
Thanks to Guido Vranken who reported an analogous issue with DSA in
https://github.com/openssl/openssl/issues/20268
When EC_KEYs are obtained through the parser, this CL is a no-op. The
corresponding public key is the point at infinity, which we'll reject at
both parse time and in EC_KEY_check_key. But as EC_KEY runs into
OpenSSL's usual API design flaw (mutable, field-by-field setters over
constructor functions for immutable objects), we should reject this in
EC_KEY_set_private_key too.
Update-Note: Systems that manually construct an EC_KEY (i.e. not from
parsing), and either omit the public key or don't call EC_KEY_check_key
will start rejecting the zero private key. If such a system *also* signs
untrusted digests, this fixes an infinite loop in ECDSA.
Change-Id: I3cc9cd2cc59eb6d16826beab3db71d66b23e83ff
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57226
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
OpenSSL's APIs are full of empty objects that aren't captured in the
type system. A DSA object may represent any of nothing, a group, a
public key, or a private key.
Since the type system doesn't capture this, better to check which type
we've got and NULL-check the fields we need for the operation.
Change-Id: I32b09ebaad58fcb2a0bfc9ad56d381f95099bf7b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57225
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
1740ff90a7 added an "algorithm" key to the output. The test expectations
need to be updated accordingly.
Also drops 3DES tests since they were removed from the regcap in
82413455b8.
Change-Id: Ibd23f6166111be361511c2f7974348400f7151e2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57467
Reviewed-by: David Benjamin <davidben@google.com>
Auto-Submit: Adam Langley <agl@google.com>
Go has a bzip2 decoder but not an encoder (because I only needed a
decoder when I wrote the package). Thus when updated ACVP tests are
written they aren't compressed. This change removes the `.bz2` suffix
from paths when writing updated tests, which makes it easier to compress
them.
Change-Id: I9de6a1845cdf1cddca2e5d253b97262b023393f6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57466
Auto-Submit: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
It happened that it used to be possible to run `acvptool` with `-json`
and not need a config file. That stopped working as of 02397c7c97 but
the test runner needs to be updated accordingly.
Change-Id: I54cf0fc7420d18749e93c3d85201ba24d0a59e15
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57465
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Ran go get -u all, followed by go mod tidy. Some tools are flagging
CVE-2021-43565 and CVE-2022-27191 in some of the Go packages. Our uses
of x/crypto are x/net are not impacted by either bug, but update anyway
to silence the tools.
Change-Id: Ia0e2757625b58d964aedd4217f21b72f293b910b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57485
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This extends the support for execute-only memory in AArch64 assembly
and uses adrp and add instead of adr.
Change-Id: I388a13ec754e7f179d7a234516f1bb4ff6a5c919
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57446
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Change-Id: I7dea47e73cbfbfa48a8924f3a633215c06730b22
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57425
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
This convention seems to break with some other tooling we have. Until we
figure out how to resolve that, remove the lines.
This partially reverts 54b04fdc21d540a6e24f9ddb7ddc3e583518e24f but
keeps the fixes to the license header comments.
Change-Id: I4f08a9f3daf65d17b4c78ac6f4ac3de234ec3436
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57366
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
The union isn't actually providing type-safety: nothing checks that we
access the correct arm of the union, and it has a void* arm anyway.
Instead, it's just adding some strict aliasing risk by relying on
type-punning: we usually write to the pointer as void*, via
EVP_PKEY_assign, but then we read from it as the underlying type.
This is allowed in C, but not in C++. And even in C, while that is
allowed, if we ever wrote &pkey->pkey.rsa, it would suddenly be a strict
aliasing violation. Just use a void*, which means we don't type-pun
pointer types against each other.
While I'm here, I made the free callbacks for EVP_PKEYs also NULL the
pointer. The one caller also NULLs it, so its fine, but some did and
some didn't do it, and this seems prudent.
Bug: 301
Change-Id: I74c76ed3984527df66f64bb2d397af44f63920bd
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57106
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
While hiding 'type' isn't such a huge deal, accessing 'pkey' without a
type check is very dangerous. The accessors are type-checked and avoid
this problem. It also gets us slightly closer to not needing to utter
CRYPTO_refcount_t in public headers, as we're currently not quite
declaring it right. And it allows us to remove another union:
https://boringssl-review.googlesource.com/c/boringssl/+/57106
This matches what upstream did in OpenSSL 1.1.0.
Update-Note: Code that reaches into the EVP_PKEY struct will no longer
compile, like in OpenSSL. I believe I've fixed all the cases. If I
missed any, the fix is to switch code to accessors. EVP_PKEY_id(pkey)
for pkey->type is the most common fix.
Change-Id: Ibe8d6b6cb8fbd141ea1cef0d02dc1ae3703e9469
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57105
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Prior to 3.12 (which we won't be requiring until July), OBJECT libraries
cannot be used with target_link_libraries. That means they cannot pick
up INTERFACE_INCLUDE_DIRECTORIES, which makes them pretty unusable in
the "modern CMake" style.
Just switch it to a static library to unbreak the build in CMake 3.10.
For some link ordering reason I don't understand, this also requires
explicitly linking boringssl_gtest to libcxx when we USE_CUSTOM_LIBCXX
is set.
Change-Id: Ia9d8351551f5da060248aa3ca73fe04473bf62aa
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57345
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Change-Id: I5682358b5564dbaa734c5910c11a8274e88ca67a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57186
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
bindgen, by default, will bind every random symbol in the libc, which is
clearly unreasonable. Now that --allowlist-file exists, we can switch to
doing what it should have done from the beginning.
This produces a pretty large diff in the bindgen output, but it's all to
exclude miscellaneous bits of libc.
Change-Id: I9a35fda10ff6f1b82449919f9fcc2ea86ad5b802
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57325
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Alex Gaynor <alex.gaynor@gmail.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Change-Id: I0b1ba546374aa8b0fe79528f56e19f261536e565
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57305
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Nothing uses this, and the code is somewhat decrepit. Instead of
fixing it and continuing to maintain it as attack surface, we
send this off to the farm where it can run and play all day with
the other unused X.509 extensions.
Update-note: This removes the proxy certificate extension from
the recognized certificate extensions. Previously by default
a certificate with a critical proxy certificate extension would
have been rejected with "proxy certificate not allowed", but
will now be rejected with an unrecognized critical extension
error.
Fixed: 568
Change-Id: I5f838d69c59517254b4fa83f6e2abe6057fa66c7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57265
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Auto-Submit: Bob Beck <bbe@google.com>
This was added with the generated symbol-prefixing header. But it
seems to be sufficient for crypto to have a dependency on the
generated header, along with some of the stray bits of delocate.
It's a little unclear from CMake documentation how these are processed;
normally .o files can be built before libraries are built or linked,
only the link step depends on.
But, empirically, if A links B, and B has a dependency on C, then CMake
seems to run C before building any of A. I tested this by making a small
project where the generation step slept for three seconds and running
with enough parallelism that we'd have tripped.
Interestingly, in the Makefile output, the individual object file
targets didn't have that dependency, but the target itself did. But this
was true on both A and B, so I think that just might not work.
Also fix the dependency in the custom target. The old formulation broke
when using an absolute path to the symbols file.
Change-Id: I2053d44949f907d465da403a5ec69c191740268f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56928
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Also stick the very verbose default install directory in a variable so
we don't have to repeat it everywhere.
Change-Id: I1a6a85c4d42d3a6e766e52b2d0ecd8e81c6ed4e3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56607
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
It's unclear to me whether doing it target-by-target is an improvement
in crypto/fipsmodule, but this otherwise does seem a bit tidier. This
aligns with CMake's documentation and "modern CMake" which prefers this
pattern.
Change-Id: I36c81842bff8b36eeaaf5dd3e0695fb45f3376c9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56585
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Compilers are fine at inlining functions nowadays. We can hide the
BN_ULLONG vs. manual carry extraction inside an inline function. I've
patterned the type signatures intentionally after Clang's builtins, in
case we want to use them in the future.
(Previously I wrote in
https://boringssl-review.googlesource.com/c/boringssl/+/56966 that the
builtins weren't good on aarch64. This wasn't quite right. Rather, they
were bad on both x86_64 and aarch64 in LLVM 13, but they're fine on both
in LLVM 14. My machine's Xcode was just a little old.)
Change-Id: I666466dce7a146d5e49e94ff372ea018b610ef34
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57245
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Also add public APIs for this, now that the specification is no longer
expected to change, and because a project external to the library wishes
to use it.
For now, I've kept the P-256 version using the generic felem_exp, but we
should update that to use the specialized field arithmetic.
Trust Tokens will presumably move to this later and, in the meantime,
another team wants this.
Bug: chromium:1414562
Change-Id: Ie38203b4439ff55659c4fb2070f45d524c55aa2a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57147
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
draft-07 to draft-16 is mostly editorial, but there were a few notable
changes:
- Empty DST values are forbidden.
- The sample implementation for map_to_curve_simple_swu has completely
changed. The new formulation has the same performance (if not a smidge
faster), and aligning with the spec seems generally useful.
- P-384 is now paired with SHA-384, not SHA-512. As this would be a
breaking change for the trust tokens code, I've left that in. A
follow-up CL will add implementations of draft-16, which is expected
to match the final draft.
Before:
Did 77000 hash-to-curve P384_XMD:SHA-512_SSWU_RO_ operations in 4025677us (19127.2 ops/sec)
Did 7156000 hash-to-scalar P384_XMD:SHA-512 operations in 4000385us (1788827.8 ops/sec)
After:
Did 77000 hash-to-curve P384_XMD:SHA-512_SSWU_RO_ operations in 4009708us (19203.4 ops/sec) [+0.4%]
Did 7327000 hash-to-scalar P384_XMD:SHA-512 operations in 4000477us (1831531.6 ops/sec) [+2.4%]
Bug: 1414562
Change-Id: Ic3c37061e325250d5d8723fd9aa263930c6023cf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57146
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
The const bool doesn't do anything. While I'm here, make the methods
const.
Change-Id: Id8c31d5fcda6d8bc244c64b02b1d758e4eff6849
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57185
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Remove all the other ERR_R_MALLOC_FAILURES from the
codebase.
Also changes cbb to push to the error stack, to correctly
report cbb failures instead of now only reporting
malloc failures. Previously it turned all cbb failures
into a malloc failure
Bug: 564
Change-Id: Ic13208bf9d9aaa470e83b2f15782fc94946bbc7b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57046
Auto-Submit: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Decoding from decimal takes quadratic time, and BN_dec2bn will happily
decode however large of input you pass in. This frustrates fuzzers.
I've added a cap to the input length in s2i_ASN1_INTEGER for now, rather
than BN_dec2bn, because we've seen people use BN for surprisingly large
calculator operations, and BN generally doesn't cap inputs to quadratic
(or worse) algorithms beyond memory limits. (We generally rely on
cryptography using fixed parameter sizes, though RSA, DSA, and DH were
misstandardized and need ad-hoc limits everywhere.)
Update-Note: The stringly-typed API for constructing X.509 extensions
now has (very generous) maximum input length for decimal integers of
8,192 digits. If anyone was relying on a higher input, this will break.
This is unlikely and should be caught by unit tests; if a project hits
this outside of tests, that means they are passing untrusted input into
this function, which is a security vulnerability in itself, and means
they especially need this change to avoid a DoS.
Bug: chromium:1415108
Change-Id: I138249d23ca6b1996f8437dba98633349bb3042b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57205
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
This will let us call ERR and thread_local from OPENSSL_malloc
without creating a circular dependency. We also make
ERR_get_error_line_data add ERR_FLAG_MALLOCED to the returned
flags value, since some projects appear to be making
assumptions about it being there.
Bug: 564
Update-Note: Any recent documentation (in all OpenSSL forks) for the ERR functions
cautions against freeing the returned ERR "data" strings, as freeing them is handled
by the error library. This change can make an existing double free bug more
obvious by being more likely to cause a crash with the double free.
Change-Id: Ie30bd3aee0b506473988b90675c48510969db31a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57045
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: Bob Beck <bbe@google.com>
This includes an internal version which allows a flag to specify
the use of system malloc, or OPENSSL_malloc - this in turn allows
us to use this function in the ERR family of functions and allow
for ERR to not call OPENSSL_malloc with a circular dependency.
Bug: 564
Change-Id: Ifd02d062fda9695cddbb0dbef2e1c1db0802a486
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57005
Auto-Submit: Bob Beck <bbe@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Especially when they were named "2214" instead of "chromium-2214", I've
seen papers and other projects treat them as releases. Add a note to
make it clear they are not releases.
Change-Id: Ie820b800de3d25a31d3083d4ceff75e1d7f74a06
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57145
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
The output of ASN1_generate_v3 is *mostly* linear with the input, except
SEQ and SET reference config sections. Sections can be referenced
multiple times, and so the structure grows exponentially.
Cap the total output size to mitigate this. As before, we don't consider
these functions safe to use with untrusted inputs, but unbounded growth
will frustrate fuzzing. This CL sets the limit to 64K, which should be
enough for anyone. (This is the size of a single X.509 extension,
whereas certificates themselves should not get that large.)
While not strictly necessary, this also rearranges the
ASN1_mbstring_copy call to pass in a maximum output. This portion does
scale linearly with the output, so it's fine, but the fuzzer discovered
an input with a 700K-byte input, which, with fuzzer instrumentation and
sanitizers, seems to be a bit slow. This change should help the fuzzer
get past those cases faster.
Update-Note: The stringly-typed API for constructing X.509 extensions
now has a maximum output size. If anyone was constructing an extension
larger than 64K, this will break. This is unlikely and should be caught
by unit tests; if a project hits this outside of tests, that means they
are passing untrusted input into this function, which is a security
vulnerability in itself, and means they especially need this change to
avoid a DoS.
Bug: oss-fuzz:55725
Change-Id: Ibb65854293f44bf48ed5855016ef7cd46d2fae77
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57125
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
This reverts commit 97873cd1a59b97ced00907e274afaff75edf4a57.
In some configurations, Clang (when building for debug) compiles
poly_mul_vec_aux with a huge (12KB) stack frame. It appears to be
expanding the code into a huge SSA graph and then assigning every value
in the graph to the stack. When optimising it can store the live values
in only a few hundreds bytes of stack, but since this function recurses,
the debug build can overflow the stack.
Unclear quite what to do about this, but this change solves the
immediate issue.
Change-Id: Ifa531f91f10bf04de92e8d07fbb14c668b366454
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57065
Auto-Submit: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Slowly reduce clutter in the top-level CMake file.
Change-Id: Ib7ca2aee7337db82ed1989c56bbaaf6ee5da0768
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56569
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We still keep time_t stuff around for calling time() and
for external interfaces that are meant to give you time_t
values, but we stop using time_t internally. For publicly
exposed and used inputs that rely on time_t, _posix versions are
added to support providing times as an int64_t, and internal
use is changed to use the _posix version.
Several legacy functions which are extensivly used and
and use pointers to time_t are retained for compatibility,
along with posix time versions of them which we use exclusively.
This fixes the tests which were disabled on 32 bit platorms
to always run.
Update-Note: This is a potentially breaking change for things
that bind to the ASN1_[UTC|GENERALIZED]TIME_set and ASN1_TIME_adj
family of functions (and can not type convert a time_t to an
int64).
Bug: 416
Change-Id: Ic4daba5a299d8f35191853742640750a1ecc53d6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54765
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
The "object reuse" mode in d2i_FOO is extremely problematic. See bug for
details. When we rewrite d2i_RSAPublicKey, etc., with CBS, we switched
dropped this fragile behavior and replaced it with freeing the old value
and replacing it with the new value. Extend this behavior to all functions
generated by crypto/asn1 templates too.
In particular, tasn_dec.c already frees the original object on error,
which means callers must already be prepared for OpenSSL to free their
reused object. The one caller I found that would be incompatible (via
both running tests and auditing callers) was actually broken due to this
error case, and has been fixed.
Update-Note: This slightly changes the calling convention of the d2i_FOO
functions. The change should be compatible with almost all valid calls.
If something goes wrong, it should hopefully be quite obvious. If
affected (or unaffected), prefer to set the output parameter to NULL
and use the return value instead.
Fixed: 550
Change-Id: Ie54cdf17f8f5a4d76fdbcddeaa27e6afd3fa9d8e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56647
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Our EVP_CIPHER_mode returns an unsigned value and including negative
numbers in switch/case when the value is unsigned causes some warnings.
This should avoid the need for https://github.com/nodejs/node/pull/46564
(Having them be positive shouldn't have compat impacts. CCM is 8, but no
cipher will report CCM, so any path checking for it will just be dead
code.)
Change-Id: I8dcf5ea55fad9732a09d6da73114cde5d69397d3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57025
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Instead, move the CRYPTO_BUFFER into ASN1_ENCODING (due to padding,
upgrading the two bitfields do a pointer doesn't increase memory usage),
and instead thread a CRYPTO_BUFFER parameter through tasn_dec.c.
Later, I want to reimplement the X509 and X509_CINF parsers with CBS/CBB
directly (https://crbug.com/boringssl/547), but that will be easier once
the whole crypto/asn1 machinery is rewritten with CBS/CBB
(https://crbug.com/boringssl/548). That, in turn, will be easier with
object reuse gone. But to get rid of object reuse, I need to remove the
one place in the library where we ourselves use it.
Bug: 550
Change-Id: Ia4df3da9280f808b124ac1f4ad58745dfe0f49e2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56646
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
We are using the CIPD copy of CMake on Windows now.
Change-Id: Idb9f62876c69333f9504540bad8321a173eaec3e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56988
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
BoringSSL never shipped the OCB-AES assembly, but took two different
strategies in disabling it for x86 versus x86_64. For x86, the
implementation was deleted, but for x86_64 it was wrapped in `if(0)`.
Since we're no longer as concerned about keeping the assembly from
diverging from upstream, be consistent in how the OCB-AES functions
are removed from both by deleting them from x86_64.
Change-Id: I5233134e3e131fed56f365ed6f43f30c39dd2e33
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56989
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The ws2_32 dependency comes from BIO, which is in libcrypto. Once it's
added there, it should get inherited by anything downstream, so we don't
need to keep listing it.
We also no longer need -lrt. We tried to remove it in
https://boringssl-review.googlesource.com/4622, but had to revert it in
https://boringssl-review.googlesource.com/4894 because of clock_gettime.
clock_gettime, per the Linux man page, is now in libc, not librt, as of
glibc 2.17. THat was released December 2012, well past our five year
watermark, so clean this part of the build up.
Change-Id: Ie6a07434b0cb02fe916b32ab8c326ec33d40bcb6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56606
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This function was involved in both CVE-2020-1971 and CVE-2023-0286. Both
times, we've had to confirm there were no external callers. Unexport it
so we can be sure of this.
Change-Id: I37b756f5bd66e389f03540872371001c85a0b5af
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56987
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
GENERAL_NAME uses a weird ASN1_SEQUENCE item type. Test that serializing
it works.
Change-Id: I8d44eb637f58a9fbe870b1998b0d75e2bfcde601
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56986
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
This fixes CVE-2023-0286.
The main impact is that GENERAL_NAME_cmp, when given x400Addresses, can
interpret a pointer with the wrong type. Applications that set
X509_V_FLAG_CRL_CHECK and take CRLs from untrusted sources should take
this patch.
Change-Id: Ib76265fa098df3cb0db075646773c14d59d0ca75
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56985
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Looks like this has since been fixed (or isn't hitting GTest anymore for
some reason).
Bug: chromium:772117
Change-Id: I2c2fb694e4429281e20fd252ef9c2c34e29a425c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56570
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
