feat: support crls in client traffic policies #7199
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7199 +/- ##
==========================================
- Coverage 71.14% 71.06% -0.09%
==========================================
Files 228 228
Lines 40827 40896 +69
==========================================
+ Hits 29047 29061 +14
- Misses 10076 10128 +52
- Partials 1704 1707 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
rudrakhp
force-pushed
the
feat_crl
branch
3 times, most recently
from
451c9ed to
353110f
Compare
| conditions: | ||
| - lastTransitionTime: null | ||
| message: |- | ||
| TLS: caCertificateRef secret [tls-secret-1] not found |
There was a problem hiding this comment.
I think this error was incorrect since I do see tls-secret-1:
| } | ||
|
|
||
| func validateCertificate(data []byte) error { | ||
| block, _ := pem.Decode(data) |
There was a problem hiding this comment.
This is a partial way to decode cert that was leading to inconsistency in parsing certificate depending on which block was being decoded first.
There was a problem hiding this comment.
If I'm understanding the code right, decodePemForBlockType only returns the first block matching blockType. In the case of multiple concatenated CA certificates or CRLs, won't validateCertificate and validateCrl only validate the first one and ignore the rest?
There was a problem hiding this comment.
You are right, we were validating only one block earlier so maintained the same behavior. But we should ideally be validating all the blocks.
There was a problem hiding this comment.
@crinjes updated the method to decode and append all certs before parsing
rudrakhp
force-pushed
the
feat_crl
branch
2 times, most recently
from
ea2e484 to
8311467
Compare
|
/retest |
rudrakhp
force-pushed
the
feat_crl
branch
5 times, most recently
from
2ac7d7c to
1a81709
Compare
Signed-off-by: Rudrakh Panigrahi <[email protected]>
|
|
||
| if tlsParams.ClientValidation.Crl != nil { | ||
| for _, crlRef := range tlsParams.ClientValidation.Crl.Refs { | ||
| crlRefKind := string(ptr.Deref(crlRef.Kind, resource.KindSecret)) |
There was a problem hiding this comment.
is it possible to dedup this logic and caCert data logic using a common func ?
4 participants
What type of PR is this?
feat: support crls in client traffic policies
What this PR does / why we need it:
Implement #6955
Which issue(s) this PR fixes:
Fixes #3021
Release Notes: Yes