TLS: BoGo tests update #4389
TLS: BoGo tests update #4389
Conversation
The default implementation simply deserializes the passed public key depending on the group identifier. By disentangling deserialization and KEM-Encaps respectively KEX-Agreement we simplify the handling of the Decoding_Error exception mapping it to TLS' illegal_parameter. Also, I can see how this might be useful for downstream users that want to use custom key pairs but don't want to re-implement the actual key agreement mechanism.
Note that the block of 'LooseErrorTests' was previously handled by a patch in the BoGo test code. For consistency, I marked them as "loose errors" (aka. ignore reported TLS alert).
|
CI failures look relevant |
reneme
force-pushed
the
tls/bogo_update
branch
from
075103c
to
c23a012
Compare
Let's try again. |
| @@ -6,7 +6,7 @@ | |||
|
|
|||
|
|
|||
| BORING_REPO = "https://github.com/randombit/boringssl.git" | |||
| BORING_BRANCH = "rene/runner-20240524" | |||
| BORING_BRANCH = "rene/runner-20241016" | |||
There was a problem hiding this comment.
This config is duplicated thrice in the repository: I had said we should centralize this.
Suggestion:
Lets add a file in src/configs/repo_config.env that can contain such configurations. Side note: Personally, I'd probably add it as a root-level file (eg. .repo_config.env), but I remember that you wanted to keep the repo root as tidy as possible.
This file could then be consumed by .github/workflows/ci.yml and .../nightly.yml as well as here and (for now) just contain:
# the fork of boringssl that should be used for BoGo tests
BORINGSSL_REPO=https://github.com/randombit/boringssl.git
# the branch in our fork of boringssl that should be used for BoGo tests
BORINGSSL_BRANCH=rene/runner-20241016We might have more such configuration that is worth to centralize like that. In any case: I'd do this adaption in an independent PR (likely after 3.6.0).
What are your thoughts on this?
There was a problem hiding this comment.
Lets add a file in src/configs/repo_config.env that can contain such configurations.
Sure sounds good.
Side note: Personally, I'd probably add it as a root-level file (eg. .repo_config.env), but I remember that you wanted to keep the repo root as tidy as possible.
Indeed so. Thank you for accomodating my autistic needs on this topic.
This updates the BoGo test suilte to boringssl's trunk of yesterday. This includes two noteworthy changes:
(its usage is disabled in this pull request and will be enabled later)
The latter required a minor refactoring in
TLS::Callbackswhere I disentangled public key deserialization and KEM-Encaps respectively KEX-Agree.TLS::Callbacks::tls_deserialize_peer_public_key()now handles the plain serialization while the pre-existingtls_kem_encapsulate()andtls_ephemeral_key_agreement()perform error re-mapping and the actual usage of the keys.Certain users may benefit from the new callback when the want to introduce custom deserialization logic but want to rely on the standard implementation of key exchange. See the updated example.
Also, a few tweaks in TLS alert usage were needed to fulfill the new tests.