From 3c084822b375d863699737d05f0f5a9a448e1a94 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Mon, 2 Oct 2023 13:30:35 +0200 Subject: [PATCH] Prevent invalid provisioner name on `step ca init` An unfortunate combination of `--provisioner acme` and the `--acme` flags on `step ca init` could lead to an invalidat CA configuration. This commit prevent this case from happening. A similar error could occur for the `sshpop` provisioner, so a fix was implemented for that case too. The fix doesn't catch all cases, e.g. it doesn't check for multiple provisioners having the same `acme-` or `sshpop-` prefix. The code that is called is intended to be only called from a `step ca init` invocation, so should work for these cases, but might not if the methods are invoked at other times. --- pki/helm.go | 28 ++++- pki/helm_test.go | 14 +++ pki/pki.go | 19 +++- ...th-acme-and-duplicate-provisioner-name.yml | 82 ++++++++++++++ ...ith-ssh-and-duplicate-provisioner-name.yml | 104 ++++++++++++++++++ 5 files changed, 241 insertions(+), 6 deletions(-) create mode 100644 pki/testdata/helm/with-acme-and-duplicate-provisioner-name.yml create mode 100644 pki/testdata/helm/with-ssh-and-duplicate-provisioner-name.yml diff --git a/pki/helm.go b/pki/helm.go index 72d95971..d2ecdf37 100644 --- a/pki/helm.go +++ b/pki/helm.go @@ -1,6 +1,7 @@ package pki import ( + "fmt" "io" "text/template" @@ -49,21 +50,42 @@ func (p *PKI) WriteHelmTemplate(w io.Writer) error { // to what's in p.GenerateConfig(), but that codepath isn't taken when // writing the Helm template. The default JWK provisioner is added earlier in // the process and that's part of the provisioners above. + // + // To prevent name clashes for the default ACME provisioner, we append "-1" to + // the name if it already exists. See https://github.com/smallstep/cli/issues/1018 + // for the reason. + // // TODO(hs): consider refactoring the initialization, so that this becomes // easier to reason about and maintain. if p.options.enableACME { + acmeProvisionerName := "acme" + for _, prov := range provisioners { + if prov.GetName() == acmeProvisionerName { + acmeProvisionerName = fmt.Sprintf("%s-1", acmeProvisionerName) + break + } + } provisioners = append(provisioners, &provisioner.ACME{ Type: "ACME", - Name: "acme", + Name: acmeProvisionerName, }) } // Add default SSHPOP provisioner if enabled. Similar to the above, this is - // the same as what happens in p.GenerateConfig(). + // the same as what happens in p.GenerateConfig(). To prevent name clashes for the + // default SSHPOP provisioner, we append "-1" to it if it already exists. See + // https://github.com/smallstep/cli/issues/1018 for the reason. if p.options.enableSSH { + sshProvisionerName := "sshpop" + for _, prov := range provisioners { + if prov.GetName() == sshProvisionerName { + sshProvisionerName = fmt.Sprintf("%s-1", sshProvisionerName) + break + } + } provisioners = append(provisioners, &provisioner.SSHPOP{ Type: "SSHPOP", - Name: "sshpop", + Name: sshProvisionerName, Claims: &provisioner.Claims{ EnableSSHCA: &p.options.enableSSH, }, diff --git a/pki/helm_test.go b/pki/helm_test.go index ea1c4acd..11c1d439 100644 --- a/pki/helm_test.go +++ b/pki/helm_test.go @@ -85,6 +85,13 @@ func TestPKI_WriteHelmTemplate(t *testing.T) { wantErr: false, } }, + "ok/with-acme-and-duplicate-provisioner-name": func(t *testing.T) test { + return test{ + pki: preparePKI(t, WithProvisioner("acme"), WithACME()), + testFile: "testdata/helm/with-acme-and-duplicate-provisioner-name.yml", + wantErr: false, + } + }, "ok/with-admin": func(t *testing.T) test { return test{ pki: preparePKI(t, WithAdmin()), @@ -99,6 +106,13 @@ func TestPKI_WriteHelmTemplate(t *testing.T) { wantErr: false, } }, + "ok/with-ssh-and-duplicate-provisioner-name": func(t *testing.T) test { + return test{ + pki: preparePKI(t, WithProvisioner("sshpop"), WithSSH()), + testFile: "testdata/helm/with-ssh-and-duplicate-provisioner-name.yml", + wantErr: false, + } + }, "ok/with-ssh-and-acme": func(t *testing.T) test { return test{ pki: preparePKI(t, WithSSH(), WithACME()), diff --git a/pki/pki.go b/pki/pki.go index 971c189b..bb299dd9 100644 --- a/pki/pki.go +++ b/pki/pki.go @@ -850,9 +850,16 @@ func (p *PKI) GenerateConfig(opt ...ConfigOption) (*authconfig.Config, error) { // Add default ACME provisioner if enabled if p.options.enableACME { + // To prevent name clashes for the default ACME provisioner, we append "-1" to + // the name if it already exists. See https://github.com/smallstep/cli/issues/1018 + // for the reason. + acmeProvisionerName := "acme" + if p.options.provisioner == acmeProvisionerName { + acmeProvisionerName = fmt.Sprintf("%s-1", acmeProvisionerName) + } provisioners = append(provisioners, &provisioner.ACME{ Type: "ACME", - Name: "acme", + Name: acmeProvisionerName, }) } @@ -867,10 +874,16 @@ func (p *PKI) GenerateConfig(opt ...ConfigOption) (*authconfig.Config, error) { EnableSSHCA: &enableSSHCA, } - // Add default SSHPOP provisioner + // Add default SSHPOP provisioner. To prevent name clashes for the default + // SSHPOP provisioner, we append "-1" to the name if it already exists. + // See https://github.com/smallstep/cli/issues/1018 for the reason. + sshProvisionerName := "sshpop" + if p.options.provisioner == sshProvisionerName { + sshProvisionerName = fmt.Sprintf("%s-1", sshProvisionerName) + } provisioners = append(provisioners, &provisioner.SSHPOP{ Type: "SSHPOP", - Name: "sshpop", + Name: sshProvisionerName, Claims: &provisioner.Claims{ EnableSSHCA: &enableSSHCA, }, diff --git a/pki/testdata/helm/with-acme-and-duplicate-provisioner-name.yml b/pki/testdata/helm/with-acme-and-duplicate-provisioner-name.yml new file mode 100644 index 00000000..f4532207 --- /dev/null +++ b/pki/testdata/helm/with-acme-and-duplicate-provisioner-name.yml @@ -0,0 +1,82 @@ +# Helm template +inject: + enabled: true + # Config contains the configuration files ca.json and defaults.json + config: + files: + ca.json: + root: /home/step/certs/root_ca.crt + federateRoots: [] + crt: /home/step/certs/intermediate_ca.crt + key: /home/step/secrets/intermediate_ca_key + address: 127.0.0.1:9000 + dnsNames: + - 127.0.0.1 + logger: + format: json + db: + type: badgerv2 + dataSource: /home/step/db + authority: + enableAdmin: false + provisioners: + - {"type":"JWK","name":"acme","key":{"use":"sig","kty":"EC","kid":"zsUmysmDVoGJ71YoPHyZ-68tNihDaDaO5Mu7xX3M-_I","crv":"P-256","alg":"ES256","x":"Pqnua4CzqKz6ua41J3yeWZ1sRkGt0UlCkbHv8H2DGuY","y":"UhoZ_2ItDen9KQTcjay-ph-SBXH0mwqhHyvrrqIFDOI"},"encryptedKey":"eyJhbGciOiJQQkVTMi1IUzI1NitBMTI4S1ciLCJjdHkiOiJqd2sranNvbiIsImVuYyI6IkEyNTZHQ00iLCJwMmMiOjEwMDAwMCwicDJzIjoiZjVvdGVRS2hvOXl4MmQtSGlMZi05QSJ9.eYA6tt3fNuUpoxKWDT7P0Lbn2juxhEbTxEnwEMbjlYLLQ3sxL-dYTA.ven-FhmdjlC9itH0.a2jRTarN9vPd6F_mWnNBlOn6KbfMjCApmci2t65XbAsLzYFzhI_79Ykm5ueMYTupWLTjBJctl-g51ZHmsSB55pStbpoyyLNAsUX2E1fTmHe-Ni8bRrspwLv15FoN1Xo1g0mpR-ufWIFxOsW-QIfnMmMIIkygVuHFXmg2tFpzTNNG5aS29K3dN2nyk0WJrdIq79hZSTqVkkBU25Yu3A46sgjcM86XcIJJ2XUEih_KWEa6T1YrkixGu96pebjVqbO0R6dbDckfPF7FqNnwPHVtb1ACFpEYoOJVIbUCMaARBpWsxYhjJZlEM__XA46l8snFQDkNY3CdN0p1_gF3ckA.JLmq9nmu1h9oUi1S8ZxYjA","options":{"x509":{},"ssh":{}}} + - {"type":"ACME","name":"acme-1"} + tls: + cipherSuites: + - TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 + - TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 + minVersion: 1.2 + maxVersion: 1.3 + renegotiation: false + + defaults.json: + ca-url: https://127.0.0.1 + ca-config: /home/step/config/ca.json + fingerprint: e543cad8e9f6417076bb5aed3471c588152118aac1e0ca7984a43ee7f76da5e3 + root: /home/step/certs/root_ca.crt + + # Certificates contains the root and intermediate certificate and + # optionally the SSH host and user public keys + certificates: + # intermediate_ca contains the text of the intermediate CA Certificate + intermediate_ca: | + -----BEGIN CERTIFICATE----- + dGhlc2UgYXJlIGp1c3Qgc29tZSBmYWtlIGludGVybWVkaWF0ZSBDQSBjZXJ0IGJ5 + dGVz + -----END CERTIFICATE----- + + + # root_ca contains the text of the root CA Certificate + root_ca: | + -----BEGIN CERTIFICATE----- + dGhlc2UgYXJlIGp1c3Qgc29tZSBmYWtlIHJvb3QgQ0EgY2VydCBieXRlcw== + -----END CERTIFICATE----- + + + # Secrets contains the root and intermediate keys and optionally the SSH + # private keys + secrets: + # ca_password contains the password used to encrypt x509.intermediate_ca_key, ssh.host_ca_key and ssh.user_ca_key + # This value must be base64 encoded. + ca_password: + provisioner_password: + + x509: + # intermediate_ca_key contains the contents of your encrypted intermediate CA key + intermediate_ca_key: | + -----BEGIN EC PRIVATE KEY----- + dGhlc2UgYXJlIGp1c3Qgc29tZSBmYWtlIGludGVybWVkaWF0ZSBDQSBrZXkgYnl0 + ZXM= + -----END EC PRIVATE KEY----- + + + # root_ca_key contains the contents of your encrypted root CA key + # Note that this value can be omitted without impacting the functionality of step-certificates + # If supplied, this should be encrypted using a unique password that is not used for encrypting + # the intermediate_ca_key, ssh.host_ca_key or ssh.user_ca_key. + root_ca_key: | + -----BEGIN EC PRIVATE KEY----- + dGhlc2UgYXJlIGp1c3Qgc29tZSBmYWtlIHJvb3QgQ0Ega2V5IGJ5dGVz + -----END EC PRIVATE KEY----- + diff --git a/pki/testdata/helm/with-ssh-and-duplicate-provisioner-name.yml b/pki/testdata/helm/with-ssh-and-duplicate-provisioner-name.yml new file mode 100644 index 00000000..d8d32dd6 --- /dev/null +++ b/pki/testdata/helm/with-ssh-and-duplicate-provisioner-name.yml @@ -0,0 +1,104 @@ +# Helm template +inject: + enabled: true + # Config contains the configuration files ca.json and defaults.json + config: + files: + ca.json: + root: /home/step/certs/root_ca.crt + federateRoots: [] + crt: /home/step/certs/intermediate_ca.crt + key: /home/step/secrets/intermediate_ca_key + ssh: + hostKey: /home/step/secrets/ssh_host_ca_key + userKey: /home/step/secrets/ssh_user_ca_key + address: 127.0.0.1:9000 + dnsNames: + - 127.0.0.1 + logger: + format: json + db: + type: badgerv2 + dataSource: /home/step/db + authority: + enableAdmin: false + provisioners: + - {"type":"JWK","name":"sshpop","key":{"use":"sig","kty":"EC","kid":"zsUmysmDVoGJ71YoPHyZ-68tNihDaDaO5Mu7xX3M-_I","crv":"P-256","alg":"ES256","x":"Pqnua4CzqKz6ua41J3yeWZ1sRkGt0UlCkbHv8H2DGuY","y":"UhoZ_2ItDen9KQTcjay-ph-SBXH0mwqhHyvrrqIFDOI"},"encryptedKey":"eyJhbGciOiJQQkVTMi1IUzI1NitBMTI4S1ciLCJjdHkiOiJqd2sranNvbiIsImVuYyI6IkEyNTZHQ00iLCJwMmMiOjEwMDAwMCwicDJzIjoiZjVvdGVRS2hvOXl4MmQtSGlMZi05QSJ9.eYA6tt3fNuUpoxKWDT7P0Lbn2juxhEbTxEnwEMbjlYLLQ3sxL-dYTA.ven-FhmdjlC9itH0.a2jRTarN9vPd6F_mWnNBlOn6KbfMjCApmci2t65XbAsLzYFzhI_79Ykm5ueMYTupWLTjBJctl-g51ZHmsSB55pStbpoyyLNAsUX2E1fTmHe-Ni8bRrspwLv15FoN1Xo1g0mpR-ufWIFxOsW-QIfnMmMIIkygVuHFXmg2tFpzTNNG5aS29K3dN2nyk0WJrdIq79hZSTqVkkBU25Yu3A46sgjcM86XcIJJ2XUEih_KWEa6T1YrkixGu96pebjVqbO0R6dbDckfPF7FqNnwPHVtb1ACFpEYoOJVIbUCMaARBpWsxYhjJZlEM__XA46l8snFQDkNY3CdN0p1_gF3ckA.JLmq9nmu1h9oUi1S8ZxYjA","claims":{"enableSSHCA":true,"disableRenewal":false,"allowRenewalAfterExpiry":false},"options":{"x509":{},"ssh":{}}} + - {"type":"SSHPOP","name":"sshpop-1","claims":{"enableSSHCA":true}} + tls: + cipherSuites: + - TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 + - TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 + minVersion: 1.2 + maxVersion: 1.3 + renegotiation: false + + defaults.json: + ca-url: https://127.0.0.1 + ca-config: /home/step/config/ca.json + fingerprint: e543cad8e9f6417076bb5aed3471c588152118aac1e0ca7984a43ee7f76da5e3 + root: /home/step/certs/root_ca.crt + + # Certificates contains the root and intermediate certificate and + # optionally the SSH host and user public keys + certificates: + # intermediate_ca contains the text of the intermediate CA Certificate + intermediate_ca: | + -----BEGIN CERTIFICATE----- + dGhlc2UgYXJlIGp1c3Qgc29tZSBmYWtlIGludGVybWVkaWF0ZSBDQSBjZXJ0IGJ5 + dGVz + -----END CERTIFICATE----- + + + # root_ca contains the text of the root CA Certificate + root_ca: | + -----BEGIN CERTIFICATE----- + dGhlc2UgYXJlIGp1c3Qgc29tZSBmYWtlIHJvb3QgQ0EgY2VydCBieXRlcw== + -----END CERTIFICATE----- + + # ssh_host_ca contains the text of the public ssh key for the SSH root CA + ssh_host_ca: ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBJ0IdS5sZm6KITBMZLEJD6b5ROVraYHcAOr3feFel8r1Wp4DRPR1oU0W00J/zjNBRBbANlJoYN4x/8WNNVZ49Ms= + + # ssh_user_ca contains the text of the public ssh key for the SSH root CA + ssh_user_ca: ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBEWA1qUxaGwVNErsvEOGe2d6TvLMF+aiVpuOiIEvpMJ3JeJmecLQctjWqeIbpSvy6/gRa7c82Ge5rLlapYmOChs= + + # Secrets contains the root and intermediate keys and optionally the SSH + # private keys + secrets: + # ca_password contains the password used to encrypt x509.intermediate_ca_key, ssh.host_ca_key and ssh.user_ca_key + # This value must be base64 encoded. + ca_password: + provisioner_password: + + x509: + # intermediate_ca_key contains the contents of your encrypted intermediate CA key + intermediate_ca_key: | + -----BEGIN EC PRIVATE KEY----- + dGhlc2UgYXJlIGp1c3Qgc29tZSBmYWtlIGludGVybWVkaWF0ZSBDQSBrZXkgYnl0 + ZXM= + -----END EC PRIVATE KEY----- + + + # root_ca_key contains the contents of your encrypted root CA key + # Note that this value can be omitted without impacting the functionality of step-certificates + # If supplied, this should be encrypted using a unique password that is not used for encrypting + # the intermediate_ca_key, ssh.host_ca_key or ssh.user_ca_key. + root_ca_key: | + -----BEGIN EC PRIVATE KEY----- + dGhlc2UgYXJlIGp1c3Qgc29tZSBmYWtlIHJvb3QgQ0Ega2V5IGJ5dGVz + -----END EC PRIVATE KEY----- + + ssh: + # ssh_host_ca_key contains the contents of your encrypted SSH Host CA key + host_ca_key: | + -----BEGIN EC PRIVATE KEY----- + ZmFrZSBzc2ggaG9zdCBrZXkgYnl0ZXM= + -----END EC PRIVATE KEY----- + + + # ssh_user_ca_key contains the contents of your encrypted SSH User CA key + user_ca_key: | + -----BEGIN EC PRIVATE KEY----- + ZmFrZSBzc2ggdXNlciBrZXkgYnl0ZXM= + -----END EC PRIVATE KEY----- +