Simplify PSK binder calculation

The PSK extension has a really annoying step. There is a list of
"binders" whose value depends on the hash of the entire ClientHello,
truncated to the binders block at the end. This means it depends on two
length prefixes that, using our CBB API, have not yet been closed out:

1. The length prefix on the extension list
2. The length prefix on the entire ClientHello message

As a result, we implemented this by filling in all zeros, and then
post-processing the message after the fact to fill them in. This got
messier with ECH, which constructs lots of different ClientHellos and
ClientHello-like structures. It will get even messier with external
PSKs, which now require us to compute potentially multiple binders.

Now that the PSK extension is treated fairly special anyway, we can
change its calling convention and fold the logic into the PSK extension.
This is effectively several steps, but the intermediate points are
awkward, so this is one CL.

First, we take care of the message header by having the binder
calculation compute the header on demand. This was already a bit hairy
on the DTLS side due to an unfortunate in-memory representation we need
to juggle in DTLS. Now that is moot.

That lets us push binder calculation into ssl_add_clienthello_tlsext, as
that is passed a CBB that will ultimately contain the ClientHello, minus
message header. But ssl_add_clienthello_tlsext bifurcates a bit between
ECH and non-ECH. Ideally we'd push it one layer deeper.

Next we change the ext_pre_shared_key_add_clienthello calling convention
to take two generations of CBB into the same function: the unfinished
ClientHello and the unfinished extensions block. This is a bit unusual
but lets it perform all three steps together:

1. Write out the PSK extension, with placeholder binder
2. Close out the extension block to make the ClientHello coherent
3. Replace the placeholder binder with the real binder, computed over
   the now coherent ClientHello.

This should make it a lot easier to add more complex PSK support as it
can all be encapsulated in that one function. We now don't need to
thread a needs_psk_binder output, though we do need to return the length
of the PSK extension to help ECH copy it in two places. (That could
probably be avoided with more math, but this seemed simpler. The main
issue is that, after ext_pre_shared_key_add_clienthello returns,
CBB_len(&extensions) is no longer usable.)

Bug: 369963041
Change-Id: I16d66567bd4eec84397b0e8e05df57bb257d3b7e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/88409
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Lily Chen <chlily@google.com>
5 files changed
tree: b7a729bbdada166fff46d1652e63bad053d1ab7f
  1. .bcr/
  2. .github/
  3. bench/
  4. cmake/
  5. crypto/
  6. decrepit/
  7. docs/
  8. fuzz/
  9. gen/
  10. include/
  11. infra/
  12. pki/
  13. rust/
  14. ssl/
  15. third_party/
  16. tool/
  17. util/
  18. .bazelignore
  19. .bazelrc
  20. .bazelversion
  21. .clang-format
  22. .clang-format-ignore
  23. .gitignore
  24. API-CONVENTIONS.md
  25. AUTHORS
  26. BREAKING-CHANGES.md
  27. BUILD.bazel
  28. build.json
  29. BUILDING.md
  30. CMakeLists.txt
  31. codereview.settings
  32. CONTRIBUTING.md
  33. FUZZING.md
  34. go.mod
  35. go.sum
  36. INCORPORATING.md
  37. LICENSE
  38. MODULE.bazel
  39. MODULE.bazel.lock
  40. PORTING.md
  41. PRESUBMIT.py
  42. PrivacyInfo.xcprivacy
  43. README.md
  44. SANDBOXING.md
  45. SECURITY.md
  46. STYLE.md
README.md

BoringSSL

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:

To file a security issue, use the Chromium process and mention in the report this is for BoringSSL. You can ignore the parts of the process that are specific to Chromium/Chrome.

There are other files in this directory which might be helpful: