From b635927566d9d0b1fbe4b29cbe8e32649c5ff108 Mon Sep 17 00:00:00 2001 From: Philipp Hossner Date: Sun, 17 Oct 2021 20:14:01 +0200 Subject: [PATCH 1/5] Let allowed_users template mix templated and non-templated parts (#10388) --- builtin/logical/ssh/backend_test.go | 76 +++++++++++++++++++++++++++++ builtin/logical/ssh/path_sign.go | 2 +- 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/builtin/logical/ssh/backend_test.go b/builtin/logical/ssh/backend_test.go index 55ba9f386e3c3..bebca7dfdb55e 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" @@ -318,6 +319,24 @@ func TestBackend_allowed_users(t *testing.T) { } } +func TestBackend_allowed_users_template(t *testing.T) { + testAllowedUsersTemplate(t, + "{{ identity.entity.metadata.ssh_username }}", + testUserName, map[string]string{ + "ssh_username": testUserName, + }, + ) +} + +func TestBackend_allowed_users_template_with_static_prefix(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 459f885896cc0..7dfe9f37e39ca 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. From 3c712c6b17084ccd49cfa3248fb5f3d1767000dd Mon Sep 17 00:00:00 2001 From: Philipp Hossner Date: Tue, 19 Oct 2021 21:05:36 +0200 Subject: [PATCH 2/5] Add documentation --- website/content/api-docs/secret/ssh.mdx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/website/content/api-docs/secret/ssh.mdx b/website/content/api-docs/secret/ssh.mdx index a5087d0ca8d9d..21cf5243348ba 100644 --- a/website/content/api-docs/secret/ssh.mdx +++ b/website/content/api-docs/secret/ssh.mdx @@ -132,7 +132,10 @@ 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 + 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. From 29df26d554981d57854f9f47834efa6f737abede Mon Sep 17 00:00:00 2001 From: Philipp Hossner Date: Tue, 19 Oct 2021 21:05:45 +0200 Subject: [PATCH 3/5] Change test function names --- builtin/logical/ssh/backend_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/logical/ssh/backend_test.go b/builtin/logical/ssh/backend_test.go index bebca7dfdb55e..488522c1976ac 100644 --- a/builtin/logical/ssh/backend_test.go +++ b/builtin/logical/ssh/backend_test.go @@ -205,7 +205,7 @@ func testSSH(user, host string, auth ssh.AuthMethod, command string) error { return err } -func TestBackend_allowed_users(t *testing.T) { +func TestBackend_AllowedUsers(t *testing.T) { config := logical.TestBackendConfig() config.StorageView = &logical.InmemStorage{} @@ -319,7 +319,7 @@ func TestBackend_allowed_users(t *testing.T) { } } -func TestBackend_allowed_users_template(t *testing.T) { +func TestBackend_AllowedUsersTemplate(t *testing.T) { testAllowedUsersTemplate(t, "{{ identity.entity.metadata.ssh_username }}", testUserName, map[string]string{ @@ -328,7 +328,7 @@ func TestBackend_allowed_users_template(t *testing.T) { ) } -func TestBackend_allowed_users_template_with_static_prefix(t *testing.T) { +func TestBackend_AllowedUsersTemplate_WithStaticPrefix(t *testing.T) { testAllowedUsersTemplate(t, "ssh-{{ identity.entity.metadata.ssh_username }}", "ssh-"+testUserName, map[string]string{ From 22ea95b3f627e2fa620d04a4b4c8b8bad6867544 Mon Sep 17 00:00:00 2001 From: Philipp Hossner Date: Tue, 19 Oct 2021 21:08:04 +0200 Subject: [PATCH 4/5] Add documentation --- website/content/api-docs/secret/ssh.mdx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/website/content/api-docs/secret/ssh.mdx b/website/content/api-docs/secret/ssh.mdx index 21cf5243348ba..36002bb4da092 100644 --- a/website/content/api-docs/secret/ssh.mdx +++ b/website/content/api-docs/secret/ssh.mdx @@ -135,9 +135,8 @@ This endpoint creates or updates a named role. 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. + 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. From 7980c9ce4a99c682148a430d86664a40f359f208 Mon Sep 17 00:00:00 2001 From: Philipp Hossner Date: Tue, 19 Oct 2021 21:13:58 +0200 Subject: [PATCH 5/5] Add changelog entry --- changelog/10886.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/10886.txt diff --git a/changelog/10886.txt b/changelog/10886.txt new file mode 100644 index 0000000000000..85453698302ba --- /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