Skip to content

Commit

Permalink
Refactoring to apply feedback
Browse files Browse the repository at this point in the history
- changes names
- enforce exact match on key length
- add test
  • Loading branch information
catsby committed Jan 11, 2019
1 parent 81c6ae9 commit 07c2178
Show file tree
Hide file tree
Showing 5 changed files with 173 additions and 151 deletions.
139 changes: 79 additions & 60 deletions builtin/logical/ssh/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ import (
"fmt"
"os/user"
"reflect"
"strconv"
"strconv"
"testing"
"time"

"golang.org/x/crypto/ssh"

"encoding/base64"
"encoding/json"
"encoding/json"
"errors"
"strings"

Expand Down Expand Up @@ -72,6 +72,9 @@ oOyBJU/HMVvBfv4g+OVFLVgSwwm6owwsouZ0+D/LasbuHqYyqYqdyPJQYzWA2Y+F
`
publicKey2 = `AAAAB3NzaC1yc2EAAAADAQABAAABAQDArgK0ilRRfk8E7HIsjz5l3BuxmwpDd8DHRCVfOhbZ4gOSVxjEOOqBwWGjygdboBIZwFXmwDlU6sWX0hBJAgpQz0Cjvbjxtq/NjkvATrYPgnrXUhTaEn2eQO0PsqRNSFH46SK/oJfTp0q8/WgojxWJ2L7FUV8PO8uIk49DzqAqPV7WXU63vFsjx+3WQOX/ILeQvHCvaqs3dWjjzEoDudRWCOdUqcHEOshV9azIzPrXlQVzRV3QAKl6u7pC+/Secorpwt6IHpMKoVPGiR0tMMuNOVH8zrAKzIxPGfy2WmNDpJopbXMTvSOGAqNcp49O4SKOQl9Fzfq2HEevJamKLrMB
`

publicKey4096 = `ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQC54Oj4YCFDYxYv69Q9KfU6rWYtUB1eByQdUW0nXFi/vr98QUIV77sEeUVhaQzZcuCojAi/GrloW7ta0Z2DaEv5jOQMAnGpXBcqLJsz3KdrHbpvl93MPNdmNaGPU0GnUEsjBVuDVn9HdIUa8CNrxShvPu7/VqoaRHKLqphGgzFb37vi4qvnQ+5VYAO/TzyVYMD6qJX6I/9Pw8d74jCfEdOh2yGKkP7rXWOghreyIl8H2zTJKg9KoZuPq9F5M8nNt7Oi3rf+DwQiYvamzIqlDP4s5oFVTZW0E9lwWvYDpyiJnUrkQqksebBK/rcyfiFG3onb4qLo2WVWXeK3si8IhGik/TEzprScyAWIf9RviT8O+l5hTA2/c+ctn3MVCLRNfez2lKpdxCoprv1MbIcySGWblTJEcY6RA+aauVJpu7FMtRxHHtZKtMpep8cLu8GKbiP6Ifq2JXBtXtNxDeIgo2MkNoMh/NHAsACJniE/dqV/+u9HvhvgrTbJ69ell0nE4ivzA7O4kZgbR/4MHlLgLFvaqC8RrWRLY6BdFagPIMxghWha7Qw16zqoIjRnolvRzUWvSXanJVg8Z6ua1VxwgirNaAH1ivmJhUh2+4lNxCX6jmZyR3zjJsWY03gjJTairvI762opjjalF8fH6Xrs15mB14JiAlNbk6+5REQcvXlGqw== [email protected]`

privateKey = `-----BEGIN RSA PRIVATE KEY-----
MIIEowIBAAKCAQEAwK4CtIpUUX5PBOxyLI8+ZdwbsZsKQ3fAx0QlXzoW2eIDklcY
xDjqgcFho8oHW6ASGcBV5sA5VOrFl9IQSQIKUM9Ao7248bavzY5LwE62D4J611IU
Expand Down Expand Up @@ -693,56 +696,72 @@ func TestBackend_OptionsOverrideDefaults(t *testing.T) {
logicaltest.Test(t, testCase)
}

func TestBackend_SignedKeyConstraints(t *testing.T) {
config := logical.TestBackendConfig()

b, err := Factory(context.Background(), config)
if err != nil {
t.Fatalf("Cannot create backend: %s", err)
}
testCase := logicaltest.TestCase{
Backend: b,
Steps: []logicaltest.TestStep{
configCaStep(),
createRoleStep("weakkey", map[string]interface{}{
"key_type": "ca",
"allow_user_certificates": true,
"signed_key_constraints": map[string]interface{}{
"rsa": json.Number(strconv.FormatInt(4096, 10)),
},
}),
logicaltest.TestStep{
Operation: logical.UpdateOperation,
Path: "sign/weakkey",
Data: map[string]interface{}{
"public_key": publicKey,
},
ErrorOk: true,
Check: func(resp *logical.Response) error {
if resp.Data["error"] != "public_key failed to meet the key requirements: your key is of an invalid size: 2048" {
return errors.New("a smaller key (2048) was allowed, when the minimum was set for 4096")
}
return nil
},
},
createRoleStep("stdkey", map[string]interface{}{
"key_type": "ca",
"allow_user_certificates": true,
"signed_key_constraints": map[string]interface{}{
"rsa": json.Number(strconv.FormatInt(2048, 10)),
},
}),
logicaltest.TestStep{
Operation: logical.UpdateOperation,
Path: "sign/stdkey",
Data: map[string]interface{}{
"public_key": publicKey,
},
},
},
}

logicaltest.Test(t, testCase)
func TestBackend_AllowedUserKeyLengths(t *testing.T) {
config := logical.TestBackendConfig()

b, err := Factory(context.Background(), config)
if err != nil {
t.Fatalf("Cannot create backend: %s", err)
}
testCase := logicaltest.TestCase{
LogicalBackend: b,
Steps: []logicaltest.TestStep{
configCaStep(),
createRoleStep("weakkey", map[string]interface{}{
"key_type": "ca",
"allow_user_certificates": true,
"allowed_user_key_lengths": map[string]interface{}{
"rsa": json.Number(strconv.FormatInt(4096, 10)),
},
}),
logicaltest.TestStep{
Operation: logical.UpdateOperation,
Path: "sign/weakkey",
Data: map[string]interface{}{
"public_key": publicKey,
},
ErrorOk: true,
Check: func(resp *logical.Response) error {
if resp.Data["error"] != "public_key failed to meet the key requirements: your key is of an invalid size: 2048" {
return errors.New("a smaller key (2048) was allowed, when the minimum was set for 4096")
}
return nil
},
},
createRoleStep("stdkey", map[string]interface{}{
"key_type": "ca",
"allow_user_certificates": true,
"allowed_user_key_lengths": map[string]interface{}{
"rsa": json.Number(strconv.FormatInt(2048, 10)),
},
}),
// Pass with 2048 key
logicaltest.TestStep{
Operation: logical.UpdateOperation,
Path: "sign/stdkey",
Data: map[string]interface{}{
"public_key": publicKey,
},
},
// Fail with 4096 key
logicaltest.TestStep{
Operation: logical.UpdateOperation,
Path: "sign/stdkey",
Data: map[string]interface{}{
"public_key": publicKey4096,
},
ErrorOk: true,
Check: func(resp *logical.Response) error {
if resp.Data["error"] != "public_key failed to meet the key requirements: your key is of an invalid size: 4096" {
return errors.New("a larger key (4096) was allowed, when the size was set for 2048")
}
return nil
},
},
},
}

logicaltest.Test(t, testCase)
}

func TestBackend_CustomKeyIDFormat(t *testing.T) {
Expand Down Expand Up @@ -853,7 +872,7 @@ func createRoleStep(name string, parameters map[string]interface{}) logicaltest.
}

func signCertificateStep(
role, keyId string, certType int, validPrincipals []string,
role, keyID string, certType int, validPrincipals []string,
criticalOptionPermissions, extensionPermissions map[string]string,
ttl time.Duration,
requestParameters map[string]interface{}) logicaltest.TestStep {
Expand Down Expand Up @@ -881,16 +900,16 @@ func signCertificateStep(
return err
}

return validateSSHCertificate(parsedKey.(*ssh.Certificate), keyId, certType, validPrincipals, criticalOptionPermissions, extensionPermissions, ttl)
return validateSSHCertificate(parsedKey.(*ssh.Certificate), keyID, certType, validPrincipals, criticalOptionPermissions, extensionPermissions, ttl)
},
}
}

func validateSSHCertificate(cert *ssh.Certificate, keyId string, certType int, validPrincipals []string, criticalOptionPermissions, extensionPermissions map[string]string,
func validateSSHCertificate(cert *ssh.Certificate, keyID string, certType int, validPrincipals []string, criticalOptionPermissions, extensionPermissions map[string]string,
ttl time.Duration) error {

if cert.KeyId != keyId {
return fmt.Errorf("incorrect KeyId: %v, wanted %v", cert.KeyId, keyId)
if cert.KeyId != keyID {
return fmt.Errorf("incorrect KeyId: %v, wanted %v", cert.KeyId, keyID)
}

if cert.CertType != uint32(certType) {
Expand All @@ -905,9 +924,9 @@ func validateSSHCertificate(cert *ssh.Certificate, keyId string, certType int, v
return fmt.Errorf("incorrect ValidBefore: %v", cert.ValidBefore)
}

actualTtl := time.Unix(int64(cert.ValidBefore), 0).Add(-30 * time.Second).Sub(time.Unix(int64(cert.ValidAfter), 0))
if actualTtl != ttl {
return fmt.Errorf("incorrect ttl: expected: %v, actualL %v", ttl, actualTtl)
actualTTL := time.Unix(int64(cert.ValidBefore), 0).Add(-30 * time.Second).Sub(time.Unix(int64(cert.ValidAfter), 0))
if actualTTL != ttl {
return fmt.Errorf("incorrect ttl: expected: %v, actualL %v", ttl, actualTTL)
}

if !reflect.DeepEqual(cert.ValidPrincipals, validPrincipals) {
Expand Down
23 changes: 13 additions & 10 deletions builtin/logical/ssh/path_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,12 @@ import (
)

const (
KeyTypeOTP = "otp"
// KeyTypeOTP is an key of type OTP
KeyTypeOTP = "otp"
// KeyTypeDynamic is dynamic key type
KeyTypeDynamic = "dynamic"
KeyTypeCA = "ca"
// KeyTypeCA is an key of type CA
KeyTypeCA = "ca"
)

// Structure that represents a role in SSH backend. This is a common role structure
Expand Down Expand Up @@ -48,7 +51,7 @@ 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"`
SignedKeyConstraints map[string]int `mapstructure:"signed_key_constraints" json:"signed_key_constraints"`
AllowedUserKeyLengths map[string]int `mapstructure:"allowed_user_key_lengths" json:"allowed_user_key_lengths"`
}

func pathListRoles(b *backend) *framework.Path {
Expand Down Expand Up @@ -280,13 +283,13 @@ func pathRoles(b *backend) *framework.Path {
'{{public_key_hash}}' - A SHA256 checksum of the public key that is being signed.
`,
},
"signed_key_constraints": &framework.FieldSchema{
Type: framework.TypeMap,
Description: `
"allowed_user_key_lengths": &framework.FieldSchema{
Type: framework.TypeMap,
Description: `
[Not applicable for Dynamic type] [Not applicable for OTP type] [Optional for CA type]
If set, allows the enforcement of key types and minimum key sizes to be signed.
`,
},
},
},

Callbacks: map[logical.Operation]framework.OperationFunc{
Expand Down Expand Up @@ -466,7 +469,7 @@ func (b *backend) createCARole(allowedUsers, defaultUser string, data *framework

defaultCriticalOptions := convertMapToStringValue(data.Get("default_critical_options").(map[string]interface{}))
defaultExtensions := convertMapToStringValue(data.Get("default_extensions").(map[string]interface{}))
signedKeyConstraints := convertMapToIntValue(data.Get("signed_key_constraints").(map[string]interface{}))
allowedUserKeyLengths := convertMapToIntValue(data.Get("allowed_user_key_lengths").(map[string]interface{}))

if ttl != 0 && maxTTL != 0 && ttl > maxTTL {
return nil, logical.ErrorResponse(
Expand All @@ -478,7 +481,7 @@ func (b *backend) createCARole(allowedUsers, defaultUser string, data *framework
role.MaxTTL = maxTTL.String()
role.DefaultCriticalOptions = defaultCriticalOptions
role.DefaultExtensions = defaultExtensions
role.SignedKeyConstraints = signedKeyConstraints
role.AllowedUserKeyLengths = allowedUserKeyLengths

return role, nil
}
Expand Down Expand Up @@ -544,7 +547,7 @@ func (b *backend) parseRole(role *sshRole) (map[string]interface{}, error) {
"key_bits": role.KeyBits,
"default_critical_options": role.DefaultCriticalOptions,
"default_extensions": role.DefaultExtensions,
"signed_key_constraints": role.SignedKeyConstraints,
"allowed_user_key_lengths": role.AllowedUserKeyLengths,
}
case KeyTypeDynamic:
result = map[string]interface{}{
Expand Down
Loading

0 comments on commit 07c2178

Please sign in to comment.