Use timestamp time as pkcs7 verification time#174
Conversation
This is based on RFC 3161 Appendix B:
B) The validity of the digital signature may then be verified in the
following way:
...
4) The date/time indicated by the TSA MUST be within the
validity period of the signer's certificate.
...
In other words, current time does *not* need to be within the validity
period, only the timestamp time.
Without the changes the included test fails with
Unable to verify pkcs7 signature: ErrorStack([Error { code: 276824181, library: "PKCS7 routines", function: "PKCS7_verify", reason: "certificate verify error", file: "crypto/pkcs7/pk7_smime.c", line: 301, data: "Verify error: certificate has expired" }])
On 32 bit systems this will start failing when the timestamp is in 2038.
4f88dfb to
e052b98
Compare
|
Maybe it's ready for review now? FYI also @haydentherapper |
woodruffw
left a comment
There was a problem hiding this comment.
LGTM, although I no longer have commit bit here 😉
facutuesca
left a comment
There was a problem hiding this comment.
LGTM! Just a small suggestion, and could you add an entry to the CHANGELOG?
|
I wrote up my thoughts on this on sigstore/rekor-tiles#399 (comment). tl;dr - The spec is underspecified on how to handle expiration of a chain. I think it's a decision we need to make as an ecosystem whether we want a TSA expiring to cause an artifact to be distrusted, or even make it a policy decision down the line. I'm OK with this interpretation from this PR, though we need all sigstore clients to do the same. We should also make it clear that the way that a client will distrust a timestamp signature going forward is by whether or not the timestamping chain is in TUF - if it's not, it's been revoked. |
I'm avoiding annotating the variable explicitly as time_t since that requires adding libc to dependencies: my assumption is that this still "works" on 32 bit.
facutuesca
left a comment
There was a problem hiding this comment.
LGTM, thanks @jku !!
Fixes #171. This is based on RFC 3161 Appendix B:
In other words, current time does not need to be within the validity period, only the timestamp time needs to within the validity time.
Without the changes the included test fails with
Unable to verify pkcs7 signature: ErrorStack([Error { code: 276824181, library: "PKCS7 routines", function: "PKCS7_verify", reason: "certificate verify error", file: "crypto/pkcs7/pk7_smime.c", line: 301, data: "Verify error: certificate has expired" }])Details:
pkcs_verify()now accepts a python datetime as argument and uses it as a verification time parameter_verify_tsr_with_chains()gets the timestamp time from the timestamp response and feeds it topkcs_verify()