diff --git a/cmd/controller/app/controller.go b/cmd/controller/app/controller.go index ad44961f9..b230fec25 100644 --- a/cmd/controller/app/controller.go +++ b/cmd/controller/app/controller.go @@ -231,7 +231,11 @@ func buildControllerContext(ctx context.Context, stopCh <-chan struct{}, opts *o DefaultAutoCertificateAnnotations: opts.DefaultAutoCertificateAnnotations, }, CertificateOptions: controller.CertificateOptions{ - EnableOwnerRef: opts.EnableCertificateOwnerRef, + EnableOwnerRef: opts.EnableCertificateOwnerRef, + ExperimentalIssuePKCS12: opts.ExperimentalIssuePKCS12, + ExperimentalPKCS12KeystorePassword: opts.ExperimentalPKCS12KeystorePassword, + ExperimentalIssueJKS: opts.ExperimentalIssueJKS, + ExperimentalJKSPassword: opts.ExperimentalJKSPassword, }, SchedulerOptions: controller.SchedulerOptions{ MaxConcurrentChallenges: opts.MaxConcurrentChallenges, diff --git a/pkg/controller/certificaterequests/sync_test.go b/pkg/controller/certificaterequests/sync_test.go index 6ba8d6226..96b10de1f 100644 --- a/pkg/controller/certificaterequests/sync_test.go +++ b/pkg/controller/certificaterequests/sync_test.go @@ -378,7 +378,7 @@ func TestSync(t *testing.T) { gen.SetCertificateRequestCertificate([]byte("a bad certificate")), )}, ExpectedEvents: []string{ - "Warning DecodeError Failed to decode returned certificate: error decoding cert PEM block", + "Warning DecodeError Failed to decode returned certificate: error decoding certificate PEM block", }, ExpectedActions: []testpkg.Action{ testpkg.NewAction(coretesting.NewUpdateSubresourceAction( @@ -391,7 +391,7 @@ func TestSync(t *testing.T) { Type: cmapi.CertificateRequestConditionReady, Status: cmmeta.ConditionFalse, Reason: "Failed", - Message: "Failed to decode returned certificate: error decoding cert PEM block", + Message: "Failed to decode returned certificate: error decoding certificate PEM block", LastTransitionTime: &nowMetaTime, }), gen.SetCertificateRequestFailureTime(nowMetaTime), diff --git a/pkg/controller/certificates/BUILD.bazel b/pkg/controller/certificates/BUILD.bazel index 868dee8dc..c582d5ce1 100644 --- a/pkg/controller/certificates/BUILD.bazel +++ b/pkg/controller/certificates/BUILD.bazel @@ -46,6 +46,7 @@ go_library( go_test( name = "go_default_test", srcs = [ + "keystore_test.go", "sync_test.go", "util_test.go", ], @@ -59,6 +60,8 @@ go_test( "//pkg/util:go_default_library", "//pkg/util/pki:go_default_library", "//test/unit/gen:go_default_library", + "@com_github_pavel_v_chernykh_keystore_go//:go_default_library", + "@com_sslmate_software_src_go_pkcs12//:go_default_library", "@io_k8s_api//core/v1:go_default_library", "@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library", "@io_k8s_apimachinery//pkg/runtime:go_default_library", diff --git a/pkg/controller/certificates/controller.go b/pkg/controller/certificates/controller.go index 4ca3402f9..3d4602b06 100644 --- a/pkg/controller/certificates/controller.go +++ b/pkg/controller/certificates/controller.go @@ -173,7 +173,7 @@ func (c *certificateRequestManager) Register(ctx *controllerpkg.Context) (workqu c.experimentalIssueJKS = ctx.CertificateOptions.ExperimentalIssueJKS c.experimentalJKSPassword = ctx.CertificateOptions.ExperimentalJKSPassword if c.experimentalIssueJKS && len(c.experimentalJKSPassword) == 0 { - return nil, nil, nil, fmt.Errorf("if experimental pkcs12 issuance is enabled, a keystore password must be provided") + return nil, nil, nil, fmt.Errorf("if experimental jks issuance is enabled, a keystore password must be provided") } c.cmClient = ctx.CMClient diff --git a/pkg/controller/certificates/keystore.go b/pkg/controller/certificates/keystore.go index e805b441f..8e47d21dc 100644 --- a/pkg/controller/certificates/keystore.go +++ b/pkg/controller/certificates/keystore.go @@ -102,7 +102,7 @@ func encodeJKSKeystore(password string, rawKey []byte, certPem []byte, caPem []b } ks := jks.KeyStore{ - "certificate": jks.PrivateKeyEntry{ + "certificate": &jks.PrivateKeyEntry{ Entry: jks.Entry{ CreationDate: time.Now(), }, @@ -117,7 +117,7 @@ func encodeJKSKeystore(password string, rawKey []byte, certPem []byte, caPem []b return nil, err } - ks["ca"] = jks.TrustedCertificateEntry{ + ks["ca"] = &jks.TrustedCertificateEntry{ Entry: jks.Entry{ CreationDate: time.Now(), }, diff --git a/pkg/controller/certificates/keystore_test.go b/pkg/controller/certificates/keystore_test.go new file mode 100644 index 000000000..78b0ef39f --- /dev/null +++ b/pkg/controller/certificates/keystore_test.go @@ -0,0 +1,225 @@ +/* +Copyright 2020 The Jetstack cert-manager contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package certificates + +import ( + "bytes" + "testing" + + jks "github.com/pavel-v-chernykh/keystore-go" + "software.sslmate.com/src/go-pkcs12" + + cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha2" + "github.com/jetstack/cert-manager/pkg/util/pki" +) + +func mustGeneratePrivateKey(t *testing.T, encoding cmapi.KeyEncoding) []byte { + pk, err := pki.GenerateRSAPrivateKey(2048) + if err != nil { + t.Fatal(err) + } + pkBytes, err := pki.EncodePrivateKey(pk, encoding) + if err != nil { + t.Fatal(err) + } + return pkBytes +} + +func mustSelfSignCertificate(t *testing.T, pkBytes []byte) []byte { + if pkBytes == nil { + pkBytes = mustGeneratePrivateKey(t, cmapi.PKCS8) + } + pk, err := pki.DecodePrivateKeyBytes(pkBytes) + if err != nil { + t.Fatal(err) + } + x509Crt, err := pki.GenerateTemplate(&cmapi.Certificate{ + Spec: cmapi.CertificateSpec{ + DNSNames: []string{"example.com"}, + }, + }) + if err != nil { + t.Fatal(err) + } + certBytes, _, err := pki.SignCertificate(x509Crt, x509Crt, pk.Public(), pk) + if err != nil { + t.Fatal(err) + } + return certBytes +} + +func TestEncodeJKSKeystore(t *testing.T) { + tests := map[string]struct { + password string + rawKey, certPEM, caPEM []byte + verify func(t *testing.T, out []byte, err error) + }{ + "encode a JKS bundle for a PKCS1 key and certificate only": { + password: "password", + rawKey: mustGeneratePrivateKey(t, cmapi.PKCS1), + certPEM: mustSelfSignCertificate(t, nil), + verify: func(t *testing.T, out []byte, err error) { + if err != nil { + t.Errorf("expected no error but got: %v", err) + return + } + buf := bytes.NewBuffer(out) + ks, err := jks.Decode(buf, []byte("password")) + if err != nil { + t.Errorf("error decoding keystore: %v", err) + return + } + if ks["certificate"] == nil { + t.Errorf("no certificate data found in keystore") + } + if ks["ca"] != nil { + t.Errorf("unexpected ca data found in keystore") + } + }, + }, + "encode a JKS bundle for a PKCS8 key and certificate only": { + password: "password", + rawKey: mustGeneratePrivateKey(t, cmapi.PKCS8), + certPEM: mustSelfSignCertificate(t, nil), + verify: func(t *testing.T, out []byte, err error) { + if err != nil { + t.Errorf("expected no error but got: %v", err) + } + buf := bytes.NewBuffer(out) + ks, err := jks.Decode(buf, []byte("password")) + if err != nil { + t.Errorf("error decoding keystore: %v", err) + return + } + if ks["certificate"] == nil { + t.Errorf("no certificate data found in keystore") + } + if ks["ca"] != nil { + t.Errorf("unexpected ca data found in keystore") + } + }, + }, + "encode a JKS bundle for a key, certificate and ca": { + password: "password", + rawKey: mustGeneratePrivateKey(t, cmapi.PKCS8), + certPEM: mustSelfSignCertificate(t, nil), + caPEM: mustSelfSignCertificate(t, nil), + verify: func(t *testing.T, out []byte, err error) { + if err != nil { + t.Errorf("expected no error but got: %v", err) + } + buf := bytes.NewBuffer(out) + ks, err := jks.Decode(buf, []byte("password")) + if err != nil { + t.Errorf("error decoding keystore: %v", err) + return + } + if ks["certificate"] == nil { + t.Errorf("no certificate data found in keystore") + } + if ks["ca"] == nil { + t.Errorf("no ca data found in keystore") + } + }, + }, + } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + out, err := encodeJKSKeystore(test.password, test.rawKey, test.certPEM, test.caPEM) + test.verify(t, out, err) + }) + } +} + +func TestEncodePKCS12Keystore(t *testing.T) { + tests := map[string]struct { + password string + rawKey, certPEM, caPEM []byte + verify func(t *testing.T, out []byte, err error) + }{ + "encode a JKS bundle for a PKCS1 key and certificate only": { + password: "password", + rawKey: mustGeneratePrivateKey(t, cmapi.PKCS1), + certPEM: mustSelfSignCertificate(t, nil), + verify: func(t *testing.T, out []byte, err error) { + if err != nil { + t.Errorf("expected no error but got: %v", err) + } + pk, cert, err := pkcs12.Decode(out, "password") + if err != nil { + t.Errorf("error decoding keystore: %v", err) + return + } + if cert == nil { + t.Errorf("no certificate data found in keystore") + } + if pk == nil { + t.Errorf("no ca data found in keystore") + } + }, + }, + "encode a JKS bundle for a PKCS8 key and certificate only": { + password: "password", + rawKey: mustGeneratePrivateKey(t, cmapi.PKCS8), + certPEM: mustSelfSignCertificate(t, nil), + verify: func(t *testing.T, out []byte, err error) { + if err != nil { + t.Errorf("expected no error but got: %v", err) + } + pk, cert, err := pkcs12.Decode(out, "password") + if err != nil { + t.Errorf("error decoding keystore: %v", err) + return + } + if cert == nil { + t.Errorf("no certificate data found in keystore") + } + if pk == nil { + t.Errorf("no ca data found in keystore") + } + }, + }, + "encode a JKS bundle for a key, certificate and ca": { + password: "password", + rawKey: mustGeneratePrivateKey(t, cmapi.PKCS8), + certPEM: mustSelfSignCertificate(t, nil), + caPEM: mustSelfSignCertificate(t, nil), + verify: func(t *testing.T, out []byte, err error) { + if err != nil { + t.Errorf("expected no error but got: %v", err) + } + // The pkcs12 package does not expose a way to decode the CA + // data that has been written. + // It will return an error when attempting to decode a file + // with more than one 'certbag', so we just ensure the error + // returned is the expected error and don't inspect the keystore + // contents. + _, _, err = pkcs12.Decode(out, "password") + if err == nil || err.Error() != "pkcs12: expected exactly two safe bags in the PFX PDU" { + t.Errorf("unexpected error string, exp=%q, got=%v", "pkcs12: expected exactly two safe bags in the PFX PDU", err) + return + } + }, + }, + } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + out, err := encodePKCS12Keystore(test.password, test.rawKey, test.certPEM, test.caPEM) + test.verify(t, out, err) + }) + } +} diff --git a/pkg/util/pki/parse.go b/pkg/util/pki/parse.go index 046ad56c5..6fe6276cd 100644 --- a/pkg/util/pki/parse.go +++ b/pkg/util/pki/parse.go @@ -110,7 +110,7 @@ func DecodeX509CertificateChainBytes(certBytes []byte) ([]*x509.Certificate, err } if len(certs) == 0 { - return nil, errors.NewInvalidData("error decoding cert PEM block") + return nil, errors.NewInvalidData("error decoding certificate PEM block") } return certs, nil