Split SSH user and cert policy configuration and execution

This commit is contained in:
Herman Slatman 2022-02-01 14:58:13 +01:00
parent a7eb27d309
commit 88c7b63c9d
No known key found for this signature in database
GPG key ID: F4D8A44EA0A75A4F
16 changed files with 285 additions and 139 deletions

View file

@ -18,7 +18,6 @@ import (
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/smallstep/certificates/errs" "github.com/smallstep/certificates/errs"
"github.com/smallstep/certificates/policy"
"go.step.sm/crypto/jose" "go.step.sm/crypto/jose"
"go.step.sm/crypto/sshutil" "go.step.sm/crypto/sshutil"
"go.step.sm/crypto/x509util" "go.step.sm/crypto/x509util"
@ -268,8 +267,8 @@ type AWS struct {
claimer *Claimer claimer *Claimer
config *awsConfig config *awsConfig
audiences Audiences audiences Audiences
x509Policy policy.X509NamePolicyEngine x509Policy x509PolicyEngine
sshPolicy policy.SSHNamePolicyEngine sshHostPolicy *hostPolicyEngine
} }
// GetID returns the provisioner unique identifier. // GetID returns the provisioner unique identifier.
@ -433,8 +432,8 @@ func (p *AWS) Init(config Config) (err error) {
return err return err
} }
// Initialize the SSH allow/deny policy engine // Initialize the SSH allow/deny policy engine for host certificates
if p.sshPolicy, err = newSSHPolicyEngine(p.Options.GetSSHOptions()); err != nil { if p.sshHostPolicy, err = newSSHHostPolicyEngine(p.Options.GetSSHOptions()); err != nil {
return err return err
} }
@ -774,6 +773,6 @@ func (p *AWS) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption,
// Require all the fields in the SSH certificate // Require all the fields in the SSH certificate
&sshCertDefaultValidator{}, &sshCertDefaultValidator{},
// Ensure that all principal names are allowed // Ensure that all principal names are allowed
newSSHNamePolicyValidator(p.sshPolicy), newSSHNamePolicyValidator(p.sshHostPolicy, nil),
), nil ), nil
} }

View file

@ -14,7 +14,6 @@ import (
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/smallstep/certificates/errs" "github.com/smallstep/certificates/errs"
"github.com/smallstep/certificates/policy"
"go.step.sm/crypto/jose" "go.step.sm/crypto/jose"
"go.step.sm/crypto/sshutil" "go.step.sm/crypto/sshutil"
"go.step.sm/crypto/x509util" "go.step.sm/crypto/x509util"
@ -99,8 +98,8 @@ type Azure struct {
config *azureConfig config *azureConfig
oidcConfig openIDConfiguration oidcConfig openIDConfiguration
keyStore *keyStore keyStore *keyStore
x509Policy policy.X509NamePolicyEngine x509Policy x509PolicyEngine
sshPolicy policy.SSHNamePolicyEngine sshHostPolicy *hostPolicyEngine
} }
// GetID returns the provisioner unique identifier. // GetID returns the provisioner unique identifier.
@ -229,8 +228,8 @@ func (p *Azure) Init(config Config) (err error) {
return err return err
} }
// Initialize the SSH allow/deny policy engine // Initialize the SSH allow/deny policy engine for host certificates
if p.sshPolicy, err = newSSHPolicyEngine(p.Options.GetSSHOptions()); err != nil { if p.sshHostPolicy, err = newSSHHostPolicyEngine(p.Options.GetSSHOptions()); err != nil {
return err return err
} }
@ -411,7 +410,7 @@ func (p *Azure) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOptio
// Require all the fields in the SSH certificate // Require all the fields in the SSH certificate
&sshCertDefaultValidator{}, &sshCertDefaultValidator{},
// Ensure that all principal names are allowed // Ensure that all principal names are allowed
newSSHNamePolicyValidator(p.sshPolicy), newSSHNamePolicyValidator(p.sshHostPolicy, nil),
), nil ), nil
} }

View file

@ -15,7 +15,6 @@ import (
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/smallstep/certificates/errs" "github.com/smallstep/certificates/errs"
"github.com/smallstep/certificates/policy"
"go.step.sm/crypto/jose" "go.step.sm/crypto/jose"
"go.step.sm/crypto/sshutil" "go.step.sm/crypto/sshutil"
"go.step.sm/crypto/x509util" "go.step.sm/crypto/x509util"
@ -93,8 +92,8 @@ type GCP struct {
config *gcpConfig config *gcpConfig
keyStore *keyStore keyStore *keyStore
audiences Audiences audiences Audiences
x509Policy policy.X509NamePolicyEngine x509Policy x509PolicyEngine
sshPolicy policy.SSHNamePolicyEngine sshHostPolicy *hostPolicyEngine
} }
// GetID returns the provisioner unique identifier. The name should uniquely // GetID returns the provisioner unique identifier. The name should uniquely
@ -224,8 +223,8 @@ func (p *GCP) Init(config Config) error {
return err return err
} }
// Initialize the SSH allow/deny policy engine // Initialize the SSH allow/deny policy engine for host certificates
if p.sshPolicy, err = newSSHPolicyEngine(p.Options.GetSSHOptions()); err != nil { if p.sshHostPolicy, err = newSSHHostPolicyEngine(p.Options.GetSSHOptions()); err != nil {
return err return err
} }
@ -453,6 +452,6 @@ func (p *GCP) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption,
// Require all the fields in the SSH certificate // Require all the fields in the SSH certificate
&sshCertDefaultValidator{}, &sshCertDefaultValidator{},
// Ensure that all principal names are allowed // Ensure that all principal names are allowed
newSSHNamePolicyValidator(p.sshPolicy), newSSHNamePolicyValidator(p.sshHostPolicy, nil),
), nil ), nil
} }

View file

@ -8,7 +8,6 @@ import (
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/smallstep/certificates/errs" "github.com/smallstep/certificates/errs"
"github.com/smallstep/certificates/policy"
"go.step.sm/crypto/jose" "go.step.sm/crypto/jose"
"go.step.sm/crypto/sshutil" "go.step.sm/crypto/sshutil"
"go.step.sm/crypto/x509util" "go.step.sm/crypto/x509util"
@ -38,8 +37,9 @@ type JWK struct {
Options *Options `json:"options,omitempty"` Options *Options `json:"options,omitempty"`
claimer *Claimer claimer *Claimer
audiences Audiences audiences Audiences
x509Policy policy.X509NamePolicyEngine x509Policy x509PolicyEngine
sshPolicy policy.SSHNamePolicyEngine sshHostPolicy *hostPolicyEngine
sshUserPolicy *userPolicyEngine
} }
// GetID returns the provisioner unique identifier. The name and credential id // GetID returns the provisioner unique identifier. The name and credential id
@ -111,8 +111,13 @@ func (p *JWK) Init(config Config) (err error) {
return err return err
} }
// Initialize the SSH allow/deny policy engine // Initialize the SSH allow/deny policy engine for user certificates
if p.sshPolicy, err = newSSHPolicyEngine(p.Options.GetSSHOptions()); err != nil { if p.sshUserPolicy, err = newSSHUserPolicyEngine(p.Options.GetSSHOptions()); err != nil {
return err
}
// Initialize the SSH allow/deny policy engine for host certificates
if p.sshHostPolicy, err = newSSHHostPolicyEngine(p.Options.GetSSHOptions()); err != nil {
return err return err
} }
@ -294,7 +299,7 @@ func (p *JWK) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption,
// Require and validate all the default fields in the SSH certificate. // Require and validate all the default fields in the SSH certificate.
&sshCertDefaultValidator{}, &sshCertDefaultValidator{},
// Ensure that all principal names are allowed // Ensure that all principal names are allowed
newSSHNamePolicyValidator(p.sshPolicy), newSSHNamePolicyValidator(p.sshHostPolicy, p.sshUserPolicy),
), nil ), nil
} }

View file

@ -11,7 +11,6 @@ import (
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/smallstep/certificates/errs" "github.com/smallstep/certificates/errs"
"github.com/smallstep/certificates/policy"
"go.step.sm/crypto/jose" "go.step.sm/crypto/jose"
"go.step.sm/crypto/pemutil" "go.step.sm/crypto/pemutil"
"go.step.sm/crypto/sshutil" "go.step.sm/crypto/sshutil"
@ -53,8 +52,9 @@ type K8sSA struct {
audiences Audiences audiences Audiences
//kauthn kauthn.AuthenticationV1Interface //kauthn kauthn.AuthenticationV1Interface
pubKeys []interface{} pubKeys []interface{}
x509Policy policy.X509NamePolicyEngine x509Policy x509PolicyEngine
sshPolicy policy.SSHNamePolicyEngine sshHostPolicy *hostPolicyEngine
sshUserPolicy *userPolicyEngine
} }
// GetID returns the provisioner unique identifier. The name and credential id // GetID returns the provisioner unique identifier. The name and credential id
@ -152,8 +152,13 @@ func (p *K8sSA) Init(config Config) (err error) {
return err return err
} }
// Initialize the SSH allow/deny policy engine // Initialize the SSH allow/deny policy engine for user certificates
if p.sshPolicy, err = newSSHPolicyEngine(p.Options.GetSSHOptions()); err != nil { if p.sshUserPolicy, err = newSSHUserPolicyEngine(p.Options.GetSSHOptions()); err != nil {
return err
}
// Initialize the SSH allow/deny policy engine for host certificates
if p.sshHostPolicy, err = newSSHHostPolicyEngine(p.Options.GetSSHOptions()); err != nil {
return err return err
} }
@ -305,7 +310,7 @@ func (p *K8sSA) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOptio
// Require and validate all the default fields in the SSH certificate. // Require and validate all the default fields in the SSH certificate.
&sshCertDefaultValidator{}, &sshCertDefaultValidator{},
// Ensure that all principal names are allowed // Ensure that all principal names are allowed
newSSHNamePolicyValidator(p.sshPolicy), newSSHNamePolicyValidator(p.sshHostPolicy, p.sshUserPolicy),
), nil ), nil
} }

View file

@ -371,7 +371,8 @@ func TestK8sSA_AuthorizeSSHSign(t *testing.T) {
case *sshDefaultDuration: case *sshDefaultDuration:
assert.Equals(t, v.Claimer, tc.p.claimer) assert.Equals(t, v.Claimer, tc.p.claimer)
case *sshNamePolicyValidator: case *sshNamePolicyValidator:
assert.Equals(t, nil, v.policyEngine) assert.Equals(t, nil, v.userPolicyEngine)
assert.Equals(t, nil, v.hostPolicyEngine)
default: default:
assert.FatalError(t, errors.Errorf("unexpected sign option of type %T", v)) assert.FatalError(t, errors.Errorf("unexpected sign option of type %T", v))
} }

View file

@ -11,7 +11,6 @@ import (
"github.com/pkg/errors" "github.com/pkg/errors"
nebula "github.com/slackhq/nebula/cert" nebula "github.com/slackhq/nebula/cert"
"github.com/smallstep/certificates/errs" "github.com/smallstep/certificates/errs"
"github.com/smallstep/certificates/policy"
"go.step.sm/crypto/jose" "go.step.sm/crypto/jose"
"go.step.sm/crypto/sshutil" "go.step.sm/crypto/sshutil"
"go.step.sm/crypto/x25519" "go.step.sm/crypto/x25519"
@ -44,8 +43,9 @@ type Nebula struct {
claimer *Claimer claimer *Claimer
caPool *nebula.NebulaCAPool caPool *nebula.NebulaCAPool
audiences Audiences audiences Audiences
x509Policy policy.X509NamePolicyEngine x509Policy x509PolicyEngine
sshPolicy policy.SSHNamePolicyEngine sshHostPolicy *hostPolicyEngine
sshUserPolicy *userPolicyEngine
} }
// Init verifies and initializes the Nebula provisioner. // Init verifies and initializes the Nebula provisioner.
@ -76,8 +76,13 @@ func (p *Nebula) Init(config Config) error {
return err return err
} }
// Initialize the SSH allow/deny policy engine // Initialize the SSH allow/deny policy engine for user certificates
if p.sshPolicy, err = newSSHPolicyEngine(p.Options.GetSSHOptions()); err != nil { if p.sshUserPolicy, err = newSSHUserPolicyEngine(p.Options.GetSSHOptions()); err != nil {
return err
}
// Initialize the SSH allow/deny policy engine for host certificates
if p.sshHostPolicy, err = newSSHHostPolicyEngine(p.Options.GetSSHOptions()); err != nil {
return err return err
} }
@ -275,7 +280,7 @@ func (p *Nebula) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOpti
// Require all the fields in the SSH certificate // Require all the fields in the SSH certificate
&sshCertDefaultValidator{}, &sshCertDefaultValidator{},
// Ensure that all principal names are allowed // Ensure that all principal names are allowed
newSSHNamePolicyValidator(p.sshPolicy), newSSHNamePolicyValidator(p.sshHostPolicy, p.sshUserPolicy),
), nil ), nil
} }

View file

@ -13,7 +13,6 @@ import (
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/smallstep/certificates/errs" "github.com/smallstep/certificates/errs"
"github.com/smallstep/certificates/policy"
"go.step.sm/crypto/jose" "go.step.sm/crypto/jose"
"go.step.sm/crypto/sshutil" "go.step.sm/crypto/sshutil"
"go.step.sm/crypto/x509util" "go.step.sm/crypto/x509util"
@ -95,8 +94,9 @@ type OIDC struct {
keyStore *keyStore keyStore *keyStore
claimer *Claimer claimer *Claimer
getIdentityFunc GetIdentityFunc getIdentityFunc GetIdentityFunc
x509Policy policy.X509NamePolicyEngine x509Policy x509PolicyEngine
sshPolicy policy.SSHNamePolicyEngine sshHostPolicy *hostPolicyEngine
sshUserPolicy *userPolicyEngine
} }
func sanitizeEmail(email string) string { func sanitizeEmail(email string) string {
@ -216,8 +216,13 @@ func (o *OIDC) Init(config Config) (err error) {
return err return err
} }
// Initialize the SSH allow/deny policy engine // Initialize the SSH allow/deny policy engine for user certificates
if o.sshPolicy, err = newSSHPolicyEngine(o.Options.GetSSHOptions()); err != nil { if o.sshUserPolicy, err = newSSHUserPolicyEngine(o.Options.GetSSHOptions()); err != nil {
return err
}
// Initialize the SSH allow/deny policy engine for host certificates
if o.sshHostPolicy, err = newSSHHostPolicyEngine(o.Options.GetSSHOptions()); err != nil {
return err return err
} }
@ -468,7 +473,7 @@ func (o *OIDC) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption
// Require all the fields in the SSH certificate // Require all the fields in the SSH certificate
&sshCertDefaultValidator{}, &sshCertDefaultValidator{},
// Ensure that all principal names are allowed // Ensure that all principal names are allowed
newSSHNamePolicyValidator(o.sshPolicy), newSSHNamePolicyValidator(o.sshHostPolicy, o.sshUserPolicy),
), nil ), nil
} }

View file

@ -1,11 +1,38 @@
package provisioner package provisioner
import ( import (
"fmt"
"github.com/smallstep/certificates/policy" "github.com/smallstep/certificates/policy"
"golang.org/x/crypto/ssh"
) )
type sshPolicyEngineType string
const (
userPolicyEngineType sshPolicyEngineType = "user"
hostPolicyEngineType sshPolicyEngineType = "host"
)
var certTypeToPolicyEngineType = map[uint32]sshPolicyEngineType{
uint32(ssh.UserCert): userPolicyEngineType,
uint32(ssh.HostCert): hostPolicyEngineType,
}
type x509PolicyEngine interface {
policy.X509NamePolicyEngine
}
type userPolicyEngine struct {
policy.SSHNamePolicyEngine
}
type hostPolicyEngine struct {
policy.SSHNamePolicyEngine
}
// newX509PolicyEngine creates a new x509 name policy engine // newX509PolicyEngine creates a new x509 name policy engine
func newX509PolicyEngine(x509Opts *X509Options) (policy.X509NamePolicyEngine, error) { func newX509PolicyEngine(x509Opts *X509Options) (x509PolicyEngine, error) {
if x509Opts == nil { if x509Opts == nil {
return nil, nil return nil, nil
@ -38,16 +65,66 @@ func newX509PolicyEngine(x509Opts *X509Options) (policy.X509NamePolicyEngine, er
return policy.New(options...) return policy.New(options...)
} }
// newSSHUserPolicyEngine creates a new SSH user certificate policy engine
func newSSHUserPolicyEngine(sshOpts *SSHOptions) (*userPolicyEngine, error) {
policyEngine, err := newSSHPolicyEngine(sshOpts, userPolicyEngineType)
if err != nil {
return nil, err
}
// ensure we're not wrapping a nil engine
if policyEngine == nil {
return nil, nil
}
return &userPolicyEngine{
SSHNamePolicyEngine: policyEngine,
}, nil
}
// newSSHHostPolicyEngine create a new SSH host certificate policy engine
func newSSHHostPolicyEngine(sshOpts *SSHOptions) (*hostPolicyEngine, error) {
policyEngine, err := newSSHPolicyEngine(sshOpts, hostPolicyEngineType)
if err != nil {
return nil, err
}
// ensure we're not wrapping a nil engine
if policyEngine == nil {
return nil, nil
}
return &hostPolicyEngine{
SSHNamePolicyEngine: policyEngine,
}, nil
}
// newSSHPolicyEngine creates a new SSH name policy engine // newSSHPolicyEngine creates a new SSH name policy engine
func newSSHPolicyEngine(sshOpts *SSHOptions) (policy.SSHNamePolicyEngine, error) { func newSSHPolicyEngine(sshOpts *SSHOptions, typ sshPolicyEngineType) (policy.SSHNamePolicyEngine, error) {
if sshOpts == nil { if sshOpts == nil {
return nil, nil return nil, nil
} }
var (
allowed *SSHNameOptions
denied *SSHNameOptions
)
// TODO: embed the type in the policy engine itself for reference?
switch typ {
case userPolicyEngineType:
if sshOpts.User != nil {
allowed = sshOpts.User.GetAllowedNameOptions()
denied = sshOpts.User.GetDeniedNameOptions()
}
case hostPolicyEngineType:
if sshOpts.Host != nil {
allowed = sshOpts.Host.AllowedNames
denied = sshOpts.Host.DeniedNames
}
default:
return nil, fmt.Errorf("unknown SSH policy engine type %s provided", typ)
}
options := []policy.NamePolicyOption{} options := []policy.NamePolicyOption{}
allowed := sshOpts.GetAllowedNameOptions()
if allowed != nil && allowed.HasNames() { if allowed != nil && allowed.HasNames() {
options = append(options, options = append(options,
policy.WithPermittedDNSDomains(allowed.DNSDomains), policy.WithPermittedDNSDomains(allowed.DNSDomains),
@ -57,7 +134,6 @@ func newSSHPolicyEngine(sshOpts *SSHOptions) (policy.SSHNamePolicyEngine, error)
) )
} }
denied := sshOpts.GetDeniedNameOptions()
if denied != nil && denied.HasNames() { if denied != nil && denied.HasNames() {
options = append(options, options = append(options,
policy.WithExcludedDNSDomains(denied.DNSDomains), policy.WithExcludedDNSDomains(denied.DNSDomains),
@ -67,5 +143,14 @@ func newSSHPolicyEngine(sshOpts *SSHOptions) (policy.SSHNamePolicyEngine, error)
) )
} }
// Return nil, because there's no policy to execute. This is
// important, because the logic that determines user vs. host certs
// are allowed depends on this fact. The two policy engines are
// not aware of eachother, so this check is performed in the
// SSH name validator, instead.
if len(options) == 0 {
return nil, nil
}
return policy.New(options...) return policy.New(options...)
} }

View file

@ -16,7 +16,6 @@ import (
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/smallstep/certificates/errs" "github.com/smallstep/certificates/errs"
"github.com/smallstep/certificates/policy"
"go.step.sm/crypto/keyutil" "go.step.sm/crypto/keyutil"
"go.step.sm/crypto/x509util" "go.step.sm/crypto/x509util"
) )
@ -408,11 +407,11 @@ func (v *validityValidator) Valid(cert *x509.Certificate, o SignOptions) error {
// x509NamePolicyValidator validates that the certificate (to be signed) // x509NamePolicyValidator validates that the certificate (to be signed)
// contains only allowed SANs. // contains only allowed SANs.
type x509NamePolicyValidator struct { type x509NamePolicyValidator struct {
policyEngine policy.X509NamePolicyEngine policyEngine x509PolicyEngine
} }
// newX509NamePolicyValidator return a new SANs allow/deny validator. // newX509NamePolicyValidator return a new SANs allow/deny validator.
func newX509NamePolicyValidator(engine policy.X509NamePolicyEngine) *x509NamePolicyValidator { func newX509NamePolicyValidator(engine x509PolicyEngine) *x509NamePolicyValidator {
return &x509NamePolicyValidator{ return &x509NamePolicyValidator{
policyEngine: engine, policyEngine: engine,
} }
@ -424,7 +423,6 @@ func (v *x509NamePolicyValidator) Valid(cert *x509.Certificate, _ SignOptions) e
if v.policyEngine == nil { if v.policyEngine == nil {
return nil return nil
} }
_, err := v.policyEngine.AreCertificateNamesAllowed(cert) _, err := v.policyEngine.AreCertificateNamesAllowed(cert)
return err return err
} }

View file

@ -4,13 +4,13 @@ import (
"crypto/rsa" "crypto/rsa"
"encoding/binary" "encoding/binary"
"encoding/json" "encoding/json"
"fmt"
"math/big" "math/big"
"strings" "strings"
"time" "time"
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/smallstep/certificates/errs" "github.com/smallstep/certificates/errs"
"github.com/smallstep/certificates/policy"
"go.step.sm/crypto/keyutil" "go.step.sm/crypto/keyutil"
"golang.org/x/crypto/ssh" "golang.org/x/crypto/ssh"
) )
@ -448,24 +448,55 @@ func (v sshDefaultPublicKeyValidator) Valid(cert *ssh.Certificate, o SignSSHOpti
// sshNamePolicyValidator validates that the certificate (to be signed) // sshNamePolicyValidator validates that the certificate (to be signed)
// contains only allowed principals. // contains only allowed principals.
type sshNamePolicyValidator struct { type sshNamePolicyValidator struct {
policyEngine policy.SSHNamePolicyEngine hostPolicyEngine *hostPolicyEngine
userPolicyEngine *userPolicyEngine
} }
// newSSHNamePolicyValidator return a new SSH allow/deny validator. // newSSHNamePolicyValidator return a new SSH allow/deny validator.
func newSSHNamePolicyValidator(engine policy.SSHNamePolicyEngine) *sshNamePolicyValidator { func newSSHNamePolicyValidator(host *hostPolicyEngine, user *userPolicyEngine) *sshNamePolicyValidator {
return &sshNamePolicyValidator{ return &sshNamePolicyValidator{
policyEngine: engine, hostPolicyEngine: host,
userPolicyEngine: user,
} }
} }
// Valid validates validates that the certificate (to be signed) // Valid validates validates that the certificate (to be signed)
// contains only allowed principals. // contains only allowed principals.
func (v *sshNamePolicyValidator) Valid(cert *ssh.Certificate, _ SignSSHOptions) error { func (v *sshNamePolicyValidator) Valid(cert *ssh.Certificate, _ SignSSHOptions) error {
if v.policyEngine == nil { if v.hostPolicyEngine == nil && v.userPolicyEngine == nil {
// no policy configured at all; allow anything
return nil return nil
} }
_, err := v.policyEngine.ArePrincipalsAllowed(cert)
// Check the policy type to execute based on type of the certificate.
// We don't allow user certs if only a host policy engine is configured and
// the same for host certs: if only a user policy engine is configured, host
// certs are denied. When both policy engines are configured, the type of
// cert determines which policy engine is used.
policyType, ok := certTypeToPolicyEngineType[cert.CertType]
if !ok {
return fmt.Errorf("unexpected SSH cert type %d", cert.CertType)
}
switch policyType {
case hostPolicyEngineType:
// when no host policy engine is configured, but a user policy engine is
// configured, we don't allow the host certificate.
if v.hostPolicyEngine == nil && v.userPolicyEngine != nil {
return errors.New("SSH host certificate not authorized") // TODO: include principals in message?
}
_, err := v.hostPolicyEngine.ArePrincipalsAllowed(cert)
return err return err
case userPolicyEngineType:
// when no user policy engine is configured, but a host policy engine is
// configured, we don't allow the user certificate.
if v.userPolicyEngine == nil && v.hostPolicyEngine != nil {
return errors.New("SSH user certificate not authorized") // TODO: include principals in message?
}
_, err := v.userPolicyEngine.ArePrincipalsAllowed(cert)
return err
default:
return fmt.Errorf("unexpected policy engine type %q", policyType) // satisfy return; shouldn't happen
}
} }
// sshCertTypeUInt32 // sshCertTypeUInt32

View file

@ -34,6 +34,15 @@ type SSHOptions struct {
// templates. // templates.
TemplateData json.RawMessage `json:"templateData,omitempty"` TemplateData json.RawMessage `json:"templateData,omitempty"`
// User contains SSH user certificate options.
User *SSHUserCertificateOptions `json:"user,omitempty"`
// Host contains SSH host certificate options.
Host *SSHHostCertificateOptions `json:"host,omitempty"`
}
// SSHUserCertificateOptions is a collection of SSH user certificate options.
type SSHUserCertificateOptions struct {
// AllowedNames contains the names the provisioner is authorized to sign // AllowedNames contains the names the provisioner is authorized to sign
AllowedNames *SSHNameOptions `json:"allow,omitempty"` AllowedNames *SSHNameOptions `json:"allow,omitempty"`
@ -41,6 +50,11 @@ type SSHOptions struct {
DeniedNames *SSHNameOptions `json:"deny,omitempty"` DeniedNames *SSHNameOptions `json:"deny,omitempty"`
} }
// SSHHostCertificateOptions is a collection of SSH host certificate options.
// It's an alias of SSHUserCertificateOptions, as the options are the same
// for both types of certificates.
type SSHHostCertificateOptions SSHUserCertificateOptions
// SSHNameOptions models the SSH name policy configuration. // SSHNameOptions models the SSH name policy configuration.
type SSHNameOptions struct { type SSHNameOptions struct {
DNSDomains []string `json:"dns,omitempty"` DNSDomains []string `json:"dns,omitempty"`
@ -56,7 +70,7 @@ func (o *SSHOptions) HasTemplate() bool {
// GetAllowedNameOptions returns the AllowedSSHNameOptions, which models the // GetAllowedNameOptions returns the AllowedSSHNameOptions, which models the
// names that a provisioner is authorized to sign SSH certificates for. // names that a provisioner is authorized to sign SSH certificates for.
func (o *SSHOptions) GetAllowedNameOptions() *SSHNameOptions { func (o *SSHUserCertificateOptions) GetAllowedNameOptions() *SSHNameOptions {
if o == nil { if o == nil {
return nil return nil
} }
@ -65,7 +79,7 @@ func (o *SSHOptions) GetAllowedNameOptions() *SSHNameOptions {
// GetDeniedNameOptions returns the DeniedSSHNameOptions, which models the // GetDeniedNameOptions returns the DeniedSSHNameOptions, which models the
// names that a provisioner is NOT authorized to sign SSH certificates for. // names that a provisioner is NOT authorized to sign SSH certificates for.
func (o *SSHOptions) GetDeniedNameOptions() *SSHNameOptions { func (o *SSHUserCertificateOptions) GetDeniedNameOptions() *SSHNameOptions {
if o == nil { if o == nil {
return nil return nil
} }

View file

@ -9,7 +9,6 @@ import (
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/smallstep/certificates/errs" "github.com/smallstep/certificates/errs"
"github.com/smallstep/certificates/policy"
"go.step.sm/crypto/jose" "go.step.sm/crypto/jose"
"go.step.sm/crypto/sshutil" "go.step.sm/crypto/sshutil"
"go.step.sm/crypto/x509util" "go.step.sm/crypto/x509util"
@ -36,8 +35,9 @@ type X5C struct {
claimer *Claimer claimer *Claimer
audiences Audiences audiences Audiences
rootPool *x509.CertPool rootPool *x509.CertPool
x509Policy policy.X509NamePolicyEngine x509Policy x509PolicyEngine
sshPolicy policy.SSHNamePolicyEngine sshHostPolicy *hostPolicyEngine
sshUserPolicy *userPolicyEngine
} }
// GetID returns the provisioner unique identifier. The name and credential id // GetID returns the provisioner unique identifier. The name and credential id
@ -133,8 +133,13 @@ func (p *X5C) Init(config Config) error {
return err return err
} }
// Initialize the SSH allow/deny policy engine // Initialize the SSH allow/deny policy engine for user certificates
if p.sshPolicy, err = newSSHPolicyEngine(p.Options.GetSSHOptions()); err != nil { if p.sshUserPolicy, err = newSSHUserPolicyEngine(p.Options.GetSSHOptions()); err != nil {
return err
}
// Initialize the SSH allow/deny policy engine for host certificates
if p.sshHostPolicy, err = newSSHHostPolicyEngine(p.Options.GetSSHOptions()); err != nil {
return err return err
} }
@ -326,6 +331,6 @@ func (p *X5C) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption,
// Require all the fields in the SSH certificate // Require all the fields in the SSH certificate
&sshCertDefaultValidator{}, &sshCertDefaultValidator{},
// Ensure that all principal names are allowed // Ensure that all principal names are allowed
newSSHNamePolicyValidator(p.sshPolicy), newSSHNamePolicyValidator(p.sshHostPolicy, p.sshUserPolicy),
), nil ), nil
} }

View file

@ -780,7 +780,8 @@ func TestX5C_AuthorizeSSHSign(t *testing.T) {
case *sshCertValidityValidator: case *sshCertValidityValidator:
assert.Equals(t, v.Claimer, tc.p.claimer) assert.Equals(t, v.Claimer, tc.p.claimer)
case *sshNamePolicyValidator: case *sshNamePolicyValidator:
assert.Equals(t, nil, v.policyEngine) assert.Equals(t, nil, v.userPolicyEngine)
assert.Equals(t, nil, v.hostPolicyEngine)
case *sshDefaultPublicKeyValidator, *sshCertDefaultValidator, sshCertificateOptionsFunc: case *sshDefaultPublicKeyValidator, *sshCertDefaultValidator, sshCertificateOptionsFunc:
default: default:
assert.FatalError(t, errors.Errorf("unexpected sign option of type %T", v)) assert.FatalError(t, errors.Errorf("unexpected sign option of type %T", v))

View file

@ -216,11 +216,11 @@ func (e *NamePolicyEngine) IsIPAllowed(ip net.IP) (bool, error) {
// ArePrincipalsAllowed verifies that all principals in an SSH certificate are allowed. // ArePrincipalsAllowed verifies that all principals in an SSH certificate are allowed.
func (e *NamePolicyEngine) ArePrincipalsAllowed(cert *ssh.Certificate) (bool, error) { func (e *NamePolicyEngine) ArePrincipalsAllowed(cert *ssh.Certificate) (bool, error) {
dnsNames, ips, emails, usernames, err := splitSSHPrincipals(cert) dnsNames, ips, emails, principals, err := splitSSHPrincipals(cert)
if err != nil { if err != nil {
return false, err return false, err
} }
if err := e.validateNames(dnsNames, ips, emails, []*url.URL{}, usernames); err != nil { if err := e.validateNames(dnsNames, ips, emails, []*url.URL{}, principals); err != nil {
return false, err return false, err
} }
return true, nil return true, nil
@ -243,32 +243,26 @@ func appendSubjectCommonName(subject pkix.Name, dnsNames *[]string, ips *[]net.I
} }
// splitPrincipals splits SSH certificate principals into DNS names, emails and usernames. // splitPrincipals splits SSH certificate principals into DNS names, emails and usernames.
func splitSSHPrincipals(cert *ssh.Certificate) (dnsNames []string, ips []net.IP, emails, usernames []string, err error) { func splitSSHPrincipals(cert *ssh.Certificate) (dnsNames []string, ips []net.IP, emails, principals []string, err error) {
dnsNames = []string{} dnsNames = []string{}
ips = []net.IP{} ips = []net.IP{}
emails = []string{} emails = []string{}
usernames = []string{} principals = []string{}
var uris []*url.URL var uris []*url.URL
switch cert.CertType { switch cert.CertType {
case ssh.HostCert: case ssh.HostCert:
dnsNames, ips, emails, uris = x509util.SplitSANs(cert.ValidPrincipals) dnsNames, ips, emails, uris = x509util.SplitSANs(cert.ValidPrincipals)
switch { if len(uris) > 0 {
case len(emails) > 0: err = fmt.Errorf("URL principals %v not expected in SSH host certificate ", uris)
err = fmt.Errorf("Email(-like) principals %v not expected in SSH Host certificate ", emails)
case len(uris) > 0:
err = fmt.Errorf("URL principals %v not expected in SSH Host certificate ", uris)
} }
case ssh.UserCert: case ssh.UserCert:
// re-using SplitSANs results in anything that can't be parsed as an IP, URI or email // re-using SplitSANs results in anything that can't be parsed as an IP, URI or email
// to be considered a username. This allows usernames like h.slatman to be present // to be considered a username principal. This allows usernames like h.slatman to be present
// in the SSH certificate. We're exluding IPs and URIs, because they can be confusing // in the SSH certificate. We're exluding IPs and URIs, because they can be confusing
// when used in a SSH user certificate. // when used in a SSH user certificate.
usernames, ips, emails, uris = x509util.SplitSANs(cert.ValidPrincipals) principals, ips, emails, uris = x509util.SplitSANs(cert.ValidPrincipals)
switch { if len(uris) > 0 {
case len(ips) > 0: err = fmt.Errorf("URL principals %v not expected in SSH user certificate ", uris)
err = fmt.Errorf("IP principals %v not expected in SSH User certificate ", ips)
case len(uris) > 0:
err = fmt.Errorf("URL principals %v not expected in SSH User certificate ", uris)
} }
default: default:
err = fmt.Errorf("unexpected SSH certificate type %d", cert.CertType) err = fmt.Errorf("unexpected SSH certificate type %d", cert.CertType)
@ -280,7 +274,7 @@ func splitSSHPrincipals(cert *ssh.Certificate) (dnsNames []string, ips []net.IP,
// validateNames verifies that all names are allowed. // validateNames verifies that all names are allowed.
// Its logic follows that of (a large part of) the (c *Certificate) isValid() function // Its logic follows that of (a large part of) the (c *Certificate) isValid() function
// in https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go // in https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go
func (e *NamePolicyEngine) validateNames(dnsNames []string, ips []net.IP, emailAddresses []string, uris []*url.URL, usernames []string) error { func (e *NamePolicyEngine) validateNames(dnsNames []string, ips []net.IP, emailAddresses []string, uris []*url.URL, principals []string) error {
// nothing to compare against; return early // nothing to compare against; return early
if e.totalNumberOfConstraints == 0 { if e.totalNumberOfConstraints == 0 {
@ -400,15 +394,15 @@ func (e *NamePolicyEngine) validateNames(dnsNames []string, ips []net.IP, emailA
} }
} }
for _, username := range usernames { for _, principal := range principals {
if e.numberOfPrincipalConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 { if e.numberOfPrincipalConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 {
return NamePolicyError{ return NamePolicyError{
Reason: NotAuthorizedForThisName, Reason: NotAuthorizedForThisName,
Detail: fmt.Sprintf("username principal %q is not explicitly permitted by any constraint", username), Detail: fmt.Sprintf("username principal %q is not explicitly permitted by any constraint", principal),
} }
} }
// TODO: some validation? I.e. allowed characters? // TODO: some validation? I.e. allowed characters?
if err := checkNameConstraints("username", username, username, if err := checkNameConstraints("principal", principal, principal,
func(parsedName, constraint interface{}) (bool, error) { func(parsedName, constraint interface{}) (bool, error) {
return matchUsernameConstraint(parsedName.(string), constraint.(string)) return matchUsernameConstraint(parsedName.(string), constraint.(string))
}, e.permittedPrincipals, e.excludedPrincipals); err != nil { }, e.permittedPrincipals, e.excludedPrincipals); err != nil {

View file

@ -2749,18 +2749,6 @@ func Test_splitSSHPrincipals(t *testing.T) {
wantErr: true, wantErr: true,
} }
}, },
"fail/user-ip": func(t *testing.T) test {
r := emptyResult()
r.wantIps = []net.IP{net.ParseIP("127.0.0.1")} // this will still be in the result
return test{
cert: &ssh.Certificate{
CertType: ssh.UserCert,
ValidPrincipals: []string{"127.0.0.1"},
},
r: r,
wantErr: true,
}
},
"fail/user-uri": func(t *testing.T) test { "fail/user-uri": func(t *testing.T) test {
r := emptyResult() r := emptyResult()
return test{ return test{
@ -2772,18 +2760,6 @@ func Test_splitSSHPrincipals(t *testing.T) {
wantErr: true, wantErr: true,
} }
}, },
"fail/host-email": func(t *testing.T) test {
r := emptyResult()
r.wantEmails = []string{"ops@work"} // this will still be in the result
return test{
cert: &ssh.Certificate{
CertType: ssh.HostCert,
ValidPrincipals: []string{"ops@work"},
},
r: r,
wantErr: true,
}
},
"fail/host-uri": func(t *testing.T) test { "fail/host-uri": func(t *testing.T) test {
r := emptyResult() r := emptyResult()
return test{ return test{
@ -2817,6 +2793,18 @@ func Test_splitSSHPrincipals(t *testing.T) {
r: r, r: r,
} }
}, },
"ok/host-email": func(t *testing.T) test {
r := emptyResult()
r.wantEmails = []string{"ops@work"}
return test{
cert: &ssh.Certificate{
CertType: ssh.HostCert,
ValidPrincipals: []string{"ops@work"},
},
r: r,
wantErr: false,
}
},
"ok/user-localhost": func(t *testing.T) test { "ok/user-localhost": func(t *testing.T) test {
r := emptyResult() r := emptyResult()
r.wantUsernames = []string{"localhost"} // when type is User cert, this is considered a username; not a DNS r.wantUsernames = []string{"localhost"} // when type is User cert, this is considered a username; not a DNS
@ -2839,6 +2827,18 @@ func Test_splitSSHPrincipals(t *testing.T) {
r: r, r: r,
} }
}, },
"ok/user-ip": func(t *testing.T) test {
r := emptyResult()
r.wantIps = []net.IP{net.ParseIP("127.0.0.1")}
return test{
cert: &ssh.Certificate{
CertType: ssh.UserCert,
ValidPrincipals: []string{"127.0.0.1"},
},
r: r,
wantErr: false,
}
},
"ok/user-maillike": func(t *testing.T) test { "ok/user-maillike": func(t *testing.T) test {
r := emptyResult() r := emptyResult()
r.wantEmails = []string{"ops@work"} r.wantEmails = []string{"ops@work"}