From 824f097a7deb197848085a9db4296e1e867c5be3 Mon Sep 17 00:00:00 2001 From: Philipp Hossner Date: Wed, 20 Oct 2021 00:00:15 +0200 Subject: [PATCH] Let allowed_users template mix templated and non-templated parts (#10886) * Let allowed_users template mix templated and non-templated parts (#10388) * Add documentation * Change test function names * Add documentation * Add changelog entry --- builtin/logical/ssh/backend_test.go | 78 ++++++++++++++++++++++++- builtin/logical/ssh/path_sign.go | 2 +- changelog/10886.txt | 3 + website/content/api-docs/secret/ssh.mdx | 8 ++- 4 files changed, 86 insertions(+), 5 deletions(-) create mode 100644 changelog/10886.txt diff --git a/builtin/logical/ssh/backend_test.go b/builtin/logical/ssh/backend_test.go index 840fee592..8e33c6b3a 100644 --- a/builtin/logical/ssh/backend_test.go +++ b/builtin/logical/ssh/backend_test.go @@ -31,6 +31,7 @@ const ( testIP = "127.0.0.1" testUserName = "vaultssh" testAdminUser = "vaultssh" + testCaKeyType = "ca" testOTPKeyType = "otp" testDynamicKeyType = "dynamic" testCIDRList = "127.0.0.1/32" @@ -204,7 +205,7 @@ func testSSH(user, host string, auth ssh.AuthMethod, command string) error { return nil } -func TestBackend_allowed_users(t *testing.T) { +func TestBackend_AllowedUsers(t *testing.T) { config := logical.TestBackendConfig() config.StorageView = &logical.InmemStorage{} @@ -318,6 +319,24 @@ func TestBackend_allowed_users(t *testing.T) { } } +func TestBackend_AllowedUsersTemplate(t *testing.T) { + testAllowedUsersTemplate(t, + "{{ identity.entity.metadata.ssh_username }}", + testUserName, map[string]string{ + "ssh_username": testUserName, + }, + ) +} + +func TestBackend_AllowedUsersTemplate_WithStaticPrefix(t *testing.T) { + testAllowedUsersTemplate(t, + "ssh-{{ identity.entity.metadata.ssh_username }}", + "ssh-"+testUserName, map[string]string{ + "ssh_username": testUserName, + }, + ) +} + func newTestingFactory(t *testing.T) func(ctx context.Context, conf *logical.BackendConfig) (logical.Backend, error) { return func(ctx context.Context, conf *logical.BackendConfig) (logical.Backend, error) { defaultLeaseTTLVal := 2 * time.Minute @@ -1614,6 +1633,63 @@ func getSshCaTestCluster(t *testing.T, userIdentity string) (*vault.TestCluster, return cluster, userpassToken } +func testAllowedUsersTemplate(t *testing.T, testAllowedUsersTemplate string, + expectedValidPrincipal string, testEntityMetadata map[string]string) { + cluster, userpassToken := getSshCaTestCluster(t, testUserName) + defer cluster.Cleanup() + client := cluster.Cores[0].Client + + // set metadata "ssh_username" to userpass username + tokenLookupResponse, err := client.Logical().Write("/auth/token/lookup", map[string]interface{}{ + "token": userpassToken, + }) + if err != nil { + t.Fatal(err) + } + entityID := tokenLookupResponse.Data["entity_id"].(string) + _, err = client.Logical().Write("/identity/entity/id/"+entityID, map[string]interface{}{ + "metadata": testEntityMetadata, + }) + if err != nil { + 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, + }) + 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, + }) + if err != nil { + t.Fatal(err) + } + + // check for the expected valid principals of certificate + signedKey := signResponse.Data["signed_key"].(string) + key, _ := base64.StdEncoding.DecodeString(strings.Split(signedKey, " ")[1]) + parsedKey, err := ssh.ParsePublicKey(key) + if err != nil { + t.Fatal(err) + } + actualPrincipals := parsedKey.(*ssh.Certificate).ValidPrincipals + if actualPrincipals[0] != expectedValidPrincipal { + t.Fatal( + fmt.Sprintf("incorrect ValidPrincipals: %v should be %v", + actualPrincipals, []string{expectedValidPrincipal}), + ) + } +} + func configCaStep(caPublicKey, caPrivateKey string) logicaltest.TestStep { return logicaltest.TestStep{ Operation: logical.UpdateOperation, diff --git a/builtin/logical/ssh/path_sign.go b/builtin/logical/ssh/path_sign.go index 459f88589..7dfe9f37e 100644 --- a/builtin/logical/ssh/path_sign.go +++ b/builtin/logical/ssh/path_sign.go @@ -220,7 +220,7 @@ func (b *backend) calculateValidPrincipals(data *framework.FieldData, req *logic for _, principal := range strutil.RemoveDuplicates(strutil.ParseStringSlice(principalsAllowedByRole, ","), false) { if role.AllowedUsersTemplate { // Look for templating markers {{ .* }} - matched, _ := regexp.MatchString(`^{{.+?}}$`, principal) + matched, _ := regexp.MatchString(`{{.+?}}`, principal) if matched { if req.EntityID != "" { // Retrieve principal based on template + entityID from request. diff --git a/changelog/10886.txt b/changelog/10886.txt new file mode 100644 index 000000000..854536983 --- /dev/null +++ b/changelog/10886.txt @@ -0,0 +1,3 @@ +```release-note:improvement +secrets/ssh: Let allowed_users template mix templated and non-templated parts. +``` \ No newline at end of file diff --git a/website/content/api-docs/secret/ssh.mdx b/website/content/api-docs/secret/ssh.mdx index 41bfc70a3..6d7c34c29 100644 --- a/website/content/api-docs/secret/ssh.mdx +++ b/website/content/api-docs/secret/ssh.mdx @@ -132,9 +132,11 @@ This endpoint creates or updates a named role. then this list enforces it. If this field is set, then credentials can only be created for `default_user` and usernames present in this list. Setting this option will enable all the users with access this role to fetch - credentials for all other usernames in this list. 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. + credentials for all other usernames in this list. + When `allowed_users_template` is set to `true`, this field can contain an identity + template with any prefix or suffix, like `ssh-{{identity.entity.id}}-user`. + 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 using identity template policies. Non-templated users are also permitted.