Implement client cert auth

[wtzhang23]

Description

• For VMs, rotation of k8s service account token (istio-token) is not reliable
• Reuse logic in Istiod which uses the peer cert to identify the workload on refresh
• Fixes Istio sidecar can only request new cert using istio-token #279

[cert-manager-prow]

Hi @wtzhang23. Thanks for your PR.

I'm waiting for a cert-manager member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[SgtCoDFish]

/ok-to-test

[SgtCoDFish]

Hey, thanks for raising this! I think this makes sense and it's a well made PR!

I don't really know how to evaluate the security impact of this (I'm not an istio expert). I don't know if having client cert auth enabled by default is a safe thing in all cases. It seems fairly safe in this context, though, since obviously the admin will have chosen which certs go into the trust store to use for checking client certs.

I'm reluctant to enable this by default just because it expands the auth surface area and as said above I'm nervous of that.

Do you think we could add a boolean flag (and a corresponding Helm chart bool) which allows users to opt-in to client cert auth? I feel like then it's super easy to merge this feature.

[wtzhang23]

Istio's own managed CA does enable it by default here, however I agree it is something that would be useful. Will try to add it.

[wtzhang23]

Added the flag in a separate authenticators block in case there comes a need for more authenticators in the future. Istio seems to support based on the highlighted code:

• client cert
• k8s sa token
• jwt
• cidrs

as well as toggles that disable authentication in a trusted network (my guess is they only need the cert for multicluster / SPIFFE federation).

[SgtCoDFish]

/lgtm
/approve

Thank you for this! Really appreciate the work you've put into it!

With this being gated behind a flag I think this is a really safe bet to add. I'm hoping to do an istio-csr release pretty soon, so please do test it when the release is out 🚀

[cert-manager-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SgtCoDFish

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [SgtCoDFish]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment