| commit | 866cccc5484c94a51f01132677ad2c7f72a9f077 | [log] [tgz] |
|---|---|---|
| author | David Benjamin <davidben@google.com> | Tue Aug 10 01:35:51 2021 -0400 |
| committer | Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> | Thu Sep 09 20:08:14 2021 +0000 |
| tree | 243f85ef034d3b69ebed6d6f3a14205e72c2d357 | |
| parent | c9b75aff28a9dac52bc92987f7c4288fbf17dc33 [diff] |
Reject missing required fields in i2d functions. See also 006906cddda37e24a66443199444ef4476697477 from OpenSSL, though this CL uses a different strategy from upstream. Upstream makes ASN1_item_ex_i2d continue to allow optionals and checks afterwards at every non-optional call site. This CL pushes down an optional parameter and says functions cannot omit items unless explicitly allowed. I think this is a better default, though it is a larger change. Fields are only optional when they come from an ASN1_TEMPLATE with the OPTIONAL flag. Upstream's strategy misses top-level calls. This CL additionally adds checks for optional ASN1_TEMPLATEs in contexts where it doesn't make sense. Only fields of SEQUENCEs and SETs may be OPTIONAL, but the ASN1_ITEM/ASN1_TEMPLATE split doesn't quite match ASN.1 itself. ASN1_TEMPLATE is additionally responsible for explicit/implicit tagging, and SEQUENCE/SET OF. That means CHOICE arms and the occasional top-level type (ASN1_ITEM_TEMPLATE) use ASN1_TEMPLATE but will get confused if marked optional. As part of this, i2d_FOO(NULL) now returns -1 rather than "successfully" writing 0 bytes. If we want to allow NULL at the top-level, that's not too hard to arrange, but our CBB-based i2d functions do not. Update-Note: Structures with missing mandatory fields can no longer be encoded. Note that, apart from the cases already handled by preceding CLs, tasn_new.c will fill in non-NULL empty objects everywhere. The main downstream impact I've seen of this particular change is in combination with other bugs. Consider a caller that does: GENERAL_NAME *name = GENERAL_NAME_new(); name->type = GEN_DNS; name->d.dNSName = DoSomethingComplicated(...); Suppose DoSomethingComplicated() was actually fallible and returned NULL, but the caller forgot to check. They'd now construct a GENERAL_NAME with a missing field. Previously, this would silently serialize some garbage (omitted field) or empty string. Now we fail to encode, but the true error was the uncaught DoSomethingComplicated() failure. (Which likely was itself a bug.) Bug: 429 Change-Id: I37fe618761be64a619be9fdc8d416f24ecbb8c46 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49350 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
BoringSSL is a fork of OpenSSL that is designed to meet Google's needs.
Although BoringSSL is an open source project, it is not intended for general use, as OpenSSL is. We don't recommend that third parties depend upon it. Doing so is likely to be frustrating because there are no guarantees of API or ABI stability.
Programs ship their own copies of BoringSSL when they use it and we update everything as needed when deciding to make API changes. This allows us to mostly avoid compromises in the name of compatibility. It works for us, but it may not work for you.
BoringSSL arose because Google used OpenSSL for many years in various ways and, over time, built up a large number of patches that were maintained while tracking upstream OpenSSL. As Google's product portfolio became more complex, more copies of OpenSSL sprung up and the effort involved in maintaining all these patches in multiple places was growing steadily.
Currently BoringSSL is the SSL library in Chrome/Chromium, Android (but it's not part of the NDK) and a number of other apps/programs.
Project links:
There are other files in this directory which might be helpful: