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

Add signed key constraints to SSH CA [continued] #6030

Merged
merged 5 commits into from
Feb 11, 2019
Merged
Show file tree
Hide file tree
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
89 changes: 81 additions & 8 deletions builtin/logical/ssh/backend_test.go
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== [email protected]`

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