Implement the AWS-LC provider interface - #3432
Conversation
|
🔒 Security Review — View Report Please review before merging. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3432 +/- ##
==========================================
+ Coverage 78.02% 78.05% +0.03%
==========================================
Files 700 700
Lines 125037 125037
Branches 17343 17355 +12
==========================================
+ Hits 97563 97602 +39
+ Misses 26605 26567 -38
+ Partials 869 868 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
caught a segfault in a python integration test, have never seen that before. rerunning to see if it's consistent. |
| return 0; | ||
| } | ||
| ctx->handle = handle; | ||
| ctx->libctx = (OSSL_LIB_CTX *)c_get_libctx(handle); |
There was a problem hiding this comment.
provider-base(7) is explicit that this cast is only valid for built-in providers: "Never cast this to OSSL_LIB_CTX in a provider that is not built-in ... Use OSSL_LIB_CTX_new_child(3) instead." It happens to work here because our OSSL_LIB_CTX_* symbols resolve to the loading libcrypto, but we'd be building later algorithm PRs on a documented misuse.
Since nothing consumes awslc_prov_ctx_libctx() yet, I'd either drop the libctx capture (and the hard requirement on OSSL_FUNC_CORE_GET_LIBCTX at L131) until something needs it, or do it the documented way:
ctx->libctx = OSSL_LIB_CTX_new_child(handle, in);
if (ctx->libctx == NULL) {
awslc_prov_clear_free(ctx, sizeof(*ctx));
return 0;
}with OSSL_LIB_CTX_free(ctx->libctx); added to awslc_prov_teardown (L94, before the clear_free). The child ctx also gives us a libctx we could hand to OSSL_PROVIDER_load later without recursing into ourselves.
There was a problem hiding this comment.
This is only substantive concern, then rest can be deferred.
There was a problem hiding this comment.
I removed the forbidden type cast (as-per the documentation) rather than use OSSL_LIB_CTX_new_child. We specifically need the OPENSSL_CORE_CTX returned by this function (for future FIPS related calls) rather than the child OSSL_LIB_CTX returned by OSSL_LIB_CTX_new_child.
| // The libctx the core handed us at init. Stored so operations that need a | ||
| // library context use the same one the core called us with. | ||
| OSSL_LIB_CTX *awslc_prov_ctx_libctx(const AWSLC_PROV_CTX *ctx); |
There was a problem hiding this comment.
Companion to the comment on provider.c L140: if we go the OSSL_LIB_CTX_new_child route, this comment should say it's a child libctx owned by the provider rather than "the libctx the core handed us", so a future caller doesn't assume it's the application's.
4c153f7 to
3cb71da
Compare
### Context and motivation Stack PRs require their branches to live on the same repository rather than a fork, so every branch push now fires the push-triggered half of CI on top of the `pull_request` half. Both halves test the same commits, so roughly half the work is thrown away. The effect is easy to see by comparing check counts. A stacked PR like #3432 runs **1054** checks, where an equivalent single-branch PR like #3475 runs **530**. The outcome we want: pushes to a working branch produce one round of CI, not two, while `main` and the `fips-*` release branches keep the push coverage they need. ### Description of changes Change the `push` trigger from `branches: ['*']` to `branches: ['main', 'fips-*']` on the 22 workflows that run against day-to-day PRs. `main` and `fips-*` are the branches where a push is the only signal we get; everywhere else the `pull_request` trigger already covers the same commits. ### Testing No new tests; this only narrows workflow trigger conditions. ### Review considerations - There is no way to factor this filter into one place, this is the simplest way to enact this change. - If there are any other branches beyond main/fips that deserve this extra scrutiny. AFAICT, all we have beyond these are stale branches that should probably be deleted. 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.
5 participants
Description of changes:
AWS-LC's provider module currently exports a stub entry point. This change implements the provider-global OpenSSL interface: initialization, teardown, provider parameters, library-context capture, and operation queries.
The change also adds the provider test fixture, an activation config using
?provider=awslc, and Linux checks for the module's symbol ownership andversioned AWS-LC imports.
Call-outs:
This is the first PR in a stack. It deliberately advertises no algorithms;
#3433 adds SHA-256, and #3434 adds the provider documentation.
OpenSSL and AWS-LC install incompatible headers under the same
openssl/namespace. The provider therefore compiles separate frontend and backend object
libraries. Frontend translation units see only OpenSSL, backend translation
units see only AWS-LC, and their shared interface uses plain C types.
The final module links AWS-LC but leaves OpenSSL core symbols for the loading
libcrypto to resolve. The linkage verifier rejects frontend references whose
names overlap AWS-LC's symbol registry and requires the final module's AWS-LC
imports to match the public, versioned symbols requested by backend objects.
Testing:
Frontend unit tests cover module loading, self-describing parameters, declining
unimplemented operations, and optional-property fallback to OpenSSL's default
provider.
tests/ci/run_aws_lc_provider_tests.shbuilds pinned OpenSSL 3.5.5 and AWS-LC'sENABLE_DIST_PKGconfiguration, builds and runs the provider test, verifiesthat the module exports only
OSSL_provider_init, checks its direct libcryptodependency, and runs the object-derived linkage verifier.
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.