Add additional x509 store verification bindings #1390
Conversation
naleba97
force-pushed
the
add-x509-verif-options
branch
7 times, most recently
from
9a3d15a to
742c7af
Compare
openssl/src/x509/store.rs
Outdated
| cfg_if! { | ||
| if #[cfg(any(ossl102, libressl261))] { | ||
| /// Sets the verification flags used to verify a certificate against a chain. | ||
| pub fn set_flags(&mut self, flags: X509VerifyFlags) -> Result<(), ErrorStack> { |
There was a problem hiding this comment.
Why would the argument type for this not be X509VerifyFlags on old versions?
There was a problem hiding this comment.
X509VerifyFlags is in the verify module which is only compiled in the top level x509 module for any(ossl102, libressl261). I could relax/remove the version condition to use X509VerifyFlags for older versions.
There was a problem hiding this comment.
If the type is used in APIs that exist in older versions, then it can be exposed on those older versions.
There was a problem hiding this comment.
Removed the top-level constraint on the verify module and added more fine-grained constraints on structs and bit flags in the verify module.
openssl/src/x509/store.rs
Outdated
| } | ||
|
|
||
| /// Sets the purpose used to verify the certificate chain. | ||
| pub fn set_purpose(&mut self, purpose: c_int) -> Result<(), ErrorStack> { |
There was a problem hiding this comment.
These should not take a raw integer.
There was a problem hiding this comment.
Will add the trust/purpose values as defined in https://github.com/openssl/openssl/blob/OpenSSL_1_1_1a/include/openssl/x509.h#L116 and https://github.com/openssl/openssl/blob/OpenSSL_1_1_1a/include/openssl/x509v3.h#L412
naleba97
force-pushed
the
add-x509-verif-options
branch
2 times, most recently
from
ac59787 to
8b4322f
Compare
openssl/src/x509/verify.rs
Outdated
| #[cfg(not(any(ossl110)))] | ||
| pub const DEFAULT: TrustId = TrustId(ffi::X509_TRUST_DEFAULT); | ||
| #[cfg(any(ossl110))] | ||
| pub const DEFAULT: TrustId = TrustId(ffi::X509_TRUST_DEFAULT); /* Only valid in purpose settings */ |
There was a problem hiding this comment.
Aren't these two definitions identical?
There was a problem hiding this comment.
Whoops, good catch, will remove.
openssl/src/x509/verify.rs
Outdated
| @@ -58,6 +59,61 @@ bitflags! { | |||
| } | |||
| } | |||
|
|
|||
| pub struct PurposeId(c_int); | |||
There was a problem hiding this comment.
These seem to fit better in the store module since that's where they're actually used. I'd call them X509Purpose and X509Trust probably I think.
There was a problem hiding this comment.
Sure, I originally put them in the verify module since trust and purpose values are also used in X509_VERIFY_PARAM, in addition to X509_STORE.
naleba97
force-pushed
the
add-x509-verif-options
branch
3 times, most recently
from
294db5a to
9ce2a66
Compare
|
What’s missing in this PR? Can I do something to make it pass review? |
naleba97
force-pushed
the
add-x509-verif-options
branch
from
9ce2a66 to
97154e4
Compare
|
Revisiting the PR: currently making fixes to build for 1.0.1 |
naleba97
force-pushed
the
add-x509-verif-options
branch
from
97154e4 to
9e9ff5e
Compare
naleba97
force-pushed
the
add-x509-verif-options
branch
from
9e9ff5e to
63be141
Compare
|
Hey @sfackler, please let me know if there's anything else blocking this PR. I've just rebased with the latest changes. Thanks! |
|
Any update on this? |
|
@sfackler |
Added some low level bindings to the X509 Store builder struct for expanded verification capabilities.
Also fixed
add_cert()to take in an&X509Refinstead of aX509.