Verify delegated & CA signed OCSP correctly #736
Conversation
|
lgtm, but please rebase on top of the master. |
|
@lziest, done |
|
@vanbroup I've fixed our tests on master; can you rebase? |
|
When resp.Certificarte is present it should contain a delegated signing certificate (not the issuer, you should already have that). When it doesn't contain a delegated signing certificate the response should not be delegated and be signed by the issuer directly. I started thinking why ParseResponseForCert in "golang.org/x/crypto/ocsp" would not already verify the signature. After verification I concluded that it does already verify the signature. I think it makes more sense to remove the OCSP signature verification from CFSSL as it has no value to do it twice. https://github.com/golang/crypto/blob/master/ocsp/ocsp.go#L524 |
Both ParseResponse and ParseResponseForCert in "golang.org/x/crypto/ocsp" do already verify the response and embedded certificates when present. Previous OCSP signature validation in this package was done incorrectly and would only be performed when ParseResponse would have verified the signature with no errors. By using ParseResponseForCert instead of ParseResponse also OCSP responses containing multiple awnsers can be handled successfully.
Codecov Report
@@ Coverage Diff @@
## master #736 +/- ##
==========================================
- Coverage 57.77% 57.71% -0.06%
==========================================
Files 76 77 +1
Lines 6643 6769 +126
==========================================
+ Hits 3838 3907 +69
- Misses 2391 2463 +72
+ Partials 414 399 -15
Continue to review full report at Codecov.
|
|
Much appreciated. |
Only check the OCSP response signature from the issuer when no delegated signing certificate is included in the response. When a delegated OCSP signing certificate is present verify this is issued by the same CA.