Checking for an error on the error queue is very tedious. You have to
check both library and reason code separately, which often means you
need to save the error code to a local variable.
Make an ERR_equals function. I've gone ahead and just made it public API
because even within BoringSSL, it seems to come up everywhere.
Change-Id: Ia963c9ae743ac5c6939846f8f64bbc98b495ce0b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81627
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
This implements just the small subset of OpenSSL's CMS API to support
the Linux kernel's sign-file.c tool. It is nowhere close to a full CMS
implementation and is not intended to become one. In particular, it does
not implement enough of CMS to support S/MIME. That requires much, much
more infrastructure than was implemented here.
CMS is, like PKCS#7, an over-engineered and cryptographically unsound
set of nestable combinators to support just about any configuration of
cryptographic operations. Profiling CMS down to a usable subset is, as a
result, more complicated, more risky, and less efficient than just
designing a bespoke structure for your use case. It is derived from
PKCS#7, and largely overlaps. However, both PKCS#7 and CMS use the v1
version number, but CMS made incompatible changes in some corner cases
that, so far, do not matter to us. (It is incompatible if you try to
layer SignedData atop another combinator, where the lack of proper
domain separation in this badly designed format is of extra risk.)
In the case of the kernel, sign-file.c wants an "external signature",
which is when the data to be signed lives elsewhere. This is, as a
result, a very, very inefficient way to concatenate an enum with a byte
string. But this is what the kernel chose, so here we are.
Because PKCS#7 and CMS are broadly the same structure, I've generalized
the internal PKCS#7 function rather than duplicating all this code. If
we ever hit the cases where PKCS#7 and CMS v1 are incompatible, plumbing
an extra boolean will be the least of our worries.
Test data was generated by compiling the actual sign-file.c against
OpenSSL and saving the output.
Change-Id: Idb0874d2b5294bfad564f3a00458c3fd044d9da5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/78452
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This reflects some assumptions we have on our headers:
- <openssl/foo.h> pulls in <openssl/base.h>. In particular, the
canonical FOO typedefs for foo_st all live forward declared in
<opessl/base.h>, but including <openssl/foo.h> is sufficient to use
FOO.
- <openssl/base.h> pulls in <stdint.h> and <stddef.h> so we don't have
to keep including it.
Add IWYU exports to reflect this so that clang's include cleaner gets
less upset. It's a bit of a blunt instrument because it also means that
<openssl/foo.h> lets you use the forward-declared BAR typedef, and
downstream projects might still prefer to explicit include <stdint.h>
when they use it (we use it so much that it would be too much), but I
think this is fine.
NB: By adding these, we're essentially promising we won't include those
transitive includes because downstream code might accidentally start
relying on it.
Change-Id: I705fe6d1026fbd302ed0f070a0cf7658a70af8ef
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/77687
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We use the standard Apache 2.0 file header, described in "APPENDIX: How
to apply the Apache License to your work."
This was primarily automated by running:
git ls-tree -r --name-only HEAD | xargs go run ./util/relicense.go
See go/boringssl-relicensing-triage for the results of triaging the
output of the tool.
As part of this, switch from taking fiat-crypto under MIT license to
Apache 2.0. (It is licensed under MIT OR Apache-2.0 OR BSD-1-Clause.)
The copyright_summary tool can also be used to confirm we didn't
accidentally drop any copyright lines:
# Run before the CL
git grep -l Copyright | xargs go run ./util/copyright_summary.go -out /tmp/old.json
# Run after the CL
git grep -l Copyright | xargs go run ./util/copyright_summary.go -compare /tmp/old.json
Bug: 364634028
Change-Id: I17c50e761e9d077a1f92e25969e50ed35e320c59
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/75852
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CPython needs this operation. See
https://github.com/openssl/openssl/issues/19848 and
https://discuss.python.org/t/error-tables-in-the-ssl-module/25431 for
details.
In principle, our functions already return the symbol names. The
differences are:
- Our library strings say "common libcrypto routines" instead of
"CRYPTO".
- The global reason codes say "internal error" instead of
"INTERNAL_ERROR". (We should consider changing this.)
- The library forwarding reason codes (ERR_R_BN_LIB) say the library
string instead of "BN_LIB". (We should consider changing this.)
- errnos report strerror
- Unknown errors return "unknown error" because we've found that
projects tend to crash when these APIs return NULL.
The new APIs consistently return the symbol name, when available. If
unavailable (ERR_LIB_SYS's errno reasons), it returns NULL because I
assume callers would rather be able to handle that case themselves.
Hopefully this will not be as common so callers can take on this one.
Change-Id: Idd9e4b1cb5a4f64513310d8066d6bf3970722c23
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66807
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: 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>
Inline functions have type signatures, unlike macros. This seems to be
compatible with existing callers and matches OpenSSL 3.0. Additionally,
while Rust bindgen cannot deal with either inline functions or macros
right now, it seems a future version will fix the former.
Change-Id: I6966ff55910cf70e23117fe5f70a0bd286e26d56
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56405
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
rust-openssl, rather than using Rust's existing error types, exposes the
OpenSSL error queue as the error type in its public callback types.
Supporting a simplified version of ERR_set_error_data is simple enough,
so it's easiest just to add this function.
Unlike OpenSSL's, we don't attempt to support non-string error data. We
also don't try to retain borrowed pointers. If the caller did not pass
ownership, make a copy internally.
Change-Id: I909eebc2867ab1f3b9975546a106ee1f762bf516
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50625
Reviewed-by: Adam Langley <agl@google.com>
Trust Token is a new protocol to issue and redeem provably anonymized
tokens. To allow for development of key storage/management for Trust
Token, this CL provides the function to generate Trust Token keys.
Bug: chromium:1014199
Change-Id: If764e027b202f07be13c64f7be66dfaff71c45e7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/40064
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
ERR_error_string_n needs to be called in a separate statement, compared
to ERR_error_string(err, NULL), which returns a buffer and is very
convenient to use in an expression. This is unfortunate because it is
not thread-safe.
Give ERR_error_string_n a return value to align. Fixing callers still
requires allocating a buffer somewhere, but the rest of the expression
can remain relatively unperturbed.
Change-Id: I273c9df97f0bb113cdc57cf3896c42195910c67a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/38964
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Originally, the only OpenSSL API to stringify errors was:
char *ERR_error_string(unsigned long e, char *buf);
This API leaves callers a choice to either be thread unsafe (buf = NULL)
or pass in a buffer with unknown size. Indeed the original
implementation was just a bunch of unchecked sprintfs with, in the buf =
NULL case, a static 256-byte buffer.
https://github.com/openssl/openssl/blob/388f2f56f213dfada0370d48cb9bcc3c7e980b32/crypto/err/err.c#L374
Then ERR_error_string was documented that the buffer must be size 120.
Nowhere in the code was 120 significant. I expect OpenSSL just made up a
number.
https://github.com/openssl/openssl/commit/388f2f56f213dfada0370d48cb9bcc3c7e980b32
Then upstream added the ERR_error_string_n API. Although the
documentation stated 120 bytes, the internal buffer was 256, so the code
actually translates ERR_error_string to ERR_error_string_n(e, buf, 256),
not ERR_error_string_n(e, buf, 120)!
https://github.com/openssl/openssl/commit/e5c84d5152c11a3dfa436041d3336a6f403baad8
So the documentation was wrong all this time! OpenSSL 1.1.0 corrected
the documentation to 256, but, alas, a lot of code used the
documentation and sized the buffer at 120. We should fix all
ERR_error_string callers to ERR_error_string_n but, in the meantime,
using 120 is probably less effort.
Note this also affects ERR_print_errors_cb right now. We don't have
function codes, so 120 bytes leaves 60 bytes for the reason code. Our
longest one, TLS_PEER_DID_NOT_RESPOND_WITH_CERTIFICATE_LIST is 46 bytes,
so it's a little tight, but, if needed, we can recover 20-ish bytes by
shrinking the library names. We can also always make ERR_print_errors_cb
use a larger buffer.
Change-Id: I472a1a802f2e6281cc7515d2a452208d6bac1200
Reviewed-on: https://boringssl-review.googlesource.com/24184
Reviewed-by: Adam Langley <agl@google.com>
This will be useful for the SSL stack to properly resurface handshake
failures. Leave this in a private header and, along the way, hide the
various types.
(ERR_NUM_ERRORS didn't change in meaning. The old documentation was
wrong.)
Bug: 206
Change-Id: I4c6ca98d162d11ad5e17e4baf439a18fbe371018
Reviewed-on: https://boringssl-review.googlesource.com/21284
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Also fully deprecate ERR_error_string. Even when passing an external
buffer, passing the length explicitly is better.
Change-Id: Id2eb5723410f4564ef5e27c54ba79672133368e7
Reviewed-on: https://boringssl-review.googlesource.com/15424
Reviewed-by: Adam Langley <agl@google.com>
Macros need a healthy dose of parentheses to avoid expression-level
misparses. Most of this comes from the clang-tidy CL here:
https://android-review.googlesource.com/c/235696/
Also switch most of the macros to use do { ... } while (0) to avoid all
the excessive comma operators and statement-level misparses.
Change-Id: I4c2ee51e347d2aa8c74a2d82de63838b03bbb0f9
Reviewed-on: https://boringssl-review.googlesource.com/11660
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
In code, structs that happened to have a '(' somewhere in their body
would cause the parser to go wrong. This change fixes that and updates
the comments on a number of structs.
Change-Id: Ia76ead266615a3d5875b64a0857a0177fec2bd00
Reviewed-on: https://boringssl-review.googlesource.com/6970
Reviewed-by: Adam Langley <agl@google.com>
Besides being a good idea anyway, this avoids clang warning about using
a non-literal format string when |ERR_add_error_dataf| calls
|BIO_vsnprintf|.
Change-Id: Iebc84d9c9d85e08e93010267d473387b661717a5
Reviewed-on: https://boringssl-review.googlesource.com/6920
Reviewed-by: David Benjamin <davidben@google.com>
This change fixes up several comments (many of which were spotted by
Kenny Root) and also changes doc.go to detect cases where comments don't
start with the correct word. (This is a common error.)
Since we have docs builders now, these errors will be found
automatically in the future.
Change-Id: I58c6dd4266bf3bd4ec748763c8762b1a67ae5ab3
Reviewed-on: https://boringssl-review.googlesource.com/6440
Reviewed-by: Adam Langley <agl@google.com>
Removing the function codes continued to sample __func__ for compatibility with
ERR_print_errors_cb, but not ERR_error_string_n. We can just emit
OPENSSL_internal for both. ERR_print_errors_cb already has the file and line
number available which is strictly more information than the function name.
(ERR_error_string_n does not, but we'd already turned that to
OPENSSL_internal.)
This shaves 100kb from a release build of the bssl tool.
In doing so, put an unused function code parameter back into ERR_put_error to
align with OpenSSL. We don't need to pass an additional string in anymore, so
OpenSSL compatibility with anything which uses ERR_LIB_USER or
ERR_get_next_error_library costs nothing. (Not that we need it.)
Change-Id: If6af34628319ade4145190b6f30a0d820e00b20d
Reviewed-on: https://boringssl-review.googlesource.com/6387
Reviewed-by: Adam Langley <agl@google.com>
Consumers sometimes use ERR_LIB_USER + <favorite number> instead of
ERR_get_next_error_library. To avoid causing them grief, keep ERR_LIB_USER
last.
Change-Id: Id19ae7836c41d5b156044bd20d417daf643bdda2
Reviewed-on: https://boringssl-review.googlesource.com/5290
Reviewed-by: Adam Langley <agl@google.com>
Running make_errors.go every time a function is renamed is incredibly
tedious. Plus we keep getting them wrong.
Instead, sample __func__ (__FUNCTION__ in MSVC) in the OPENSSL_PUT_ERROR macro
and store it alongside file and line number. This doesn't change the format of
ERR_print_errors, however ERR_error_string_n now uses the placeholder
"OPENSSL_internal" rather than an actual function name since that only takes
the uint32_t packed error code as input.
This updates err scripts to not emit the function string table. The
OPENSSL_PUT_ERROR invocations, for now, still include the extra
parameter. That will be removed in a follow-up.
BUG=468039
Change-Id: Iaa2ef56991fb58892fa8a1283b3b8b995fbb308d
Reviewed-on: https://boringssl-review.googlesource.com/5275
Reviewed-by: Adam Langley <agl@google.com>
Some of the documentation had the right explanation but the incorrect
function names attached.
Change-Id: I7b479dae6d71a5ac7bc86df5a3890508c3b3d09f
Reviewed-on: https://boringssl-review.googlesource.com/5090
Reviewed-by: Adam Langley <agl@google.com>
Beyond generally eliminating unnecessary includes, eliminate as many
includes of headers that declare/define particularly error-prone
functionality like strlen, malloc, and free. crypto/err/internal.h was
added to remove the dependency on openssl/thread.h from the public
openssl/err.h header. The include of <stdlib.h> in openssl/mem.h was
retained since it defines OPENSSL_malloc and friends as macros around
the stdlib.h functions. The public x509.h, x509v3.h, and ssl.h headers
were not changed in order to minimize breakage of source compatibility
with external code.
Change-Id: I0d264b73ad0a720587774430b2ab8f8275960329
Reviewed-on: https://boringssl-review.googlesource.com/4220
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>
Since ERR will soon have thread-local storage, we don't need to worry
about high-performance implementations and thus don't need to be able to
switch two different implementations at run-time.
Change-Id: I0598054ee8a8b499ac686ea635a96f5d03c754e0
Previously, error strings were kept in arrays for each subdirectory and
err.c would iterate over them all and insert them at init time to a hash
table.
This means that, even if you have a shared library and lots of processes
using that, each process has ~30KB of private memory from building that
hash table.
This this change, all the error strings are built into a sorted list and
are thus static data. This means that processes can share the error
information and it actually saves binary space because of all the
pointer overhead in the old scheme. Also it saves the time taken
building the hash table at startup.
This removes support for externally-supplied error string data.
Change-Id: Ifca04f335c673a048e1a3e76ff2b69c7264635be
As feared, 2bca0988 did cause some leak checkers to get upset about the
state_hash pointer getting cleared.
This change makes err_shutdown free all the error queues to try and
avoid this. Hopefully this doesn't upset TSAN in turn.
BUG=448296
Change-Id: I827da63c793dcabc73168ece052cdcd3d3cc64e3
Reviewed-on: https://boringssl-review.googlesource.com/2890
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Add some missing headers and ensure each header has a short description. doc.go
gets confused at declarations that break before the first (, so avoid doing
that. Also skip a/an/deprecated: in markupFirstWord and process pipe words in
the table of contents.
Change-Id: Ia08ec5ae8e496dd617e377e154eeea74f4abf435
Reviewed-on: https://boringssl-review.googlesource.com/2839
Reviewed-by: Adam Langley <agl@google.com>
The extra free in ex_data_impl.c is fixing a mistake: when calling
|CRYPTO_cleanup_all_ex_data| the |EX_CLASS_ITEM| itself wouldn't be
freed.
The change in err_impl.c is to free the thread-id hash also. This allows
programs to free absolutely all memory allocated by BoringSSL, which
allows fuzz testing to find any memory leaks.
Change-Id: I1e518adf2b3e0efa7d7f00f7ab4e65e1dc70161e
Reviewed-on: https://boringssl-review.googlesource.com/2670
Reviewed-by: Adam Langley <agl@google.com>
Some parts of Android can't be updated yet so this change adds
declarations (only) for some functions that will be stubbed in
Android-specific code. (That Android-specific code will live in the
Android repo, not the BoringSSL repo.)
Trying to use these functions outside of Android will result in a link
error.
Change-Id: Iaa9b956e6408d21cd8fc34d90d9c15657e429877
Reviewed-on: https://boringssl-review.googlesource.com/2760
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
I misunderstood the OpenSSL semantics here. When receiving an error data
pointer via ERR_get_error_line_data and friends, although the error is
cleared, OpenSSL retains ownership of the data pointer. It's kept in the
cleared error until another error overrides it, or the whole error queue
is cleared.
It's pretty odd to have live pointers in empty errors so this change
allows an error queue to retain one data pointer. Thus the pointer
returned from ERR_get_error_line_data is valid until the next call to
ERR_get_error_line_data, or until the queue is freed.
From reviewing uses of the API, this is sufficient for all of them.
Change-Id: I73cb8e9c792452ae3c1a934ac8bbe8b5353b65b2
Reviewed-on: https://boringssl-review.googlesource.com/1880
Reviewed-by: Adam Langley <agl@google.com>
The corresponding constants are ints, so these should match. This
appeases MSVC on some Chromium DCHECK.
Change-Id: I7a5db41fa072c2850841a102917163af5e90d860
Reviewed-on: https://boringssl-review.googlesource.com/1867
Reviewed-by: Adam Langley <agl@google.com>
David Benjamin