CI/Test: Build and test with --disable-modules=tls13#5309
CI/Test: Build and test with --disable-modules=tls13#5309reneme merged 4 commits intorandombit:masterfrom
--disable-modules=tls13#5309Conversation
There was a problem hiding this comment.
Pull request overview
Adds a CI configuration to build Botan with TLS 1.3 disabled and still run the BoringSSL BoGo integration tests using a dedicated shim config.
Changes:
- Introduce a new CI target
no_tls13that builds Botan with--disable-modules=tls13and runs BoGo tests. - Add a BoGo shim configuration file (
config_no_tls13.json) that disables TLS 1.3-related tests. - Extend CI workflow and package selection to support the new target.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/scripts/ci_build.py |
Adds no_tls13 target, disables tls13 module, and selects the TLS1.2-only BoGo shim config when running BoGo tests. |
src/scripts/ci/gha_linux_packages.py |
Ensures no_tls13 installs Boost headers (needed for the build configuration). |
src/editors/vscode/scripts/bogo.py |
Adds a --without-tls-13 flag to select the TLS1.2-only shim config when running BoGo locally. |
src/bogo_shim/config_no_tls13.json |
New BoGo shim config that disables TLS 1.3 / DTLS 1.3 and related tests. |
src/bogo_shim/bogo_shim.cpp |
Adds one error mapping and compiles out TLS 1.3-specific extension modification when TLS 1.3 is disabled. |
.github/workflows/ci.yml |
Adds no_tls13 to the CI matrix and checks out the BoringSSL fork for this target. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b07293f to
07450a1
Compare
|
@randombit This is independent of the other TLS related PRs. I'd suggest, let's merge this now and already benefit from it while merging the stack of open PRs. I'll open another one with a TLS 1.3-only test setup at the end. |
|
Hang on... disabling TLS 1.2 tests would be a huge list of tests in the config.json, globbing test names only takes us so far. I'm going to add a patch to the |
... in particular this runs the BoGo test suite without TLS 1.3 specific tests.
07450a1 to
a4c933c
Compare
|
|
||
| # The branch in our fork of boringssl that should be used for BoGo tests | ||
| BORINGSSL_BRANCH="rene/runner-20241016" | ||
| BORINGSSL_BRANCH="rene/runner-20241016-1" |
There was a problem hiding this comment.
This version of the boringssl test suite now contains a patch adding -skip-dtls, -skip-tls12 and -skip-tls13. That way, our BoGo integration can fairly easily disable most of these tests. Unfortunately, this isn't perfect. Some tests are actually meant to test TLS 1.2 but assume that the shim offers/expects TLS 1.3 or vice versa. These tests still have to be manually disabled via the config.json file, but they are very few and this is hopefully much less brittle.
|
@randombit Ready to go now. Sorry for the back-and-forth. |
See here for an overview of the related pull requests to disentangle TLS 1.2 and 1.3.
In the context of allowing to disable TLS 1.2 at build time I think it makes sense to test the library with TLS 1.2 or TLS 1.3 disabled respectively. This adds a boringSSL test suite configuration that doesn't require TLS 1.3 and runs it in CI with a build of Botan that is configured with
--disable-modules=tls13. Once TLS 1.2 can be disabled independently at build time I'll add this as a test configuration as well.