From 21a10e09b68ede4550f8e9d50a8bc16962f953f5 Mon Sep 17 00:00:00 2001 From: Jakob Beckmann <32326425+f4z3r@users.noreply.github.com> Date: Tue, 16 Aug 2022 21:59:29 +0200 Subject: [PATCH] fix bug with allowed_users_template and add allowed_domains_template for SSH role (#16056) * impr(ssh): fix bug with allowed_users_template and add allowed_domains_template field in SSH role configuration, closes #10943 * chore: add changelog entry --- builtin/logical/ssh/backend_test.go | 59 ++++++++++++++++++++----- builtin/logical/ssh/path_issue_sign.go | 8 ++-- builtin/logical/ssh/path_roles.go | 12 +++++ changelog/16056.txt | 3 ++ website/content/api-docs/secret/ssh.mdx | 6 ++- 5 files changed, 71 insertions(+), 17 deletions(-) create mode 100644 changelog/16056.txt diff --git a/builtin/logical/ssh/backend_test.go b/builtin/logical/ssh/backend_test.go index 6f2741185..25c025642 100644 --- a/builtin/logical/ssh/backend_test.go +++ b/builtin/logical/ssh/backend_test.go @@ -322,6 +322,31 @@ func TestBackend_AllowedUsers(t *testing.T) { } } +func TestBackend_AllowedDomainsTemplate(t *testing.T) { + testAllowedDomainsTemplate := "{{ identity.entity.metadata.ssh_username }}.example.com" + expectedValidPrincipal := "foo." + testUserName + ".example.com" + testAllowedPrincipalsTemplate( + t, testAllowedDomainsTemplate, + expectedValidPrincipal, + map[string]string{ + "ssh_username": testUserName, + }, + map[string]interface{}{ + "key_type": testCaKeyType, + "algorithm_signer": "rsa-sha2-256", + "allow_host_certificates": true, + "allow_subdomains": true, + "allowed_domains": testAllowedDomainsTemplate, + "allowed_domains_template": true, + }, + map[string]interface{}{ + "cert_type": "host", + "public_key": testCAPublicKey, + "valid_principals": expectedValidPrincipal, + }, + ) +} + func TestBackend_AllowedUsersTemplate(t *testing.T) { testAllowedUsersTemplate(t, "{{ identity.entity.metadata.ssh_username }}", @@ -2093,9 +2118,9 @@ func testDefaultUserTemplate(t *testing.T, testDefaultUserTemplate string, } } -func testAllowedUsersTemplate(t *testing.T, testAllowedUsersTemplate string, +func testAllowedPrincipalsTemplate(t *testing.T, testAllowedDomainsTemplate string, expectedValidPrincipal string, testEntityMetadata map[string]string, -) { + roleConfigPayload map[string]interface{}, signingPayload map[string]interface{}) { cluster, userpassToken := getSshCaTestCluster(t, testUserName) defer cluster.Cleanup() client := cluster.Cores[0].Client @@ -2115,22 +2140,14 @@ func testAllowedUsersTemplate(t *testing.T, testAllowedUsersTemplate string, t.Fatal(err) } - _, err = client.Logical().Write("ssh/roles/my-role", map[string]interface{}{ - "key_type": testCaKeyType, - "allow_user_certificates": true, - "allowed_users": testAllowedUsersTemplate, - "allowed_users_template": true, - }) + _, err = client.Logical().Write("ssh/roles/my-role", roleConfigPayload) if err != nil { t.Fatal(err) } // sign SSH key as userpass user client.SetToken(userpassToken) - signResponse, err := client.Logical().Write("ssh/sign/my-role", map[string]interface{}{ - "public_key": testCAPublicKey, - "valid_principals": expectedValidPrincipal, - }) + signResponse, err := client.Logical().Write("ssh/sign/my-role", signingPayload) if err != nil { t.Fatal(err) } @@ -2151,6 +2168,24 @@ func testAllowedUsersTemplate(t *testing.T, testAllowedUsersTemplate string, } } +func testAllowedUsersTemplate(t *testing.T, testAllowedUsersTemplate string, + expectedValidPrincipal string, testEntityMetadata map[string]string) { + testAllowedPrincipalsTemplate( + t, testAllowedUsersTemplate, + expectedValidPrincipal, testEntityMetadata, + map[string]interface{}{ + "key_type": testCaKeyType, + "allow_user_certificates": true, + "allowed_users": testAllowedUsersTemplate, + "allowed_users_template": true, + }, + map[string]interface{}{ + "public_key": testCAPublicKey, + "valid_principals": expectedValidPrincipal, + }, + ) +} + func configCaStep(caPublicKey, caPrivateKey string) logicaltest.TestStep { return logicaltest.TestStep{ Operation: logical.UpdateOperation, diff --git a/builtin/logical/ssh/path_issue_sign.go b/builtin/logical/ssh/path_issue_sign.go index d98b81009..e6f225ffd 100644 --- a/builtin/logical/ssh/path_issue_sign.go +++ b/builtin/logical/ssh/path_issue_sign.go @@ -65,7 +65,7 @@ func (b *backend) pathSignIssueCertificateHelper(ctx context.Context, req *logic var parsedPrincipals []string if certificateType == ssh.HostCert { - parsedPrincipals, err = b.calculateValidPrincipals(data, req, role, "", role.AllowedDomains, validateValidPrincipalForHosts(role)) + parsedPrincipals, err = b.calculateValidPrincipals(data, req, role, "", role.AllowedDomains, role.AllowedDomainsTemplate, validateValidPrincipalForHosts(role)) if err != nil { return logical.ErrorResponse(err.Error()), nil } @@ -77,7 +77,7 @@ func (b *backend) pathSignIssueCertificateHelper(ctx context.Context, req *logic return nil, err } } - parsedPrincipals, err = b.calculateValidPrincipals(data, req, role, defaultPrincipal, role.AllowedUsers, strutil.StrListContains) + parsedPrincipals, err = b.calculateValidPrincipals(data, req, role, defaultPrincipal, role.AllowedUsers, role.AllowedUsersTemplate, strutil.StrListContains) if err != nil { return logical.ErrorResponse(err.Error()), nil } @@ -160,7 +160,7 @@ func (b *backend) renderPrincipal(principal string, req *logical.Request) (strin return principal, nil } -func (b *backend) calculateValidPrincipals(data *framework.FieldData, req *logical.Request, role *sshRole, defaultPrincipal, principalsAllowedByRole string, validatePrincipal func([]string, string) bool) ([]string, error) { +func (b *backend) calculateValidPrincipals(data *framework.FieldData, req *logical.Request, role *sshRole, defaultPrincipal, principalsAllowedByRole string, enableTemplating bool, validatePrincipal func([]string, string) bool) ([]string, error) { validPrincipals := "" validPrincipalsRaw, ok := data.GetOk("valid_principals") if ok { @@ -173,7 +173,7 @@ func (b *backend) calculateValidPrincipals(data *framework.FieldData, req *logic // Build list of allowed Principals from template and static principalsAllowedByRole var allowedPrincipals []string for _, principal := range strutil.RemoveDuplicates(strutil.ParseStringSlice(principalsAllowedByRole, ","), false) { - if role.AllowedUsersTemplate { + if enableTemplating { rendered, err := b.renderPrincipal(principal, req) if err != nil { return nil, err diff --git a/builtin/logical/ssh/path_roles.go b/builtin/logical/ssh/path_roles.go index 123696b7a..595839301 100644 --- a/builtin/logical/ssh/path_roles.go +++ b/builtin/logical/ssh/path_roles.go @@ -48,6 +48,7 @@ type sshRole struct { AllowedUsers string `mapstructure:"allowed_users" json:"allowed_users"` AllowedUsersTemplate bool `mapstructure:"allowed_users_template" json:"allowed_users_template"` AllowedDomains string `mapstructure:"allowed_domains" json:"allowed_domains"` + AllowedDomainsTemplate bool `mapstructure:"allowed_domains_template" json:"allowed_domains_template"` KeyOptionSpecs string `mapstructure:"key_option_specs" json:"key_option_specs"` MaxTTL string `mapstructure:"max_ttl" json:"max_ttl"` TTL string `mapstructure:"ttl" json:"ttl"` @@ -223,6 +224,15 @@ func pathRoles(b *backend) *framework.Path { valid host. If only certain domains are allowed, then this list enforces it. `, }, + "allowed_domains_template": { + Type: framework.TypeBool, + Description: ` + [Not applicable for Dynamic type] [Not applicable for OTP type] [Optional for CA type] + If set, Allowed domains can be specified using identity template policies. + Non-templated domains are also permitted. + `, + Default: false, + }, "key_option_specs": { Type: framework.TypeString, Description: ` @@ -567,6 +577,7 @@ func (b *backend) createCARole(allowedUsers, defaultUser, signer string, data *f AllowedUsers: allowedUsers, AllowedUsersTemplate: data.Get("allowed_users_template").(bool), AllowedDomains: data.Get("allowed_domains").(string), + AllowedDomainsTemplate: data.Get("allowed_domains_template").(bool), DefaultUser: defaultUser, DefaultUserTemplate: data.Get("default_user_template").(bool), AllowBareDomains: data.Get("allow_bare_domains").(bool), @@ -750,6 +761,7 @@ func (b *backend) parseRole(role *sshRole) (map[string]interface{}, error) { "allowed_users": role.AllowedUsers, "allowed_users_template": role.AllowedUsersTemplate, "allowed_domains": role.AllowedDomains, + "allowed_domains_template": role.AllowedDomainsTemplate, "default_user": role.DefaultUser, "default_user_template": role.DefaultUserTemplate, "ttl": int64(ttl.Seconds()), diff --git a/changelog/16056.txt b/changelog/16056.txt new file mode 100644 index 000000000..1652726ce --- /dev/null +++ b/changelog/16056.txt @@ -0,0 +1,3 @@ +```release-note:improvement +secrets/ssh: Add allowed_domains_template to allow templating of allowed_domains. +``` diff --git a/website/content/api-docs/secret/ssh.mdx b/website/content/api-docs/secret/ssh.mdx index 60258b44c..75a8facd7 100644 --- a/website/content/api-docs/secret/ssh.mdx +++ b/website/content/api-docs/secret/ssh.mdx @@ -146,7 +146,7 @@ This endpoint creates or updates a named role. Use with caution. N.B.: if the type is `ca`, an empty list does not allow any user; instead you must use `*` to enable this behavior. -- `allowed_users_template` `(bool: false)` - If set, allowed_users can be specified +- `allowed_users_template` `(bool: false)` - If set, `allowed_users` can be specified using identity template policies. Non-templated users are also permitted. - `allowed_domains` `(string: "")` – The list of domains for which a client can @@ -154,6 +154,10 @@ This endpoint creates or updates a named role. credentials can be created for any domain. See also `allow_bare_domains` and `allow_subdomains`. +- `allowed_domains_template` `(bool: false)` - If set, `allowed_domains` can be + specified using identity template policies. Non-templated domains are also + permitted. + - `key_option_specs` `(string: "")` – Specifies a comma separated option specification which will be prefixed to RSA keys in the remote host's authorized_keys file. N.B.: Vault does not check this string for validity.