Skip to content

Commit

Permalink
respect cert TTL in client side for k8s RA (istio#34484)
Browse files Browse the repository at this point in the history
* respect cert TTL in client side for k8s RA

* fix lint

* address review comments

* move constant def into chiron
  • Loading branch information
irisdingbj authored Aug 17, 2021
1 parent 8d71acb commit af892f1
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 18 deletions.
2 changes: 1 addition & 1 deletion pilot/pkg/bootstrap/certcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func (s *Server) initDNSCerts(hostname, namespace string) error {
if features.PilotCertProvider == constants.CertProviderKubernetes {
log.Infof("Generating K8S-signed cert for %v", s.dnsNames)
certChain, keyPEM, _, err = chiron.GenKeyCertK8sCA(s.kubeClient,
strings.Join(s.dnsNames, ","), hostnamePrefix+".csr.secret", namespace, defaultCACertPath, "", true)
strings.Join(s.dnsNames, ","), hostnamePrefix+".csr.secret", namespace, defaultCACertPath, "", true, SelfSignedCACertTTL.Get())
if err != nil {
return fmt.Errorf("failed generating key and cert by kubernetes: %v", err)
}
Expand Down
11 changes: 10 additions & 1 deletion pkg/test/csrctrl/controllers/csr_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

"istio.io/istio/pkg/test/csrctrl/signer"
"istio.io/istio/security/pkg/k8s/chiron"
"istio.io/istio/security/pkg/pki/util"
"istio.io/pkg/log"
)
Expand Down Expand Up @@ -76,7 +77,15 @@ func (r *CertificateSigningRequestSigningReconciler) Reconcile(_ context.Context
if signer, ok = r.Signers[csr.Spec.SignerName]; !ok {
return ctrl.Result{}, fmt.Errorf("error no signer can sign this csr: %v", err)
}
cert, err := signer.Sign(x509cr, csr.Spec.Usages)
requestedLifeTime := signer.CertTTL
requestedDuration, ok := csr.Annotations[chiron.RequestLifeTimeAnnotationForCertManager]
if ok {
duration, err := time.ParseDuration(requestedDuration)
if err == nil {
requestedLifeTime = duration
}
}
cert, err := signer.Sign(x509cr, csr.Spec.Usages, requestedLifeTime)
if err != nil {
return ctrl.Result{}, fmt.Errorf("error auto signing csr: %v", err)
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/test/csrctrl/signer/signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (

type Signer struct {
caProvider *caProvider
certTTL time.Duration
CertTTL time.Duration
}

func NewSigner(signerRoot, signerName string, certificateDuration time.Duration) (*Signer, error) {
Expand All @@ -41,18 +41,18 @@ func NewSigner(signerRoot, signerName string, certificateDuration time.Duration)

ret := &Signer{
caProvider: caProvider,
certTTL: certificateDuration,
CertTTL: certificateDuration,
}
return ret, nil
}

func (s *Signer) Sign(x509cr *x509.CertificateRequest, usages []capi.KeyUsage) ([]byte, error) {
func (s *Signer) Sign(x509cr *x509.CertificateRequest, usages []capi.KeyUsage, requestedLifetime time.Duration) ([]byte, error) {
currCA, err := s.caProvider.currentCA()
if err != nil {
return nil, err
}
der, err := currCA.Sign(x509cr.Raw, authority.PermissiveSigningPolicy{
TTL: s.certTTL,
TTL: requestedLifetime,
Usages: usages,
})
if err != nil {
Expand Down
6 changes: 4 additions & 2 deletions security/pkg/k8s/chiron/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,9 @@ func (wc *WebhookController) upsertSecret(secretName, dnsName, secretNamespace s
return nil
}

requestedLifetime := time.Duration(0)
// Now we know the secret does not exist yet. So we create a new one.
chain, key, caCert, err := GenKeyCertK8sCA(wc.clientset, dnsName, secretName, secretNamespace, wc.k8sCaCertFile, wc.certIssuer, true)
chain, key, caCert, err := GenKeyCertK8sCA(wc.clientset, dnsName, secretName, secretNamespace, wc.k8sCaCertFile, wc.certIssuer, true, requestedLifetime)
if err != nil {
log.Errorf("failed to generate key and certificate for secret %v in namespace %v (error %v)",
secretName, secretNamespace, err)
Expand Down Expand Up @@ -327,7 +328,8 @@ func (wc *WebhookController) refreshSecret(scrt *v1.Secret) error {
return fmt.Errorf("failed to find the service name for the secret (%v) to refresh", scrtName)
}

chain, key, caCert, err := GenKeyCertK8sCA(wc.clientset, dnsName, scrtName, namespace, wc.k8sCaCertFile, wc.certIssuer, true)
requestedLifetime := time.Duration(0)
chain, key, caCert, err := GenKeyCertK8sCA(wc.clientset, dnsName, scrtName, namespace, wc.k8sCaCertFile, wc.certIssuer, true, requestedLifetime)
if err != nil {
return err
}
Expand Down
19 changes: 14 additions & 5 deletions security/pkg/k8s/chiron/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ import (
const (
randomLength = 18
csrRetriesMax = 3
// cert-manager use below annotation on kubernetes CSR to control TTL for the generated cert.
RequestLifeTimeAnnotationForCertManager = "experimental.cert-manager.io/request-duration"
)

type CsrNameGenerator func(string, string) string
Expand All @@ -58,7 +60,7 @@ func GenCsrName() string {
// 2. Call SignCSRK8sCA to finish rest of the flow
func GenKeyCertK8sCA(client clientset.Interface, dnsName,
secretName, secretNamespace, caFilePath string,
signerName string, approveCsr bool) ([]byte, []byte, []byte, error) {
signerName string, approveCsr bool, requestedLifetime time.Duration) ([]byte, []byte, []byte, error) {
// 1. Generate a CSR

options := util.CertOptions{
Expand All @@ -82,7 +84,7 @@ func GenKeyCertK8sCA(client clientset.Interface, dnsName,
signerName = "kubernetes.io/legacy-unknown"
}
certChain, caCert, err := SignCSRK8s(client, csrPEM,
signerName, nil, usages, dnsName, caFilePath, approveCsr, true)
signerName, nil, usages, dnsName, caFilePath, approveCsr, true, requestedLifetime)

return certChain, keyPEM, caCert, err
}
Expand All @@ -96,13 +98,13 @@ func SignCSRK8s(client clientset.Interface,
csrData []byte, signerName string, requestedDuration *time.Duration,
usages []certv1.KeyUsage,
dnsName, caFilePath string,
approveCsr bool, appendCaCert bool) ([]byte, []byte, error) {
approveCsr bool, appendCaCert bool, requestedLifetime time.Duration) ([]byte, []byte, error) {
var err error
var v1Req bool = false

// 1. Submit the CSR

csrName, v1CsrReq, v1Beta1CsrReq, err := submitCSR(client, csrData, signerName, usages, csrRetriesMax)
csrName, v1CsrReq, v1Beta1CsrReq, err := submitCSR(client, csrData, signerName, usages, csrRetriesMax, requestedLifetime)
if err != nil {
return nil, nil, fmt.Errorf("unable to submit CSR request (%v). Error: %v", csrName, err)
}
Expand Down Expand Up @@ -189,7 +191,8 @@ func reloadCACert(wc *WebhookController) (bool, error) {

func submitCSR(clientset clientset.Interface,
csrData []byte, signerName string,
usages []certv1.KeyUsage, numRetries int) (string, *certv1.CertificateSigningRequest, *certv1beta1.CertificateSigningRequest, error) {
usages []certv1.KeyUsage, numRetries int, requestedLifetime time.Duration) (string, *certv1.CertificateSigningRequest,
*certv1beta1.CertificateSigningRequest, error) {
var err error = fmt.Errorf("unable to submit csr")
var useV1 bool = true
var csrName string = ""
Expand All @@ -211,6 +214,9 @@ func submitCSR(clientset clientset.Interface,
SignerName: signerName,
},
}
if requestedLifetime != time.Duration(0) {
csr.ObjectMeta.Annotations = map[string]string{RequestLifeTimeAnnotationForCertManager: requestedLifetime.String()}
}
v1req, err := clientset.CertificatesV1().CertificateSigningRequests().Create(context.TODO(), csr, metav1.CreateOptions{})
if err == nil {
return csrName, v1req, nil, nil
Expand Down Expand Up @@ -239,6 +245,9 @@ func submitCSR(clientset clientset.Interface,
for _, usage := range usages {
v1beta1csr.Spec.Usages = append(v1beta1csr.Spec.Usages, certv1beta1.KeyUsage(usage))
}
if requestedLifetime != time.Duration(0) {
v1beta1csr.ObjectMeta.Annotations = map[string]string{RequestLifeTimeAnnotationForCertManager: requestedLifetime.String()}
}
// create v1beta1 certificate request
v1beta1req, err := clientset.CertificatesV1beta1().CertificateSigningRequests().Create(context.TODO(), v1beta1csr, metav1.CreateOptions{})
if err == nil {
Expand Down
5 changes: 3 additions & 2 deletions security/pkg/k8s/chiron/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ SpAJos6OfJqyok7JXDdOYRDD5/hBerj68R9llWzNJd27/1jZ0NF2sIE1W4QFddy/
e+5z6MTAO6ktvHdQlSuH6ARn47bJrZOlkttAhg==
-----END CERTIFICATE-----
`
DefaulCertTTL = 24 * time.Hour
)

type mockTLSServer struct {
Expand Down Expand Up @@ -127,7 +128,7 @@ func TestGenKeyCertK8sCA(t *testing.T) {
}

_, _, _, err = GenKeyCertK8sCA(wc.clientset, tc.dnsNames[0], tc.secretNames[0],
tc.serviceNamespaces[0], wc.k8sCaCertFile, "testSigner", true)
tc.serviceNamespaces[0], wc.k8sCaCertFile, "testSigner", true, DefaulCertTTL)
if tc.expectFail {
if err == nil {
t.Errorf("should have failed")
Expand Down Expand Up @@ -387,7 +388,7 @@ func TestSubmitCSR(t *testing.T) {
}

_, r, _, err := submitCSR(wc.clientset, []byte("test-pem"), "test-signer",
usages, numRetries)
usages, numRetries, DefaulCertTTL)
if tc.expectFail {
if err == nil {
t.Errorf("test case (%s) should have failed", tcName)
Expand Down
9 changes: 6 additions & 3 deletions security/pkg/pki/ra/k8s_ra.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package ra

import (
"fmt"
"time"

cert "k8s.io/api/certificates/v1"
clientset "k8s.io/client-go/kubernetes"
Expand Down Expand Up @@ -47,7 +48,8 @@ func NewKubernetesRA(raOpts *IstioRAOptions) (*KubernetesRA, error) {
return istioRA, nil
}

func (r *KubernetesRA) kubernetesSign(csrPEM []byte, caCertFile string, certSigner string) ([]byte, error) {
func (r *KubernetesRA) kubernetesSign(csrPEM []byte, caCertFile string, certSigner string,
requestedLifetime time.Duration) ([]byte, error) {
certSignerDomain := r.raOpts.CertSignerDomain
if certSignerDomain == "" && certSigner != "" {
return nil, raerror.NewError(raerror.CertGenError, fmt.Errorf("certSignerDomain is requiered for signer %s", certSigner))
Expand All @@ -64,7 +66,7 @@ func (r *KubernetesRA) kubernetesSign(csrPEM []byte, caCertFile string, certSign
cert.UsageClientAuth,
}
certChain, _, err := chiron.SignCSRK8s(r.csrInterface, csrPEM, certSigner,
nil, usages, "", caCertFile, true, false)
nil, usages, "", caCertFile, true, false, requestedLifetime)
if err != nil {
return nil, raerror.NewError(raerror.CertGenError, err)
}
Expand All @@ -78,7 +80,8 @@ func (r *KubernetesRA) Sign(csrPEM []byte, certOpts ca.CertOpts) ([]byte, error)
return nil, err
}
certSigner := certOpts.CertSigner
return r.kubernetesSign(csrPEM, r.raOpts.CaCertFile, certSigner)

return r.kubernetesSign(csrPEM, r.raOpts.CaCertFile, certSigner, certOpts.TTL)
}

// SignWithCertChain is similar to Sign but returns the leaf cert and the entire cert chain.
Expand Down

0 comments on commit af892f1

Please sign in to comment.