Although this is only used by <openssl/pem.h>, one existing caller
expects the free functions to be defined in <openssl/x509.h>. It's not
really worth it to put it in the other header, so just move it back.
Change-Id: I7e719d51110b567296fcd797f72d13aa41de73af
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64287
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
The only user of the tiny utility class to clear the error stack
was verify_certificate_chain, which can have it in it's anonymous
namespace.
It was however, used to great effect as a horrible dirty cheat to get
us openssl/base.h during the conversion. That time is past, so just
let us include <openssl/base.h> normally, and get rid of this.
Bug: 668
Change-Id: Ie8537d5b1d6789acb35182f3d781f7505ad79109
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64188
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: Bob Beck <bbe@google.com>
x509.h isn't ready for doc.go yet, but fix a few mistakes caught by
previewing it.
Bug: 426
Change-Id: I79630cc1cbe5737cea96143b54c2fa42882077a0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64140
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Some things of note:
- Anyone calling X509_OBJECT_up_ref_count is breaking X509_OBJECT's
internal invariants, or relying on someone else handing back an
X509_OBJECT with broken invariants.
- X509_LOOKUP_by_subject hands back an X509_OBJECT with broken internal
invariants. Fortunately, it is never called, so unexport it as a the
first step to cleaning this up.
Change-Id: Ia67693f802671cf857bf51aec6e20f27d1525212
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64130
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This is a bit of a mess. The immediate motivation here is that there is
no legitimate reason to ever call X509_OBJECT_free_contents outside of
the library. Unsurprisingly, this means rust-openssl uses it.
rust-openssl uses it because they want to be able to free X509_OBJECTs.
Add OpenSSL 1.1.x's X509_OBJECT_free, which is what they should be using
it instead. As it turns out, they don't *actually* need to free
X509_OBJECTs. This is just some design mistake that cause them to need
free functions for types they never free. On top of that, the only
reason rust-openssl references X509_OBJECT is for
X509_STORE_get0_objects, but their use of that API is a Rust safety
violation anyway. It's all a mess.
As for whether freeing it ever makes sense, the question is whether
X509_STORE_get_by_subject needs to be a public API. In so far as it is
public, callers would need to create empty X509_OBJECTs as an output,
now that X509_OBJECT is opaque. There are also other users of
X509_STORE_get0_objects that might benefit from an
X509_STORE_get1_objects, in which case X509_OBJECT_free will be useful.
For now just to unblock fixing the more immediate rust-openssl mistake
(rather than the underlying mistake), add the APIs that
X509_STORE_get_by_subject callers would need if they existed.
There's quite a bit to clean up around X509_OBJECT, but start by adding
these APIs.
As part of this, since rust-openssl prevents us from removing
X509_OBJECT_free_contents, deprecate it and fix it to leave the
X509_OBJECT in a self-consistent state. (This is moot because
rust-openssl will never call it, but still.)
Change-Id: I78708f2d2464eb9a18844fef0d62cb0a727b9f47
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64129
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This wasn't possible when X509_STORE_CTX was stack-allocated because
X509_STORE_CTX_init needed to account for an uninitialized struct. But
now it is always initialized, so we can avoid this footgun. This also
matches what OpenSSL does nowadays.
Change-Id: I266be58204b8cd374fa4896c1c66a35ffaa762ea
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64141
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
It ceased being a service long ago. The relevant piece to
get the test data is moved to the anonymous namespace in
test_helpers which it the only remaining user.
Bug: 668
Change-Id: I23d8a7166ed61e83f12a7e82473beec316c56d86
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64169
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Move these functions into the library bssl::string_util namespace
Bug: 668
Change-Id: I1665176217fb25164cb321a4092391bf60592a88
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64187
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: Bob Beck <bbe@google.com>
Move the file reader into the anonymous namespace in test_helpers
which is the only user.
Bug: 668
Change-Id: Idd650d14fb7f9e0b7b15a7fd08e21f9a7081cc14
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64168
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: Bob Beck <bbe@google.com>
We move the last piece of used stuff here into the library
string_utils.
Bug: 668
Change-Id: Idde14a497204a3b40a602ed6c03a5859eee80811
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64167
Auto-Submit: Bob Beck <bbe@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
This was never used externally. It's a remnant of when we supported
stack-allocated X509_STOREs, but now its opaque.
Change-Id: Idb997237ca81f4c35795cfc8c9d2ee222629e1ce
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64128
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Bug: 659
Change-Id: I48eeda0bcd0de45d70644c321138225f83cb6c60
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64107
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: Bob Beck <bbe@google.com>
Before making further changes. This pass is without
InsertBraces, will follow up with InsertBraces
for separate review
Bug: 659
Change-Id: Ie311dcd932aec5f98c16573f9326b1918a639adf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64067
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: Bob Beck <bbe@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
X509_INFO only exists to be a return value to PEM_X509_INFO_read. There
is no use in letting callers create these objects, since they cannot do
anything with it. Only X509_INFO_free is needed.
Also cut a ton of unused fields from X509_PKEY.
Change-Id: I322589f04883903e1fe5c23c3966ecf631e85b7f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64127
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
These probably shouldn't be public API, but ah well.
Bug: 426
Change-Id: I4c5a81c70d3b2d5866ef494ac2a6710a662103c8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63947
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The only ASN1_ITEMs from x509.h that are referenced externally are
X509_NAME, X509_ALGOR, and RSA_PSS_PARAMS. (RSA_PSS_PARAMS only by way
of an ASN1_item_pack call.) The others don't actually need ASN1_ITEMs.
This will cut down on the number of compatibility schemes we need when
the parsers are rewritten.
Also remove (d2i|i2d)_X509_NAME_ENTRY. It seems people only need to
create and destroy them, not serialize them individually.
It also means we can actually remove X509's ASN1_ITEM, as it's already
unused inside the library. I've replaced EmbedX509 with tests for the
types that are actually embedded. Those have not yet been rewritten,
but the tests are now ready for when they are.
Update-Note: Fewer types can be parsed generically through the ASN1_ITEM
system now. If someone was relying on a removed ASN1_ITEM, it will
appear as a compile error and we can restore it.
Bug: 443
Change-Id: Ib2a75d40c2e93dcf1c0474cf0df4a96190aac9a4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63946
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Now that this is the source of truth, this isn't really doing anything.
Update-Note: _BORINGSSL_LIBPKI_ in build files can be removed.
Bug: 658
Change-Id: I6daacf692bf4bf51d9822d1b91237625b83d7849
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64027
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
crl_akid_check now always, on success, leaves CRL_SCORE_SAME_PATH in the
score. (Note CRL_SCORE_ISSUER_CERT contains CRL_SCORE_SAME_PATH.) This
means the recursive validation logic in check_crl_path never runs, and
we can remove a very worrying re-entrant call in the validator. The CRL
must be issued by either the issuer, or some ancestor in the chain.
(This also matches the behavior of the new validator.)
Bug: 601
Change-Id: Ie5c0feb5bb5ade3bfd49e338a637196fce29fd2a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63942
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This is part of indirect CRLs, though it wasn't correctly conditioned on
them. All CRL entries are now expected to relate to the CRL issuer. This
removes the very complicated delta encoding this extension uses. (This
extension applies to not just the current CRL entry, but all subsequent
CRL entries that don't override it. This also means sorting a CRL is
destructive.)
Bug: 601
Change-Id: I388baee64719007648fb3b5defea82a4661735d2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63941
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
ASN1_BOOLEAN is int, so this is a no-op. But they're parsed as
ASN1_FBOOLEAN (boolean with default false), so we should use the
typedef.
Also leave some TODOs for future cleanup opportunities.
Change-Id: I7b060e7530f0ef1afb3818c043727feb6fd4fcbf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63940
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Avoid polluting the global namespace a bit. The idp_flags field itself
is private, so it is impossible for a caller to do anything useful with
it.
Change-Id: I5853e72a3f37a93c75fb73ed5d7ca467b9ba01a3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63939
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We'll never accept CRLs where the issuers don't match. That means
CRL_SCORE_ISSUER_NAME is always set, so we can remove code that
conditions on it.
Update-Note: This also makes a corresponding distribution point change
to ignore distribution points with a CRLissuer field. Before, we would
check for it to match the CRL issuer, but this field is only meant to be
used with indirect CRLs (RFC 5280, section 6.3.3, step b.1). The old
code didn't include this, so I think it isn't *quite* a no-op on some
invalid DP/CRL pairs, but it matches the new verifier from Chromium.
Bug: 601
Change-Id: Ibe409b88cae1c2b78b3924e61270884d6e0eb436
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63938
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We never set CRL_SCORE_TIME_DELTA.
Bug: 601
Change-Id: Ic7497492565bda9f3e9d7091f5e16b81e111cfa1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63937
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Now that we only process CRLs that cover all reasons:
1. A successful get_crl will always set current_reasons to
CRLDP_ALL_REASONS.
2. The last_reasons == current_reasons will never happen.
3. The loop always makes exactly one iteration.
A footnote on point 1: it is also possible for the caller to override
get_crl. In that case, the caller's get_crl was previously responsible
for setting current_reasons, but there was no way to do so. In reality,
that callback was actually impossible to use correctly. See
https://github.com/openssl/openssl/issues/21679 and
https://github.com/openssl/openssl/issues/10211.
I previously attempted to remove those first, but gRPC did not notice it
was unusable and use it anyway. Instead, they're suppressing
X509_V_ERR_UNABLE_TO_GET_CRL via the callback, which is probably working
around the bug in their get_crl implementation.
Later, when we tackle the callback, we'll probably need to unwind the
gRPC mess and, in the process, add a X509_STORE_CTX_set_current_reasons
for them to call for OpenSSL compatibility. For now, this change has the
side effect of removing the need for them to call that.
Bug: 601
Change-Id: Icc5c0fb195d9f66991d0e560911f304e82afa5fd
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63936
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This isn't *quite* a no-op, but it's the other half of removing support
for partitioned CRLs. The distribution point's reasons field and the
CRL's onlySomeReasons field are masked together to determine which
reasons we have covered so far. This is used in some complex logic from
RFC 5280, section 6.3.3 to loop through a bunch of CRLs before
determining that we've covered evertything.
OpenSSL's "extended CRL" feature skipped all CRLs with an
onlySomeReasons field, but did not condition on the distribution point,
so the loop was still active in some cases.
The new verifier from Chromium doesn't support either. If the
distribution point has a reasons field, we ignore it. Align with
Chromium. Now the reasons field is always the special all-reasons value.
As part of this, this removed the dp_reasons field from DIST_POINT. This
is a public struct, but was unused outside of cl/581761514, so we can
stop computing it.
Update-Note: See above.
Bug: 601
Change-Id: I9b0a9766b281d3486874e1b6d4d415a51e50ba59
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63935
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
This is part of onlySomeReasons handling, which was removed with
extended CRL support. Demonstrating that the code to be removed is a
no-op takes a few steps, so I'm splitting this into a couple CLs for
ease of review.
First, every CRL either has the IDP_REASONS set, or idp_reasons is
CRLDP_ALL_REASONS. The only thing that reads idp_reasons is
crl_crldp_check, which is run after get_crl_score skips all IDP_REASONS
certificates. Thus, the field can be assumed to be CRLDP_ALL_REASONS.
Bug: 601
Change-Id: I4d41b5665d35db10dd9752e47553ae90f46f1e44
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63934
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
This is part of delta CRLs, where OpenSSL would return 2 instead of 1 to
signal a removedFromCRL entry. The logic that caught that was removed in
the preceeding CL.
Bug: 601
Change-Id: I5ac0b32a864b09d303198ca99ecbfc1219906e36
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63933
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This is now always NULL.
Bug: 601
Change-Id: I030b41fd65973ca13b96c426962430ea8a6ae198
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63932
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
These extensions were only parsed for delta CRL support. Instead, ignore
CRL numbers and treat critical delta CRL extensions as unrecognized
critical extensions. This trims a pile of dead, untested code from the
verifier.
Update-Note: While this is broadly a no-op, this may change behavior
slightly at the edges. Invalid CRL number extensions will now be ignored
instead of treated as a parse error. A delta CRL that incorrectly marks
its delta CRL extension as non-critical will be interpreted as a normal
CRL. (This is the expected behavior for an implementation which does not
implement delta CRLs. Extensions like this are supposed to be marked
critical.)
Bug: 601
Change-Id: Iece9a8acce83a7c59e129499517fc8eb0d048e98
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63931
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Update-Note: Though exported, this was an internal flag to the delta CRL
implementation. Remove it.
Bug: 601
Change-Id: Ic7f99da94391aea861fd7ea9ad79a3fb66cc649e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63930
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Update-Note: The X509_V_FLAG_EXTENDED_CRL_SUPPORT and
X509_V_FLAG_USE_DELTAS flags now cause verification to fail. They
weren't enabled by any caller.
This broadly is meant to disable:
- Delta CRLs
- Indirect CRLs (When the CRL's issuer is somehow different from the
certificate. The security properties for this is very interesting,
since it refers to just any other random name under the same trust
anchor. Very clearly a remnant of when X.509 was meant to authenticate
a global directory. See the rather worrisome comment over
check_crl_chain.)
- Merging together multiple CRLs that are partitioned by reasons
There's some other code we can now unwind, which will be handled in
follow-up changes. This CL is meant to be a minimal change to disable
them. Though even this minimal change requires we delete a bunch of
functions.
Bug: 601
Change-Id: I319ab793f480c6b99de86da6077b616f18edf06b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63929
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We've been effectively relying on Clang (on x86_64) to do it for us. But
other compiler/arch platforms don't unroll it as readily. I originally
did this to play with the 32-bit bit interleaving trick, which requires
this, but actually it's a significant win on its own.
Clang (NDK), aarch32, Pixel 5A
Before:
Did 4836 Kyber generate + decap operations in 2036618us (2374.5 ops/sec)
Did 6237 Kyber parse + encap operations in 2055784us (3033.9 ops/sec)
After:
Did 8610 Kyber generate + decap operations in 2051048us (4197.9 ops/sec) [+76.8%]
Did 12138 Kyber parse + encap operations in 2042363us (5943.1 ops/sec) [+95.9%]
Clang (NDK), aarch64, Pixel 5A
Before:
Did 16720 Kyber generate + decap operations in 2011039us (8314.1 ops/sec)
Did 30000 Kyber parse + encap operations in 2023170us (14828.2 ops/sec)
AFter:
Did 17080 Kyber generate + decap operations in 2005310us (8517.4 ops/sec) [+2.4%]
Did 31000 Kyber parse + encap operations in 2059104us (15055.1 ops/sec) [+1.5%]
GCC, x86_64
Before:
Did 14535 Kyber generate + decap operations in 2015051us (7213.2 ops/sec)
Did 21000 Kyber parse + encap operations in 2017842us (10407.2 ops/sec)
After:
Did 19900 Kyber generate + decap operations in 2016747us (9867.4 ops/sec) [+36.8%]
Did 34000 Kyber parse + encap operations in 2059643us (16507.7 ops/sec) [+58.6%]
Clang, x86_64
Before:
Did 19584 Kyber generate + decap operations in 2006839us (9758.6 ops/sec)
Did 34000 Kyber parse + encap operations in 2050513us (16581.2 ops/sec)
After:
Did 19928 Kyber generate + decap operations in 2020249us (9864.1 ops/sec) [+1.1%]
Did 34000 Kyber parse + encap operations in 2052970us (16561.4 ops/sec) [-0.1%]
Change-Id: Iee9315667c1d2044785faa9370815a3c7555c259
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63992
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
When squeezing a multiple of the rate bytes (e.g. in the Kyber XOF), we
were running the Keccak permutation one more time than necessary.
Before:
Did 18900 Kyber generate + decap operations in 2001506us (9442.9 ops/sec)
Did 32000 Kyber parse + encap operations in 2041500us (15674.7 ops/sec)
After:
Did 19796 Kyber generate + decap operations in 2017501us (9812.1 ops/sec) [+3.9%]
Did 34000 Kyber parse + encap operations in 2032085us (16731.6 ops/sec) [+6.7%]
Change-Id: I69787536508c4eadcc37a2f752c3678c60906c38
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64007
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Update-Note: Removed an unused function. This has no callers and is only
useful to create delta CRLs, which are similarly unused and being
removed.
Bug: 601
Change-Id: I22abf36e723d19b9759bcabf28fddf7f2ffe7379
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63928
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Bug: 426
Change-Id: I82820de3048af0d9280d37b89ebf98cb07c746d8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63927
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Also move a few functions into the correct sections.
Bug: 426
Change-Id: I81c4e65bd7f248251a2a85b9934abe500798532a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63926
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
This'll probably need another pass once we figure out what to do with
X509_TRUST, but put it with the other aux functions.
Bug: 426
Change-Id: I6ae2e45b94bace40307dd4dcc1c8702fc8baa8eb
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63925
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Anonymous unions are now standard in C, as of C11, but not in C++.
Enabling sufficiently strict warnings in GCC and Clang flag this.
I considered whether we should just remove this and go back to the
OpenSSL formulation, but we actually rely on this being an array when
calling sha1_block_data_order. Upstream types these as taking pointers
to the context, which would work, but taking a pointer to the state is a
bit more accurate. (The assembly function should not touch the buffering
inside the context.)
This anonymous union does mean wpa_supplicant's behavior is slightly
questionable from a strict aliasing perspective, but ah well.
wpa_supplicant uses this to implement an old FIPS 186-2 PRF, which was
based on SHA-1's underlying permutation. Ideally we would either
implement this PRF for them, or have them use their own SHA-1
implementation. They've actually done the latter for OpenSSL 3.0, but
it's a little silly to duplicate the code.
strongswan and go/another-fips-186-2-prf seems to do this too. I'm not
positive what strongswan is doing. I've filed crbug.com/boringssl/667
for follow-up work.
Bug: 667
Fixed: 566
Change-Id: Ife32cc8c278e0dbbd95401ccdd3bd62945e10cf2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63967
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
OPENSSL_memcpy already internally checks for empty lengths.
Change-Id: I0015758fd5410e036b532ae727341ae0c0edbdbf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63826
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The AES-GCM EVP_CIPHER has a bunch of machinery for internally managing
a fixed/counter IV split. OpenSSH uses these APIs, so test it.
(OpenSSH barely uses this. It only ever passes -1 as a length to
EVP_CTRL_AEAD_SET_IV_FIXED, and then uses EVP_CTRL_GCM_IV_GEN to
internally increment a counter. But may as well just test the whole
thing, at least until we decide to remove it.)
Change-Id: I688616c586208d27a431836c81d3412799d830bd
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63825
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
The actual maximum allowed for this function is INT_MAX - 1. However,
elsewhere we bound parsers to INT_MAX / 2, to guard against overflows
elsewhere in crypto/asn1.
As parsing code is rewritten with CBS, it will naturally handle size_t.
Rather than manually pepper bounds checks in all the functions, we can
catch it at ASN1_STRING_set time. (Whether we'll still need this hack by
the end, I'm not sure, but let's keep it for now.)
Bug: 547
Change-Id: Ia24c9d77d198ba1b5200825a347ca91850f470a2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63526
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
David Benjamin
Bob Beck