ESSCertIDv2 Update for RFC 3161 (ts module)#771
ESSCertIDv2 Update for RFC 3161 (ts module)#771kleinmrk wants to merge 12 commits intoopenssl:masterfrom
Conversation
|
Do we need both? Just asking because I don't know anything about TS. |
|
Yes, we do need both since it is written in RFC5816 that the certificate identifier is either ESSCertID or ESSCertIDv2. |
|
Thanks for the clarification. |
crypto/ts/ts_conf.c
Outdated
There was a problem hiding this comment.
do we really need these define's?
There was a problem hiding this comment.
I have simplified the logic and now only one .conf variable is needed.
|
a few minor style issues. I didn't always comment on all of them, so please make sure to check that there's a blank line after variable declarations and don't use ! for pointer checking. |
|
@richsalz thank you for the review. |
|
No, this MR is fine. Just go to the branch, rebase on mater and push it to github again (-f flag) |
|
+1 will merge after anothe rreview. thanks. |
crypto/ts/ts_rsp_sign.c
Outdated
There was a problem hiding this comment.
merge 1035/1306, put the linebreak after the paren.
|
any chance of tests? |
|
I've added ess_cert_id_alg variable into existing tsa tests. I believe this is enough. |
|
ok thanks |
|
It's very good to see TS module is getting some attention - I've been waiting five years for inclusion of my last patch 😃. Any chance of getting this merged to 1.1? |
|
Yes, the OpenSSL has not care very much about TS. Sorry about that. |
include/openssl/ts.h
Outdated
There was a problem hiding this comment.
Define TS_F_TS_RESP_CTX_SET_ESS_CERT_ID_V2_DIGEST is not used anywhere in the code.
richsalz
removed
the
issue: feature request
|
For starters, it needs a rebase on top of a fresh master... |
crypto/ts/ts_rsp_sign.c
Outdated
There was a problem hiding this comment.
Inconsistent naming... why capitalize some function names and not others?
doc/man1/ts.pod
Outdated
There was a problem hiding this comment.
I think we concluded elsewhere that pointed lists should be =over 2, so please revert the change on this line.
|
Sigh. I feel kinda bad for the submittor. It's like we keep adding things for him to do. |
|
He'll be rewarded with eternal glory. |
…tialized after calling ESS_SIGNING_CERT_V2_new() Removed unnecessary check: issuer_serial within ESS_CERT_ID_V2 is set to NULL after calling ESS_CERT_ID_V2_new()
|
@levitte static functions are now consistently in lowercase. |
|
Cool! Going in... |
Reviewed-by: Rich Salz <[email protected]> Reviewed-by: Richard Levitte <[email protected]> (Merged from #771)
|
Merged. I smashed it all together into one commit, f0ef20b |
Reviewed-by: Rich Salz <[email protected]> Reviewed-by: Richard Levitte <[email protected]> (Merged from openssl#771)
Zeitstempel.dfn.de currently fails verification due to a mismatch in signatures (sha1 vs. sha2) in the new configuation of their server. See openssl/openssl#771
9 participants
This patch introduces new .conf variables (ess_cert_id_v2 and ess_cert_id_v2_alg) and adds support for ESSCertIDv2 to ts module as defined in RFC5816, thus it removes another hardcoded SHA-1 usage from ts module.
Original behavior (ESSCertID) is preserved.