Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow identity templates in ssh backend default_user field #16351

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
200 changes: 200 additions & 0 deletions builtin/logical/ssh/backend_test.go
Original file line number Diff line number Diff line change
@@ -340,6 +340,147 @@ func TestBackend_AllowedUsersTemplate_WithStaticPrefix(t *testing.T) {
)
}

func TestBackend_DefaultUserTemplate(t *testing.T) {
testDefaultUserTemplate(t,
"{{ identity.entity.metadata.ssh_username }}",
testUserName,
map[string]string{
"ssh_username": testUserName,
},
)
}

func TestBackend_DefaultUserTemplate_WithStaticPrefix(t *testing.T) {
testDefaultUserTemplate(t,
"user-{{ identity.entity.metadata.ssh_username }}",
"user-"+testUserName,
map[string]string{
"ssh_username": testUserName,
},
)
}

func TestBackend_DefaultUserTemplateFalse_AllowedUsersTemplateTrue(t *testing.T) {
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": map[string]string{
"ssh_username": testUserName,
},
})
if err != nil {
t.Fatal(err)
}

_, err = client.Logical().Write("ssh/roles/my-role", map[string]interface{}{
"key_type": testCaKeyType,
"allow_user_certificates": true,
"default_user": "{{identity.entity.metadata.ssh_username}}",
// disable user templating but not allowed_user_template and the request should fail
"default_user_template": false,
"allowed_users": "{{identity.entity.metadata.ssh_username}}",
"allowed_users_template": true,
})
if err != nil {
t.Fatal(err)
}

// sign SSH key as userpass user
client.SetToken(userpassToken)
_, err = client.Logical().Write("ssh/sign/my-role", map[string]interface{}{
"public_key": testCAPublicKey,
})
if err == nil {
t.Errorf("signing request should fail when default_user is not in the allowed_users list, because allowed_users_template is true and default_user_template is not")
}

expectedErrStr := "{{identity.entity.metadata.ssh_username}} is not a valid value for valid_principals"
if !strings.Contains(err.Error(), expectedErrStr) {
t.Errorf("expected error to include %q but it was: %q", expectedErrStr, err.Error())
}
}

func TestBackend_DefaultUserTemplateFalse_AllowedUsersTemplateFalse(t *testing.T) {
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": map[string]string{
"ssh_username": testUserName,
},
})
if err != nil {
t.Fatal(err)
}

_, err = client.Logical().Write("ssh/roles/my-role", map[string]interface{}{
"key_type": testCaKeyType,
"allow_user_certificates": true,
"default_user": "{{identity.entity.metadata.ssh_username}}",
"default_user_template": false,
"allowed_users": "{{identity.entity.metadata.ssh_username}}",
"allowed_users_template": false,
})
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,
})
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 len(actualPrincipals) < 1 {
t.Fatal(
fmt.Sprintf("No ValidPrincipals returned: should have been %v",
[]string{"{{identity.entity.metadata.ssh_username}}"}),
)
}
if len(actualPrincipals) > 1 {
t.Error(
fmt.Sprintf("incorrect number ValidPrincipals, expected only 1: %v should be %v",
actualPrincipals, []string{"{{identity.entity.metadata.ssh_username}}"}),
)
}
if actualPrincipals[0] != "{{identity.entity.metadata.ssh_username}}" {
t.Fatal(
fmt.Sprintf("incorrect ValidPrincipals: %v should be %v",
actualPrincipals, []string{"{{identity.entity.metadata.ssh_username}}"}),
)
}
}

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
@@ -1893,6 +2034,65 @@ func getSshCaTestCluster(t *testing.T, userIdentity string) (*vault.TestCluster,
return cluster, userpassToken
}

func testDefaultUserTemplate(t *testing.T, testDefaultUserTemplate 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,
"default_user": testDefaultUserTemplate,
"default_user_template": true,
"allowed_users": testDefaultUserTemplate,
"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,
})
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 testAllowedUsersTemplate(t *testing.T, testAllowedUsersTemplate string,
expectedValidPrincipal string, testEntityMetadata map[string]string,
) {
47 changes: 30 additions & 17 deletions builtin/logical/ssh/path_issue_sign.go
Original file line number Diff line number Diff line change
@@ -70,7 +70,14 @@ func (b *backend) pathSignIssueCertificateHelper(ctx context.Context, req *logic
return logical.ErrorResponse(err.Error()), nil
}
} else {
parsedPrincipals, err = b.calculateValidPrincipals(data, req, role, role.DefaultUser, role.AllowedUsers, strutil.StrListContains)
defaultPrincipal := role.DefaultUser
if role.DefaultUserTemplate {
defaultPrincipal, err = b.renderPrincipal(role.DefaultUser, req)
if err != nil {
return logical.ErrorResponse(err.Error()), nil
}
}
parsedPrincipals, err = b.calculateValidPrincipals(data, req, role, defaultPrincipal, role.AllowedUsers, strutil.StrListContains)
if err != nil {
return logical.ErrorResponse(err.Error()), nil
}
@@ -136,6 +143,23 @@ func (b *backend) pathSignIssueCertificateHelper(ctx context.Context, req *logic
return response, nil
}

func (b *backend) renderPrincipal(principal string, req *logical.Request) (string, error) {
// Look for templating markers {{ .* }}
matched := containsTemplateRegex.MatchString(principal)
if matched {
if req.EntityID != "" {
// Retrieve principal based on template + entityID from request.
renderedPrincipal, err := framework.PopulateIdentityTemplate(principal, req.EntityID, b.System())
if err != nil {
return "", fmt.Errorf("template '%s' could not be rendered -> %s", principal, err)
}
return renderedPrincipal, nil
}
}
// Static principal
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) {
validPrincipals := ""
validPrincipalsRaw, ok := data.GetOk("valid_principals")
@@ -150,23 +174,12 @@ func (b *backend) calculateValidPrincipals(data *framework.FieldData, req *logic
var allowedPrincipals []string
for _, principal := range strutil.RemoveDuplicates(strutil.ParseStringSlice(principalsAllowedByRole, ","), false) {
if role.AllowedUsersTemplate {
// Look for templating markers {{ .* }}
matched := containsTemplateRegex.MatchString(principal)
if matched {
if req.EntityID != "" {
// Retrieve principal based on template + entityID from request.
templatePrincipal, err := framework.PopulateIdentityTemplate(principal, req.EntityID, b.System())
if err == nil {
// Template returned a principal
allowedPrincipals = append(allowedPrincipals, templatePrincipal)
} else {
return nil, fmt.Errorf("template '%s' could not be rendered -> %s", principal, err)
}
}
} else {
// Static principal or err template
allowedPrincipals = append(allowedPrincipals, principal)
rendered, err := b.renderPrincipal(principal, req)
if err != nil {
return nil, fmt.Errorf("template '%s' could not be rendered -> %s", principal, err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is inconsistent with above (lines 77) -- I believe the err should be returned here and this message should be preserved as-is as we already contain a copy of the error message prefix you have here (so it'll be duplicated).

IOW, I think 77 is wrong too (return nil, err) to preserve existing behavior-ish.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops good catch @cipherboy. I'll merge this PR and address this in another myself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

previously (when only the allowed_users field could be templated) the template rendering happened in calculateValidPrincipals and the error on this line (return nil, fmt.Errorf("template '%s' could not be rendered -> %s", principal, err), on line 163 in the previous version) could be returned by that func,

That return value would then be caught on the previous version's line 74 which contained the logical.ErrorResponse wrapping, i.e. lines 73-76 prior to my changes were:

parsedPrincipals, err = b.calculateValidPrincipals(data, req, role, role.DefaultUser, role.AllowedUsers, strutil.StrListContains)
if err != nil {
	return logical.ErrorResponse(err.Error()), nil
} 

I was aiming to match the failure behaviors for templating errors in default_user to the existing behavior for allowed_users -- that all said, 👍 with any changes to it, y'all know the details on that better than me!

}
// Template returned a principal
allowedPrincipals = append(allowedPrincipals, rendered)
} else {
// Static principal
allowedPrincipals = append(allowedPrincipals, principal)
12 changes: 12 additions & 0 deletions builtin/logical/ssh/path_roles.go
Original file line number Diff line number Diff line change
@@ -40,6 +40,7 @@ type sshRole struct {
KeyBits int `mapstructure:"key_bits" json:"key_bits"`
AdminUser string `mapstructure:"admin_user" json:"admin_user"`
DefaultUser string `mapstructure:"default_user" json:"default_user"`
DefaultUserTemplate bool `mapstructure:"default_user_template" json:"default_user_template"`
CIDRList string `mapstructure:"cidr_list" json:"cidr_list"`
ExcludeCIDRList string `mapstructure:"exclude_cidr_list" json:"exclude_cidr_list"`
Port int `mapstructure:"port" json:"port"`
@@ -122,6 +123,15 @@ func pathRoles(b *backend) *framework.Path {
Name: "Default Username",
},
},
"default_user_template": {
Type: framework.TypeBool,
Description: `
[Not applicable for Dynamic type] [Not applicable for OTP type] [Optional for CA type]
If set, Default user can be specified using identity template policies.
Non-templated users are also permitted.
`,
Default: false,
},
"cidr_list": {
Type: framework.TypeString,
Description: `
@@ -558,6 +568,7 @@ func (b *backend) createCARole(allowedUsers, defaultUser, signer string, data *f
AllowedUsersTemplate: data.Get("allowed_users_template").(bool),
AllowedDomains: data.Get("allowed_domains").(string),
DefaultUser: defaultUser,
DefaultUserTemplate: data.Get("default_user_template").(bool),
AllowBareDomains: data.Get("allow_bare_domains").(bool),
AllowSubdomains: data.Get("allow_subdomains").(bool),
AllowUserKeyIDs: data.Get("allow_user_key_ids").(bool),
@@ -740,6 +751,7 @@ func (b *backend) parseRole(role *sshRole) (map[string]interface{}, error) {
"allowed_users_template": role.AllowedUsersTemplate,
"allowed_domains": role.AllowedDomains,
"default_user": role.DefaultUser,
"default_user_template": role.DefaultUserTemplate,
"ttl": int64(ttl.Seconds()),
"max_ttl": int64(maxTTL.Seconds()),
"allowed_critical_options": role.AllowedCriticalOptions,
3 changes: 3 additions & 0 deletions changelog/16351.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
secrets/ssh: Allow the use of Identity templates in the `default_user` field
```
6 changes: 6 additions & 0 deletions website/content/api-docs/secret/ssh.mdx
Original file line number Diff line number Diff line change
@@ -99,9 +99,15 @@ This endpoint creates or updates a named role.
create individual roles for each username to ensure absolute isolation between
usernames. This is required for Dynamic Key type and OTP type.

When `default_user_template` is set to `true`, this field can contain an identity
template with any prefix or suffix, like `ssh-{{identity.entity.id}}-user`.

For the CA type, if you wish this to be a valid principal, it must also be
in `allowed_users`.

- `default_user_template` `(bool: false)` - If set, `default_users` can be specified
using identity template values. A non-templated user is also permitted.

- `cidr_list` `(string: "")` – Specifies a comma separated list of CIDR blocks
for which the role is applicable for. It is possible that a same set of CIDR
blocks are part of multiple roles. This is a required parameter, unless the