impl SSL_CTX_set_default_verify_paths and friends #2
Conversation
cpu
force-pushed
the
cpu-set-default-paths
branch
2 times, most recently
from
c7b4bd9
to
e122674
Compare
|
@ctz This probably needs a bit more work but seemed like a good spot to check in for feedback. In terms of test coverage: do you have any thoughts? I was thinking of updating |
| entry! { | ||
| pub fn _SSL_CTX_set_default_verify_paths(ctx: *mut SSL_CTX) -> c_int { | ||
| let ctx = try_clone_arc!(ctx); | ||
| match ctx | ||
| .lock() | ||
| .map_err(|_| Error::cannot_lock()) | ||
| .map(|mut ctx| ctx.set_default_verify_paths()) | ||
| { | ||
| Err(e) => e.raise().into(), | ||
| Ok(()) => C_INT_SUCCESS, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This pattern of casting an arc from the pointer, juggling the lock, invoking a fn with the unlocked ctx, and then juggling a c_int error return might be worth refactoring into something shared 🤔
There was a problem hiding this comment.
Yeah, agreed. Should be possible to take a closure that gets a &mut SslContext. Seems like a good refactoring for later.
| entry! { | ||
| pub fn _SSL_CTX_set_default_verify_paths(ctx: *mut SSL_CTX) -> c_int { | ||
| let ctx = try_clone_arc!(ctx); | ||
| match ctx | ||
| .lock() | ||
| .map_err(|_| Error::cannot_lock()) | ||
| .map(|mut ctx| ctx.set_default_verify_paths()) | ||
| { | ||
| Err(e) => e.raise().into(), | ||
| Ok(()) => C_INT_SUCCESS, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Yeah, agreed. Should be possible to take a closure that gets a &mut SslContext. Seems like a good refactoring for later.
Yeah adding things to |
cpu
force-pushed
the
cpu-set-default-paths
branch
3 times, most recently
from
24b6627
to
ca1dcd5
Compare
I sorted the first part of this by adding a "default" magic option for the ca cert. The second part is addressed by setting a |
cpu
force-pushed
the
cpu-set-default-paths
branch
2 times, most recently
from
6e305d3
to
da431da
Compare
cpu
force-pushed
the
cpu-set-default-paths
branch
from
da431da
to
3b293c8
Compare
The `load_verify_files` fn in `entry.rs` had two jobs: 1. Converting a `impl Iterator<Item = PathBuf>` to a `Vec<CertificateDer<'a>>`. 2. Unlocking a `SSL_CTX` mutex and calling `add_trusted_certs` with the `Vec`. The first of these jobs is useful in other contexts, and so this commit lifts it into `x509.rs` where it can be shared.
The `SSL_new` entry point is described as returning `NULL` for error conditions. Prior to this commit the only possible error was from the `SSL_CTX` Mutex being poisoned - the `SSL::new` constructor was infallible. To support loading certs on-demand from default locations when constructing a `SSL` from a `SSL_CTX` this commit updates the `SSL::new` constructor fn to be fallible. We convert any error to a `NULL` return in the entry-point wrapper.
Adds: * SSL_CTX_set_default_verify_paths * SSL_CTX_set_default_verify_dir * SSL_CTX_set_default_verify_file Stubs: * SSL_CTX_set_default_verify_store We take a dep on `openssl-probe` in order to get convenient handling of the `SSL_CERT_DIR` and `SSL_CERT_FILE` env vars, and default locations for the verify paths. There is likely some fine-tuning to do (e.g. with respect to the `X509_LOOKUP` API surface), but is a step in the right direction for the simple cases.
Keeps CLion/Rust-Rover users from checking in IDE settings by mistake.
Previously the CA cert positional arg to the client could be either a path to a PEM bundle, or "insecure" to disable cert. validation. This commit updates this handling to add "default" as an option that will invoke `SSL_CTX_set_default_verify_paths` to load default system CA certs.
Previously the client test wrote "hello" to the socket and expected to receive an echoed response. This matches what our test server infrastructure does, but precludes using the test client against a real HTTPS server. We want to be able to do that to test using the system CA bundle to verify a server with a cert from the web PKI. This commit adds a check for a `NO_ECHO` env var. When set, we clean up the connection without trying to write a message or expect a response. In the future we may choose to do something more complex like writing a HTTP request and asserting a 200 response. By default when `NO_ECHO` is not set the test client operates as it did before.
This commit adds a unit test that uses the client test binary to connect to https://example.com, validating the presented certificate chain using the system default CA bundle. We do this with `NO_ECHO=1` to avoid trying to write a non-HTTP request and asserting on an echoed response.
cpu
force-pushed
the
cpu-set-default-paths
branch
from
3b293c8
to
5859eba
Compare
Adds:
SSL_CTX_set_default_verify_pathsSSL_CTX_set_default_verify_dirSSL_CTX_set_default_verify_fileStubs:
SSL_CTX_set_default_verify_storeWe take a dep on
openssl-probein order to get convenient handling of theSSL_CERT_DIRandSSL_CERT_FILEenv vars, and default locations for the verify paths.There is likely some fine-tuning to do (e.g. with respect to the
X509_LOOKUPAPI surface), but it's a step in the right direction for the simple cases. See man SSL_CTX_set_default_verify_paths for more information.With this branch in-place I'm able to use a build of curl 7.81.0 that previously complained about the lack of a
SSL_CTX_set_default_verify_pathssymbol to do a HTTPS connection toexample.comwithout explicit specification of CA cert file/dir paths.With tip of main:
With this branch: