luxolus
/
open-vault
2
0
Fork 0
Code Issues 1 Pull Requests Releases Activity

Allow specifying multiple allowed SSH key lengths (#13991) 

* Allow specifying multiple allowed SSH key lengths

In the ssh secrets engine, only a single allowed key length was allowed
for each algorithm type. However, many algorithms have multiple safe
values (such as RSA and ECDSA); allowing a single role to have multiple
values for a single algorithm is thus helpful.

On creation or update, roles can now specify multiple types using a list
or comma separated string of allowed values:

    allowed_user_key_lengths: map[string][]int{"rsa": []int{2048, 4096}}

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add changelog entry

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Break out ssh upgrade logic into separate function

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Update parseutil for optional lists of integers

    go get -u github.com/hashicorp/go-secure-stdlib/parseutil
    go mod tidy

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Simplify parse logic using new parseutil

The newly introduced parseutil.ParseIntSlice handles the more
complicated optional int-like slice logic for us.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
This commit is contained in:
Alexander Scheel 2022-02-17 14:36:56 -06:00 committed by GitHub
parent c6e64f51b6
commit 45c028a2fb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 188 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
builtin/logical/ssh/backend_test.go
View File

@ -4,12 +4,10 @@ import (
"bytes"
"context"
"encoding/base64"
"encoding/json"
"errors"
"fmt"
"net"
"reflect"
"strconv"
"strings"
"testing"
"time"
@ -76,6 +74,8 @@ oOyBJU/HMVvBfv4g+OVFLVgSwwm6owwsouZ0+D/LasbuHqYyqYqdyPJQYzWA2Y+F
publicKey2 = `AAAAB3NzaC1yc2EAAAADAQABAAABAQDArgK0ilRRfk8E7HIsjz5l3BuxmwpDd8DHRCVfOhbZ4gOSVxjEOOqBwWGjygdboBIZwFXmwDlU6sWX0hBJAgpQz0Cjvbjxtq/NjkvATrYPgnrXUhTaEn2eQO0PsqRNSFH46SK/oJfTp0q8/WgojxWJ2L7FUV8PO8uIk49DzqAqPV7WXU63vFsjx+3WQOX/ILeQvHCvaqs3dWjjzEoDudRWCOdUqcHEOshV9azIzPrXlQVzRV3QAKl6u7pC+/Secorpwt6IHpMKoVPGiR0tMMuNOVH8zrAKzIxPGfy2WmNDpJopbXMTvSOGAqNcp49O4SKOQl9Fzfq2HEevJamKLrMB
`
publicKey3072 = `ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDlsMr3K1d0nzE1TjUULPRuVjEGETmOqHtWq4gVPq3HiuNVHE/e/BJnkXc40BoClQ2Z5ZZPJZ6izF9PnlzNDjpq8DrILUrn/6KrzCHvRwnkYMAXbfM/Br09z5QGptbOe1EMLeVe0b/udmUicbYAGPxMruZk+ljyr4vXkO+gOAIrxeSIQSdMVLU4g0pCPQuDCOx5IQpDYSlOB3091frpN8npfMueKPflNYzxnqqYgAVeDKAIqMCGOMOHUeIZJ7A7HuynEAVOsOkJwC9nesy9D6ppdWNduGl42IkzlwVdDMZtUAEznMUT/dnHNG1Krx9SuNZ/S9fGjxGVsT+jzUmizrWB9/6XIEHDxPBzcqlWFuwYTGz1OL8bfZ+HldOGPcnqZn9hKntWwjUc3whcvWt+NCmXpHSVLSxf+WN8pdmfEsCqn8mpvo2MXa+iJrtAVPX4i0u8AQUuqC3NuXHv4Cn0LNwtziBT544UjgbWkAZqzFZJREYA09OHscc3akEIrTnPehk= demo@example.com`
publicKey4096 = `ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQC54Oj4YCFDYxYv69Q9KfU6rWYtUB1eByQdUW0nXFi/vr98QUIV77sEeUVhaQzZcuCojAi/GrloW7ta0Z2DaEv5jOQMAnGpXBcqLJsz3KdrHbpvl93MPNdmNaGPU0GnUEsjBVuDVn9HdIUa8CNrxShvPu7/VqoaRHKLqphGgzFb37vi4qvnQ+5VYAO/TzyVYMD6qJX6I/9Pw8d74jCfEdOh2yGKkP7rXWOghreyIl8H2zTJKg9KoZuPq9F5M8nNt7Oi3rf+DwQiYvamzIqlDP4s5oFVTZW0E9lwWvYDpyiJnUrkQqksebBK/rcyfiFG3onb4qLo2WVWXeK3si8IhGik/TEzprScyAWIf9RviT8O+l5hTA2/c+ctn3MVCLRNfez2lKpdxCoprv1MbIcySGWblTJEcY6RA+aauVJpu7FMtRxHHtZKtMpep8cLu8GKbiP6Ifq2JXBtXtNxDeIgo2MkNoMh/NHAsACJniE/dqV/+u9HvhvgrTbJ69ell0nE4ivzA7O4kZgbR/4MHlLgLFvaqC8RrWRLY6BdFagPIMxghWha7Qw16zqoIjRnolvRzUWvSXanJVg8Z6ua1VxwgirNaAH1ivmJhUh2+4lNxCX6jmZyR3zjJsWY03gjJTairvI762opjjalF8fH6Xrs15mB14JiAlNbk6+5REQcvXlGqw== dummy@example.com`
testCAPrivateKey = `-----BEGIN RSA PRIVATE KEY-----
@ -1198,7 +1198,7 @@ func TestBackend_AllowedUserKeyLengths(t *testing.T) {
"key_type": "ca",
"allow_user_certificates": true,
"allowed_user_key_lengths": map[string]interface{}{
"rsa": json.Number(strconv.FormatInt(4096, 10)),
"rsa": 4096,
},
}),
{
@ -1219,7 +1219,7 @@ func TestBackend_AllowedUserKeyLengths(t *testing.T) {
"key_type": "ca",
"allow_user_certificates": true,
"allowed_user_key_lengths": map[string]interface{}{
"rsa": json.Number(strconv.FormatInt(2048, 10)),
"rsa": 2048,
},
}),
// Pass with 2048 key
@ -1245,6 +1245,44 @@ func TestBackend_AllowedUserKeyLengths(t *testing.T) {
return nil
},
},
createRoleStep("multikey", map[string]interface{}{
"key_type": "ca",
"allow_user_certificates": true,
"allowed_user_key_lengths": map[string]interface{}{
"rsa": []int{2048, 4096},
},
}),
// Pass with 2048-bit key
{
Operation: logical.UpdateOperation,
Path: "sign/multikey",
Data: map[string]interface{}{
"public_key": testCAPublicKey,
},
},
// Pass with 4096-bit key
{
Operation: logical.UpdateOperation,
Path: "sign/multikey",
Data: map[string]interface{}{
"public_key": publicKey4096,
},
},
// Fail with 3072-bit key
{
Operation: logical.UpdateOperation,
Path: "sign/multikey",
Data: map[string]interface{}{
"public_key": publicKey3072,
},
ErrorOk: true,
Check: func(resp *logical.Response) error {
if resp.Data["error"] != "public_key failed to meet the key requirements: key is of an invalid size: 3072" {
return errors.New("a larger key (3072) was allowed, when the size was set for 2048")
}
return nil
},
},
},
}

74
builtin/logical/ssh/path_roles.go
View File

@ -9,6 +9,7 @@ import (
"github.com/hashicorp/go-secure-stdlib/parseutil"
"github.com/hashicorp/vault/sdk/framework"
"github.com/hashicorp/vault/sdk/helper/cidrutil"
"github.com/hashicorp/vault/sdk/helper/consts"
"github.com/hashicorp/vault/sdk/logical"
"golang.org/x/crypto/ssh"
)
@ -20,6 +21,11 @@ const (
KeyTypeDynamic = "dynamic"
// KeyTypeCA is an key of type CA
KeyTypeCA = "ca"
// 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 = 1
)
// Structure that represents a role in SSH backend. This is a common role structure
@ -52,8 +58,10 @@ type sshRole struct {
AllowSubdomains bool `mapstructure:"allow_subdomains" json:"allow_subdomains"`
AllowUserKeyIDs bool `mapstructure:"allow_user_key_ids" json:"allow_user_key_ids"`
KeyIDFormat string `mapstructure:"key_id_format" json:"key_id_format"`
AllowedUserKeyLengths map[string]int `mapstructure:"allowed_user_key_lengths" json:"allowed_user_key_lengths"`
OldAllowedUserKeyLengths map[string]int `mapstructure:"allowed_user_key_lengths" json:"allowed_user_key_lengths,omitempty"`
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"`
}
func pathListRoles(b *backend) *framework.Path {
@ -431,6 +439,7 @@ func (b *backend) pathRoleWrite(ctx context.Context, req *logical.Request, d *fr
KeyType: KeyTypeOTP,
Port: port,
AllowedUsers: allowedUsers,
Version: roleEntryVersion,
}
} else if keyType == KeyTypeDynamic {
defaultUser := d.Get("default_user").(string)
@ -484,6 +493,7 @@ func (b *backend) pathRoleWrite(ctx context.Context, req *logical.Request, d *fr
InstallScript: installScript,
AllowedUsers: allowedUsers,
KeyOptionSpecs: keyOptionSpecs,
Version: roleEntryVersion,
}
} else if keyType == KeyTypeCA {
algorithmSigner := ""
@ -539,6 +549,7 @@ func (b *backend) createCARole(allowedUsers, defaultUser, signer string, data *f
KeyIDFormat: data.Get("key_id_format").(string),
KeyType: KeyTypeCA,
AlgorithmSigner: signer,
Version: roleEntryVersion,
}
if !role.AllowUserCertificates && !role.AllowHostCertificates {
@ -547,7 +558,7 @@ func (b *backend) createCARole(allowedUsers, defaultUser, signer string, data *f
defaultCriticalOptions := convertMapToStringValue(data.Get("default_critical_options").(map[string]interface{}))
defaultExtensions := convertMapToStringValue(data.Get("default_extensions").(map[string]interface{}))
allowedUserKeyLengths, err := convertMapToIntValue(data.Get("allowed_user_key_lengths").(map[string]interface{}))
allowedUserKeyLengths, err := convertMapToIntSlice(data.Get("allowed_user_key_lengths").(map[string]interface{}))
if err != nil {
return nil, logical.ErrorResponse(fmt.Sprintf("error processing allowed_user_key_lengths: %s", err.Error()))
}
@ -562,7 +573,7 @@ func (b *backend) createCARole(allowedUsers, defaultUser, signer string, data *f
role.MaxTTL = maxTTL.String()
role.DefaultCriticalOptions = defaultCriticalOptions
role.DefaultExtensions = defaultExtensions
role.AllowedUserKeyLengths = allowedUserKeyLengths
role.AllowedUserKeyTypesLengths = allowedUserKeyLengths
return role, nil
}
@ -581,9 +592,64 @@ func (b *backend) getRole(ctx context.Context, s logical.Storage, n string) (*ss
return nil, err
}
if err := b.checkUpgrade(ctx, s, n, &result); err != nil {
return nil, err
}
return &result, nil
}
func (b *backend) checkUpgrade(ctx context.Context, s logical.Storage, n string, result *sshRole) error {
modified := false
// NOTE: When introducing a new migration, increment roleEntryVersion and
// check if the version is less than the version this change was introduced
// at and perform the change. At the end, set modified and update the
// version to the version this migration was introduced at! Additionally,
// add new migrations after all existing migrations.
//
// Otherwise, past or future migrations may not execute!
if result.Version == roleEntryVersion {
return nil
}
// Role version introduced at version 1, migrating OldAllowedUserKeyLengths
// to the newer AllowedUserKeyTypesLengths field.
if result.Version < 1 {
// Only migrate if we have old data and no new data to avoid clobbering.
//
// This change introduced the first role version, value of 1.
if len(result.OldAllowedUserKeyLengths) > 0 && len(result.AllowedUserKeyTypesLengths) == 0 {
result.AllowedUserKeyTypesLengths = make(map[string][]int)
for k, v := range result.OldAllowedUserKeyLengths {
result.AllowedUserKeyTypesLengths[k] = []int{v}
}
result.OldAllowedUserKeyLengths = nil
}
result.Version = 1
modified = true
}
// Add new migrations just before here.
//
// Condition copied from PKI builtin.
if modified && (b.System().LocalMount() || !b.System().ReplicationState().HasState(consts.ReplicationPerformanceSecondary)) {
jsonEntry, err := logical.StorageEntryJSON("roles/"+n, &result)
if err != nil {
return err
}
if err := s.Put(ctx, jsonEntry); err != nil {
// Only perform upgrades on replication primary
if !strings.Contains(err.Error(), logical.ErrReadOnly.Error()) {
return err
}
}
}
return nil
}
// parseRole converts a sshRole object into its map[string]interface representation,
// with appropriate values for each KeyType. If the KeyType is invalid, it will return
// an error.
@ -630,7 +696,7 @@ func (b *backend) parseRole(role *sshRole) (map[string]interface{}, error) {
"default_critical_options": role.DefaultCriticalOptions,
"default_extensions": role.DefaultExtensions,
"default_extensions_template": role.DefaultExtensionsTemplate,
"allowed_user_key_lengths": role.AllowedUserKeyLengths,
"allowed_user_key_lengths": role.AllowedUserKeyTypesLengths,
"algorithm_signer": role.AlgorithmSigner,
}
case KeyTypeDynamic:

11
builtin/logical/ssh/path_sign.go
View File

@ -447,7 +447,7 @@ func (b *backend) calculateTTL(data *framework.FieldData, role *sshRole) (time.D
}
func (b *backend) validateSignedKeyRequirements(publickey ssh.PublicKey, role *sshRole) error {
if len(role.AllowedUserKeyLengths) != 0 {
if len(role.AllowedUserKeyTypesLengths) != 0 {
var kstr string
var kbits int
@ -473,31 +473,36 @@ func (b *backend) validateSignedKeyRequirements(publickey ssh.PublicKey, role *s
return fmt.Errorf("pubkey not suitable for crypto (expected ssh.CryptoPublicKey but found %T)", k)
}
if value, ok := role.AllowedUserKeyLengths[kstr]; ok {
if allowed_values, ok := role.AllowedUserKeyTypesLengths[kstr]; ok {
var pass bool
for _, value := range allowed_values {
switch kstr {
case "rsa":
if kbits == value {
pass = true
break
}
case "dsa":
if kbits == value {
pass = true
break
}
case "ecdsa":
if kbits == value {
pass = true
break
}
case "ed25519":
// ed25519 public keys are always 256 bits in length,
// so there is no need to inspect their value
pass = true
break
}
}
if !pass {
return fmt.Errorf("key is of an invalid size: %v", kbits)
}
} else {
return fmt.Errorf("key type of %s is not allowed", kstr)
}

14
builtin/logical/ssh/util.go
View File

@ -220,15 +220,21 @@ func convertMapToStringValue(initial map[string]interface{}) map[string]string {
return result
}
func convertMapToIntValue(initial map[string]interface{}) (map[string]int, error) {
result := map[string]int{}
func convertMapToIntSlice(initial map[string]interface{}) (map[string][]int, error) {
result := map[string][]int{}
for key, value := range initial {
v, err := parseutil.ParseInt(value)
sliced, err := parseutil.ParseIntSlice(value)
if err != nil {
return nil, err
}
result[key] = int(v)
result[key] = make([]int, 0, len(sliced))
for _, value := range sliced {
result[key] = append(result[key], int(value))
}
}
return result, nil
}

3
changelog/13991.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:improvement
secrets/ssh: Allow specifying multiple approved key lengths for a single algorithm
```

6
go.mod
View File

@ -76,10 +76,10 @@ require (
github.com/hashicorp/go-secure-stdlib/gatedwriter v0.1.1
github.com/hashicorp/go-secure-stdlib/kv-builder v0.1.1
github.com/hashicorp/go-secure-stdlib/mlock v0.1.1
github.com/hashicorp/go-secure-stdlib/parseutil v0.1.2
github.com/hashicorp/go-secure-stdlib/parseutil v0.1.3
github.com/hashicorp/go-secure-stdlib/password v0.1.1
github.com/hashicorp/go-secure-stdlib/reloadutil v0.1.1
github.com/hashicorp/go-secure-stdlib/strutil v0.1.1
github.com/hashicorp/go-secure-stdlib/strutil v0.1.2
github.com/hashicorp/go-secure-stdlib/tlsutil v0.1.1
github.com/hashicorp/go-sockaddr v1.0.2
github.com/hashicorp/go-syslog v1.0.0
@ -138,7 +138,7 @@ require (
github.com/mitchellh/go-testing-interface v1.14.0
github.com/mitchellh/go-wordwrap v1.0.0
github.com/mitchellh/gox v1.0.1
github.com/mitchellh/mapstructure v1.4.2
github.com/mitchellh/mapstructure v1.4.3
github.com/mitchellh/reflectwalk v1.0.2
github.com/mongodb/go-client-mongodb-atlas v0.1.2
github.com/natefinch/atomic v0.0.0-20150920032501-a62ce929ffcc

9
go.sum
View File

@ -868,14 +868,16 @@ github.com/hashicorp/go-secure-stdlib/kv-builder v0.1.1/go.mod h1:rf5JPE13wi+Nwj
github.com/hashicorp/go-secure-stdlib/mlock v0.1.1 h1:cCRo8gK7oq6A2L6LICkUZ+/a5rLiRXFMf1Qd4xSwxTc=
github.com/hashicorp/go-secure-stdlib/mlock v0.1.1/go.mod h1:zq93CJChV6L9QTfGKtfBxKqD7BqqXx5O04A/ns2p5+I=
github.com/hashicorp/go-secure-stdlib/parseutil v0.1.1/go.mod h1:QmrqtbKuxxSWTN3ETMPuB+VtEiBJ/A9XhoYGv8E1uD8=
github.com/hashicorp/go-secure-stdlib/parseutil v0.1.2 h1:Tz6v3Jb2DRnDCfifRSjYKG0m8dLdNq6bcDkB41en7nw=
github.com/hashicorp/go-secure-stdlib/parseutil v0.1.2/go.mod h1:QmrqtbKuxxSWTN3ETMPuB+VtEiBJ/A9XhoYGv8E1uD8=
github.com/hashicorp/go-secure-stdlib/parseutil v0.1.3 h1:geBw3SBrxQq+buvbf4K+Qltv1gjaXJxy8asD4CjGYow=
github.com/hashicorp/go-secure-stdlib/parseutil v0.1.3/go.mod h1:QmrqtbKuxxSWTN3ETMPuB+VtEiBJ/A9XhoYGv8E1uD8=
github.com/hashicorp/go-secure-stdlib/password v0.1.1 h1:6JzmBqXprakgFEHwBgdchsjaA9x3GyjdI568bXKxa60=
github.com/hashicorp/go-secure-stdlib/password v0.1.1/go.mod h1:9hH302QllNwu1o2TGYtSk8I8kTAN0ca1EHpwhm5Mmzo=
github.com/hashicorp/go-secure-stdlib/reloadutil v0.1.1 h1:SMGUnbpAcat8rIKHkBPjfv81yC46a8eCNZ2hsR2l1EI=
github.com/hashicorp/go-secure-stdlib/reloadutil v0.1.1/go.mod h1:Ch/bf00Qnx77MZd49JRgHYqHQjtEmTgGU2faufpVZb0=
github.com/hashicorp/go-secure-stdlib/strutil v0.1.1 h1:nd0HIW15E6FG1MsnArYaHfuw9C2zgzM8LxkG5Ty/788=
github.com/hashicorp/go-secure-stdlib/strutil v0.1.1/go.mod h1:gKOamz3EwoIoJq7mlMIRBpVTAUn8qPCrEclOKKWhD3U=
github.com/hashicorp/go-secure-stdlib/strutil v0.1.2 h1:kes8mmyCpxJsI7FTwtzRqEy9CdjCtrXrXGuOpxEA7Ts=
github.com/hashicorp/go-secure-stdlib/strutil v0.1.2/go.mod h1:Gou2R9+il93BqX25LAKCLuM+y9U2T4hlwvT1yprcna4=
github.com/hashicorp/go-secure-stdlib/tlsutil v0.1.1 h1:Yc026VyMyIpq1UWRnakHRG01U8fJm+nEfEmjoAb00n8=
github.com/hashicorp/go-secure-stdlib/tlsutil v0.1.1/go.mod h1:l8slYwnJA26yBz+ErHpp2IRCLr0vuOMGBORIz4rRiAs=
github.com/hashicorp/go-slug v0.7.0 h1:8HIi6oreWPtnhpYd8lIGQBgp4rXzDWQTOhfILZm+nok=
@ -1181,8 +1183,9 @@ github.com/mitchellh/mapstructure v0.0.0-20160808181253-ca63d7c062ee/go.mod h1:F
github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
github.com/mitchellh/mapstructure v1.3.3/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
github.com/mitchellh/mapstructure v1.4.1/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
github.com/mitchellh/mapstructure v1.4.2 h1:6h7AQ0yhTcIsmFmnAwQls75jp2Gzs4iB8W7pjMO+rqo=
github.com/mitchellh/mapstructure v1.4.2/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
github.com/mitchellh/mapstructure v1.4.3 h1:OVowDSCllw/YjdLkam3/sm7wEtOy59d8ndGgCcyj8cs=
github.com/mitchellh/mapstructure v1.4.3/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
github.com/mitchellh/osext v0.0.0-20151018003038-5e2d6d41470f/go.mod h1:OkQIRizQZAeMln+1tSwduZz7+Af5oFlKirV/MSYes2A=
github.com/mitchellh/pointerstructure v1.2.0 h1:O+i9nHnXS3l/9Wu7r4NrEdwA2VFTicjUEN1uBnDo34A=
github.com/mitchellh/pointerstructure v1.2.0/go.mod h1:BRAsLI5zgXmw97Lf6s25bs8ohIXc3tViBH44KcwB2g4=

6
website/content/api-docs/secret/ssh.mdx
View File

@ -210,8 +210,10 @@ This endpoint creates or updates a named role.
'{{public_key_hash}}' - A SHA256 checksum of the public key that is being signed.
e.g. "custom-keyid-{{token_display_name}}"
- `allowed_user_key_lengths` `(map<string|int>: "")` – Specifies a map of ssh key types
and their expected sizes which are allowed to be signed by the CA type.
- `allowed_user_key_lengths` `(map<string|(int|[]int|string)>: "")` – Specifies a
map of ssh key types and their expected sizes which are allowed to be signed by
the CA type. To specify multiple sizes, either use a comma-separated list or an
array of allowed key widths.
- `algorithm_signer` `(string: "")` - Algorithm to sign keys with. Valid
values are `ssh-rsa`, `rsa-sha2-256`, and `rsa-sha2-512`. This value may be left