Add netty-tcnative patches and tests #2006
Conversation
skmcgrail
changed the title
skmcgrail
force-pushed
the
netty-tcnative
branch
4 times, most recently
from
908e58f to
0308878
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2006 +/- ##
==========================================
- Coverage 78.90% 78.90% -0.01%
==========================================
Files 594 594
Lines 102415 102414 -1
Branches 14517 14517
==========================================
- Hits 80812 80805 -7
- Misses 20953 20958 +5
- Partials 650 651 +1 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
skmcgrail
force-pushed
the
netty-tcnative
branch
5 times, most recently
from
983fc3e to
45b03b3
Compare
skmcgrail
force-pushed
the
netty-tcnative
branch
from
45b03b3 to
522463c
Compare
skmcgrail
changed the title
| mkdir -p "${INTERROGATE_DIR}" | ||
| pushd "${INTERROGATE_DIR}" | ||
| unzip "${TARGET_JAR}" | ||
| nm "META-INF/native/libnetty_tcnative_${UNAME_S}_${UNAME_P}.so" | grep awslc_api_version_num |
There was a problem hiding this comment.
Double checking: if this grep doesn't find awslc_api_version_num does the whole GitHub job fail?
There was a problem hiding this comment.
Yes because of the pipefail option configured above.
| # Shared Build | ||
| wrapper_aws_lc_build "-DBUILD_SHARED_LIBS=1" | ||
|
|
||
| build_netty_tcnative -am -pl openssl-dynamic clean verify | ||
|
|
||
| # Shared FIPS Build | ||
| wrapper_aws_lc_build "-DBUILD_SHARED_LIBS=1" "-DFIPS=1" | ||
|
|
||
| build_netty_tcnative -am -pl openssl-dynamic clean verify |
There was a problem hiding this comment.
Is there anyway to detect if these shared builds are using the system OpenSSL or AWS-LC?
| # Shared Build | ||
| wrapper_aws_lc_build "-DBUILD_SHARED_LIBS=1" | ||
|
|
||
| build_netty_tcnative -am -pl openssl-dynamic clean verify |
There was a problem hiding this comment.
Does verify run all of tcnative's tests?
There was a problem hiding this comment.
Correct, which there are very sparse/few tests.
| -#ifdef OPENSSL_IS_BORINGSSL | ||
| +#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC) | ||
| sk_CRYPTO_BUFFER_pop_free(chain, CRYPTO_BUFFER_free); | ||
| #else | ||
| sk_X509_pop_free(chain, X509_free); | ||
| X509_free(cert); | ||
| -#endif // OPENSSL_IS_BORINGSSL | ||
| +#endif // defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC) |
There was a problem hiding this comment.
I don't get some of these if/def's we have sk_X509_pop_free and X509_free
There was a problem hiding this comment.
This is due to the code in netty-tcnative leveraging the TLS_with_buffers_method when constructing the SSL_CTX with SSL_CTX_new. This is a feature that BoringSSL added that Netty is taking advantage of to avoid usage of the more expensive X509 structure. This retains the certificate in a CRYPTO_BUFFER, thus alternative functions need to be used for the differing types. See our porting guide.
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.