From 469ad6d09a3d0a0c7f0417b2299a5aab0031b6b2 Mon Sep 17 00:00:00 2001 From: Gabriel Santos Date: Thu, 12 May 2022 13:50:40 +0100 Subject: [PATCH] not_before_duration added to SSH (#15250) * add-not-before-duration-to-ssh * Missing field * Adding tests * changelog file * Backend test * Requested changes * Update builtin/logical/ssh/path_roles.go Co-authored-by: Alexander Scheel --- builtin/logical/ssh/backend_test.go | 94 ++++++++++++++++++++++ builtin/logical/ssh/path_config_ca_test.go | 1 + builtin/logical/ssh/path_roles.go | 21 ++++- builtin/logical/ssh/path_sign.go | 2 +- changelog/15250.txt | 3 + ui/app/models/role-ssh.js | 1 + website/content/api-docs/secret/ssh.mdx | 2 + 7 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 changelog/15250.txt diff --git a/builtin/logical/ssh/backend_test.go b/builtin/logical/ssh/backend_test.go index adee82aa1..d830103e8 100644 --- a/builtin/logical/ssh/backend_test.go +++ b/builtin/logical/ssh/backend_test.go @@ -1682,6 +1682,100 @@ func TestBackend_DefExtTemplatingDisabled(t *testing.T) { } } +func TestSSHBackend_ValidateNotBeforeDuration(t *testing.T) { + config := logical.TestBackendConfig() + + b, err := Factory(context.Background(), config) + if err != nil { + t.Fatalf("Cannot create backend: %s", err) + } + testCase := logicaltest.TestCase{ + LogicalBackend: b, + Steps: []logicaltest.TestStep{ + configCaStep(testCAPublicKey, testCAPrivateKey), + + createRoleStep("testing", map[string]interface{}{ + "key_type": "ca", + "allow_host_certificates": true, + "allowed_domains": "example.com,example.org", + "allow_subdomains": true, + "default_critical_options": map[string]interface{}{ + "option": "value", + }, + "default_extensions": map[string]interface{}{ + "extension": "extended", + }, + "not_before_duration": "300s", + }), + + signCertificateStep("testing", "vault-root-22608f5ef173aabf700797cb95c5641e792698ec6380e8e1eb55523e39aa5e51", ssh.HostCert, []string{"dummy.example.org", "second.example.com"}, map[string]string{ + "option": "value", + }, map[string]string{ + "extension": "extended", + }, + 2*time.Hour+5*time.Minute-30*time.Second, map[string]interface{}{ + "public_key": publicKey2, + "ttl": "2h", + "cert_type": "host", + "valid_principals": "dummy.example.org,second.example.com", + }), + + createRoleStep("testing", map[string]interface{}{ + "key_type": "ca", + "allow_host_certificates": true, + "allowed_domains": "example.com,example.org", + "allow_subdomains": true, + "default_critical_options": map[string]interface{}{ + "option": "value", + }, + "default_extensions": map[string]interface{}{ + "extension": "extended", + }, + "not_before_duration": "2h", + }), + + signCertificateStep("testing", "vault-root-22608f5ef173aabf700797cb95c5641e792698ec6380e8e1eb55523e39aa5e51", ssh.HostCert, []string{"dummy.example.org", "second.example.com"}, map[string]string{ + "option": "value", + }, map[string]string{ + "extension": "extended", + }, + 4*time.Hour-30*time.Second, map[string]interface{}{ + "public_key": publicKey2, + "ttl": "2h", + "cert_type": "host", + "valid_principals": "dummy.example.org,second.example.com", + }), + createRoleStep("testing", map[string]interface{}{ + "key_type": "ca", + "allow_host_certificates": true, + "allowed_domains": "example.com,example.org", + "allow_subdomains": true, + "default_critical_options": map[string]interface{}{ + "option": "value", + }, + "default_extensions": map[string]interface{}{ + "extension": "extended", + }, + "not_before_duration": "30s", + }), + + signCertificateStep("testing", "vault-root-22608f5ef173aabf700797cb95c5641e792698ec6380e8e1eb55523e39aa5e51", ssh.HostCert, []string{"dummy.example.org", "second.example.com"}, map[string]string{ + "option": "value", + }, map[string]string{ + "extension": "extended", + }, + 2*time.Hour, map[string]interface{}{ + "public_key": publicKey2, + "ttl": "2h", + "cert_type": "host", + "valid_principals": "dummy.example.org,second.example.com", + }), + }, + } + + logicaltest.Test(t, testCase) +} + func getSshCaTestCluster(t *testing.T, userIdentity string) (*vault.TestCluster, string) { coreConfig := &vault.CoreConfig{ CredentialBackends: map[string]logical.Factory{ diff --git a/builtin/logical/ssh/path_config_ca_test.go b/builtin/logical/ssh/path_config_ca_test.go index 1a04d9dbe..651ed42ce 100644 --- a/builtin/logical/ssh/path_config_ca_test.go +++ b/builtin/logical/ssh/path_config_ca_test.go @@ -255,6 +255,7 @@ func TestSSH_ConfigCAKeyTypes(t *testing.T) { "allowed_users": "*", "key_type": "ca", "ttl": "30s", + "not_before_duration": "2h", } roleReq := &logical.Request{ Operation: logical.UpdateOperation, diff --git a/builtin/logical/ssh/path_roles.go b/builtin/logical/ssh/path_roles.go index d46ad622e..79c3947fe 100644 --- a/builtin/logical/ssh/path_roles.go +++ b/builtin/logical/ssh/path_roles.go @@ -28,7 +28,7 @@ const ( // Present version of the sshRole struct; when adding a new field or are // needing to perform a migration, increment this struct and read the note // in checkUpgrade(...). - roleEntryVersion = 2 + roleEntryVersion = 3 ) // Structure that represents a role in SSH backend. This is a common role structure @@ -65,6 +65,7 @@ type sshRole struct { AllowedUserKeyTypesLengths map[string][]int `mapstructure:"allowed_user_key_types_lengths" json:"allowed_user_key_types_lengths"` AlgorithmSigner string `mapstructure:"algorithm_signer" json:"algorithm_signer"` Version int `mapstructure:"role_version" json:"role_version"` + NotBeforeDuration time.Duration `mapstructure:"not_before_duration" json:"not_before_duration"` } func pathListRoles(b *backend) *framework.Path { @@ -363,6 +364,16 @@ func pathRoles(b *backend) *framework.Path { Name: "Signing Algorithm", }, }, + "not_before_duration": { + Type: framework.TypeDurationSecond, + Default: 30, + Description: ` + The duration that the SSH certificate should be backdated by at issuance.`, + DisplayAttrs: &framework.DisplayAttributes{ + Name: "Not before duration", + Value: 30, + }, + }, }, Callbacks: map[logical.Operation]framework.OperationFunc{ @@ -555,6 +566,7 @@ func (b *backend) createCARole(allowedUsers, defaultUser, signer string, data *f KeyType: KeyTypeCA, AlgorithmSigner: signer, Version: roleEntryVersion, + NotBeforeDuration: time.Duration(data.Get("not_before_duration").(int)) * time.Second, } if !role.AllowUserCertificates && !role.AllowHostCertificates { @@ -672,6 +684,12 @@ func (b *backend) checkUpgrade(ctx context.Context, s logical.Storage, n string, err = nil } + if result.Version < 3 { + modified = true + result.NotBeforeDuration = 30 * time.Second + result.Version = 3 + } + // Add new migrations just before here. // // Condition copied from PKI builtin. @@ -739,6 +757,7 @@ func (b *backend) parseRole(role *sshRole) (map[string]interface{}, error) { "default_extensions_template": role.DefaultExtensionsTemplate, "allowed_user_key_lengths": role.AllowedUserKeyTypesLengths, "algorithm_signer": role.AlgorithmSigner, + "not_before_duration": role.NotBeforeDuration, } case KeyTypeDynamic: result = map[string]interface{}{ diff --git a/builtin/logical/ssh/path_sign.go b/builtin/logical/ssh/path_sign.go index df6923feb..39d384055 100644 --- a/builtin/logical/ssh/path_sign.go +++ b/builtin/logical/ssh/path_sign.go @@ -580,7 +580,7 @@ func (b *creationBundle) sign() (retCert *ssh.Certificate, retErr error) { Key: b.PublicKey, KeyId: b.KeyID, ValidPrincipals: b.ValidPrincipals, - ValidAfter: uint64(now.Add(-30 * time.Second).In(time.UTC).Unix()), + ValidAfter: uint64(now.Add(-b.Role.NotBeforeDuration).In(time.UTC).Unix()), ValidBefore: uint64(now.Add(b.TTL).In(time.UTC).Unix()), CertType: b.CertificateType, Permissions: ssh.Permissions{ diff --git a/changelog/15250.txt b/changelog/15250.txt new file mode 100644 index 000000000..36d6d03b8 --- /dev/null +++ b/changelog/15250.txt @@ -0,0 +1,3 @@ +```release-note:improvement +secrets/ssh: Support for `add_before_duration` in SSH +``` \ No newline at end of file diff --git a/ui/app/models/role-ssh.js b/ui/app/models/role-ssh.js index 91ef357b6..53221d3a9 100644 --- a/ui/app/models/role-ssh.js +++ b/ui/app/models/role-ssh.js @@ -36,6 +36,7 @@ const CA_FIELDS = [ 'allowSubdomains', 'allowUserKeyIds', 'keyIdFormat', + 'notBeforeDuration' ]; export default Model.extend({ diff --git a/website/content/api-docs/secret/ssh.mdx b/website/content/api-docs/secret/ssh.mdx index a76a97bf1..eec5ccd08 100644 --- a/website/content/api-docs/secret/ssh.mdx +++ b/website/content/api-docs/secret/ssh.mdx @@ -831,6 +831,8 @@ to the restrictions contained in the role named in the endpoint. - `extensions` `(map: "")` – Specifies a map of the extensions that the certificate should be signed for. Defaults to none. +- `not_before_duration` `(duration: "30s")` – Specifies the duration by which to backdate the `ValidAfter` property. + ### Sample Payload ```json