-
Notifications
You must be signed in to change notification settings - Fork 753
test(integration): add BoringSSL cohort to expand mTLS coverage #5659
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| # NOTE: BoringSSL is disabled on macOS to avoid symbol collisions with | ||
| # OpenSSL; see https://github.com/aws/s2n-tls/pull/5659 for details. | ||
| [target.'cfg(not(target_os = "macos"))'.dependencies.boring] | ||
| git = "https://github.com/kaukabrizvi/boring.git" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with this, but can we just open an issue to find a way to "time bomb" ourselves? I'd like our CI to fail if your fork goes more than e.g. 6 months without a commit. "trust but verify" 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve installed https://wei.github.io/pull/ on the fork, which should help keep it in check. That said, an additional guardrail would be useful - I’ll open a follow-up issue after this lands to add a CI “time-bomb” so the fork doesn’t silently go stale.
4cf7c36 to
5f5cf65
Compare
Goal
This PR does two things:
Why
The mTLS tests were introduced to ensure that s2n can interop with peers for basic, synchronous callback, and asynchronous callback cases. This PR continues that work by expanding coverage. Currently, the tests only assert on interoperability with rustls, this PR adds cases for BoringSSL. This also sets the foundation to add more integration test coverage with BoringSSL as a peer in the future.
How
Adds a dependency on my fork of BoringSSL (see callouts section) for the integration and tls-harness crates. Adds a module for the boringssl harness which matches most of the flow in the OpenSSL harness. Integrates BoringSSL into the mTLS test, following the same flow of existing tests, adding cases for BoringSSL as the peer.
Callouts
Once we have a reliable macOS path (potentially via Add symbol prefixing feature for BoringSSL cloudflare/boring#401), we can remove this target-specific gate.
Testing
Related
mTLS coverage is the primary goal of this PR, though future work could include expanding other tests now that BoringSSL is supported in the harness.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.