Istio CA should return a cert chain instead of a single cert #59
Conversation
certmanager/ca.go
Outdated
|
|
||
| return NewIstioCA(cert, key) | ||
| opts := &IstioCAOptions{ | ||
| CertChainBytes: pemCert, |
There was a problem hiding this comment.
In self-signed root certificate case, you don't need to include the root cert here.
Basically the only certificates needed to be included are the one before root certificate, since the root certificate is already distributed to the other end.
https://tools.ietf.org/html/rfc5246#section-7.4.2
It is just an optimization, it is ok to include, but not necessary.
There was a problem hiding this comment.
Done. We are on longer sending root cert in the cert chain in self-signed case.
In non-self-signed case we have no control over what's in the chain file. So we'll blindly send over what's in the file.
| // Pass in unmatched chain and cert to make sure the `verify` method yeilds an error. | ||
| func TestInvalidIstioCAOptions(t *testing.T) { | ||
| rootCert := ` | ||
| -----BEGIN CERTIFICATE----- |
There was a problem hiding this comment.
Is there any timestamp in the cert? If so, would it still work when the cert expires?
There was a problem hiding this comment.
Updated the timestamps to make the certs long-lived so expiration shouldn't be an issue.
* Fix tests * Make testing certs long-lived
|
lgtm |
Codecov Report
@@ Coverage Diff @@
## master #59 +/- ##
==========================================
+ Coverage 68.45% 70.73% +2.28%
==========================================
Files 6 6
Lines 317 352 +35
==========================================
+ Hits 217 249 +32
- Misses 81 84 +3
Partials 19 19
Continue to review full report at Codecov.
|
Fix #56