Skip to content

Commit

Permalink
Support multiple root certs for multicluster self-signed Citadel (ist…
Browse files Browse the repository at this point in the history
  • Loading branch information
lei-tang authored and istio-testing committed Mar 15, 2019
1 parent 3ee929c commit fed6b1f
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 14 deletions.
6 changes: 4 additions & 2 deletions security/cmd/istio_ca/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,8 @@ func init() {
flags.StringVar(&opts.certChainFile, "cert-chain", "", "Path to the certificate chain file")
flags.StringVar(&opts.signingCertFile, "signing-cert", "", "Path to the CA signing certificate file")
flags.StringVar(&opts.signingKeyFile, "signing-key", "", "Path to the CA signing key file")

// Both self-signed or non-self-signed Citadel may take a root certificate file with a list of root certificates.
flags.StringVar(&opts.rootCertFile, "root-cert", "", "Path to the root certificate file")

// Configuration if Citadel acts as a self signed CA.
Expand Down Expand Up @@ -286,7 +288,7 @@ func runCA() {
sc, err := controller.NewSecretController(ca,
opts.workloadCertTTL,
opts.workloadCertGracePeriodRatio, opts.workloadCertMinGracePeriod, opts.dualUse,
cs.CoreV1(), opts.signCACerts, opts.listenedNamespace, webhooks)
cs.CoreV1(), opts.signCACerts, opts.listenedNamespace, webhooks, opts.rootCertFile)
if err != nil {
fatalf("Failed to create secret controller: %v", err)
}
Expand Down Expand Up @@ -399,7 +401,7 @@ func createCA(client corev1.CoreV1Interface) *ca.IstioCA {
}
caOpts, err = ca.NewSelfSignedIstioCAOptions(ctx, opts.selfSignedCACertTTL, opts.workloadCertTTL,
opts.maxWorkloadCertTTL, spiffe.GetTrustDomain(), opts.dualUse,
opts.istioCaStorageNamespace, checkInterval, client)
opts.istioCaStorageNamespace, checkInterval, client, opts.rootCertFile)
if err != nil {
fatalf("Failed to create a self-signed Citadel (error: %v)", err)
}
Expand Down
2 changes: 1 addition & 1 deletion security/pkg/k8s/controller/workloadsecret.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ type SecretController struct {
func NewSecretController(ca ca.CertificateAuthority, certTTL time.Duration,
gracePeriodRatio float32, minGracePeriod time.Duration, dualUse bool,
core corev1.CoreV1Interface, forCA bool, namespace string,
dnsNames map[string]*DNSNameEntry) (*SecretController, error) {
dnsNames map[string]*DNSNameEntry, rootCertFile string) (*SecretController, error) {

if gracePeriodRatio < 0 || gracePeriodRatio > 1 {
return nil, fmt.Errorf("grace period ratio %f should be within [0, 1]", gracePeriodRatio)
Expand Down
9 changes: 5 additions & 4 deletions security/pkg/k8s/controller/workloadsecret_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func TestSecretController(t *testing.T) {
}
controller, err := NewSecretController(createFakeCA(), defaultTTL,
tc.gracePeriodRatio, defaultMinGracePeriod, false, client.CoreV1(), false,
metav1.NamespaceAll, webhooks)
metav1.NamespaceAll, webhooks, "")
if tc.shouldFail {
if err == nil {
t.Errorf("should have failed to create secret controller")
Expand Down Expand Up @@ -179,7 +179,7 @@ func TestSecretContent(t *testing.T) {
client := fake.NewSimpleClientset()
controller, err := NewSecretController(createFakeCA(), defaultTTL,
defaultGracePeriodRatio, defaultMinGracePeriod, false, client.CoreV1(), false,
metav1.NamespaceAll, map[string]*DNSNameEntry{})
metav1.NamespaceAll, map[string]*DNSNameEntry{}, "")
if err != nil {
t.Errorf("Failed to create secret controller: %v", err)
}
Expand All @@ -201,7 +201,7 @@ func TestDeletedIstioSecret(t *testing.T) {
client := fake.NewSimpleClientset()
controller, err := NewSecretController(createFakeCA(), defaultTTL,
defaultGracePeriodRatio, defaultMinGracePeriod, false, client.CoreV1(), false,
metav1.NamespaceAll, nil)
metav1.NamespaceAll, nil, "")
if err != nil {
t.Errorf("failed to create secret controller: %v", err)
}
Expand Down Expand Up @@ -319,7 +319,8 @@ func TestUpdateSecret(t *testing.T) {
for k, tc := range testCases {
client := fake.NewSimpleClientset()
controller, err := NewSecretController(createFakeCA(), time.Hour,
tc.gracePeriodRatio, tc.minGracePeriod, false, client.CoreV1(), false, metav1.NamespaceAll, nil)
tc.gracePeriodRatio, tc.minGracePeriod, false, client.CoreV1(), false, metav1.NamespaceAll,
nil, "")
if err != nil {
t.Errorf("failed to create secret controller: %v", err)
}
Expand Down
40 changes: 37 additions & 3 deletions security/pkg/pki/ca/ca.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"encoding/pem"
"fmt"
"io/ioutil"
"strings"
"time"

v1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -101,9 +102,33 @@ type IstioCA struct {
livenessProbe *probe.Probe
}

// Append root certificates in rootCertFile to the input certificate.
func appendRootCerts(pemCert []byte, rootCertFile string) ([]byte, error) {
var rootCerts []byte
if len(pemCert) > 0 {
// Copy the input certificate
rootCerts = make([]byte, len(pemCert))
copy(rootCerts, pemCert)
}
if len(rootCertFile) > 0 {
log.Debugf("append root certificates from %v", rootCertFile)
certBytes, err := ioutil.ReadFile(rootCertFile)
if err != nil {
return rootCerts, fmt.Errorf("failed to read root certificates (%v)", err)
}
log.Debugf("The root certificates to be appended is: %v", rootCertFile[:])
if len(rootCerts) > 0 {
// Append a newline after the last cert
rootCerts = []byte(strings.TrimSuffix(string(rootCerts[:]), "\n") + "\n")
}
rootCerts = append(rootCerts, certBytes...)
}
return rootCerts, nil
}

// NewSelfSignedIstioCAOptions returns a new IstioCAOptions instance using self-signed certificate.
func NewSelfSignedIstioCAOptions(ctx context.Context, caCertTTL, certTTL, maxCertTTL time.Duration, org string, dualUse bool,
namespace string, readCertRetryInterval time.Duration, client corev1.CoreV1Interface) (caOpts *IstioCAOptions, err error) {
namespace string, readCertRetryInterval time.Duration, client corev1.CoreV1Interface, rootCertFile string) (caOpts *IstioCAOptions, err error) {
// For the first time the CA is up, if readSigningCertOnly is unset,
// it generates a self-signed key/cert pair and write it to CASecret.
// For subsequent restart, CA will reads key/cert from CASecret.
Expand Down Expand Up @@ -146,7 +171,12 @@ func NewSelfSignedIstioCAOptions(ctx context.Context, caCertTTL, certTTL, maxCer
return nil, fmt.Errorf("unable to generate CA cert and key for self-signed CA (%v)", ckErr)
}

if caOpts.KeyCertBundle, err = util.NewVerifiedKeyCertBundleFromPem(pemCert, pemKey, nil, pemCert); err != nil {
rootCerts, err := appendRootCerts(pemCert, rootCertFile)
if err != nil {
return nil, fmt.Errorf("failed to append root certificates (%v)", err)
}

if caOpts.KeyCertBundle, err = util.NewVerifiedKeyCertBundleFromPem(pemCert, pemKey, nil, rootCerts); err != nil {
return nil, fmt.Errorf("failed to create CA KeyCertBundle (%v)", err)
}

Expand All @@ -157,8 +187,12 @@ func NewSelfSignedIstioCAOptions(ctx context.Context, caCertTTL, certTTL, maxCer
}
} else {
log.Infof("Load signing key and cert from existing secret %s:%s", caSecret.Namespace, caSecret.Name)
rootCerts, err := appendRootCerts(caSecret.Data[caCertID], rootCertFile)
if err != nil {
return nil, fmt.Errorf("failed to append root certificates (%v)", err)
}
if caOpts.KeyCertBundle, err = util.NewVerifiedKeyCertBundleFromPem(caSecret.Data[caCertID],
caSecret.Data[caPrivateKeyID], nil, caSecret.Data[caCertID]); err != nil {
caSecret.Data[caPrivateKeyID], nil, rootCerts); err != nil {
return nil, fmt.Errorf("failed to create CA KeyCertBundle (%v)", err)
}
}
Expand Down
43 changes: 39 additions & 4 deletions security/pkg/pki/ca/ca_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,10 @@ func TestCreateSelfSignedIstioCAWithoutSecret(t *testing.T) {
dualUse := false
caNamespace := "default"
client := fake.NewSimpleClientset()
rootCertFile := ""

caopts, err := NewSelfSignedIstioCAOptions(context.Background(), caCertTTL, defaultCertTTL, maxCertTTL,
org, dualUse, caNamespace, -1, client.CoreV1())
org, dualUse, caNamespace, -1, client.CoreV1(), rootCertFile)
if err != nil {
t.Fatalf("Failed to create a self-signed CA Options: %v", err)
}
Expand Down Expand Up @@ -180,9 +181,10 @@ func TestCreateSelfSignedIstioCAWithSecret(t *testing.T) {
org := "test.ca.org"
caNamespace := "default"
dualUse := false
rootCertFile := ""

caopts, err := NewSelfSignedIstioCAOptions(context.Background(), caCertTTL, certTTL, maxCertTTL,
org, dualUse, caNamespace, -1, client.CoreV1())
org, dualUse, caNamespace, -1, client.CoreV1(), rootCertFile)
if err != nil {
t.Fatalf("Failed to create a self-signed CA Options: %v", err)
}
Expand Down Expand Up @@ -242,6 +244,7 @@ func TestCreateSelfSignedIstioCAReadSigningCertOnly(t *testing.T) {
org := "test.ca.org"
caNamespace := "default"
dualUse := false
rootCertFile := ""

client := fake.NewSimpleClientset()

Expand All @@ -250,7 +253,7 @@ func TestCreateSelfSignedIstioCAReadSigningCertOnly(t *testing.T) {
ctx0, cancel0 := context.WithTimeout(context.Background(), time.Millisecond*50)
defer cancel0()
_, err := NewSelfSignedIstioCAOptions(ctx0, caCertTTL, certTTL, maxCertTTL,
org, dualUse, caNamespace, time.Millisecond*10, client.CoreV1())
org, dualUse, caNamespace, time.Millisecond*10, client.CoreV1(), rootCertFile)
if err == nil {
t.Errorf("Expected error, but succeeded.")
}
Expand All @@ -269,7 +272,7 @@ func TestCreateSelfSignedIstioCAReadSigningCertOnly(t *testing.T) {
ctx1, cancel1 := context.WithCancel(context.Background())
defer cancel1()
caopts, err := NewSelfSignedIstioCAOptions(ctx1, caCertTTL, certTTL, maxCertTTL,
org, dualUse, caNamespace, time.Millisecond*10, client.CoreV1())
org, dualUse, caNamespace, time.Millisecond*10, client.CoreV1(), rootCertFile)
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
Expand Down Expand Up @@ -498,6 +501,38 @@ func TestSignCSRTTLError(t *testing.T) {
}
}

func TestAppendRootCerts(t *testing.T) {
root1 := "root-cert-1"
expRootCerts := `root-cert-1
root-cert-2
root-cert-3`
rootCerts, err := appendRootCerts([]byte(root1), "./root-certs-for-testing.pem")
if err != nil {
t.Errorf("appendRootCerts() returns an error: %v", err)
} else {
if expRootCerts != string(rootCerts[:]) {
t.Errorf("the root certificates do not match. Expect:%v. Actual:%v.",
expRootCerts, string(rootCerts[:]))
}
}
}

func TestAppendRootCertsToNullCert(t *testing.T) {
// nil certificate
var root1 []byte
expRootCerts := `root-cert-2
root-cert-3`
rootCerts, err := appendRootCerts(root1, "./root-certs-for-testing.pem")
if err != nil {
t.Errorf("appendRootCerts() returns an error: %v", err)
} else {
if expRootCerts != string(rootCerts[:]) {
t.Errorf("the root certificates do not match. Expect:%v. Actual:%v.",
expRootCerts, string(rootCerts[:]))
}
}
}

// nolint: unparam
func createCA(maxTTL time.Duration, multicluster bool) (*IstioCA, error) {
// Generate root CA key and cert.
Expand Down
2 changes: 2 additions & 0 deletions security/pkg/pki/ca/root-certs-for-testing.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
root-cert-2
root-cert-3

0 comments on commit fed6b1f

Please sign in to comment.