-
Notifications
You must be signed in to change notification settings - Fork 749
test(integration): add async cert verify and offload 'stress' test #5653
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
base: main
Are you sure you want to change the base?
Conversation
bindings/rust/standard/integration/src/mtls/async_verify_and_offload.rs
Outdated
Show resolved
Hide resolved
f3e37e4 to
9095a05
Compare
bindings/rust/standard/integration/src/mtls/async_verify_and_offload.rs
Outdated
Show resolved
Hide resolved
bindings/rust/standard/integration/src/mtls/async_verify_and_offload.rs
Outdated
Show resolved
Hide resolved
| invoked: Arc::clone(&invoked), | ||
| sender: tx, | ||
| }); | ||
| let ctx_ptr = Box::into_raw(ctx) as *mut c_void; |
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.
Hmm, this is technically leaking memory.
Not a huge deal because it's just a test, but that's a slippery slope, and it's generally a good idea to get in the habit of writing leak-free code 😆
Option 1: use Box::from_raw in the callback. This will take back ownership from the pointer, and it will be freed after the callback has been used. This is probably the neatest option. It does mean that the callback can only be invoked once, but I think that's fine for the test.
Option 2: "store" the context for the duration of the program.
fn my_function() {
// first, put the object on the heap (not technically necessary, but easier)
let thing_ffi_needs = Box::new(5);
// DANGER: The application is responsible for ensuring that ptr_for_ffi is never
// references after `thing_ffi_needs` goes out of scope.
let ptr_for_ffi = &this_ffi_needs as *mut c_void;
// pass ptr_for_ffi into a function, or whatever
// at the end of the function, thing_ffi_needs goes out of scope, and the memory is cleaned up.
}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.
You’re right, this is leaking memory (noted in the comment below). I will change this to keep it leak-free by reclaiming ownership with Box::from_raw in the callback, and apply the same change in the mTLS test as well:
| let ctx_ptr = Box::into_raw(ctx) as *mut c_void; |
Goal
Add an mTLS “stress” integration test that exercises the full async path: async certificate validation + async pkey verify offload, using rustls as client and s2n-tls as server (TLS 1.3).
Why
This combination adds additional complexity to the mTLS handshake path and ensures that async callback ordering, offload behavior, and multi-message handshake handling remain correct. In addition to the async cert verify callback tests introduced in #5638, applications may configure mTLS with both asynchronous certificate validation and asynchronous public-key verification, so it’s valuable to have integration coverage for that full path.
How
Callouts
Testing
I validated the test as follows:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.