Skip to content

Commit

Permalink
Add signed key constraints to SSH CA [continued] (#6030)
Browse files Browse the repository at this point in the history 
* Adds the ability to enforce particular ssh key types and minimum key
lengths when using Signed SSH Certificates via the SSH Secret Engine.
  • Loading branch information
@catsby @jefferai
catsby authored and jefferai committed Feb 11, 2019
1 parent a0ede31 commit 7c78575
Show file tree
Hide file tree
Showing 5 changed files with 193 additions and 19 deletions.
89 changes: 81 additions & 8 deletions builtin/logical/ssh/backend_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ import (
"fmt"
"os/user"
"reflect"
"strconv"
"testing"
"time"

"golang.org/x/crypto/ssh"

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

Expand Down Expand Up @@ -70,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
Expand Down Expand Up @@ -691,6 +696,74 @@ func TestBackend_OptionsOverrideDefaults(t *testing.T) {
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: 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: 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) {
config := logical.TestBackendConfig()

Expand Down Expand Up @@ -799,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 @@ -827,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 @@ -851,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
21 changes: 19 additions & 2 deletions builtin/logical/ssh/path_roles.go
View file
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,6 +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"`
AllowedUserKeyLengths map[string]int `mapstructure:"allowed_user_key_lengths" json:"allowed_user_key_lengths"`
}

func pathListRoles(b *backend) *framework.Path {
Expand Down Expand Up @@ -279,6 +283,13 @@ func pathRoles(b *backend) *framework.Path {
'{{public_key_hash}}' - A SHA256 checksum of the public key that is being signed.
`,
},
"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 @@ -458,6 +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, 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(
Expand All @@ -469,6 +484,7 @@ func (b *backend) createCARole(allowedUsers, defaultUser string, data *framework
role.MaxTTL = maxTTL.String()
role.DefaultCriticalOptions = defaultCriticalOptions
role.DefaultExtensions = defaultExtensions
role.AllowedUserKeyLengths = allowedUserKeyLengths

return role, nil
}
Expand Down Expand Up @@ -534,6 +550,7 @@ func (b *backend) parseRole(role *sshRole) (map[string]interface{}, error) {
"key_bits": role.KeyBits,
"default_critical_options": role.DefaultCriticalOptions,
"default_extensions": role.DefaultExtensions,
"allowed_user_key_lengths": role.AllowedUserKeyLengths,
}
case KeyTypeDynamic:
result = map[string]interface{}{
Expand Down
84 changes: 76 additions & 8 deletions builtin/logical/ssh/path_sign.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ package ssh

import (
"context"
"crypto/dsa"
"crypto/ecdsa"
"crypto/rand"
"crypto/rsa"
"crypto/sha256"
"errors"
"fmt"
Expand All @@ -16,11 +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/ssh"
)

type creationBundle struct {
KeyId string
KeyID string
ValidPrincipals []string
PublicKey ssh.PublicKey
CertificateType uint32
Expand Down Expand Up @@ -110,9 +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
}

// 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
}
Expand Down Expand Up @@ -164,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,
Expand Down Expand Up @@ -266,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}}"
Expand Down Expand Up @@ -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.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("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("key is of an invalid size: %v", kbits)
}

} else {
return fmt.Errorf("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 {
Expand All @@ -405,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()),
Expand Down
13 changes: 13 additions & 0 deletions builtin/logical/ssh/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,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"
Expand Down Expand Up @@ -215,6 +216,18 @@ 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{}
for key, value := range initial {
v, err := parseutil.ParseInt(value)
if err != nil {
return nil, err
}
result[key] = int(v)
}
return result, nil
}

// Serve a template processor for custom format inputs
func substQuery(tpl string, data map[string]string) string {
for k, v := range data {
Expand Down
5 changes: 4 additions & 1 deletion website/source/api/secret/ssh/index.html.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}}"

- `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.

### Sample Payload

Expand Down

0 comments on commit 7c78575

Please sign in to comment.