Add pq-tls interop test with BoringSSL #2199
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2199 +/- ##
==========================================
- Coverage 79.05% 79.04% -0.01%
==========================================
Files 612 612
Lines 106159 106164 +5
Branches 15002 15004 +2
==========================================
- Hits 83923 83921 -2
- Misses 21582 21590 +8
+ Partials 654 653 -1 ☔ View full report in Codecov by Sentry. |
| echo "build boring-ssl with aws-lc" | ||
| cd "$BSSL_SRC_FOLDER" | ||
| cmake . "-B$BSSL_BUILD_FOLDER" -GNinja \ | ||
| -DCMAKE_BUILD_TYPE=Release \ | ||
| -DCMAKE_PREFIX_PATH="$AWS_LC_INSTALL_FOLDER" |
There was a problem hiding this comment.
Why build BoringSSL with AWS-LC, shouldn't they be independent? This looks like you're trying to build BoringSSL with AWS-LC's headers/libraries which doesn't make sense.
There was a problem hiding this comment.
Thanks for catching. It was a copy over from s2n-tls build. Verified that CMAKE_PREFIX_PATH is a no-op for boring SSL builds as it doesn't have to find headers or libraries using that parameter like s2n-tls does. Will remove it.
There was a problem hiding this comment.
LGTM. Some food for thought though. I checked the logs for this task and it's not debuggable in practice. CodeBuild truncates the logs at 10k lines in the UI. Those 10k lines are consumed by the LC and S2N builds. To get to the actual logs of this test, you have to figure out how to download the full logs and open it locally without crashing your text editor. Suggest looking into ways to silence some of this irrelevant logging.
| cat "$AWS_LC_BUILD_FOLDER"/s_server_out | ||
| cat "$BSSL_BUILD_FOLDER"/s_client_out | ||
| grep "Connected" "$AWS_LC_BUILD_FOLDER"/s_server_out | ||
| grep "ECDHE group" "$AWS_LC_BUILD_FOLDER"/s_server_out | grep "$GROUP" |
There was a problem hiding this comment.
nit: Why'd you switch to calling this an ECDHE group here? Earlier you used KEM groups. More generally, we should use the more generic "TLS group" terminology.
There was a problem hiding this comment.
The test is just validating what the tool prints in its server/client logs.
s2n-tls tool used the term KEM group. bssl tool uses ECDHE group. If either tool is updated to use the generic term, we need to update the test at that time.
There was a problem hiding this comment.
Reg log output, we could perhaps not log the build output (suppress stdout while keeping stderr logs).
The current output is around 3K lines most of which is the lc/s2n/bssl build stdout logs.
|
Suppressing s2n-tls and boring-ssl build stdouts almost halved the log size. |
Description of changes:
Adding cross library PQ TLS interop test with BoringSSL
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.