aboutsummaryrefslogtreecommitdiff
path: root/include
AgeCommit message (Collapse)AuthorFilesLines
2021-09-13Clarify that TLS sessions are not application sessions.David Benjamin1-0/+18
Having APIs named "session" and "ID" appears to be far too tempting for developers, mistaking it as some application-level notion of session. Update the documentation, in hopes of discouraging this mistake. Change-Id: Ifd9516287092371d4701114771eff6640df1bcb0 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49405 Reviewed-by: Adam Langley <agl@google.com>
2021-09-13Fix BN_prime_checks_for_validation to align with false-positive rate.jakemas1-3/+3
This doesn't affect RSA key generation, which uses BN_prime_checks_for_generation. Change-Id: Ibf32c0c4bc9fed369e8f8a1efea72c5bd39185a9 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49426 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2021-09-10Add maskHash to RSA_PSS_PARAMS for compatShelley Vohr3-2/+12
This CL adds a maskHash member to the rsa_pss_params_st struct for increased compatibility with OpenSSL: https://source.chromium.org/chromium/chromium/src/+/main:third_party/perl/c/include/openssl/rsa.h;l=282-289 Node.js recently began to make use of this member in https://github.com/nodejs/node/pull/39851 and without this member Electron sees compilation errors. Change-Id: Ibd18a31605b0a715edb279a3bca4b4f05e679767 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49365 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2021-09-09Remove ASN1_OP_I2D_* callbacks.David Benjamin1-2/+2
These are a little odd with the ASN1_ENCODING paths. And there were some bugs previously around CHOICE types. Nothing defines them, inside or outside BoringSSL, so remove them. Change-Id: Id2954fef8ee9637f36f7511b51dc0adc2557e3ba Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49352 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-09-07Check for __TRUSTY__ instead of TRUSTY.David Benjamin1-2/+2
Meant to do this shortly after filing the bug but forgot. Bug: 377 Change-Id: Ic5a5c167a7b6745599e3a32c4792b66ebbb2dee0 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49265 Reviewed-by: Adam Langley <agl@google.com>
2021-09-02Fix calculation of draft-13 ECH confirmation signal.David Benjamin1-0/+4
Apparently both we and Go flipped the parameter order for HKDF-Extract relative to the HKDF spec. (The spec orders the salt before the key.) Not sure how that happened. Found doing interop testing with Stephen Farrell's implementation. https://pkg.go.dev/golang.org/x/crypto/hkdf#Extract https://datatracker.ietf.org/doc/html/rfc5869#section-2.2 https://datatracker.ietf.org/doc/html/draft-ietf-tls-esni-13#section-7.2 Bug: 275 Change-Id: I40a7d53b45cb548e93e6a7ae235e98e55dec4a7a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49185 Reviewed-by: Adam Langley <agl@google.com>
2021-09-01Update to draft-ietf-tls-esni-13.David Benjamin2-5/+6
Later CLs will clean up the ClientHello construction a bit (draft-12 avoids computing ClientHelloOuter twice). I suspect the transcript handling on the client can also be simpler, but I'll see what's convenient after I've changed how ClientHelloOuter is constructed. Changes of note between draft-10 and draft-13: - There is now an ECH confirmation signal in both HRR and SH. We don't actually make much use of this in our client right now, but it resolves a bunch of weird issues around HRR, including edge cases if HRR applies to one ClientHello but not the other. - The confirmation signal no longer depends on key_share and PSK, so we don't have to work around a weird ordering issue. - ech_is_inner is now folded into the main encrypted_client_hello code point. This works better with some stuff around HRR. - Padding is moved from the padding extension, computed with ClientHelloInner, to something we fill in afterwards. This makes it easier to pad up the whole thing to a multiple of 32. I've accordingly updated to the latest recommended padding construction, and updated the GREASE logic to match. - ech_outer_extensions is much easier to process because the order is required to be consistent. We were doing that anyway, and now a simple linear scan works. - ClientHelloOuterAAD now uses an all zero placeholder payload of the same length. This lets us simplify the server code, but, for now, I've kept the client code the same. I'll follow this up with a CL to avoid computing ClientHelloOuter twice. - ClientHelloOuterAAD is allowed to contain a placeholder PSK. I haven't filled that in and will do it in a follow-up CL. Bug: 275 Change-Id: I7464345125c53968b2fe692f9268e392120fc2eb Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48912 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-09-01Reword SSL_get0_ech_name_override documentation.David Benjamin1-5/+5
Hopefully it's a little clearer that this may be called whether or not ECH is offered. (And whether or not it's a server.) Bug: 275 Change-Id: I39c8ce5758543a0cfda84652b3fc0a5b9669fd0a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49165 Reviewed-by: Matt Mueller <mattm@google.com> Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2021-09-01Remove SSL_set_verify_result.David Benjamin1-6/+0
Follow-up from https://boringssl-review.googlesource.com/10485 that I forgot about. It's been removed from netty-tcnative. Change-Id: Ic4b97b30787962b78a69911a6e3cd28647546f59 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49145 Reviewed-by: Adam Langley <agl@google.com>
2021-08-31Make most of crypto/x509 opaque.David Benjamin3-188/+2
This unexports X509, X509_CINF, X509_NAME_ENTRY, X509_NAME, X509_OBJECT, X509_LOOKUP_METHOD, X509_STORE, X509_LOOKUP, and X509_STORE_CTX. Note this means X509_STORE_CTX can no longer be stack-allocated. Update-Note: Patch cl/390055173 into the roll that includes this. This unexports most of the X.509 structs, aligning with OpenSSL. Use the accessor APIs instead. Bug: 425 Change-Id: I53e915bfae3b8dc4b67642279d0e54dc606f2297 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48985 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-08-30Remove V_ASN1_APP_CHOOSE.David Benjamin1-4/+0
V_ASN1_APP_CHOOSE has been discouraged by OpenSSL since 2000: https://git.openssl.org/gitweb/?p=openssl.git;a=blob;f=CHANGES;h=824f421b8d331ba2a2009dbda333a57493bedb1e;hb=fb047ebc87b18bdc4cf9ddee9ee1f5ed93e56aff#l10848 Instead, upstream recommends an MBSTRING_* constant. https://www.openssl.org/docs/man1.1.1/man3/X509_NAME_add_entry_by_NID.html This function is a bit overloaded: MBSTRING_* means "Decode my input from this format and then re-encode it using whatever string type best suits the NID (usually UTF8String, but some NIDs require PrintableString)". V_ASN1_APP_CHOOSE means "This is a Latin-1 string. Without looking at the NID, pick one of PrintableString, IA5String, or T61String". The latter is almost certainly not what callers want. If they want a particular type, they can always force it by passing a particular V_ASN1_* constant. This removes the only use of ASN1_PRINTABLE_type within the library, though there is one external use still. Update-Note: V_ASN1_APP_CHOOSE is removed. I only found one use, which has been fixed. Change-Id: Id36376dd0ec68559bbbb366e2305d42be5ddac67 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49067 Reviewed-by: Adam Langley <agl@google.com>
2021-08-30Rewrite ASN1_PRINTABLE_type and add tests.David Benjamin1-3/+5
The old loop read one byte past the length. It also stopped the loop too early on interior NUL. See also upstream's https://github.com/openssl/openssl/pull/16433, though I've opted to rewrite the function entirely rather than use their fix. Also deduplicate the PrintableString check. Change-Id: Ia8bd282047c2a2ed1d5e71a68a3947c7c108df95 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49066 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-08-26Work around yet another MSVC 2015 SFINAE bug.David Benjamin1-14/+17
Although we defined a CBS -> Span<const uint8_t> conversion, MSVC 2015 keeps trying to call the Span(const Container&) constructor. It seems to not correctly SFINAE the existence of data() and size() members unless the expression is inlined into the default template argument. Change-Id: I4e88f820b78ce72ad1b014b5bae0830bc7d099d4 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48945 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-08-25Fix some error-handling in i2v functions.David Benjamin1-1/+39
See upstream commits: 32f3b98d1302d4c0950dc1bf94b50269b6edbd95 432f8688bb72e21939845ac7a69359ca718c6676 7bb50cbc4af78a0c8d36fdf2c141ad1330125e2f 8c74c9d1ade0fbdab5b815ddb747351b8b839641 Change-Id: Iff614260c1b1582856edb4ae7a226f2e07537698 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49045 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-08-25Fix typo.David Benjamin1-0/+2
Subsequent CLs will add some fuzzers, etc., that'll help with catching this. Change-Id: I10a8e4b2f23ffd07b124e725c1f7454e7ea6f2dd Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49025 Reviewed-by: Adam Langley <agl@google.com>
2021-08-25Rewrite name constraints matching with CBS.David Benjamin1-0/+5
See also 8393de42498f8be75cf0353f5c9f906a43a748d2 from upstream and CBS-2021-3712. But rather than do that, I've rewritten it with CBS, so it's a bit clearer. The previous commit added tests. Change-Id: Ie52e28f07b9bf805c8730eab7be5d40cb5d558b6 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49008 Reviewed-by: Adam Langley <agl@google.com>
2021-08-25Add some tests for name constraints.David Benjamin1-0/+1
Change-Id: I51606bb7e4674716ffb6688b3a8e69db3f014546 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49007 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-08-24Fix i2v_GENERAL_NAME to not assume NUL terminated stringsDavid Benjamin1-0/+1
See also 174ba8048a7f2f5e1fca31cfb93b1730d9db8300 from upstream. This differs from the upstream CL in that: - We don't silently drop trailing NULs. - As a NUL-terminated C string, the empty string is a non-NULL pointer to an array containing a zero byte. Use the latter consistently. Change-Id: I99c6c4c26be5a1771c56c6ab356425f1b85be41d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49006 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-08-24Add a CBB_add_zeros helper.David Benjamin1-0/+4
We fill in placeholder values of all zeros fairly often in TLS now, as workarounds for messages being constructed in the wrong order. draft-12 of ECH adds even more of these. Add a helper so we don't need to interrupt an || chain with a memset. Change-Id: Id4f9d988ee67598645a01637cc9515b475c1aec2 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48909 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-08-20Refer to RFCs consistently.David Benjamin4-50/+50
We were a mix of "RFC1234" and "RFC 1234". Apparently there is actually an answer for this, which is with a space textually and without a space in the citation/reference tag: https://datatracker.ietf.org/doc/html/rfc7322#section-3.5 Change-Id: I0c44023163fe3a2a3ffe28cbc644d4c952dc8f1e Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48965 Reviewed-by: Adam Langley <agl@google.com>
2021-08-16Add Span::first() and Span::last().David Benjamin1-1/+20
absl::Span, base::span, and std::span have first() and last() methods which give prefixes and suffixes. first() just saves 5 characters, but last() is nicer to write than subspan() for suffixes. Unlike subspan(), they also do not have clipping behavior, so we're guaranteed the length is correct. The clipping behavior comes from absl::Span::subspan() and is not present in std::span or base::span. I've left it in, in case we switch to absl::Span in the future, but I imagine absl::Span will need to migrate this at some point. Change-Id: I042dd6c566b6d753ec6de9d84e8c09ac7c270267 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48905 Reviewed-by: Adam Langley <agl@google.com>
2021-08-11Add a test for ASN1_mbstring_copy and clean up.David Benjamin1-11/+13
In writing the tests, I noticed that the documentation was wrong. First, the maximum lengths are measured in codepoints, not bytes. Second, the TODO was wrong. We actually do handle this correctly, *almost*. Rather, the bug is that the function assumes |mask| contains no extraneous bits. If it does, all extraneous bits are interpreted as B_ASN1_UTF8STRING. This seems like a bug, so I've gone ahead and fixed that, with a test. Change-Id: I7ba8fa700a8e21e6d25cb7ce879dace685eecf7e Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48825 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-08-11Remove ASN1_TFLG_SET_ORDER.David Benjamin1-7/+0
ASN1_TFLG_SET_ORDER was used in OpenSSL's CMS and PKCS#7 implementations, which we've removed. Fields that use it not only get the DER SET sorting but, when serialized, go back and mutate the original object to match. This is unused, so remove it. This removes one of the sources of non-const behavior in i2d functions. Bug: 407 Change-Id: I6b2bf8d11c30a41b53d14ad475c26a1a30dfd31f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48786 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-08-11Document ASN.1 printing functions.David Benjamin1-79/+97
ASN1_STRING_print_ex is extremely complex and attempting to implement RFC2253, so write some tests for it. Along the way, unexport CHARTYPE_*, which are internal book-keeping used in ASN1_STRING_print_ex. Change-Id: Idb27cd40fb66dc099d1fd6d039a00404608c2063 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48776 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-08-10Remove OPENSSL_NO_FP_API ifdefs.David Benjamin3-26/+0
We've never tested this and plenty of files depend on FILE* APIs without ifdefs. Change-Id: I8c51c043e068b30bdde1723c3810d3e890eabfca Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48771 Reviewed-by: Adam Langley <agl@google.com>
2021-08-10Move X509_ALGOR to x509.h.David Benjamin2-7/+7
This matches OpenSSL and the name. Also accessors like X509_ALGOR_get0 are in x509.h. Change-Id: Ic7583edcf04627cbfae822df11e75eebdd9ad7aa Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48770 Reviewed-by: Adam Langley <agl@google.com>
2021-08-10Unexport BIT_STRING_BITNAME.David Benjamin2-9/+0
This type does not appear in any public APIs. Change-Id: Ie57c7662e691ea05ff2133beda9760832ea0d0de Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48769 Reviewed-by: Adam Langley <agl@google.com>
2021-08-10Unexport ub_* constants.David Benjamin1-11/+0
These constants aren't suitably namespaced and, moreover, are redefined in a_strnid.c. (The constants aren't especially useful because an X509_NAME doesn't check the upper bound.) Update-Note: Removed some unnamespaced constants. Change-Id: I7d15ae731628d3665119081289947600e7f38065 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48768 Reviewed-by: Adam Langley <agl@google.com>
2021-08-10Always use an ASN1_STRING_TABLE global mask of UTF8String.David Benjamin1-0/+5
ASN1_STRING_set_by_NID is very complex and depends on a "global mask" for most NIDs. (Some NIDs use a single type and use STABLE_NO_MASK to disable the global mask.) Historically, it defaulted to allowing all types, but it switched to UTF8String in OpenSSL 1.0.2. Updating the global mask is not thread-safe, and it's 2021. Let's just always use UTF-8. The only callers I found set it to UTF-8 anyway (with the exception of some test script we don't use, and some code that wasn't compiled). No-op writes in the C/C++ memory model are still race conditions, so this CL fixes some bugs in those callers. Update-Note: The global mask for ASN1_STRING_set_by_NID is now always UTF-8. Callers that want another type should reconsider and, if UTF-8 is still unsuitable, just pass the actual desired type into ASN1_mbstring_copy, X509_NAME_ENTRY_set_data, etc Change-Id: I679e99c57da9a48c805460abcb3af5b2f938c93f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48766 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-08-10Document ASN1_mbstring_copy.David Benjamin1-38/+61
Change-Id: Ia2cb9d969b25d1815d8157dd74125d60b138138f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48765 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-08-04Remove ASN1_STRING_FLAG_MSTRING.David Benjamin2-15/+0
This flag is set when an ASN1_STRING is created from a codepath that is aware it is an "mstring" (CHOICE of multiple string or string-like types). With setters like X509_set_notBefore, it is very easy to accidentally lose the flag on some field that normally has it. The only place the flag is checked is X509_time_adj_ex. X509_time_adj_ex usually transparently picks UTCTime vs GeneralizedTime, as in the X.509 CHOICE type. But if writing to an existing object AND if the object lacks the flag, it will lock to whichever type the object was previously. It is likely any caller hitting this codepath is doing so unintentionally and has a latent bug that won't trip until 2050. In fact, one of the ways callers might accidentally lose the ASN1_STRING_FLAG_MSTRING flag is by using X509_time_adj_ex! X509_time_adj_ex(NULL) does not use an mstring-aware constructor. This CL avoids needing such a notion in the first place. Looking through callers, the one place that wants the old behavior is a call site within OpenSSL, to set the producedAt field in OCSP. That field is a GeneralizedTime, rather than a UTCTime/GeneralizedTime CHOICE. We dropped that code, but I'm making a note of it to remember when filing upstream. Update-Note: ASN1_STRING_FLAG_MSTRING is no longer defined and X509_time_adj_ex now behaves more predictably. Callers that actually wanted to lock to a specific type should call ASN1_UTCTIME_adj or ASN1_GENERALIZEDTIME_adj instead. Change-Id: Ib9e1c9dbd0c694e1e69f938da3992d1ffc9bd060 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48668 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-08-03Document another batch of functions.David Benjamin2-30/+166
This covers most of the ASN.1 time functions and a handful more of x509.h. Also remove some code under #if 0. I'm running out of a easy ones to do, which is probably a good thing. Change-Id: I085b1e2a54d191a7a5f18c801b3c135cfda7bd88 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48665 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: Adam Langley <agl@google.com>
2021-08-03Clarify BIO_new_mum_buf's lifetime rules.David Benjamin1-1/+3
It is not obvious from "It does not take ownership of |buf|" whether the function makes a copy or not. It does not make a copy (maybe it should...), so callers are obligated to manage their lifetimes. Change-Id: I7df9a5814321fd833fcb8d009d9e0318d6668dd4 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48669 Reviewed-by: Adam Langley <agl@google.com>
2021-07-16Add convenience functions to malloc EVP_HPKE_CTX and EVP_HPKE_KEY.David Benjamin1-0/+25
Some callers want the value to be heap-allocated. It's a little annoying that this returns an empty value (if we only supported heap-allocated ones, I'd have merged init into new), but since we have multiple constructor functions, this is probably the least fuss. Change-Id: I42f586e39850954fb6743f8be50a7cfffa0755ba Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48526 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-07-16Document that SSL_PRIVATE_KEY_METHOD should configure signing prefs.David Benjamin1-0/+5
Otherwise BoringSSL may select one the private key does not support. Change-Id: Ia0a57657bd6dedaa6653c23cc850bb6b6fa8f219 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48525 Reviewed-by: Adam Langley <agl@google.com>
2021-07-15hrss: use less stack space.Adam Langley1-12/+14
The stack consumption of the HRSS functions is causing issues in stack-constrained environments. Therefore allocate many variables on the heap. This means that several HRSS_ functions now allocate, and thus can fail, where they couldn't before. Callers that ignore the return value and don't have crash-on-failure mallocs will still be safe, although things will fail to decrypt later on. Somehow, this actually makes key generation _faster_ on my machine. (I don't know. Better alignment? Fewer L1 collisions?) The other operations are slightly slower, as expected. Before: Did 17390 HRSS generate operations in 3054088us (5694.0 ops/sec) Did 225000 HRSS encap operations in 3000512us (74987.2 ops/sec) Did 87000 HRSS decap operations in 3014525us (28860.3 ops/sec) After: Did 21300 HRSS generate operations in 3026637us (7037.5 ops/sec) Did 221000 HRSS encap operations in 3008911us (73448.5 ops/sec) Did 84000 HRSS decap operations in 3007622us (27929.0 ops/sec) Change-Id: I2312df8909af7d8d250c7c483c65038123f21ad9 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48345 Commit-Queue: Adam Langley <agl@google.com> Reviewed-by: David Benjamin <davidben@google.com>
2021-07-15Make X509_EXTENSION opaque.David Benjamin1-7/+1
I've switched a few things to the accessors where it was easy, but X509_EXTENSION is, in us and upstream, not const-correct right now, so it's a little goofy. Update-Note: Use X509_EXTENSION_get_* instead. Change-Id: Ife9636051a924a950b1c739b7720baf12e35f9c7 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48505 Reviewed-by: Adam Langley <agl@google.com>
2021-07-15Make X509_CRL opaque.David Benjamin2-35/+0
Update-Note: Use accessors instead. Change-Id: I7b41eb7c724d94d3e6d26498063e045a1850c671 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48465 Reviewed-by: Adam Langley <agl@google.com>
2021-07-09Remove unused field in X509_NAME_ENTRY.David Benjamin1-1/+0
This is not used anywhere inside or outside the library. Update-Note: Removed unused field in struct. Change-Id: I244d8af819e84412956fecb929678404fdfcc38f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48427 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-07-07Remove X509at_get0_data_by_OBJ.David Benjamin1-6/+0
This function's behavior differs from all the other lastpos functions. It does not appear to be used anywhere, so remove it. (lastpos = -1 returns the first match, lastpos = -2 additionally fails if there are duplicates, lastpos = -3 additionally fails if the attribute is multiply-valued.) Update-Note: X509at_get0_data_by_OBJ is removed. We found no callers of this function. Change-Id: I8547bac6626623e43827e2490f04850eb148e317 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48367 Reviewed-by: Adam Langley <agl@google.com>
2021-07-07Document a batch of extension-related functions in x509.h.David Benjamin1-0/+181
Change-Id: Iaa5971f6a09a4267be95ea1820b72f7b619b53e1 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48366 Reviewed-by: Adam Langley <agl@google.com>
2021-07-02conf: don't crash when parsing.Adam Langley1-5/+8
lh_strhash mapped nullptr to zero. ec8c67dfbc switched CONF's use to OPENSSL_strhash, which crashes on nullptr. But CONF depends on the nullptr handling. Change-Id: I131c752aa089fb99b01c9e406b6994f3a6236976 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48385 Commit-Queue: Adam Langley <agl@google.com> Reviewed-by: David Benjamin <davidben@google.com>
2021-07-01Add some OpenSSL compatibility aliases.David Benjamin2-0/+9
EVP_MD_nid, in OpenSSL, is the same as EVP_MD_type. EVP_MD_type seems to be the preferred spelling, so put EVP_MD_nid in the deprecated bucket. Also add an EVP_MD_do_all alias to EVP_MD_do_all_sorted. Change-Id: I4e7b800902459ac5cb9ef0df65d73da94afdf927 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48365 Reviewed-by: Adam Langley <agl@google.com>
2021-06-30Make ASN1_OBJECT opaque.David Benjamin2-27/+5
This cleans up the story with https://boringssl-review.googlesource.com/c/boringssl/+/46164. None of our exported functions mutate ASN1_OBJECTS, with the exception of ASN1_OBJECT_free, the object reuse mode of c2i_ASN1_OBJECT, and their callers. Those functions check flags to correctly handle static ASN1_OBJECTs. For now, I've kept the struct definition in crypto/asn1 even though ASN1_OBJECT is partially in crypto/obj. Since we prefer to cut dependencies to crypto/asn1, we probably should rearrange this later. I've also, for now, kept crypto/asn1/internal.h at C-style comments, though our style story here is weird. (Maybe it's time to clang-format crypto/asn1 and crypto/x509? Patches from upstream rarely directly apply anyway, since we're a mix of 1.0.2 and 1.1.1 in crypto/x509.) Update-Note: ASN1_OBJECT is now opaque. Callers should use accessors. Change-Id: I655e6bd8afda98a2d1e676c3abeb873aa8de6691 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48326 Reviewed-by: Adam Langley <agl@google.com>
2021-06-24Implement ClientHelloOuter handshakes.David Benjamin2-6/+54
If a client offers ECH, but the server rejects it, the client completes the handshake with ClientHelloOuter in order to authenticate retry keys. Implement this flow. This is largely allowing the existing handshake to proceed, but with some changes: - Certificate verification uses the other name. This CL routes this up to the built-in verifier and adds SSL_get0_ech_name_override for the callback. - We need to disable False Start to pick up server Finished in TLS 1.2. - Client certificates, notably in TLS 1.3 where they're encrypted, should only be revealed to the true server. Fortunately, not sending client certs is always an option, so do that. Channel ID has a similar issue. I've just omitted the extension in ClientHelloOuter because it's deprecated and is unlikely to be used with ECH at this point. ALPS may be worth some pondering but, the way it's currently used, is not sensitive. (Possibly we should change the draft to terminate the handshake before even sending that flight...) - The session is never offered in ClientHelloOuter, but our internal book-keeping doesn't quite notice. I had to replace ech_accept with a tri-state ech_status to correctly handle an edge case in SSL_get0_ech_name_override: when ECH + 0-RTT + reverify_on_resume are all enabled, the first certificate verification is for the 0-RTT session and should be against the true name, yet we have selected_ech_config && !ech_accept. A tri-state tracks when ECH is actually rejected. I've maintained this on the server as well, though the server never actually cares. Bug: 275 Change-Id: Ie55966ca3dc4ffcc8c381479f0fe9bcacd34d0f8 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48135 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-06-23Remove old ASN.1 SET macros.David Benjamin3-19/+0
These macros aren't consumed by anything anymore. Change-Id: Id9616fa0962ae0dbf27bc884c6883dcad9755eb2 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48229 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-06-23Document some ASN1_INTEGER and ASN1_ENUMERATED functions.David Benjamin1-26/+75
Change-Id: If192e1f77d93a216e964b5422cb7d13d153ac328 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48228 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-06-23Document ASN1_STRING_to_UTF8.David Benjamin1-2/+7
We already had a test, but move it to asn1_test.cc since it's part of the ASN.1 library. Also, since it's easy, test it using public APIs rather than stack-allocating an ASN1_STRING. Change-Id: Ic77494e6c8f74584d159a600e334416197761475 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48227 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-06-23Const-correct ASN1_item_verify a bit more.David Benjamin1-1/+2
Change-Id: I188feff6d62986554e34a10d148108b19a4eae0b Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48226 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2021-06-22Compute ASN.1 BIT STRING sizes more consistently.David Benjamin2-18/+80
OpenSSL's BIT STRING representation has two modes, one where it implicitly trims trailing zeros and the other where the number of unused bits is explicitly set. This means logic in ASN1_item_verify, or elsewhere in callers, that checks flags and ASN1_STRING_length is inconsistent with i2c_ASN1_BIT_STRING. Add ASN1_BIT_STRING_num_bytes for code that needs to deal with X.509 using BIT STRING for some fields instead of OCTET STRING. Switch ASN1_item_verify to it. Some external code does this too, so export it as public API. This is mostly a theoretical issue. All parsed BIT STRINGS use explicit byte strings, and there are no APIs (apart from not-yet-opaquified structs) to specify the ASN1_STRING in X509, etc., structures. We intentionally made X509_set1_signature_value, etc., internally construct the ASN1_STRING. Still having an API is more consistent and helps nudge callers towards rejecting excess bits when they want bytes. It may also be worth a public API for consistently accessing the bit count. I've left it alone for now because I've not seen callers that need it, and it saves worrying about bytes-to-bits overflows. This also fixes a bug in the original version of the truncating logic when the entire string was all zeros, and const-corrects a few parameters. Change-Id: I9d29842a3d3264b0cde61ca8cfea07d02177dbc2 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48225 Commit-Queue: David Benjamin <davidben@google.com> Commit-Queue: Adam Langley <agl@google.com> Reviewed-by: Adam Langley <agl@google.com>