Add support for X509_VERIFY_PARAM_set_time and X509_VERIFY_PARAM_set_… #1710
Conversation
alexanderjordanbaker
force-pushed
the
X509VerifyTimeDepth
branch
8 times, most recently
from
f699099
to
b5fabee
Compare
alexanderjordanbaker
force-pushed
the
X509VerifyTimeDepth
branch
2 times, most recently
from
a237506
to
969f955
Compare
openssl/src/x509/tests.rs
Outdated
| assert!(context | ||
| .init(&store, &cert, &chain, |c| c.verify_cert()) | ||
| .unwrap()); | ||
| assert!(context |
There was a problem hiding this comment.
I might be missing something, but why are you calling assert twice with the same arguments? I'd also be much happier with these tests if they also checked a failing cert or depth (you can do it in the same function).
There was a problem hiding this comment.
test_verify_cert in this file calls it twice, I assumed that was best practice. On the failing side, I do checking failing certs with respect to set_time in test_verify_param_set_time_fails_verification. For set_depth, I don't believe there are any test certs in the repo with a 3-deep chain. What is best practice for creating such a chain? Should I just add it to the repo?
There was a problem hiding this comment.
test_verify_cert calling it twice is somewhere between a fluke of history and a test about cleanup, in this case you only need one. Yes, just create a new certificate and throw it in the directory with the rest of them (openssl/test), I think this change would be fine without that test but if you're willing to do that it would be nice to have the coverage.
There was a problem hiding this comment.
Removed the double call, and added new certificates into the repository to create a 3-long chain, which is then used in the set_depth tests to check failure
alexanderjordanbaker
force-pushed
the
X509VerifyTimeDepth
branch
6 times, most recently
from
449bd45
to
9ad8458
Compare
alexanderjordanbaker
force-pushed
the
X509VerifyTimeDepth
branch
from
9ad8458
to
808b951
Compare
|
@sfackler Anything else I need to do for this PR? |
|
Thanks! Sorry for the delay. |
…depth