From 753b6f97759398a622bf9350c3477f6c32029a85 Mon Sep 17 00:00:00 2001 From: Brian Nuszkowski Date: Thu, 26 Apr 2018 16:26:22 -0400 Subject: [PATCH 1/5] Adds the ability to enforce particular ssh key types and minimum key lengths when using Signed SSH Certificates via the SSH Secret Engine. --- builtin/logical/ssh/path_roles.go | 11 ++++ builtin/logical/ssh/path_sign.go | 68 +++++++++++++++++++++ builtin/logical/ssh/util.go | 10 +++ website/source/api/secret/ssh/index.html.md | 5 +- 4 files changed, 93 insertions(+), 1 deletion(-) diff --git a/builtin/logical/ssh/path_roles.go b/builtin/logical/ssh/path_roles.go index 76fda0e0ea40..93985af04ab9 100644 --- a/builtin/logical/ssh/path_roles.go +++ b/builtin/logical/ssh/path_roles.go @@ -48,6 +48,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"` } func pathListRoles(b *backend) *framework.Path { @@ -279,6 +280,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: ` + [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{ @@ -458,6 +466,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{})) if ttl != 0 && maxTTL != 0 && ttl > maxTTL { return nil, logical.ErrorResponse( @@ -469,6 +478,7 @@ func (b *backend) createCARole(allowedUsers, defaultUser string, data *framework role.MaxTTL = maxTTL.String() role.DefaultCriticalOptions = defaultCriticalOptions role.DefaultExtensions = defaultExtensions + role.SignedKeyConstraints = signedKeyConstraints return role, nil } @@ -534,6 +544,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, } case KeyTypeDynamic: result = map[string]interface{}{ diff --git a/builtin/logical/ssh/path_sign.go b/builtin/logical/ssh/path_sign.go index 68bd1efe6410..cb7a9a1dd008 100644 --- a/builtin/logical/ssh/path_sign.go +++ b/builtin/logical/ssh/path_sign.go @@ -2,7 +2,10 @@ package ssh import ( "context" + "crypto/dsa" + "crypto/ecdsa" "crypto/rand" + "crypto/rsa" "crypto/sha256" "errors" "fmt" @@ -16,6 +19,7 @@ import ( "github.com/hashicorp/vault/helper/strutil" "github.com/hashicorp/vault/logical" "github.com/hashicorp/vault/logical/framework" + "golang.org/x/crypto/ed25519" "golang.org/x/crypto/ssh" ) @@ -110,6 +114,11 @@ func (b *backend) pathSignCertificate(ctx context.Context, req *logical.Request, return logical.ErrorResponse(fmt.Sprintf("failed to parse public_key as SSH key: %s", err)), nil } + err = b.validateSignedKeyRequirements(userPublicKey, role) + if err != nil { + return logical.ErrorResponse(fmt.Sprintf("public_key failed to meet the key requirements: %s", err)), nil + } + // Note that these various functions always return "user errors" so we pass // them as 4xx values keyId, err := b.calculateKeyId(data, req, role, userPublicKey) @@ -384,6 +393,65 @@ func (b *backend) calculateTTL(data *framework.FieldData, role *sshRole) (time.D return ttl, nil } +func (b *backend) validateSignedKeyRequirements(publickey ssh.PublicKey, role *sshRole) error { + if len(role.SignedKeyConstraints) != 0 { + var kstr string + var kbits int + + switch k := publickey.(type) { + case ssh.CryptoPublicKey: + ff := k.CryptoPublicKey() + switch k := ff.(type) { + case *rsa.PublicKey: + kstr = "rsa" + kbits = k.N.BitLen() + case *dsa.PublicKey: + kstr = "dsa" + kbits = k.Parameters.P.BitLen() + case *ecdsa.PublicKey: + kstr = "ecdsa" + kbits = k.Curve.Params().BitSize + case ed25519.PublicKey: + kstr = "ed25519" + default: + return fmt.Errorf("your public key type of %s is not allowed", kstr) + } + default: + return fmt.Errorf("pubkey not suitable for crypto (expected ssh.CryptoPublicKey but found %T)", k) + } + + if value, ok := role.SignedKeyConstraints[kstr]; ok { + var pass bool + switch kstr { + case "rsa": + if kbits >= value { + pass = true + } + case "dsa": + if kbits >= value { + pass = true + } + case "ecdsa": + if kbits >= value { + pass = true + } + case "ed25519": + // ed25519 public keys are always 256 bits in length, + // so there is no need to inspect their value + pass = true + } + + if !pass { + return fmt.Errorf("your key is of an invalid size: %v", kbits) + } + + } else { + return fmt.Errorf("your key type of %s is not allowed", kstr) + } + } + return nil +} + func (b *creationBundle) sign() (retCert *ssh.Certificate, retErr error) { defer func() { if r := recover(); r != nil { diff --git a/builtin/logical/ssh/util.go b/builtin/logical/ssh/util.go index 98a7036a46a6..5ac80dd4f703 100644 --- a/builtin/logical/ssh/util.go +++ b/builtin/logical/ssh/util.go @@ -7,6 +7,7 @@ import ( "crypto/rsa" "crypto/x509" "encoding/base64" + "encoding/json" "encoding/pem" "fmt" "net" @@ -215,6 +216,15 @@ func convertMapToStringValue(initial map[string]interface{}) map[string]string { return result } +func convertMapToIntValue(initial map[string]interface{}) map[string]int { + result := map[string]int{} + for key, value := range initial { + i, _ := value.(json.Number).Int64() + result[key] = int(i) + } + return result +} + // Serve a template processor for custom format inputs func substQuery(tpl string, data map[string]string) string { for k, v := range data { diff --git a/website/source/api/secret/ssh/index.html.md b/website/source/api/secret/ssh/index.html.md index 1e434f2c1702..5c7abc4bc4c3 100644 --- a/website/source/api/secret/ssh/index.html.md +++ b/website/source/api/secret/ssh/index.html.md @@ -206,7 +206,10 @@ This endpoint creates or updates a named role. available for use: '{{token_display_name}}' - The display name of the token used to make the request. '{{role_name}}' - The name of the role signing the request. '{{public_key_hash}}' - A SHA256 checksum of the public key that is being signed. - e.g. "custom-keyid-{{token_display_name}}", + e.g. "custom-keyid-{{token_display_name}}" + +- `signed_key_constraints` `(map: "")` – Specifies a map of ssh key types + and their minimum sizes which are allowed to be signed by the CA type. ### Sample Payload From 81c6ae998441a594a327e471b2698cfdd566caae Mon Sep 17 00:00:00 2001 From: Brian Nuszkowski Date: Thu, 26 Apr 2018 18:18:51 -0400 Subject: [PATCH 2/5] Add tests for signed key constraints --- builtin/logical/ssh/backend_test.go | 54 +++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/builtin/logical/ssh/backend_test.go b/builtin/logical/ssh/backend_test.go index 85888839bf70..68a48776e537 100644 --- a/builtin/logical/ssh/backend_test.go +++ b/builtin/logical/ssh/backend_test.go @@ -5,12 +5,14 @@ import ( "fmt" "os/user" "reflect" + "strconv" "testing" "time" "golang.org/x/crypto/ssh" "encoding/base64" + "encoding/json" "errors" "strings" @@ -691,6 +693,58 @@ 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_CustomKeyIDFormat(t *testing.T) { config := logical.TestBackendConfig() From 07c2178c25fde0397db483fdcf875544430fa19c Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Fri, 11 Jan 2019 13:10:55 -0600 Subject: [PATCH 3/5] Refactoring to apply feedback - changes names - enforce exact match on key length - add test --- builtin/logical/ssh/backend_test.go | 139 +++++++++++-------- builtin/logical/ssh/path_roles.go | 23 ++-- builtin/logical/ssh/path_sign.go | 144 ++++++++++---------- builtin/logical/ssh/util.go | 14 +- website/source/api/secret/ssh/index.html.md | 4 +- 5 files changed, 173 insertions(+), 151 deletions(-) diff --git a/builtin/logical/ssh/backend_test.go b/builtin/logical/ssh/backend_test.go index 68a48776e537..1869a12fa361 100644 --- a/builtin/logical/ssh/backend_test.go +++ b/builtin/logical/ssh/backend_test.go @@ -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" @@ -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== dummy@example.com` + privateKey = `-----BEGIN RSA PRIVATE KEY----- MIIEowIBAAKCAQEAwK4CtIpUUX5PBOxyLI8+ZdwbsZsKQ3fAx0QlXzoW2eIDklcY xDjqgcFho8oHW6ASGcBV5sA5VOrFl9IQSQIKUM9Ao7248bavzY5LwE62D4J611IU @@ -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) { @@ -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 { @@ -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) { @@ -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) { diff --git a/builtin/logical/ssh/path_roles.go b/builtin/logical/ssh/path_roles.go index 93985af04ab9..7ce1320df0ea 100644 --- a/builtin/logical/ssh/path_roles.go +++ b/builtin/logical/ssh/path_roles.go @@ -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 @@ -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 { @@ -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{ @@ -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( @@ -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 } @@ -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{}{ diff --git a/builtin/logical/ssh/path_sign.go b/builtin/logical/ssh/path_sign.go index cb7a9a1dd008..20833dcb52a1 100644 --- a/builtin/logical/ssh/path_sign.go +++ b/builtin/logical/ssh/path_sign.go @@ -2,10 +2,10 @@ package ssh import ( "context" - "crypto/dsa" - "crypto/ecdsa" + "crypto/dsa" + "crypto/ecdsa" "crypto/rand" - "crypto/rsa" + "crypto/rsa" "crypto/sha256" "errors" "fmt" @@ -19,12 +19,12 @@ import ( "github.com/hashicorp/vault/helper/strutil" "github.com/hashicorp/vault/logical" "github.com/hashicorp/vault/logical/framework" - "golang.org/x/crypto/ed25519" + "golang.org/x/crypto/ed25519" "golang.org/x/crypto/ssh" ) type creationBundle struct { - KeyId string + KeyID string ValidPrincipals []string PublicKey ssh.PublicKey CertificateType uint32 @@ -114,14 +114,14 @@ func (b *backend) pathSignCertificate(ctx context.Context, req *logical.Request, return logical.ErrorResponse(fmt.Sprintf("failed to parse public_key as SSH key: %s", err)), nil } - err = b.validateSignedKeyRequirements(userPublicKey, role) - if err != nil { - return logical.ErrorResponse(fmt.Sprintf("public_key failed to meet the key requirements: %s", err)), nil - } + err = b.validateSignedKeyRequirements(userPublicKey, role) + if err != nil { + return logical.ErrorResponse(fmt.Sprintf("public_key failed to meet the key requirements: %s", err)), nil + } // Note that these various functions always return "user errors" so we pass // them as 4xx values - keyId, err := b.calculateKeyId(data, req, role, userPublicKey) + keyID, err := b.calculateKeyID(data, req, role, userPublicKey) if err != nil { return logical.ErrorResponse(err.Error()), nil } @@ -173,7 +173,7 @@ func (b *backend) pathSignCertificate(ctx context.Context, req *logical.Request, } cBundle := creationBundle{ - KeyId: keyId, + KeyID: keyID, PublicKey: userPublicKey, Signer: signer, ValidPrincipals: parsedPrincipals, @@ -275,14 +275,14 @@ func (b *backend) calculateCertificateType(data *framework.FieldData, role *sshR return certificateType, nil } -func (b *backend) calculateKeyId(data *framework.FieldData, req *logical.Request, role *sshRole, pubKey ssh.PublicKey) (string, error) { - reqId := data.Get("key_id").(string) +func (b *backend) calculateKeyID(data *framework.FieldData, req *logical.Request, role *sshRole, pubKey ssh.PublicKey) (string, error) { + reqID := data.Get("key_id").(string) - if reqId != "" { + if reqID != "" { if !role.AllowUserKeyIDs { return "", fmt.Errorf("setting key_id is not allowed by role") } - return reqId, nil + return reqID, nil } keyIDFormat := "vault-{{token_display_name}}-{{public_key_hash}}" @@ -394,62 +394,62 @@ func (b *backend) calculateTTL(data *framework.FieldData, role *sshRole) (time.D } func (b *backend) validateSignedKeyRequirements(publickey ssh.PublicKey, role *sshRole) error { - if len(role.SignedKeyConstraints) != 0 { - var kstr string - var kbits int - - switch k := publickey.(type) { - case ssh.CryptoPublicKey: - ff := k.CryptoPublicKey() - switch k := ff.(type) { - case *rsa.PublicKey: - kstr = "rsa" - kbits = k.N.BitLen() - case *dsa.PublicKey: - kstr = "dsa" - kbits = k.Parameters.P.BitLen() - case *ecdsa.PublicKey: - kstr = "ecdsa" - kbits = k.Curve.Params().BitSize - case ed25519.PublicKey: - kstr = "ed25519" - default: - return fmt.Errorf("your public key type of %s is not allowed", kstr) - } - default: - return fmt.Errorf("pubkey not suitable for crypto (expected ssh.CryptoPublicKey but found %T)", k) - } - - if value, ok := role.SignedKeyConstraints[kstr]; ok { - var pass bool - switch kstr { - case "rsa": - if kbits >= value { - pass = true - } - case "dsa": - if kbits >= value { - pass = true - } - case "ecdsa": - if kbits >= value { - pass = true - } - case "ed25519": - // ed25519 public keys are always 256 bits in length, - // so there is no need to inspect their value - pass = true - } - - if !pass { - return fmt.Errorf("your key is of an invalid size: %v", kbits) - } - - } else { - return fmt.Errorf("your key type of %s is not allowed", kstr) - } - } - return nil + if len(role.AllowedUserKeyLengths) != 0 { + var kstr string + var kbits int + + switch k := publickey.(type) { + case ssh.CryptoPublicKey: + ff := k.CryptoPublicKey() + switch k := ff.(type) { + case *rsa.PublicKey: + kstr = "rsa" + kbits = k.N.BitLen() + case *dsa.PublicKey: + kstr = "dsa" + kbits = k.Parameters.P.BitLen() + case *ecdsa.PublicKey: + kstr = "ecdsa" + kbits = k.Curve.Params().BitSize + case ed25519.PublicKey: + kstr = "ed25519" + default: + return fmt.Errorf("your public key type of %s is not allowed", kstr) + } + default: + return fmt.Errorf("pubkey not suitable for crypto (expected ssh.CryptoPublicKey but found %T)", k) + } + + if value, ok := role.AllowedUserKeyLengths[kstr]; ok { + var pass bool + switch kstr { + case "rsa": + if kbits == value { + pass = true + } + case "dsa": + if kbits == value { + pass = true + } + case "ecdsa": + if kbits == value { + pass = true + } + case "ed25519": + // ed25519 public keys are always 256 bits in length, + // so there is no need to inspect their value + pass = true + } + + if !pass { + return fmt.Errorf("your key is of an invalid size: %v", kbits) + } + + } else { + return fmt.Errorf("your key type of %s is not allowed", kstr) + } + } + return nil } func (b *creationBundle) sign() (retCert *ssh.Certificate, retErr error) { @@ -473,7 +473,7 @@ func (b *creationBundle) sign() (retCert *ssh.Certificate, retErr error) { certificate := &ssh.Certificate{ Serial: serialNumber.Uint64(), Key: b.PublicKey, - KeyId: b.KeyId, + KeyId: b.KeyID, ValidPrincipals: b.ValidPrincipals, ValidAfter: uint64(now.Add(-30 * time.Second).In(time.UTC).Unix()), ValidBefore: uint64(now.Add(b.TTL).In(time.UTC).Unix()), diff --git a/builtin/logical/ssh/util.go b/builtin/logical/ssh/util.go index 5ac80dd4f703..3833c16236b9 100644 --- a/builtin/logical/ssh/util.go +++ b/builtin/logical/ssh/util.go @@ -7,7 +7,7 @@ import ( "crypto/rsa" "crypto/x509" "encoding/base64" - "encoding/json" + "encoding/json" "encoding/pem" "fmt" "net" @@ -217,12 +217,12 @@ func convertMapToStringValue(initial map[string]interface{}) map[string]string { } func convertMapToIntValue(initial map[string]interface{}) map[string]int { - result := map[string]int{} - for key, value := range initial { - i, _ := value.(json.Number).Int64() - result[key] = int(i) - } - return result + result := map[string]int{} + for key, value := range initial { + i, _ := value.(json.Number).Int64() + result[key] = int(i) + } + return result } // Serve a template processor for custom format inputs diff --git a/website/source/api/secret/ssh/index.html.md b/website/source/api/secret/ssh/index.html.md index 5c7abc4bc4c3..8a514091926d 100644 --- a/website/source/api/secret/ssh/index.html.md +++ b/website/source/api/secret/ssh/index.html.md @@ -208,8 +208,8 @@ 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}}" -- `signed_key_constraints` `(map: "")` – Specifies a map of ssh key types - and their minimum sizes which are allowed to be signed by the CA type. +- `allowed_user_key_lengths` `(map: "")` – Specifies a map of ssh key types + and their expected sizes which are allowed to be signed by the CA type. ### Sample Payload From a8aab7d5c3b59c19d6e071cf128bf450067f65b8 Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Thu, 7 Feb 2019 16:17:47 -0600 Subject: [PATCH 4/5] update convertMapToIntValue function to return an error if invalid input. Other minor updates --- builtin/logical/ssh/backend_test.go | 4 ++-- builtin/logical/ssh/path_roles.go | 5 ++++- builtin/logical/ssh/path_sign.go | 6 +++--- builtin/logical/ssh/util.go | 14 +++++++++++--- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/builtin/logical/ssh/backend_test.go b/builtin/logical/ssh/backend_test.go index 1869a12fa361..dd84a92a7734 100644 --- a/builtin/logical/ssh/backend_test.go +++ b/builtin/logical/ssh/backend_test.go @@ -722,7 +722,7 @@ func TestBackend_AllowedUserKeyLengths(t *testing.T) { }, 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" { + if resp.Data["error"] != "public_key failed to meet the key requirements: 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 @@ -752,7 +752,7 @@ func TestBackend_AllowedUserKeyLengths(t *testing.T) { }, 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" { + if resp.Data["error"] != "public_key failed to meet the key requirements: 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 diff --git a/builtin/logical/ssh/path_roles.go b/builtin/logical/ssh/path_roles.go index 7ce1320df0ea..932d6edf65d2 100644 --- a/builtin/logical/ssh/path_roles.go +++ b/builtin/logical/ssh/path_roles.go @@ -469,7 +469,10 @@ 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{})) - allowedUserKeyLengths := convertMapToIntValue(data.Get("allowed_user_key_lengths").(map[string]interface{})) + allowedUserKeyLengths, err := convertMapToIntValue(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())) + } if ttl != 0 && maxTTL != 0 && ttl > maxTTL { return nil, logical.ErrorResponse( diff --git a/builtin/logical/ssh/path_sign.go b/builtin/logical/ssh/path_sign.go index 20833dcb52a1..376a5dad43f1 100644 --- a/builtin/logical/ssh/path_sign.go +++ b/builtin/logical/ssh/path_sign.go @@ -414,7 +414,7 @@ func (b *backend) validateSignedKeyRequirements(publickey ssh.PublicKey, role *s case ed25519.PublicKey: kstr = "ed25519" default: - return fmt.Errorf("your public key type of %s is not allowed", kstr) + return fmt.Errorf("public key type of %s is not allowed", kstr) } default: return fmt.Errorf("pubkey not suitable for crypto (expected ssh.CryptoPublicKey but found %T)", k) @@ -442,11 +442,11 @@ func (b *backend) validateSignedKeyRequirements(publickey ssh.PublicKey, role *s } if !pass { - return fmt.Errorf("your key is of an invalid size: %v", kbits) + return fmt.Errorf("key is of an invalid size: %v", kbits) } } else { - return fmt.Errorf("your key type of %s is not allowed", kstr) + return fmt.Errorf("key type of %s is not allowed", kstr) } } return nil diff --git a/builtin/logical/ssh/util.go b/builtin/logical/ssh/util.go index 3833c16236b9..2f20bd9a1542 100644 --- a/builtin/logical/ssh/util.go +++ b/builtin/logical/ssh/util.go @@ -216,13 +216,21 @@ func convertMapToStringValue(initial map[string]interface{}) map[string]string { return result } -func convertMapToIntValue(initial map[string]interface{}) map[string]int { +func convertMapToIntValue(initial map[string]interface{}) (map[string]int, error) { result := map[string]int{} for key, value := range initial { - i, _ := value.(json.Number).Int64() + v, ok := value.(json.Number) + if !ok { + return nil, fmt.Errorf("invalid integer value for type (%s): %v", key, value) + } + + i, err := v.Int64() + if err != nil { + return nil, err + } result[key] = int(i) } - return result + return result, nil } // Serve a template processor for custom format inputs From 225443cd3e3ed01241784450db7070d1c02d0027 Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Thu, 7 Feb 2019 16:27:07 -0600 Subject: [PATCH 5/5] simplify parsing by using parseutil --- builtin/logical/ssh/util.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/builtin/logical/ssh/util.go b/builtin/logical/ssh/util.go index 2f20bd9a1542..70cc8801539b 100644 --- a/builtin/logical/ssh/util.go +++ b/builtin/logical/ssh/util.go @@ -7,7 +7,6 @@ import ( "crypto/rsa" "crypto/x509" "encoding/base64" - "encoding/json" "encoding/pem" "fmt" "net" @@ -15,6 +14,7 @@ import ( "time" "github.com/hashicorp/errwrap" + "github.com/hashicorp/vault/helper/parseutil" "github.com/hashicorp/vault/logical" log "github.com/hashicorp/go-hclog" @@ -219,16 +219,11 @@ func convertMapToStringValue(initial map[string]interface{}) map[string]string { func convertMapToIntValue(initial map[string]interface{}) (map[string]int, error) { result := map[string]int{} for key, value := range initial { - v, ok := value.(json.Number) - if !ok { - return nil, fmt.Errorf("invalid integer value for type (%s): %v", key, value) - } - - i, err := v.Int64() + v, err := parseutil.ParseInt(value) if err != nil { return nil, err } - result[key] = int(i) + result[key] = int(v) } return result, nil }