From acdf080308f82d0d2f9990b3fd6fa39e624a40b6 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Thu, 29 Sep 2022 15:08:32 +0200 Subject: [PATCH 01/18] Add `enableAdmin` and `enableACME` to Helm values.yml generation --- pki/helm.go | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/pki/helm.go b/pki/helm.go index e13bb97c..7651d8ef 100644 --- a/pki/helm.go +++ b/pki/helm.go @@ -17,6 +17,7 @@ type helmVariables struct { Defaults *linkedca.Defaults Password string EnableSSH bool + EnableAdmin bool TLS authconfig.TLSOptions Provisioners []provisioner.Interface } @@ -35,7 +36,11 @@ func (p *PKI) WriteHelmTemplate(w io.Writer) error { } // Convert provisioner to ca.json - provisioners := make([]provisioner.Interface, len(p.Authority.Provisioners)) + numberOfProvisioners := len(p.Authority.Provisioners) + if p.options.enableACME { + numberOfProvisioners++ + } + provisioners := make([]provisioner.Interface, numberOfProvisioners) for i, p := range p.Authority.Provisioners { pp, err := authority.ProvisionerToCertificates(p) if err != nil { @@ -44,11 +49,25 @@ func (p *PKI) WriteHelmTemplate(w io.Writer) error { provisioners[i] = pp } + // Add default ACME provisioner if enabled. Note that this logic is similar + // 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. + // TODO(hs): consider refactoring the initialization, so that this becomes + // easier to reason about and maintain. + if p.options.enableACME { + provisioners[len(provisioners)-1] = &provisioner.ACME{ + Type: "ACME", + Name: "acme", + } + } + if err := tmpl.Execute(w, helmVariables{ Configuration: &p.Configuration, Defaults: &p.Defaults, Password: "", EnableSSH: p.options.enableSSH, + EnableAdmin: p.options.enableAdmin, TLS: authconfig.DefaultTLSOptions, Provisioners: provisioners, }); err != nil { @@ -88,6 +107,7 @@ inject: type: badgerv2 dataSource: /home/step/db authority: + enableAdmin: {{ .EnableAdmin }} provisioners: {{- range .Provisioners }} - {{ . | toJson }} From cebb7d7ef0c2962654adf958482b026e17532370 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Thu, 6 Oct 2022 17:14:02 +0200 Subject: [PATCH 02/18] Add automatic migration of provisioners Provisioners stored in the CA configuration file are automatically migrated to the database. Currently no cleanup of the provisioners in the configuration file yet. In certain situations this may not work as expected, for example if the CA can't write to the file. But it's probalby good to try it, so that we can keep the configuration state of the CA consistent. --- authority/authority.go | 53 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 46 insertions(+), 7 deletions(-) diff --git a/authority/authority.go b/authority/authority.go index 5271842d..a667cfa8 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -600,20 +600,59 @@ func (a *Authority) init() error { return admin.WrapErrorISE(err, "error loading provisioners to initialize authority") } if len(provs) == 0 && !strings.EqualFold(a.config.AuthorityConfig.DeploymentType, "linked") { - // Create First Provisioner - prov, err := CreateFirstProvisioner(ctx, a.adminDB, string(a.password)) - if err != nil { - return admin.WrapErrorISE(err, "error creating first provisioner") + + var firstJWKProvisioner *linkedca.Provisioner + if len(a.config.AuthorityConfig.Provisioners) > 0 { + log.Printf("Starting migration of provisioners") + // Existing provisioners detected; try migrating them to DB storage + for _, p := range a.config.AuthorityConfig.Provisioners { + lp, err := ProvisionerToLinkedca(p) + if err != nil { + return admin.WrapErrorISE(err, "error transforming provisioner %q while migrating", p.GetName()) + } + + // Store the provisioner to be migrated + if err := a.adminDB.CreateProvisioner(ctx, lp); err != nil { + return admin.WrapErrorISE(err, "error creating provisioner %q while migrating", p.GetName()) + } + + // Mark the first JWK provisioner, so that it can be used for administration purposes + if firstJWKProvisioner == nil && lp.Type == linkedca.Provisioner_JWK { + firstJWKProvisioner = lp + log.Printf("Migrated JWK provisioner %q with admin permissions", p.GetName()) // TODO(hs): change the wording? + } else { + log.Printf("Migrated %s provisioner %q", p.GetType(), p.GetName()) + } + } + + // TODO(hs): try to update ca.json to remove migrated provisioners from the + // file? This may not always be possible though, so we shouldn't fail hard on + // every error. The next time the CA runs, it won't have perform the migration, + // because there'll be at least a JWK provisioner. + + log.Printf("Finished migrating provisioners") } - // Create first admin + // Create first JWK provisioner for remote administration purposes if none exists yet + if firstJWKProvisioner == nil { + firstJWKProvisioner, err = CreateFirstProvisioner(ctx, a.adminDB, string(a.password)) + if err != nil { + return admin.WrapErrorISE(err, "error creating first provisioner") + } + log.Printf("Created JWK provisioner %q with admin permissions", firstJWKProvisioner.GetName()) // TODO(hs): change the wording? + } + + // Create first super admin, belonging to the first JWK provisioner + firstSuperAdminSubject := "step" if err := a.adminDB.CreateAdmin(ctx, &linkedca.Admin{ - ProvisionerId: prov.Id, - Subject: "step", + ProvisionerId: firstJWKProvisioner.Id, + Subject: firstSuperAdminSubject, Type: linkedca.Admin_SUPER_ADMIN, }); err != nil { return admin.WrapErrorISE(err, "error creating first admin") } + + log.Printf("Created super admin %q for JWK provisioner %q", firstSuperAdminSubject, firstJWKProvisioner.GetName()) } } From c9ee4a9f9d309236f839551725a8584e23d4fb60 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Tue, 11 Oct 2022 12:19:29 +0200 Subject: [PATCH 03/18] Disable initialization log output if started with `--quiet` --- authority/authority.go | 33 ++++++++++++++++++++++++--------- authority/options.go | 8 ++++++++ ca/ca.go | 4 ++++ 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/authority/authority.go b/authority/authority.go index a667cfa8..188633c3 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -73,7 +73,7 @@ type Authority struct { sshCAUserFederatedCerts []ssh.PublicKey sshCAHostFederatedCerts []ssh.PublicKey - // Do not re-initialize + // If true, do not re-initialize initOnce bool startTime time.Time @@ -91,8 +91,11 @@ type Authority struct { adminMutex sync.RWMutex - // Do Not initialize the authority + // If true, do not initialize the authority skipInit bool + + // If true, does not output initialization logs + quietInit bool } // Info contains information about the authority. @@ -600,10 +603,9 @@ func (a *Authority) init() error { return admin.WrapErrorISE(err, "error loading provisioners to initialize authority") } if len(provs) == 0 && !strings.EqualFold(a.config.AuthorityConfig.DeploymentType, "linked") { - var firstJWKProvisioner *linkedca.Provisioner if len(a.config.AuthorityConfig.Provisioners) > 0 { - log.Printf("Starting migration of provisioners") + a.initLogf("Starting migration of provisioners") // Existing provisioners detected; try migrating them to DB storage for _, p := range a.config.AuthorityConfig.Provisioners { lp, err := ProvisionerToLinkedca(p) @@ -619,9 +621,9 @@ func (a *Authority) init() error { // Mark the first JWK provisioner, so that it can be used for administration purposes if firstJWKProvisioner == nil && lp.Type == linkedca.Provisioner_JWK { firstJWKProvisioner = lp - log.Printf("Migrated JWK provisioner %q with admin permissions", p.GetName()) // TODO(hs): change the wording? + a.initLogf("Migrated JWK provisioner %q with admin permissions", p.GetName()) // TODO(hs): change the wording? } else { - log.Printf("Migrated %s provisioner %q", p.GetType(), p.GetName()) + a.initLogf("Migrated %s provisioner %q", p.GetType(), p.GetName()) } } @@ -630,7 +632,12 @@ func (a *Authority) init() error { // every error. The next time the CA runs, it won't have perform the migration, // because there'll be at least a JWK provisioner. - log.Printf("Finished migrating provisioners") + // 1. check if prerequisites for writing files look OK (user/group, permission bits, etc) + // 2. update the configuration to write (internal representation; do a deep copy first?) + // 3. try writing the new ca.json + // 4. on failure, perform rollback of the write (restore original in internal representation) + + a.initLogf("Finished migrating provisioners") } // Create first JWK provisioner for remote administration purposes if none exists yet @@ -639,7 +646,7 @@ func (a *Authority) init() error { if err != nil { return admin.WrapErrorISE(err, "error creating first provisioner") } - log.Printf("Created JWK provisioner %q with admin permissions", firstJWKProvisioner.GetName()) // TODO(hs): change the wording? + a.initLogf("Created JWK provisioner %q with admin permissions", firstJWKProvisioner.GetName()) // TODO(hs): change the wording? } // Create first super admin, belonging to the first JWK provisioner @@ -652,7 +659,7 @@ func (a *Authority) init() error { return admin.WrapErrorISE(err, "error creating first admin") } - log.Printf("Created super admin %q for JWK provisioner %q", firstSuperAdminSubject, firstJWKProvisioner.GetName()) + a.initLogf("Created super admin %q for JWK provisioner %q", firstSuperAdminSubject, firstJWKProvisioner.GetName()) } } @@ -702,6 +709,14 @@ func (a *Authority) init() error { return nil } +// initLogf is used to log initialization information. The output +// can be disabled by starting the CA with the `--quiet` flag. +func (a *Authority) initLogf(format string, v ...any) { + if !a.quietInit { + log.Printf(format, v...) + } +} + // GetID returns the define authority id or a zero uuid. func (a *Authority) GetID() string { const zeroUUID = "00000000-0000-0000-0000-000000000000" diff --git a/authority/options.go b/authority/options.go index f332d4a9..bf443ed6 100644 --- a/authority/options.go +++ b/authority/options.go @@ -86,6 +86,14 @@ func WithDatabase(d db.AuthDB) Option { } } +// WithQuietInit disables log output when the authority is initialized. +func WithQuietInit() Option { + return func(a *Authority) error { + a.quietInit = true + return nil + } +} + // WithWebhookClient sets the http.Client to be used for outbound requests. func WithWebhookClient(c *http.Client) Option { return func(a *Authority) error { diff --git a/ca/ca.go b/ca/ca.go index ab2aa8ac..98f845b0 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -156,6 +156,10 @@ func (ca *CA) Init(cfg *config.Config) (*CA, error) { opts = append(opts, authority.WithDatabase(ca.opts.database)) } + if ca.opts.quiet { + opts = append(opts, authority.WithQuietInit()) + } + webhookTransport := http.DefaultTransport.(*http.Transport).Clone() opts = append(opts, authority.WithWebhookClient(&http.Client{Transport: webhookTransport})) From 674206320c4280f8a11b76872138285a2c179902 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Tue, 11 Oct 2022 14:12:06 +0200 Subject: [PATCH 04/18] Write updated CA configuration after migrating provisioners --- authority/authority.go | 31 ++++++++++++++++++++++--------- authority/config/config.go | 25 +++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/authority/authority.go b/authority/authority.go index 188633c3..e3bc3473 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -605,8 +605,8 @@ func (a *Authority) init() error { if len(provs) == 0 && !strings.EqualFold(a.config.AuthorityConfig.DeploymentType, "linked") { var firstJWKProvisioner *linkedca.Provisioner if len(a.config.AuthorityConfig.Provisioners) > 0 { - a.initLogf("Starting migration of provisioners") // Existing provisioners detected; try migrating them to DB storage + a.initLogf("Starting migration of provisioners") for _, p := range a.config.AuthorityConfig.Provisioners { lp, err := ProvisionerToLinkedca(p) if err != nil { @@ -627,15 +627,28 @@ func (a *Authority) init() error { } } - // TODO(hs): try to update ca.json to remove migrated provisioners from the - // file? This may not always be possible though, so we shouldn't fail hard on - // every error. The next time the CA runs, it won't have perform the migration, - // because there'll be at least a JWK provisioner. + // TODO(hs): test if this works with LinkedCA too. Also could be useful + // for printing out where the configuration is read from in case of LinkedCA. + c := a.config + if c.WasLoadedFromFile() { + // TODO(hs): check if prerequisites for writing files look OK (user/group, permission bits, etc) as + // extra safety check before trying to write at all? - // 1. check if prerequisites for writing files look OK (user/group, permission bits, etc) - // 2. update the configuration to write (internal representation; do a deep copy first?) - // 3. try writing the new ca.json - // 4. on failure, perform rollback of the write (restore original in internal representation) + // Remove the existing provisioners from the authority configuration + // and commit it to the existing configuration file. NOTE: committing + // the configuration at this point also writes other properties that + // have been initialized with default values, such as the `backdate` and + // `template` settings in the `authority`. + oldProvisioners := c.AuthorityConfig.Provisioners + c.AuthorityConfig.Provisioners = []provisioner.Interface{} + if err := c.Commit(); err != nil { + // Restore the provisioners in in-memory representation for consistency + // when writing the updated configuration fails. This is considered a soft + // error, so execution can continue. + c.AuthorityConfig.Provisioners = oldProvisioners + a.initLogf("Failed removing provisioners from configuration: %v", err) + } + } a.initLogf("Finished migrating provisioners") } diff --git a/authority/config/config.go b/authority/config/config.go index c5e74b39..79228e98 100644 --- a/authority/config/config.go +++ b/authority/config/config.go @@ -73,6 +73,9 @@ type Config struct { Templates *templates.Templates `json:"templates,omitempty"` CommonName string `json:"commonName,omitempty"` SkipValidation bool `json:"-"` + + // Keeps record of the filename the Config is read from + loadedFromFilename string } // ASN1DN contains ASN1.DN attributes that are used in Subject and Issuer @@ -163,6 +166,10 @@ func LoadConfiguration(filename string) (*Config, error) { return nil, errors.Wrapf(err, "error parsing %s", filename) } + // store filename that was read to populate Config + c.loadedFromFilename = filename + + // initialize the Config c.Init() return &c, nil @@ -199,6 +206,24 @@ func (c *Config) Save(filename string) error { return errors.Wrapf(enc.Encode(c), "error writing %s", filename) } +// Commit saves the current configuration to the same +// file it was initially loaded from. +// +// TODO(hs): rename Save() to WriteTo() and replace this +// with Save()? Or is Commit clear enough. +func (c *Config) Commit() error { + if !c.WasLoadedFromFile() { + return errors.New("cannot commit configuration if not loaded from file") + } + return c.Save(c.loadedFromFilename) +} + +// WasLoadedFromFile returns whether or not the Config was +// read from a file. +func (c *Config) WasLoadedFromFile() bool { + return c.loadedFromFilename != "" +} + // Validate validates the configuration. func (c *Config) Validate() error { switch { From 8616d3160fa38d87810758628ab67a9205ad7289 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Tue, 11 Oct 2022 17:18:19 +0200 Subject: [PATCH 05/18] Add tests for writing the Helm template --- pki/helm_test.go | 104 +++++++++++++++++++++++++++++++ pki/testdata/helm/simple.yml | 66 ++++++++++++++++++++ pki/testdata/helm/with-acme.yml | 67 ++++++++++++++++++++ pki/testdata/helm/with-admin.yml | 66 ++++++++++++++++++++ pki/testdata/helm/with-ssh.yml | 82 ++++++++++++++++++++++++ 5 files changed, 385 insertions(+) create mode 100644 pki/helm_test.go create mode 100644 pki/testdata/helm/simple.yml create mode 100644 pki/testdata/helm/with-acme.yml create mode 100644 pki/testdata/helm/with-admin.yml create mode 100644 pki/testdata/helm/with-ssh.yml diff --git a/pki/helm_test.go b/pki/helm_test.go new file mode 100644 index 00000000..703e6cb8 --- /dev/null +++ b/pki/helm_test.go @@ -0,0 +1,104 @@ +package pki + +import ( + "bytes" + "os" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/assert" + + "github.com/smallstep/certificates/cas/apiv1" +) + +func TestPKI_WriteHelmTemplate(t *testing.T) { + type fields struct { + casOptions apiv1.Options + pkiOptions []Option + } + tests := []struct { + name string + fields fields + testFile string + wantErr bool + }{ + { + name: "ok/simple", + fields: fields{ + pkiOptions: []Option{ + WithHelm(), + }, + casOptions: apiv1.Options{ + Type: "softcas", + IsCreator: true, + }, + }, + testFile: "testdata/helm/simple.yml", + wantErr: false, + }, + { + name: "ok/with-acme", + fields: fields{ + pkiOptions: []Option{ + WithHelm(), + WithACME(), + }, + casOptions: apiv1.Options{ + Type: "softcas", + IsCreator: true, + }, + }, + testFile: "testdata/helm/with-acme.yml", + wantErr: false, + }, + { + name: "ok/with-admin", + fields: fields{ + pkiOptions: []Option{ + WithHelm(), + WithAdmin(), + }, + casOptions: apiv1.Options{ + Type: "softcas", + IsCreator: true, + }, + }, + testFile: "testdata/helm/with-admin.yml", + wantErr: false, + }, + { + name: "ok/with-ssh", + fields: fields{ + pkiOptions: []Option{ + WithHelm(), + WithSSH(), + }, + casOptions: apiv1.Options{ + Type: "softcas", + IsCreator: true, + }, + }, + testFile: "testdata/helm/with-ssh.yml", + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + o := tt.fields.casOptions + opts := tt.fields.pkiOptions + p, err := New(o, opts...) + assert.NoError(t, err) + w := &bytes.Buffer{} + if err := p.WriteHelmTemplate(w); (err != nil) != tt.wantErr { + t.Errorf("PKI.WriteHelmTemplate() error = %v, wantErr %v", err, tt.wantErr) + return + } + wantBytes, err := os.ReadFile(tt.testFile) + assert.NoError(t, err) + if diff := cmp.Diff(wantBytes, w.Bytes()); diff != "" { + t.Logf("Generated Helm template did not match reference %q\n", tt.testFile) + t.Errorf("Diff follows:\n%s\n", diff) + } + }) + } +} diff --git a/pki/testdata/helm/simple.yml b/pki/testdata/helm/simple.yml new file mode 100644 index 00000000..1c3049c3 --- /dev/null +++ b/pki/testdata/helm/simple.yml @@ -0,0 +1,66 @@ +# 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: + 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: + 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: | + + + # root_ca contains the text of the root CA Certificate + root_ca: | + + + # 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: | + + + # 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: | + diff --git a/pki/testdata/helm/with-acme.yml b/pki/testdata/helm/with-acme.yml new file mode 100644 index 00000000..17ff6f81 --- /dev/null +++ b/pki/testdata/helm/with-acme.yml @@ -0,0 +1,67 @@ +# 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":"ACME","name":"acme"} + 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: + 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: | + + + # root_ca contains the text of the root CA Certificate + root_ca: | + + + # 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: | + + + # 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: | + diff --git a/pki/testdata/helm/with-admin.yml b/pki/testdata/helm/with-admin.yml new file mode 100644 index 00000000..75fd1999 --- /dev/null +++ b/pki/testdata/helm/with-admin.yml @@ -0,0 +1,66 @@ +# 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: true + provisioners: + 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: + 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: | + + + # root_ca contains the text of the root CA Certificate + root_ca: | + + + # 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: | + + + # 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: | + diff --git a/pki/testdata/helm/with-ssh.yml b/pki/testdata/helm/with-ssh.yml new file mode 100644 index 00000000..b2ba96f6 --- /dev/null +++ b/pki/testdata/helm/with-ssh.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 + 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: + 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: + 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: | + + + # root_ca contains the text of the root CA Certificate + root_ca: | + + # ssh_host_ca contains the text of the public ssh key for the SSH root CA + ssh_host_ca: + + # ssh_user_ca contains the text of the public ssh key for the SSH root CA + ssh_user_ca: + + # 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: | + + + # 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: | + + ssh: + # ssh_host_ca_key contains the contents of your encrypted SSH Host CA key + host_ca_key: | + + + # ssh_user_ca_key contains the contents of your encrypted SSH User CA key + user_ca_key: | + From 317efa456860d8265fa3730c513343db768249a0 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Tue, 11 Oct 2022 17:39:35 +0200 Subject: [PATCH 06/18] Add some TODOs for improvingin PKI initialization maintainability --- pki/helm_test.go | 6 ++++++ pki/pki.go | 3 +++ 2 files changed, 9 insertions(+) diff --git a/pki/helm_test.go b/pki/helm_test.go index 703e6cb8..f83a8370 100644 --- a/pki/helm_test.go +++ b/pki/helm_test.go @@ -86,6 +86,12 @@ func TestPKI_WriteHelmTemplate(t *testing.T) { t.Run(tt.name, func(t *testing.T) { o := tt.fields.casOptions opts := tt.fields.pkiOptions + // TODO(hs): invoking `New` doesn't perform all operations that are executed + // when `ca init --helm` is executed. The list of provisioners on the authority + // is not populated, for example, resulting in this test not being entirely + // realistic. Ideally this logic should be handled in one place and probably + // inside of the PKI initialization, but if that becomes messy, some more + // logic needs to be performed here to get the PKI instance in good shape. p, err := New(o, opts...) assert.NoError(t, err) w := &bytes.Buffer{} diff --git a/pki/pki.go b/pki/pki.go index c05eadbd..5bbd42a1 100644 --- a/pki/pki.go +++ b/pki/pki.go @@ -307,6 +307,9 @@ type PKI struct { // New creates a new PKI configuration. func New(o apiv1.Options, opts ...Option) (*PKI, error) { + // TODO(hs): invoking `New` with a context active will use values from + // that CA context while generating the context. Thay may or may not + // be fully expected and/or what we want. Check that. currentCtx := step.Contexts().GetCurrent() caService, err := cas.New(context.Background(), o) if err != nil { From 6516384160b5ddf23a154a1c465ea841f8025f44 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Wed, 12 Oct 2022 15:54:32 +0200 Subject: [PATCH 07/18] Trigger CI From 1a5523f5c023af64e7ff2289256d1b92cfd30ece Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Fri, 14 Oct 2022 00:09:32 +0200 Subject: [PATCH 08/18] Add default JWK to the Helm tests --- pki/helm_test.go | 77 ++++++++++++++++++++++++++ pki/testdata/helm/simple.yml | 1 + pki/testdata/helm/with-acme.yml | 1 + pki/testdata/helm/with-admin.yml | 1 + pki/testdata/helm/with-provisioner.yml | 67 ++++++++++++++++++++++ pki/testdata/helm/with-ssh.yml | 1 + 6 files changed, 148 insertions(+) create mode 100644 pki/testdata/helm/with-provisioner.yml diff --git a/pki/helm_test.go b/pki/helm_test.go index f83a8370..0a383614 100644 --- a/pki/helm_test.go +++ b/pki/helm_test.go @@ -2,12 +2,16 @@ package pki import ( "bytes" + "encoding/json" "os" "testing" "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" + "go.step.sm/crypto/jose" + "go.step.sm/linkedca" + "github.com/smallstep/certificates/cas/apiv1" ) @@ -36,6 +40,21 @@ func TestPKI_WriteHelmTemplate(t *testing.T) { testFile: "testdata/helm/simple.yml", wantErr: false, }, + { + name: "ok/with-provisioner", + fields: fields{ + pkiOptions: []Option{ + WithHelm(), + WithProvisioner("a-provisioner"), + }, + casOptions: apiv1.Options{ + Type: "softcas", + IsCreator: true, + }, + }, + testFile: "testdata/helm/with-provisioner.yml", + wantErr: false, + }, { name: "ok/with-acme", fields: fields{ @@ -94,11 +113,21 @@ func TestPKI_WriteHelmTemplate(t *testing.T) { // logic needs to be performed here to get the PKI instance in good shape. p, err := New(o, opts...) assert.NoError(t, err) + + // setKeyPairs sets a predefined JWK and a default JWK provisioner. This is one + // of the things performed in the `ca init` code that's not part of `New`, but + // performed after that in p.GenerateKeyPairs`. We're currently using the same + // JWK for every test to keep test variance small: we're not testing JWK generation + // here after all. It's a bit dangerous to redefine the function here, but it's + // the simplest way to make this fully testable without refactoring the init now. + setKeyPairs(t, p) + w := &bytes.Buffer{} if err := p.WriteHelmTemplate(w); (err != nil) != tt.wantErr { t.Errorf("PKI.WriteHelmTemplate() error = %v, wantErr %v", err, tt.wantErr) return } + wantBytes, err := os.ReadFile(tt.testFile) assert.NoError(t, err) if diff := cmp.Diff(wantBytes, w.Bytes()); diff != "" { @@ -108,3 +137,51 @@ func TestPKI_WriteHelmTemplate(t *testing.T) { }) } } + +func setKeyPairs(t *testing.T, p *PKI) { + t.Helper() + + var err error + + p.ottPublicKey, err = jose.ParseKey([]byte(`{"use":"sig","kty":"EC","kid":"zsUmysmDVoGJ71YoPHyZ-68tNihDaDaO5Mu7xX3M-_I","crv":"P-256","alg":"ES256","x":"Pqnua4CzqKz6ua41J3yeWZ1sRkGt0UlCkbHv8H2DGuY","y":"UhoZ_2ItDen9KQTcjay-ph-SBXH0mwqhHyvrrqIFDOI"}`)) + if err != nil { + t.Fatal(err) + } + + p.ottPrivateKey, err = jose.ParseEncrypted("eyJhbGciOiJQQkVTMi1IUzI1NitBMTI4S1ciLCJjdHkiOiJqd2sranNvbiIsImVuYyI6IkEyNTZHQ00iLCJwMmMiOjEwMDAwMCwicDJzIjoiZjVvdGVRS2hvOXl4MmQtSGlMZi05QSJ9.eYA6tt3fNuUpoxKWDT7P0Lbn2juxhEbTxEnwEMbjlYLLQ3sxL-dYTA.ven-FhmdjlC9itH0.a2jRTarN9vPd6F_mWnNBlOn6KbfMjCApmci2t65XbAsLzYFzhI_79Ykm5ueMYTupWLTjBJctl-g51ZHmsSB55pStbpoyyLNAsUX2E1fTmHe-Ni8bRrspwLv15FoN1Xo1g0mpR-ufWIFxOsW-QIfnMmMIIkygVuHFXmg2tFpzTNNG5aS29K3dN2nyk0WJrdIq79hZSTqVkkBU25Yu3A46sgjcM86XcIJJ2XUEih_KWEa6T1YrkixGu96pebjVqbO0R6dbDckfPF7FqNnwPHVtb1ACFpEYoOJVIbUCMaARBpWsxYhjJZlEM__XA46l8snFQDkNY3CdN0p1_gF3ckA.JLmq9nmu1h9oUi1S8ZxYjA") + if err != nil { + t.Fatal(err) + } + + var claims *linkedca.Claims + if p.options.enableSSH { + claims = &linkedca.Claims{ + Ssh: &linkedca.SSHClaims{ + Enabled: true, + }, + } + } + + // Add JWK provisioner to the configuration. + publicKey, err := json.Marshal(p.ottPublicKey) + if err != nil { + t.Fatal(err) + } + encryptedKey, err := p.ottPrivateKey.CompactSerialize() + if err != nil { + t.Fatal(err) + } + p.Authority.Provisioners = append(p.Authority.Provisioners, &linkedca.Provisioner{ + Type: linkedca.Provisioner_JWK, + Name: p.options.provisioner, + Claims: claims, + Details: &linkedca.ProvisionerDetails{ + Data: &linkedca.ProvisionerDetails_JWK{ + JWK: &linkedca.JWKProvisioner{ + PublicKey: publicKey, + EncryptedPrivateKey: []byte(encryptedKey), + }, + }, + }, + }) +} diff --git a/pki/testdata/helm/simple.yml b/pki/testdata/helm/simple.yml index 1c3049c3..8b1f053e 100644 --- a/pki/testdata/helm/simple.yml +++ b/pki/testdata/helm/simple.yml @@ -20,6 +20,7 @@ inject: authority: enableAdmin: false provisioners: + - {"type":"JWK","name":"step-cli","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":{}}} tls: cipherSuites: - TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 diff --git a/pki/testdata/helm/with-acme.yml b/pki/testdata/helm/with-acme.yml index 17ff6f81..cf135946 100644 --- a/pki/testdata/helm/with-acme.yml +++ b/pki/testdata/helm/with-acme.yml @@ -20,6 +20,7 @@ inject: authority: enableAdmin: false provisioners: + - {"type":"JWK","name":"step-cli","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"} tls: cipherSuites: diff --git a/pki/testdata/helm/with-admin.yml b/pki/testdata/helm/with-admin.yml index 75fd1999..5a88e071 100644 --- a/pki/testdata/helm/with-admin.yml +++ b/pki/testdata/helm/with-admin.yml @@ -20,6 +20,7 @@ inject: authority: enableAdmin: true provisioners: + - {"type":"JWK","name":"step-cli","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":{}}} tls: cipherSuites: - TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 diff --git a/pki/testdata/helm/with-provisioner.yml b/pki/testdata/helm/with-provisioner.yml new file mode 100644 index 00000000..257a4623 --- /dev/null +++ b/pki/testdata/helm/with-provisioner.yml @@ -0,0 +1,67 @@ +# 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":"a-provisioner","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":{}}} + 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: + 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: | + + + # root_ca contains the text of the root CA Certificate + root_ca: | + + + # 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: | + + + # 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: | + diff --git a/pki/testdata/helm/with-ssh.yml b/pki/testdata/helm/with-ssh.yml index b2ba96f6..a44192cd 100644 --- a/pki/testdata/helm/with-ssh.yml +++ b/pki/testdata/helm/with-ssh.yml @@ -23,6 +23,7 @@ inject: authority: enableAdmin: false provisioners: + - {"type":"JWK","name":"step-cli","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":{}}} tls: cipherSuites: - TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 From 3262ffd43bf381d80dea56991bd5390ece9d8153 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Fri, 14 Oct 2022 01:06:43 +0200 Subject: [PATCH 09/18] Add X.509 intermedaite and root certificates to Helm tests --- pki/helm_test.go | 21 ++++++++++++++++++--- pki/testdata/helm/simple.yml | 7 +++++++ pki/testdata/helm/with-acme.yml | 7 +++++++ pki/testdata/helm/with-admin.yml | 7 +++++++ pki/testdata/helm/with-provisioner.yml | 7 +++++++ pki/testdata/helm/with-ssh.yml | 7 +++++++ 6 files changed, 53 insertions(+), 3 deletions(-) diff --git a/pki/helm_test.go b/pki/helm_test.go index 0a383614..6d684c29 100644 --- a/pki/helm_test.go +++ b/pki/helm_test.go @@ -2,6 +2,7 @@ package pki import ( "bytes" + "crypto/x509" "encoding/json" "os" "testing" @@ -114,13 +115,19 @@ func TestPKI_WriteHelmTemplate(t *testing.T) { p, err := New(o, opts...) assert.NoError(t, err) - // setKeyPairs sets a predefined JWK and a default JWK provisioner. This is one + // setKeyPair sets a predefined JWK and a default JWK provisioner. This is one // of the things performed in the `ca init` code that's not part of `New`, but // performed after that in p.GenerateKeyPairs`. We're currently using the same // JWK for every test to keep test variance small: we're not testing JWK generation // here after all. It's a bit dangerous to redefine the function here, but it's // the simplest way to make this fully testable without refactoring the init now. - setKeyPairs(t, p) + // The password for the predefined encrypted key is \x01\x03\x03\x07. + setKeyPair(t, p) + + // setFiles sets some static intermediate and root CA certificate bytes. It + // replaces the logic executed in `p.GenerateRootCertificate`, `p.WriteRootCertificate`, + // and `p.GenerateIntermediateCertificate`. + setFiles(t, p) w := &bytes.Buffer{} if err := p.WriteHelmTemplate(w); (err != nil) != tt.wantErr { @@ -133,12 +140,14 @@ func TestPKI_WriteHelmTemplate(t *testing.T) { if diff := cmp.Diff(wantBytes, w.Bytes()); diff != "" { t.Logf("Generated Helm template did not match reference %q\n", tt.testFile) t.Errorf("Diff follows:\n%s\n", diff) + t.Errorf("Full output:\n%s\n", w.Bytes()) } }) } } -func setKeyPairs(t *testing.T, p *PKI) { +// setKeyPair sets a predefined JWK and a default JWK provisioner. +func setKeyPair(t *testing.T, p *PKI) { t.Helper() var err error @@ -185,3 +194,9 @@ func setKeyPairs(t *testing.T, p *PKI) { }, }) } + +// setFiles sets some static, gibberish intermediate and root CA certificate bytes. +func setFiles(t *testing.T, p *PKI) { + p.Files["/home/step/certs/root_ca.crt"] = encodeCertificate(&x509.Certificate{Raw: []byte("these are just some fake root CA cert bytes")}) + p.Files["/home/step/certs/intermediate_ca.crt"] = encodeCertificate(&x509.Certificate{Raw: []byte("these are just some fake intermediate CA cert bytes")}) +} diff --git a/pki/testdata/helm/simple.yml b/pki/testdata/helm/simple.yml index 8b1f053e..c0f5f993 100644 --- a/pki/testdata/helm/simple.yml +++ b/pki/testdata/helm/simple.yml @@ -40,10 +40,17 @@ inject: 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 diff --git a/pki/testdata/helm/with-acme.yml b/pki/testdata/helm/with-acme.yml index cf135946..393a7a01 100644 --- a/pki/testdata/helm/with-acme.yml +++ b/pki/testdata/helm/with-acme.yml @@ -41,10 +41,17 @@ inject: 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 diff --git a/pki/testdata/helm/with-admin.yml b/pki/testdata/helm/with-admin.yml index 5a88e071..28896e73 100644 --- a/pki/testdata/helm/with-admin.yml +++ b/pki/testdata/helm/with-admin.yml @@ -40,10 +40,17 @@ inject: 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 diff --git a/pki/testdata/helm/with-provisioner.yml b/pki/testdata/helm/with-provisioner.yml index 257a4623..9095aa27 100644 --- a/pki/testdata/helm/with-provisioner.yml +++ b/pki/testdata/helm/with-provisioner.yml @@ -40,10 +40,17 @@ inject: 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 diff --git a/pki/testdata/helm/with-ssh.yml b/pki/testdata/helm/with-ssh.yml index a44192cd..770da794 100644 --- a/pki/testdata/helm/with-ssh.yml +++ b/pki/testdata/helm/with-ssh.yml @@ -43,10 +43,17 @@ inject: 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: From 459bfc4c4fa71fd413d747689b7dfbf5d9b7b59e Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Fri, 14 Oct 2022 01:45:07 +0200 Subject: [PATCH 10/18] Add gibberish test key bytes to Helm tests --- pki/helm_test.go | 23 +++++++++++++++++++++-- pki/testdata/helm/simple.yml | 4 ++-- pki/testdata/helm/with-acme.yml | 4 ++-- pki/testdata/helm/with-admin.yml | 4 ++-- pki/testdata/helm/with-provisioner.yml | 4 ++-- pki/testdata/helm/with-ssh.yml | 12 ++++++------ 6 files changed, 35 insertions(+), 16 deletions(-) diff --git a/pki/helm_test.go b/pki/helm_test.go index 6d684c29..aeffb5ca 100644 --- a/pki/helm_test.go +++ b/pki/helm_test.go @@ -129,6 +129,10 @@ func TestPKI_WriteHelmTemplate(t *testing.T) { // and `p.GenerateIntermediateCertificate`. setFiles(t, p) + // setSSHSigningKeys sets predefined SSH user and host certificate and key bytes. + // This replaces the logic in `p.GenerateSSHSigningKeys` + setSSHSigningKeys(t, p) + w := &bytes.Buffer{} if err := p.WriteHelmTemplate(w); (err != nil) != tt.wantErr { t.Errorf("PKI.WriteHelmTemplate() error = %v, wantErr %v", err, tt.wantErr) @@ -197,6 +201,21 @@ func setKeyPair(t *testing.T, p *PKI) { // setFiles sets some static, gibberish intermediate and root CA certificate bytes. func setFiles(t *testing.T, p *PKI) { - p.Files["/home/step/certs/root_ca.crt"] = encodeCertificate(&x509.Certificate{Raw: []byte("these are just some fake root CA cert bytes")}) - p.Files["/home/step/certs/intermediate_ca.crt"] = encodeCertificate(&x509.Certificate{Raw: []byte("these are just some fake intermediate CA cert bytes")}) + p.Files[p.Root[0]] = encodeCertificate(&x509.Certificate{Raw: []byte("these are just some fake root CA cert bytes")}) + p.Files[p.RootKey[0]] = []byte("these are just some fake root CA key bytes") + p.Files[p.Intermediate] = encodeCertificate(&x509.Certificate{Raw: []byte("these are just some fake intermediate CA cert bytes")}) + p.Files[p.IntermediateKey] = []byte("these are just some fake intermediate CA key bytes") +} + +// setSSHSigningKeys sets some static, gibberish ssh user and host CA certificate and key bytes. +func setSSHSigningKeys(t *testing.T, p *PKI) { + + if !p.options.enableSSH { + return + } + + p.Files[p.Ssh.HostKey] = []byte("fake ssh host key bytes") + p.Files[p.Ssh.HostPublicKey] = []byte("fake ssh host cert bytes") + p.Files[p.Ssh.UserKey] = []byte("fake ssh user key bytes") + p.Files[p.Ssh.UserPublicKey] = []byte("fake ssh user cert bytes") } diff --git a/pki/testdata/helm/simple.yml b/pki/testdata/helm/simple.yml index c0f5f993..9cc82806 100644 --- a/pki/testdata/helm/simple.yml +++ b/pki/testdata/helm/simple.yml @@ -64,11 +64,11 @@ inject: x509: # intermediate_ca_key contains the contents of your encrypted intermediate CA key intermediate_ca_key: | - + these are just some fake intermediate CA key bytes # 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: | - + these are just some fake root CA key bytes diff --git a/pki/testdata/helm/with-acme.yml b/pki/testdata/helm/with-acme.yml index 393a7a01..4f9d5761 100644 --- a/pki/testdata/helm/with-acme.yml +++ b/pki/testdata/helm/with-acme.yml @@ -65,11 +65,11 @@ inject: x509: # intermediate_ca_key contains the contents of your encrypted intermediate CA key intermediate_ca_key: | - + these are just some fake intermediate CA key bytes # 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: | - + these are just some fake root CA key bytes diff --git a/pki/testdata/helm/with-admin.yml b/pki/testdata/helm/with-admin.yml index 28896e73..b90647ea 100644 --- a/pki/testdata/helm/with-admin.yml +++ b/pki/testdata/helm/with-admin.yml @@ -64,11 +64,11 @@ inject: x509: # intermediate_ca_key contains the contents of your encrypted intermediate CA key intermediate_ca_key: | - + these are just some fake intermediate CA key bytes # 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: | - + these are just some fake root CA key bytes diff --git a/pki/testdata/helm/with-provisioner.yml b/pki/testdata/helm/with-provisioner.yml index 9095aa27..75788acc 100644 --- a/pki/testdata/helm/with-provisioner.yml +++ b/pki/testdata/helm/with-provisioner.yml @@ -64,11 +64,11 @@ inject: x509: # intermediate_ca_key contains the contents of your encrypted intermediate CA key intermediate_ca_key: | - + these are just some fake intermediate CA key bytes # 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: | - + these are just some fake root CA key bytes diff --git a/pki/testdata/helm/with-ssh.yml b/pki/testdata/helm/with-ssh.yml index 770da794..1f922994 100644 --- a/pki/testdata/helm/with-ssh.yml +++ b/pki/testdata/helm/with-ssh.yml @@ -56,10 +56,10 @@ inject: -----END CERTIFICATE----- # ssh_host_ca contains the text of the public ssh key for the SSH root CA - ssh_host_ca: + ssh_host_ca: fake ssh host cert bytes # ssh_user_ca contains the text of the public ssh key for the SSH root CA - ssh_user_ca: + ssh_user_ca: fake ssh user cert bytes # Secrets contains the root and intermediate keys and optionally the SSH # private keys @@ -72,19 +72,19 @@ inject: x509: # intermediate_ca_key contains the contents of your encrypted intermediate CA key intermediate_ca_key: | - + these are just some fake intermediate CA key bytes # 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: | - + these are just some fake root CA key bytes ssh: # ssh_host_ca_key contains the contents of your encrypted SSH Host CA key host_ca_key: | - + fake ssh host key bytes # ssh_user_ca_key contains the contents of your encrypted SSH User CA key user_ca_key: | - + fake ssh user key bytes From c423e2f664543561c8212ab190fede588c6c97b4 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Fri, 14 Oct 2022 13:52:27 +0200 Subject: [PATCH 11/18] Improve Helm test data to be more realistic --- pki/helm.go | 3 ++ pki/helm_test.go | 52 +++++++++++++++++--------- pki/pki.go | 2 +- pki/testdata/helm/simple.yml | 13 +++++-- pki/testdata/helm/with-acme.yml | 13 +++++-- pki/testdata/helm/with-admin.yml | 13 +++++-- pki/testdata/helm/with-provisioner.yml | 13 +++++-- pki/testdata/helm/with-ssh.yml | 27 +++++++++---- 8 files changed, 99 insertions(+), 37 deletions(-) diff --git a/pki/helm.go b/pki/helm.go index 7651d8ef..3fbadb40 100644 --- a/pki/helm.go +++ b/pki/helm.go @@ -62,6 +62,9 @@ func (p *PKI) WriteHelmTemplate(w io.Writer) error { } } + // TODO(hs): add default SSHPOP provisioner if SSH is configured, similar + // as the ACME one above. + if err := tmpl.Execute(w, helmVariables{ Configuration: &p.Configuration, Defaults: &p.Defaults, diff --git a/pki/helm_test.go b/pki/helm_test.go index aeffb5ca..1eb621a8 100644 --- a/pki/helm_test.go +++ b/pki/helm_test.go @@ -2,9 +2,13 @@ package pki import ( "bytes" + "crypto/sha256" "crypto/x509" + "encoding/hex" "encoding/json" + "encoding/pem" "os" + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -106,12 +110,12 @@ func TestPKI_WriteHelmTemplate(t *testing.T) { t.Run(tt.name, func(t *testing.T) { o := tt.fields.casOptions opts := tt.fields.pkiOptions + // TODO(hs): invoking `New` doesn't perform all operations that are executed - // when `ca init --helm` is executed. The list of provisioners on the authority - // is not populated, for example, resulting in this test not being entirely - // realistic. Ideally this logic should be handled in one place and probably - // inside of the PKI initialization, but if that becomes messy, some more - // logic needs to be performed here to get the PKI instance in good shape. + // when `ca init --helm` is executed. Ideally this logic should be handled + // in one place and probably inside of the PKI initialization. For testing + // purposes the missing operations to fill a Helm template fully are faked + // by `setKeyPair`, `setCertificates` and `setSSHSigningKeys` p, err := New(o, opts...) assert.NoError(t, err) @@ -124,10 +128,10 @@ func TestPKI_WriteHelmTemplate(t *testing.T) { // The password for the predefined encrypted key is \x01\x03\x03\x07. setKeyPair(t, p) - // setFiles sets some static intermediate and root CA certificate bytes. It + // setCertificates sets some static intermediate and root CA certificate bytes. It // replaces the logic executed in `p.GenerateRootCertificate`, `p.WriteRootCertificate`, // and `p.GenerateIntermediateCertificate`. - setFiles(t, p) + setCertificates(t, p) // setSSHSigningKeys sets predefined SSH user and host certificate and key bytes. // This replaces the logic in `p.GenerateSSHSigningKeys` @@ -175,7 +179,6 @@ func setKeyPair(t *testing.T, p *PKI) { } } - // Add JWK provisioner to the configuration. publicKey, err := json.Marshal(p.ottPublicKey) if err != nil { t.Fatal(err) @@ -199,12 +202,21 @@ func setKeyPair(t *testing.T, p *PKI) { }) } -// setFiles sets some static, gibberish intermediate and root CA certificate bytes. -func setFiles(t *testing.T, p *PKI) { - p.Files[p.Root[0]] = encodeCertificate(&x509.Certificate{Raw: []byte("these are just some fake root CA cert bytes")}) - p.Files[p.RootKey[0]] = []byte("these are just some fake root CA key bytes") +// setCertificates sets some static, gibberish intermediate and root CA certificate and key bytes. +func setCertificates(t *testing.T, p *PKI) { + raw := []byte("these are just some fake root CA cert bytes") + p.Files[p.Root[0]] = encodeCertificate(&x509.Certificate{Raw: raw}) + p.Files[p.RootKey[0]] = pem.EncodeToMemory(&pem.Block{ + Type: "EC PRIVATE KEY", + Bytes: []byte("these are just some fake root CA key bytes"), + }) p.Files[p.Intermediate] = encodeCertificate(&x509.Certificate{Raw: []byte("these are just some fake intermediate CA cert bytes")}) - p.Files[p.IntermediateKey] = []byte("these are just some fake intermediate CA key bytes") + p.Files[p.IntermediateKey] = pem.EncodeToMemory(&pem.Block{ + Type: "EC PRIVATE KEY", + Bytes: []byte("these are just some fake intermediate CA key bytes"), + }) + sum := sha256.Sum256(raw) + p.Defaults.Fingerprint = strings.ToLower(hex.EncodeToString(sum[:])) } // setSSHSigningKeys sets some static, gibberish ssh user and host CA certificate and key bytes. @@ -214,8 +226,14 @@ func setSSHSigningKeys(t *testing.T, p *PKI) { return } - p.Files[p.Ssh.HostKey] = []byte("fake ssh host key bytes") - p.Files[p.Ssh.HostPublicKey] = []byte("fake ssh host cert bytes") - p.Files[p.Ssh.UserKey] = []byte("fake ssh user key bytes") - p.Files[p.Ssh.UserPublicKey] = []byte("fake ssh user cert bytes") + p.Files[p.Ssh.HostKey] = pem.EncodeToMemory(&pem.Block{ + Type: "EC PRIVATE KEY", + Bytes: []byte("fake ssh host key bytes"), + }) + p.Files[p.Ssh.HostPublicKey] = []byte("ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBJ0IdS5sZm6KITBMZLEJD6b5ROVraYHcAOr3feFel8r1Wp4DRPR1oU0W00J/zjNBRBbANlJoYN4x/8WNNVZ49Ms=") + p.Files[p.Ssh.UserKey] = pem.EncodeToMemory(&pem.Block{ + Type: "EC PRIVATE KEY", + Bytes: []byte("fake ssh user key bytes"), + }) + p.Files[p.Ssh.UserPublicKey] = []byte("ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBEWA1qUxaGwVNErsvEOGe2d6TvLMF+aiVpuOiIEvpMJ3JeJmecLQctjWqeIbpSvy6/gRa7c82Ge5rLlapYmOChs=") } diff --git a/pki/pki.go b/pki/pki.go index 5bbd42a1..a4a64344 100644 --- a/pki/pki.go +++ b/pki/pki.go @@ -648,7 +648,7 @@ func (p *PKI) GetCertificateAuthority() error { // SSH user certificates and a private key used for signing host certificates. func (p *PKI) GenerateSSHSigningKeys(password []byte) error { // Enable SSH - p.options.enableSSH = true + p.options.enableSSH = true // TODO(hs): change this function to not mutate configuration state // Create SSH key used to sign host certificates. Using // kmsapi.UnspecifiedSignAlgorithm will default to the default algorithm. diff --git a/pki/testdata/helm/simple.yml b/pki/testdata/helm/simple.yml index 9cc82806..8a7e369f 100644 --- a/pki/testdata/helm/simple.yml +++ b/pki/testdata/helm/simple.yml @@ -32,7 +32,7 @@ inject: defaults.json: ca-url: https://127.0.0.1 ca-config: /home/step/config/ca.json - fingerprint: + fingerprint: e543cad8e9f6417076bb5aed3471c588152118aac1e0ca7984a43ee7f76da5e3 root: /home/step/certs/root_ca.crt # Certificates contains the root and intermediate certificate and @@ -64,11 +64,18 @@ inject: x509: # intermediate_ca_key contains the contents of your encrypted intermediate CA key intermediate_ca_key: | - these are just some fake intermediate CA key bytes + -----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: | - these are just some fake root CA key bytes + -----BEGIN EC PRIVATE KEY----- + dGhlc2UgYXJlIGp1c3Qgc29tZSBmYWtlIHJvb3QgQ0Ega2V5IGJ5dGVz + -----END EC PRIVATE KEY----- + diff --git a/pki/testdata/helm/with-acme.yml b/pki/testdata/helm/with-acme.yml index 4f9d5761..488bc32f 100644 --- a/pki/testdata/helm/with-acme.yml +++ b/pki/testdata/helm/with-acme.yml @@ -33,7 +33,7 @@ inject: defaults.json: ca-url: https://127.0.0.1 ca-config: /home/step/config/ca.json - fingerprint: + fingerprint: e543cad8e9f6417076bb5aed3471c588152118aac1e0ca7984a43ee7f76da5e3 root: /home/step/certs/root_ca.crt # Certificates contains the root and intermediate certificate and @@ -65,11 +65,18 @@ inject: x509: # intermediate_ca_key contains the contents of your encrypted intermediate CA key intermediate_ca_key: | - these are just some fake intermediate CA key bytes + -----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: | - these are just some fake root CA key bytes + -----BEGIN EC PRIVATE KEY----- + dGhlc2UgYXJlIGp1c3Qgc29tZSBmYWtlIHJvb3QgQ0Ega2V5IGJ5dGVz + -----END EC PRIVATE KEY----- + diff --git a/pki/testdata/helm/with-admin.yml b/pki/testdata/helm/with-admin.yml index b90647ea..790fbdd4 100644 --- a/pki/testdata/helm/with-admin.yml +++ b/pki/testdata/helm/with-admin.yml @@ -32,7 +32,7 @@ inject: defaults.json: ca-url: https://127.0.0.1 ca-config: /home/step/config/ca.json - fingerprint: + fingerprint: e543cad8e9f6417076bb5aed3471c588152118aac1e0ca7984a43ee7f76da5e3 root: /home/step/certs/root_ca.crt # Certificates contains the root and intermediate certificate and @@ -64,11 +64,18 @@ inject: x509: # intermediate_ca_key contains the contents of your encrypted intermediate CA key intermediate_ca_key: | - these are just some fake intermediate CA key bytes + -----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: | - these are just some fake root CA key bytes + -----BEGIN EC PRIVATE KEY----- + dGhlc2UgYXJlIGp1c3Qgc29tZSBmYWtlIHJvb3QgQ0Ega2V5IGJ5dGVz + -----END EC PRIVATE KEY----- + diff --git a/pki/testdata/helm/with-provisioner.yml b/pki/testdata/helm/with-provisioner.yml index 75788acc..de17ef0a 100644 --- a/pki/testdata/helm/with-provisioner.yml +++ b/pki/testdata/helm/with-provisioner.yml @@ -32,7 +32,7 @@ inject: defaults.json: ca-url: https://127.0.0.1 ca-config: /home/step/config/ca.json - fingerprint: + fingerprint: e543cad8e9f6417076bb5aed3471c588152118aac1e0ca7984a43ee7f76da5e3 root: /home/step/certs/root_ca.crt # Certificates contains the root and intermediate certificate and @@ -64,11 +64,18 @@ inject: x509: # intermediate_ca_key contains the contents of your encrypted intermediate CA key intermediate_ca_key: | - these are just some fake intermediate CA key bytes + -----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: | - these are just some fake root CA key bytes + -----BEGIN EC PRIVATE KEY----- + dGhlc2UgYXJlIGp1c3Qgc29tZSBmYWtlIHJvb3QgQ0Ega2V5IGJ5dGVz + -----END EC PRIVATE KEY----- + diff --git a/pki/testdata/helm/with-ssh.yml b/pki/testdata/helm/with-ssh.yml index 1f922994..e1ce4143 100644 --- a/pki/testdata/helm/with-ssh.yml +++ b/pki/testdata/helm/with-ssh.yml @@ -35,7 +35,7 @@ inject: defaults.json: ca-url: https://127.0.0.1 ca-config: /home/step/config/ca.json - fingerprint: + fingerprint: e543cad8e9f6417076bb5aed3471c588152118aac1e0ca7984a43ee7f76da5e3 root: /home/step/certs/root_ca.crt # Certificates contains the root and intermediate certificate and @@ -56,10 +56,10 @@ inject: -----END CERTIFICATE----- # ssh_host_ca contains the text of the public ssh key for the SSH root CA - ssh_host_ca: fake ssh host cert bytes + 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: fake ssh user cert bytes + ssh_user_ca: ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBEWA1qUxaGwVNErsvEOGe2d6TvLMF+aiVpuOiIEvpMJ3JeJmecLQctjWqeIbpSvy6/gRa7c82Ge5rLlapYmOChs= # Secrets contains the root and intermediate keys and optionally the SSH # private keys @@ -72,19 +72,32 @@ inject: x509: # intermediate_ca_key contains the contents of your encrypted intermediate CA key intermediate_ca_key: | - these are just some fake intermediate CA key bytes + -----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: | - these are just some fake root CA key bytes + -----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: | - fake ssh host key bytes + -----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: | - fake ssh user key bytes + -----BEGIN EC PRIVATE KEY----- + ZmFrZSBzc2ggdXNlciBrZXkgYnl0ZXM= + -----END EC PRIVATE KEY----- + From 57001168a5e6757f0472eea24a20a58460fea04d Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Fri, 14 Oct 2022 14:05:39 +0200 Subject: [PATCH 12/18] Add default `SSHPOP` provisioner to Helm template output --- pki/helm.go | 29 ++++--- pki/helm_test.go | 16 ++++ pki/testdata/helm/with-ssh-and-acme.yml | 105 ++++++++++++++++++++++++ pki/testdata/helm/with-ssh.yml | 1 + 4 files changed, 139 insertions(+), 12 deletions(-) create mode 100644 pki/testdata/helm/with-ssh-and-acme.yml diff --git a/pki/helm.go b/pki/helm.go index 3fbadb40..72d95971 100644 --- a/pki/helm.go +++ b/pki/helm.go @@ -35,18 +35,14 @@ func (p *PKI) WriteHelmTemplate(w io.Writer) error { p.Ssh = nil } - // Convert provisioner to ca.json - numberOfProvisioners := len(p.Authority.Provisioners) - if p.options.enableACME { - numberOfProvisioners++ - } - provisioners := make([]provisioner.Interface, numberOfProvisioners) - for i, p := range p.Authority.Provisioners { + // Convert provisioners to ca.json representation + provisioners := []provisioner.Interface{} + for _, p := range p.Authority.Provisioners { pp, err := authority.ProvisionerToCertificates(p) if err != nil { return err } - provisioners[i] = pp + provisioners = append(provisioners, pp) } // Add default ACME provisioner if enabled. Note that this logic is similar @@ -56,14 +52,23 @@ func (p *PKI) WriteHelmTemplate(w io.Writer) error { // TODO(hs): consider refactoring the initialization, so that this becomes // easier to reason about and maintain. if p.options.enableACME { - provisioners[len(provisioners)-1] = &provisioner.ACME{ + provisioners = append(provisioners, &provisioner.ACME{ Type: "ACME", Name: "acme", - } + }) } - // TODO(hs): add default SSHPOP provisioner if SSH is configured, similar - // as the ACME one above. + // Add default SSHPOP provisioner if enabled. Similar to the above, this is + // the same as what happens in p.GenerateConfig(). + if p.options.enableSSH { + provisioners = append(provisioners, &provisioner.SSHPOP{ + Type: "SSHPOP", + Name: "sshpop", + Claims: &provisioner.Claims{ + EnableSSHCA: &p.options.enableSSH, + }, + }) + } if err := tmpl.Execute(w, helmVariables{ Configuration: &p.Configuration, diff --git a/pki/helm_test.go b/pki/helm_test.go index 1eb621a8..21d6d4db 100644 --- a/pki/helm_test.go +++ b/pki/helm_test.go @@ -105,6 +105,22 @@ func TestPKI_WriteHelmTemplate(t *testing.T) { testFile: "testdata/helm/with-ssh.yml", wantErr: false, }, + { + name: "ok/with-ssh-and-acme", + fields: fields{ + pkiOptions: []Option{ + WithHelm(), + WithACME(), + WithSSH(), + }, + casOptions: apiv1.Options{ + Type: "softcas", + IsCreator: true, + }, + }, + testFile: "testdata/helm/with-ssh-and-acme.yml", + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pki/testdata/helm/with-ssh-and-acme.yml b/pki/testdata/helm/with-ssh-and-acme.yml new file mode 100644 index 00000000..639aca6a --- /dev/null +++ b/pki/testdata/helm/with-ssh-and-acme.yml @@ -0,0 +1,105 @@ +# 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":"step-cli","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":"ACME","name":"acme"} + - {"type":"SSHPOP","name":"sshpop","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----- + diff --git a/pki/testdata/helm/with-ssh.yml b/pki/testdata/helm/with-ssh.yml index e1ce4143..2e4845f0 100644 --- a/pki/testdata/helm/with-ssh.yml +++ b/pki/testdata/helm/with-ssh.yml @@ -24,6 +24,7 @@ inject: enableAdmin: false provisioners: - {"type":"JWK","name":"step-cli","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","claims":{"enableSSHCA":true}} tls: cipherSuites: - TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 From d981b9e0dc5cf1d430a4c5db3f1c8a8e0d05c265 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Fri, 14 Oct 2022 16:01:18 +0200 Subject: [PATCH 13/18] Add `--admin-subject` flag to `ca init` The first super admin subject can now be provided through the `--admin-subject` flag when initializing a CA. It's not yet possible to configure the subject of the first super admin when provisioners are migrated from `ca.json` to the database. This effectively limits usage of the flag to scenarios in which the provisioners are written to the database immediately, so when `--remote-management` is enabled. It currently also doesn't work with Helm deployments, because there's no mechanism yet to pass this type of option to the Helm chart. This commit partially addresses https://github.com/smallstep/cli/issues/697 --- authority/authority.go | 8 ++ pki/helm_test.go | 225 ++++++++++++++++++----------------------- pki/pki.go | 45 ++++++--- 3 files changed, 141 insertions(+), 137 deletions(-) diff --git a/authority/authority.go b/authority/authority.go index e3bc3473..ae8b9a56 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -663,6 +663,14 @@ func (a *Authority) init() error { } // Create first super admin, belonging to the first JWK provisioner + // TODO(hs): pass a user-provided first super admin subject to here. With `ca init` it's + // added to the DB immediately if using remote management. But when migrating from + // ca.json to the DB, this option doesn't exist. Adding a flag just to do it during + // migration isn't nice. We could opt for a user to change it afterwards. There exist + // cases in which creation of `step` could lock out a user from API access. This is the + // case if `step` isn't allowed to be signed by Name Constraints or the X.509 policy. + // We have protection for that when creating and updating a policy, but if a policy or + // Name Constraints are in use at the time of migration, that could lock the user out. firstSuperAdminSubject := "step" if err := a.adminDB.CreateAdmin(ctx, &linkedca.Admin{ ProvisionerId: firstJWKProvisioner.Id, diff --git a/pki/helm_test.go b/pki/helm_test.go index 21d6d4db..ea1c4acd 100644 --- a/pki/helm_test.go +++ b/pki/helm_test.go @@ -21,148 +21,125 @@ import ( ) func TestPKI_WriteHelmTemplate(t *testing.T) { - type fields struct { - casOptions apiv1.Options - pkiOptions []Option + var preparePKI = func(t *testing.T, opts ...Option) *PKI { + o := apiv1.Options{ + Type: "softcas", + IsCreator: true, + } + + // Add default WithHelm option + opts = append(opts, WithHelm()) + + // TODO(hs): invoking `New` doesn't perform all operations that are executed + // when `ca init --helm` is executed. Ideally this logic should be handled + // in one place and probably inside of the PKI initialization. For testing + // purposes the missing operations to fill a Helm template fully are faked + // by `setKeyPair`, `setCertificates` and `setSSHSigningKeys` + p, err := New(o, opts...) + assert.NoError(t, err) + + // setKeyPair sets a predefined JWK and a default JWK provisioner. This is one + // of the things performed in the `ca init` code that's not part of `New`, but + // performed after that in p.GenerateKeyPairs`. We're currently using the same + // JWK for every test to keep test variance small: we're not testing JWK generation + // here after all. It's a bit dangerous to redefine the function here, but it's + // the simplest way to make this fully testable without refactoring the init now. + // The password for the predefined encrypted key is \x01\x03\x03\x07. + setKeyPair(t, p) + + // setCertificates sets some static intermediate and root CA certificate bytes. It + // replaces the logic executed in `p.GenerateRootCertificate`, `p.WriteRootCertificate`, + // and `p.GenerateIntermediateCertificate`. + setCertificates(t, p) + + // setSSHSigningKeys sets predefined SSH user and host certificate and key bytes. + // This replaces the logic in `p.GenerateSSHSigningKeys` + setSSHSigningKeys(t, p) + + return p } - tests := []struct { - name string - fields fields + type test struct { + pki *PKI testFile string wantErr bool - }{ - { - name: "ok/simple", - fields: fields{ - pkiOptions: []Option{ - WithHelm(), - }, - casOptions: apiv1.Options{ - Type: "softcas", - IsCreator: true, - }, - }, - testFile: "testdata/helm/simple.yml", - wantErr: false, + } + var tests = map[string]func(t *testing.T) test{ + "ok/simple": func(t *testing.T) test { + return test{ + pki: preparePKI(t), + testFile: "testdata/helm/simple.yml", + wantErr: false, + } }, - { - name: "ok/with-provisioner", - fields: fields{ - pkiOptions: []Option{ - WithHelm(), - WithProvisioner("a-provisioner"), - }, - casOptions: apiv1.Options{ - Type: "softcas", - IsCreator: true, - }, - }, - testFile: "testdata/helm/with-provisioner.yml", - wantErr: false, + "ok/with-provisioner": func(t *testing.T) test { + return test{ + pki: preparePKI(t, WithProvisioner("a-provisioner")), + testFile: "testdata/helm/with-provisioner.yml", + wantErr: false, + } }, - { - name: "ok/with-acme", - fields: fields{ - pkiOptions: []Option{ - WithHelm(), - WithACME(), - }, - casOptions: apiv1.Options{ - Type: "softcas", - IsCreator: true, - }, - }, - testFile: "testdata/helm/with-acme.yml", - wantErr: false, + "ok/with-acme": func(t *testing.T) test { + return test{ + pki: preparePKI(t, WithACME()), + testFile: "testdata/helm/with-acme.yml", + wantErr: false, + } }, - { - name: "ok/with-admin", - fields: fields{ - pkiOptions: []Option{ - WithHelm(), - WithAdmin(), - }, - casOptions: apiv1.Options{ - Type: "softcas", - IsCreator: true, - }, - }, - testFile: "testdata/helm/with-admin.yml", - wantErr: false, + "ok/with-admin": func(t *testing.T) test { + return test{ + pki: preparePKI(t, WithAdmin()), + testFile: "testdata/helm/with-admin.yml", + wantErr: false, + } }, - { - name: "ok/with-ssh", - fields: fields{ - pkiOptions: []Option{ - WithHelm(), - WithSSH(), - }, - casOptions: apiv1.Options{ - Type: "softcas", - IsCreator: true, - }, - }, - testFile: "testdata/helm/with-ssh.yml", - wantErr: false, + "ok/with-ssh": func(t *testing.T) test { + return test{ + pki: preparePKI(t, WithSSH()), + testFile: "testdata/helm/with-ssh.yml", + wantErr: false, + } }, - { - name: "ok/with-ssh-and-acme", - fields: fields{ - pkiOptions: []Option{ - WithHelm(), - WithACME(), - WithSSH(), + "ok/with-ssh-and-acme": func(t *testing.T) test { + return test{ + pki: preparePKI(t, WithSSH(), WithACME()), + testFile: "testdata/helm/with-ssh-and-acme.yml", + wantErr: false, + } + }, + "fail/authority.ProvisionerToCertificates": func(t *testing.T) test { + pki := preparePKI(t) + pki.Authority.Provisioners = append(pki.Authority.Provisioners, + &linkedca.Provisioner{ + Type: linkedca.Provisioner_JWK, + Name: "Broken JWK", + Details: nil, }, - casOptions: apiv1.Options{ - Type: "softcas", - IsCreator: true, - }, - }, - testFile: "testdata/helm/with-ssh-and-acme.yml", - wantErr: false, + ) + return test{ + pki: pki, + wantErr: true, + } }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - o := tt.fields.casOptions - opts := tt.fields.pkiOptions - - // TODO(hs): invoking `New` doesn't perform all operations that are executed - // when `ca init --helm` is executed. Ideally this logic should be handled - // in one place and probably inside of the PKI initialization. For testing - // purposes the missing operations to fill a Helm template fully are faked - // by `setKeyPair`, `setCertificates` and `setSSHSigningKeys` - p, err := New(o, opts...) - assert.NoError(t, err) - - // setKeyPair sets a predefined JWK and a default JWK provisioner. This is one - // of the things performed in the `ca init` code that's not part of `New`, but - // performed after that in p.GenerateKeyPairs`. We're currently using the same - // JWK for every test to keep test variance small: we're not testing JWK generation - // here after all. It's a bit dangerous to redefine the function here, but it's - // the simplest way to make this fully testable without refactoring the init now. - // The password for the predefined encrypted key is \x01\x03\x03\x07. - setKeyPair(t, p) - - // setCertificates sets some static intermediate and root CA certificate bytes. It - // replaces the logic executed in `p.GenerateRootCertificate`, `p.WriteRootCertificate`, - // and `p.GenerateIntermediateCertificate`. - setCertificates(t, p) - - // setSSHSigningKeys sets predefined SSH user and host certificate and key bytes. - // This replaces the logic in `p.GenerateSSHSigningKeys` - setSSHSigningKeys(t, p) + for name, run := range tests { + tc := run(t) + t.Run(name, func(t *testing.T) { w := &bytes.Buffer{} - if err := p.WriteHelmTemplate(w); (err != nil) != tt.wantErr { - t.Errorf("PKI.WriteHelmTemplate() error = %v, wantErr %v", err, tt.wantErr) + if err := tc.pki.WriteHelmTemplate(w); (err != nil) != tc.wantErr { + t.Errorf("PKI.WriteHelmTemplate() error = %v, wantErr %v", err, tc.wantErr) return } - wantBytes, err := os.ReadFile(tt.testFile) + if tc.wantErr { + // don't compare output if an error was expected on output + return + } + + wantBytes, err := os.ReadFile(tc.testFile) assert.NoError(t, err) if diff := cmp.Diff(wantBytes, w.Bytes()); diff != "" { - t.Logf("Generated Helm template did not match reference %q\n", tt.testFile) + t.Logf("Generated Helm template did not match reference %q\n", tc.testFile) t.Errorf("Diff follows:\n%s\n", diff) t.Errorf("Full output:\n%s\n", w.Bytes()) } diff --git a/pki/pki.go b/pki/pki.go index a4a64344..cf7c7d09 100644 --- a/pki/pki.go +++ b/pki/pki.go @@ -175,18 +175,19 @@ func GetProvisionerKey(caURL, rootFile, kid string) (string, error) { } type options struct { - provisioner string - pkiOnly bool - enableACME bool - enableSSH bool - enableAdmin bool - noDB bool - isHelm bool - deploymentType DeploymentType - rootKeyURI string - intermediateKeyURI string - hostKeyURI string - userKeyURI string + provisioner string + firstSuperAdminSubject string + pkiOnly bool + enableACME bool + enableSSH bool + enableAdmin bool + noDB bool + isHelm bool + deploymentType DeploymentType + rootKeyURI string + intermediateKeyURI string + hostKeyURI string + userKeyURI string } // Option is the type of a configuration option on the pki constructor. @@ -220,6 +221,15 @@ func WithProvisioner(s string) Option { } } +// WithFirstSuperAdminSubject defines the subject of the first +// super admin for use with the Admin API. The admin will belong +// to the first JWK provisioner. +func WithFirstSuperAdminSubject(s string) Option { + return func(p *PKI) { + p.options.firstSuperAdminSubject = s + } +} + // WithPKIOnly will only generate the PKI without the step-ca config files. func WithPKIOnly() Option { return func(p *PKI) { @@ -886,6 +896,11 @@ func (p *PKI) GenerateConfig(opt ...ConfigOption) (*authconfig.Config, error) { // // Note that we might want to be able to define the database as a // flag in `step ca init` so we can write to the proper place. + // + // TODO(hs): the logic for creating the provisioners and the super admin + // is similar to what's done when automatically migrating the provisioners. + // This is related to the existing comment above. Refactor this to exist in + // a single place and ensure it happensonly once. _db, err := db.New(cfg.DB) if err != nil { return nil, err @@ -909,9 +924,13 @@ func (p *PKI) GenerateConfig(opt ...ConfigOption) (*authconfig.Config, error) { } } // Add the first provisioner as an admin. + firstSuperAdminSubject := "step" + if p.options.firstSuperAdminSubject != "" { + firstSuperAdminSubject = p.options.firstSuperAdminSubject + } if err := adminDB.CreateAdmin(context.Background(), &linkedca.Admin{ AuthorityId: admin.DefaultAuthorityID, - Subject: "step", + Subject: firstSuperAdminSubject, Type: linkedca.Admin_SUPER_ADMIN, ProvisionerId: adminID, }); err != nil { From 49718f1bbb96fc5e889a249408dfb32e265b2eca Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Fri, 21 Oct 2022 11:11:42 +0200 Subject: [PATCH 14/18] Fix some comments --- authority/authority.go | 14 ++++++++------ pki/pki.go | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/authority/authority.go b/authority/authority.go index ae8b9a56..c3155d96 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -94,7 +94,7 @@ type Authority struct { // If true, do not initialize the authority skipInit bool - // If true, does not output initialization logs + // If true, do not output initialization logs quietInit bool } @@ -603,9 +603,13 @@ func (a *Authority) init() error { return admin.WrapErrorISE(err, "error loading provisioners to initialize authority") } if len(provs) == 0 && !strings.EqualFold(a.config.AuthorityConfig.DeploymentType, "linked") { + // Migration will currently only be kicked off once, because either one or more provisioners + // are migrated or a default JWK provisioner will be created in the DB. It won't run for + // linked or hosted deployments. Not for linked, because that case is explicitly checked + // for above. Not for hosted, because there'll be at least an existing OIDC provisioner. var firstJWKProvisioner *linkedca.Provisioner if len(a.config.AuthorityConfig.Provisioners) > 0 { - // Existing provisioners detected; try migrating them to DB storage + // Existing provisioners detected; try migrating them to DB storage. a.initLogf("Starting migration of provisioners") for _, p := range a.config.AuthorityConfig.Provisioners { lp, err := ProvisionerToLinkedca(p) @@ -621,14 +625,12 @@ func (a *Authority) init() error { // Mark the first JWK provisioner, so that it can be used for administration purposes if firstJWKProvisioner == nil && lp.Type == linkedca.Provisioner_JWK { firstJWKProvisioner = lp - a.initLogf("Migrated JWK provisioner %q with admin permissions", p.GetName()) // TODO(hs): change the wording? + a.initLogf("Migrated JWK provisioner %q with admin permissions", p.GetName()) } else { a.initLogf("Migrated %s provisioner %q", p.GetType(), p.GetName()) } } - // TODO(hs): test if this works with LinkedCA too. Also could be useful - // for printing out where the configuration is read from in case of LinkedCA. c := a.config if c.WasLoadedFromFile() { // TODO(hs): check if prerequisites for writing files look OK (user/group, permission bits, etc) as @@ -659,7 +661,7 @@ func (a *Authority) init() error { if err != nil { return admin.WrapErrorISE(err, "error creating first provisioner") } - a.initLogf("Created JWK provisioner %q with admin permissions", firstJWKProvisioner.GetName()) // TODO(hs): change the wording? + a.initLogf("Created JWK provisioner %q with admin permissions", firstJWKProvisioner.GetName()) } // Create first super admin, belonging to the first JWK provisioner diff --git a/pki/pki.go b/pki/pki.go index cf7c7d09..df65a721 100644 --- a/pki/pki.go +++ b/pki/pki.go @@ -900,7 +900,7 @@ func (p *PKI) GenerateConfig(opt ...ConfigOption) (*authconfig.Config, error) { // TODO(hs): the logic for creating the provisioners and the super admin // is similar to what's done when automatically migrating the provisioners. // This is related to the existing comment above. Refactor this to exist in - // a single place and ensure it happensonly once. + // a single place and ensure it happens only once. _db, err := db.New(cfg.DB) if err != nil { return nil, err From fd38dd34f945bedb9728139a38573806d73c6b59 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Mon, 24 Oct 2022 14:51:27 +0200 Subject: [PATCH 15/18] Fix PR comments --- authority/authority.go | 28 ++++++++-------------------- pki/pki.go | 40 ++++++++++++++++++++-------------------- 2 files changed, 28 insertions(+), 40 deletions(-) diff --git a/authority/authority.go b/authority/authority.go index c3155d96..47a1de1c 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -633,23 +633,11 @@ func (a *Authority) init() error { c := a.config if c.WasLoadedFromFile() { - // TODO(hs): check if prerequisites for writing files look OK (user/group, permission bits, etc) as - // extra safety check before trying to write at all? - - // Remove the existing provisioners from the authority configuration - // and commit it to the existing configuration file. NOTE: committing - // the configuration at this point also writes other properties that - // have been initialized with default values, such as the `backdate` and - // `template` settings in the `authority`. - oldProvisioners := c.AuthorityConfig.Provisioners - c.AuthorityConfig.Provisioners = []provisioner.Interface{} - if err := c.Commit(); err != nil { - // Restore the provisioners in in-memory representation for consistency - // when writing the updated configuration fails. This is considered a soft - // error, so execution can continue. - c.AuthorityConfig.Provisioners = oldProvisioners - a.initLogf("Failed removing provisioners from configuration: %v", err) - } + // The provisioners in the configuration file can be deleted from + // the file by editing it. Automatic rewriting of the file was considered + // to be too surprising for users and not the right solution for all + // use cases, so we leave it up to users to this themselves. + a.initLogf("Provisioners that were migrated can now be removed from `ca.json` by editing it.") } a.initLogf("Finished migrating provisioners") @@ -673,16 +661,16 @@ func (a *Authority) init() error { // case if `step` isn't allowed to be signed by Name Constraints or the X.509 policy. // We have protection for that when creating and updating a policy, but if a policy or // Name Constraints are in use at the time of migration, that could lock the user out. - firstSuperAdminSubject := "step" + superAdminSubject := "step" if err := a.adminDB.CreateAdmin(ctx, &linkedca.Admin{ ProvisionerId: firstJWKProvisioner.Id, - Subject: firstSuperAdminSubject, + Subject: superAdminSubject, Type: linkedca.Admin_SUPER_ADMIN, }); err != nil { return admin.WrapErrorISE(err, "error creating first admin") } - a.initLogf("Created super admin %q for JWK provisioner %q", firstSuperAdminSubject, firstJWKProvisioner.GetName()) + a.initLogf("Created super admin %q for JWK provisioner %q", superAdminSubject, firstJWKProvisioner.GetName()) } } diff --git a/pki/pki.go b/pki/pki.go index df65a721..cee3f06a 100644 --- a/pki/pki.go +++ b/pki/pki.go @@ -175,19 +175,19 @@ func GetProvisionerKey(caURL, rootFile, kid string) (string, error) { } type options struct { - provisioner string - firstSuperAdminSubject string - pkiOnly bool - enableACME bool - enableSSH bool - enableAdmin bool - noDB bool - isHelm bool - deploymentType DeploymentType - rootKeyURI string - intermediateKeyURI string - hostKeyURI string - userKeyURI string + provisioner string + superAdminSubject string + pkiOnly bool + enableACME bool + enableSSH bool + enableAdmin bool + noDB bool + isHelm bool + deploymentType DeploymentType + rootKeyURI string + intermediateKeyURI string + hostKeyURI string + userKeyURI string } // Option is the type of a configuration option on the pki constructor. @@ -221,12 +221,12 @@ func WithProvisioner(s string) Option { } } -// WithFirstSuperAdminSubject defines the subject of the first +// WithSuperAdminSubject defines the subject of the first // super admin for use with the Admin API. The admin will belong // to the first JWK provisioner. -func WithFirstSuperAdminSubject(s string) Option { +func WithSuperAdminSubject(s string) Option { return func(p *PKI) { - p.options.firstSuperAdminSubject = s + p.options.superAdminSubject = s } } @@ -924,13 +924,13 @@ func (p *PKI) GenerateConfig(opt ...ConfigOption) (*authconfig.Config, error) { } } // Add the first provisioner as an admin. - firstSuperAdminSubject := "step" - if p.options.firstSuperAdminSubject != "" { - firstSuperAdminSubject = p.options.firstSuperAdminSubject + superAdminSubject := "step" + if p.options.superAdminSubject != "" { + superAdminSubject = p.options.superAdminSubject } if err := adminDB.CreateAdmin(context.Background(), &linkedca.Admin{ AuthorityId: admin.DefaultAuthorityID, - Subject: firstSuperAdminSubject, + Subject: superAdminSubject, Type: linkedca.Admin_SUPER_ADMIN, ProvisionerId: adminID, }); err != nil { From 54c560f62027b12f917244d654e8e33e5a82a069 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Mon, 24 Oct 2022 15:22:37 +0200 Subject: [PATCH 16/18] Improve configuration file initialization log output --- authority/config/config.go | 16 +++++++++++----- ca/ca.go | 9 ++++++++- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/authority/config/config.go b/authority/config/config.go index 79228e98..7ec1d12e 100644 --- a/authority/config/config.go +++ b/authority/config/config.go @@ -75,7 +75,7 @@ type Config struct { SkipValidation bool `json:"-"` // Keeps record of the filename the Config is read from - loadedFromFilename string + loadedFromFilepath string } // ASN1DN contains ASN1.DN attributes that are used in Subject and Issuer @@ -167,7 +167,7 @@ func LoadConfiguration(filename string) (*Config, error) { } // store filename that was read to populate Config - c.loadedFromFilename = filename + c.loadedFromFilepath = filename // initialize the Config c.Init() @@ -215,13 +215,19 @@ func (c *Config) Commit() error { if !c.WasLoadedFromFile() { return errors.New("cannot commit configuration if not loaded from file") } - return c.Save(c.loadedFromFilename) + return c.Save(c.loadedFromFilepath) } // WasLoadedFromFile returns whether or not the Config was -// read from a file. +// loaded from a file. func (c *Config) WasLoadedFromFile() bool { - return c.loadedFromFilename != "" + return c.loadedFromFilepath != "" +} + +// Filepath returns the path to the file the Config was +// loaded from. +func (c *Config) Filepath() string { + return c.loadedFromFilepath } // Validate validates the configuration. diff --git a/ca/ca.go b/ca/ca.go index 98f845b0..880f7e46 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -349,7 +349,7 @@ func (ca *CA) Run() error { if step.Contexts().GetCurrent() != nil { log.Printf("Current context: %s", step.Contexts().GetCurrent().Name) } - log.Printf("Config file: %s", ca.opts.configFile) + log.Printf("Config file: %s", ca.getConfigFileOutput()) baseURL := fmt.Sprintf("https://%s%s", authorityInfo.DNSNames[0], ca.config.Address[strings.LastIndex(ca.config.Address, ":"):]) @@ -569,3 +569,10 @@ func dumpRoutes(mux chi.Routes) { fmt.Printf("Logging err: %s\n", err.Error()) } } + +func (ca *CA) getConfigFileOutput() string { + if ca.config.WasLoadedFromFile() { + return ca.config.Filepath() + } + return "loaded from token" +} From 9d04e7d1dcf089d981001e5c6ac7c5b423fd95f0 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Mon, 24 Oct 2022 15:33:48 +0200 Subject: [PATCH 17/18] Remove period in log output --- authority/authority.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/authority/authority.go b/authority/authority.go index 47a1de1c..1efe1a7c 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -637,7 +637,7 @@ func (a *Authority) init() error { // the file by editing it. Automatic rewriting of the file was considered // to be too surprising for users and not the right solution for all // use cases, so we leave it up to users to this themselves. - a.initLogf("Provisioners that were migrated can now be removed from `ca.json` by editing it.") + a.initLogf("Provisioners that were migrated can now be removed from `ca.json` by editing it") } a.initLogf("Finished migrating provisioners") From e90fe4bfa05b39080597b34059c8745b96fad273 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Mon, 24 Oct 2022 16:34:34 +0200 Subject: [PATCH 18/18] Update CHANGELOG.md with provisioner migration --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e7172eda..59829021 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Added name constraints evaluation and enforcement when issuing or renewing X.509 certificates. - Added provisioner webhooks for augmenting template data and authorizing certificate requests before signing. +- Added automatic migration of provisioners when enabling remote managment. ### Fixed - MySQL DSN parsing issues fixed with upgrade to [smallstep/nosql@v0.5.0](https://github.com/smallstep/nosql/releases/tag/v0.5.0).