TrueCloudLab/certificates
17
0
Fork 2
Code Issues 1 Pull requests 1 Releases Activity Actions

Fix (most) PR comments

Browse source
This commit is contained in:
Herman Slatman 2022-03-31 16:12:29 +02:00
parent bfa4d809fd
commit 571b21abbc
No known key found for this signature in database
GPG key ID: F4D8A44EA0A75A4F
25 changed files with 682 additions and 714 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
api/read/read.go
View file

@ -34,7 +34,7 @@ func ProtoJSON(r io.Reader, m proto.Message) error {
}
// ProtoJSONWithCheck reads JSON from the request body and stores it in the value
// pointed to by v. Returns false if an error was written; true if not.
// pointed to by m. Returns false if an error was written; true if not.
func ProtoJSONWithCheck(w http.ResponseWriter, r io.Reader, m proto.Message) bool {
data, err := io.ReadAll(r)
if err != nil {
@ -57,6 +57,7 @@ func ProtoJSONWithCheck(w http.ResponseWriter, r io.Reader, m proto.Message) boo
if err := protojson.Unmarshal(data, m); err != nil {
if errors.Is(err, proto.Error) {
var wrapper = struct {
// TODO(hs): more properties in the error response?
Message string `json:"message"`
}{
Message: err.Error(),

2
authority/admin/api/acme.go
View file

@ -53,7 +53,7 @@ func (h *Handler) requireEABEnabled(next http.HandlerFunc) http.HandlerFunc {
return
}
next(w, r.WithContext(ctx))
next(w, r)
}
}

52
authority/admin/api/handler.go
View file

@ -35,10 +35,6 @@ func (h *Handler) Route(r api.Router) {
return h.extractAuthorizeTokenAdmin(h.requireAPIEnabled(next))
}
requireEABEnabled := func(next http.HandlerFunc) http.HandlerFunc {
return h.requireEABEnabled(next)
}
enabledInStandalone := func(next http.HandlerFunc) http.HandlerFunc {
return h.checkAction(next, true)
}
@ -47,6 +43,22 @@ func (h *Handler) Route(r api.Router) {
return h.checkAction(next, false)
}
acmeEABMiddleware := func(next http.HandlerFunc) http.HandlerFunc {
return authnz(h.loadProvisionerByName(h.requireEABEnabled(next)))
}
authorityPolicyMiddleware := func(next http.HandlerFunc) http.HandlerFunc {
return authnz(enabledInStandalone(next))
}
provisionerPolicyMiddleware := func(next http.HandlerFunc) http.HandlerFunc {
return authnz(disabledInStandalone(h.loadProvisionerByName(next)))
}
acmePolicyMiddleware := func(next http.HandlerFunc) http.HandlerFunc {
return authnz(disabledInStandalone(h.loadProvisionerByName(h.requireEABEnabled(next))))
}
// Provisioners
r.MethodFunc("GET", "/provisioners/{name}", authnz(h.GetProvisioner))
r.MethodFunc("GET", "/provisioners", authnz(h.GetProvisioners))
@ -62,26 +74,26 @@ func (h *Handler) Route(r api.Router) {
r.MethodFunc("DELETE", "/admins/{id}", authnz(h.DeleteAdmin))
// ACME External Account Binding Keys
r.MethodFunc("GET", "/acme/eab/{provisionerName}/{reference}", authnz(h.loadProvisionerByName(requireEABEnabled(h.acmeResponder.GetExternalAccountKeys))))
r.MethodFunc("GET", "/acme/eab/{provisionerName}", authnz(h.loadProvisionerByName(requireEABEnabled(h.acmeResponder.GetExternalAccountKeys))))
r.MethodFunc("POST", "/acme/eab/{provisionerName}", authnz(h.loadProvisionerByName(requireEABEnabled(h.acmeResponder.CreateExternalAccountKey))))
r.MethodFunc("DELETE", "/acme/eab/{provisionerName}/{id}", authnz(h.loadProvisionerByName(requireEABEnabled(h.acmeResponder.DeleteExternalAccountKey))))
r.MethodFunc("GET", "/acme/eab/{provisionerName}/{reference}", acmeEABMiddleware(h.acmeResponder.GetExternalAccountKeys))
r.MethodFunc("GET", "/acme/eab/{provisionerName}", acmeEABMiddleware(h.acmeResponder.GetExternalAccountKeys))
r.MethodFunc("POST", "/acme/eab/{provisionerName}", acmeEABMiddleware(h.acmeResponder.CreateExternalAccountKey))
r.MethodFunc("DELETE", "/acme/eab/{provisionerName}/{id}", acmeEABMiddleware(h.acmeResponder.DeleteExternalAccountKey))
// Policy - Authority
r.MethodFunc("GET", "/policy", authnz(enabledInStandalone(h.policyResponder.GetAuthorityPolicy)))
r.MethodFunc("POST", "/policy", authnz(enabledInStandalone(h.policyResponder.CreateAuthorityPolicy)))
r.MethodFunc("PUT", "/policy", authnz(enabledInStandalone(h.policyResponder.UpdateAuthorityPolicy)))
r.MethodFunc("DELETE", "/policy", authnz(enabledInStandalone(h.policyResponder.DeleteAuthorityPolicy)))
r.MethodFunc("GET", "/policy", authorityPolicyMiddleware(h.policyResponder.GetAuthorityPolicy))
r.MethodFunc("POST", "/policy", authorityPolicyMiddleware(h.policyResponder.CreateAuthorityPolicy))
r.MethodFunc("PUT", "/policy", authorityPolicyMiddleware(h.policyResponder.UpdateAuthorityPolicy))
r.MethodFunc("DELETE", "/policy", authorityPolicyMiddleware(h.policyResponder.DeleteAuthorityPolicy))
// Policy - Provisioner
r.MethodFunc("GET", "/provisioners/{provisionerName}/policy", authnz(disabledInStandalone(h.loadProvisionerByName(h.policyResponder.GetProvisionerPolicy))))
r.MethodFunc("POST", "/provisioners/{provisionerName}/policy", authnz(disabledInStandalone(h.loadProvisionerByName(h.policyResponder.CreateProvisionerPolicy))))
r.MethodFunc("PUT", "/provisioners/{provisionerName}/policy", authnz(disabledInStandalone(h.loadProvisionerByName(h.policyResponder.UpdateProvisionerPolicy))))
r.MethodFunc("DELETE", "/provisioners/{provisionerName}/policy", authnz(disabledInStandalone(h.loadProvisionerByName(h.policyResponder.DeleteProvisionerPolicy))))
r.MethodFunc("GET", "/provisioners/{provisionerName}/policy", provisionerPolicyMiddleware(h.policyResponder.GetProvisionerPolicy))
r.MethodFunc("POST", "/provisioners/{provisionerName}/policy", provisionerPolicyMiddleware(h.policyResponder.CreateProvisionerPolicy))
r.MethodFunc("PUT", "/provisioners/{provisionerName}/policy", provisionerPolicyMiddleware(h.policyResponder.UpdateProvisionerPolicy))
r.MethodFunc("DELETE", "/provisioners/{provisionerName}/policy", provisionerPolicyMiddleware(h.policyResponder.DeleteProvisionerPolicy))
// Policy - ACME Account
r.MethodFunc("GET", "/acme/policy/{provisionerName}/{accountID}", authnz(disabledInStandalone(h.loadProvisionerByName(h.requireEABEnabled(h.policyResponder.GetACMEAccountPolicy)))))
r.MethodFunc("POST", "/acme/policy/{provisionerName}/{accountID}", authnz(disabledInStandalone(h.loadProvisionerByName(h.requireEABEnabled(h.policyResponder.CreateACMEAccountPolicy)))))
r.MethodFunc("PUT", "/acme/policy/{provisionerName}/{accountID}", authnz(disabledInStandalone(h.loadProvisionerByName(h.requireEABEnabled(h.policyResponder.UpdateACMEAccountPolicy)))))
r.MethodFunc("DELETE", "/acme/policy/{provisionerName}/{accountID}", authnz(disabledInStandalone(h.loadProvisionerByName(h.requireEABEnabled(h.policyResponder.DeleteACMEAccountPolicy)))))
r.MethodFunc("GET", "/acme/policy/{provisionerName}/{accountID}", acmePolicyMiddleware(h.policyResponder.GetACMEAccountPolicy))
r.MethodFunc("POST", "/acme/policy/{provisionerName}/{accountID}", acmePolicyMiddleware(h.policyResponder.CreateACMEAccountPolicy))
r.MethodFunc("PUT", "/acme/policy/{provisionerName}/{accountID}", acmePolicyMiddleware(h.policyResponder.UpdateACMEAccountPolicy))
r.MethodFunc("DELETE", "/acme/policy/{provisionerName}/{accountID}", acmePolicyMiddleware(h.policyResponder.DeleteACMEAccountPolicy))
}

9
authority/admin/api/middleware.go
View file

@ -48,7 +48,7 @@ func (h *Handler) extractAuthorizeTokenAdmin(next http.HandlerFunc) http.Handler
}
}
// loadProvisioner is a middleware that searches for a provisioner
// loadProvisionerByName is a middleware that searches for a provisioner
// by name and stores it in the context.
func (h *Handler) loadProvisionerByName(next http.HandlerFunc) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
@ -81,6 +81,13 @@ func (h *Handler) loadProvisionerByName(next http.HandlerFunc) http.HandlerFunc
func (h *Handler) checkAction(next http.HandlerFunc, supportedInStandalone bool) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
// temporarily only support the admin nosql DB
if _, ok := h.adminDB.(*nosql.DB); !ok {
render.Error(w, admin.NewError(admin.ErrorNotImplementedType,
"operation not supported"))
return
}
// actions allowed in standalone mode are always supported
if supportedInStandalone {
next(w, r)

22
authority/admin/api/policy.go
View file

@ -64,12 +64,7 @@ func (par *PolicyAdminResponder) CreateAuthorityPolicy(w http.ResponseWriter, r
ctx := r.Context()
policy, err := par.auth.GetAuthorityPolicy(ctx)
shouldWriteError := false
if ae, ok := err.(*admin.Error); ok {
shouldWriteError = !ae.IsType(admin.ErrorNotFoundType)
}
if shouldWriteError {
if ae, ok := err.(*admin.Error); ok && !ae.IsType(admin.ErrorNotFoundType) {
render.Error(w, admin.WrapErrorISE(err, "error retrieving authority policy"))
return
}
@ -103,12 +98,7 @@ func (par *PolicyAdminResponder) UpdateAuthorityPolicy(w http.ResponseWriter, r
ctx := r.Context()
policy, err := par.auth.GetAuthorityPolicy(ctx)
shouldWriteError := false
if ae, ok := err.(*admin.Error); ok {
shouldWriteError = !ae.IsType(admin.ErrorNotFoundType)
}
if shouldWriteError {
if ae, ok := err.(*admin.Error); ok && !ae.IsType(admin.ErrorNotFoundType) {
render.Error(w, admin.WrapErrorISE(err, "error retrieving authority policy"))
return
}
@ -256,17 +246,17 @@ func (par *PolicyAdminResponder) DeleteProvisionerPolicy(w http.ResponseWriter,
}
func (par *PolicyAdminResponder) GetACMEAccountPolicy(w http.ResponseWriter, r *http.Request) {
render.JSON(w, "not implemented yet")
render.JSONStatus(w, "not implemented yet", http.StatusNotImplemented)
}
func (par *PolicyAdminResponder) CreateACMEAccountPolicy(w http.ResponseWriter, r *http.Request) {
render.JSON(w, "not implemented yet")
render.JSONStatus(w, "not implemented yet", http.StatusNotImplemented)
}
func (par *PolicyAdminResponder) UpdateACMEAccountPolicy(w http.ResponseWriter, r *http.Request) {
render.JSON(w, "not implemented yet")
render.JSONStatus(w, "not implemented yet", http.StatusNotImplemented)
}
func (par *PolicyAdminResponder) DeleteACMEAccountPolicy(w http.ResponseWriter, r *http.Request) {
render.JSON(w, "not implemented yet")
render.JSONStatus(w, "not implemented yet", http.StatusNotImplemented)
}

32
authority/admin/db/nosql/policy.go
View file

@ -41,9 +41,6 @@ func (db *DB) unmarshalDBAuthorityPolicy(data []byte, authorityID string) (*dbAu
if err := json.Unmarshal(data, dba); err != nil {
return nil, errors.Wrapf(err, "error unmarshaling admin %s into dbAdmin", authorityID)
}
// if !dba.DeletedAt.IsZero() {
// return nil, admin.NewError(admin.ErrorDeletedType, "admin %s is deleted", authorityID)
// }
if dba.AuthorityID != db.authorityID {
return nil, admin.NewError(admin.ErrorAuthorityMismatchType,
"admin %s is not owned by authority %s", dba.ID, db.authorityID)
@ -63,14 +60,6 @@ func (db *DB) getDBAuthorityPolicy(ctx context.Context, authorityID string) (*db
return dbap, nil
}
// func (db *DB) unmarshalAuthorityPolicy(data []byte, authorityID string) (*linkedca.Policy, error) {
// dbap, err := db.unmarshalDBAuthorityPolicy(data, authorityID)
// if err != nil {
// return nil, err
// }
// return dbap.convert(), nil
// }
func (db *DB) CreateAuthorityPolicy(ctx context.Context, policy *linkedca.Policy) error {
dbap := &dbAuthorityPolicy{
@ -88,27 +77,6 @@ func (db *DB) CreateAuthorityPolicy(ctx context.Context, policy *linkedca.Policy
}
func (db *DB) GetAuthorityPolicy(ctx context.Context) (*linkedca.Policy, error) {
// policy := &linkedca.Policy{
// X509: &linkedca.X509Policy{
// Allow: &linkedca.X509Names{
// Dns: []string{".localhost"},
// },
// Deny: &linkedca.X509Names{
// Dns: []string{"denied.localhost"},
// },
// },
// Ssh: &linkedca.SSHPolicy{
// User: &linkedca.SSHUserPolicy{
// Allow: &linkedca.SSHUserNames{},
// Deny: &linkedca.SSHUserNames{},
// },
// Host: &linkedca.SSHHostPolicy{
// Allow: &linkedca.SSHHostNames{},
// Deny: &linkedca.SSHHostNames{},
// },
// },
// }
dbap, err := db.getDBAuthorityPolicy(ctx, db.authorityID)
if err != nil {
return nil, err

4
authority/admin/db/nosql/provisioner.go
View file

@ -19,7 +19,6 @@ type dbProvisioner struct {
Type linkedca.Provisioner_Type `json:"type"`
Name string `json:"name"`
Claims *linkedca.Claims `json:"claims"`
Policy *linkedca.Policy `json:"policy"`
Details []byte `json:"details"`
X509Template *linkedca.Template `json:"x509Template"`
SSHTemplate *linkedca.Template `json:"sshTemplate"`
@ -44,7 +43,6 @@ func (dbp *dbProvisioner) convert2linkedca() (*linkedca.Provisioner, error) {
Type: dbp.Type,
Name: dbp.Name,
Claims: dbp.Claims,
Policy: dbp.Policy,
Details: details,
X509Template: dbp.X509Template,
SshTemplate: dbp.SSHTemplate,
@ -162,7 +160,6 @@ func (db *DB) CreateProvisioner(ctx context.Context, prov *linkedca.Provisioner)
Type: prov.Type,
Name: prov.Name,
Claims: prov.Claims,
Policy: prov.Policy,
Details: details,
X509Template: prov.X509Template,
SSHTemplate: prov.SshTemplate,
@ -190,7 +187,6 @@ func (db *DB) UpdateProvisioner(ctx context.Context, prov *linkedca.Provisioner)
}
nu.Name = prov.Name
nu.Claims = prov.Claims
nu.Policy = prov.Policy
nu.Details, err = json.Marshal(prov.Details.GetData())
if err != nil {
return admin.WrapErrorISE(err, "error marshaling details when updating provisioner %s", prov.Name)

19
authority/authority.go
View file

@ -12,6 +12,11 @@ import (
"time"
"github.com/pkg/errors"
"golang.org/x/crypto/ssh"
"go.step.sm/crypto/pemutil"
"go.step.sm/linkedca"
"github.com/smallstep/certificates/authority/admin"
adminDBNosql "github.com/smallstep/certificates/authority/admin/db/nosql"
"github.com/smallstep/certificates/authority/administrator"
@ -27,9 +32,6 @@ import (
"github.com/smallstep/certificates/scep"
"github.com/smallstep/certificates/templates"
"github.com/smallstep/nosql"
"go.step.sm/crypto/pemutil"
"go.step.sm/linkedca"
"golang.org/x/crypto/ssh"
)
// Authority implements the Certificate Authority internal interface.
@ -220,6 +222,17 @@ func (a *Authority) reloadPolicyEngines(ctx context.Context) error {
)
// if admin API is enabled, the CA is running in linked mode
if a.config.AuthorityConfig.EnableAdmin {
// temporarily disable policy loading when LinkedCA is in use
if _, ok := a.adminDB.(*linkedCaClient); ok {
return nil
}
// temporarily only support the admin nosql DB
if _, ok := a.adminDB.(*adminDBNosql.DB); !ok {
return nil
}
linkedPolicy, err := a.adminDB.GetAuthorityPolicy(ctx)
if err != nil {
return admin.WrapErrorISE(err, "error getting policy to (re)load policy engines")

4
authority/config/config.go
View file

@ -8,13 +8,15 @@ import (
"time"
"github.com/pkg/errors"
"go.step.sm/linkedca"
"github.com/smallstep/certificates/authority/policy"
"github.com/smallstep/certificates/authority/provisioner"
cas "github.com/smallstep/certificates/cas/apiv1"
"github.com/smallstep/certificates/db"
kms "github.com/smallstep/certificates/kms/apiv1"
"github.com/smallstep/certificates/templates"
"go.step.sm/linkedca"
)
const (

12
authority/linkedca.go
View file

@ -15,16 +15,18 @@ import (
"time"
"github.com/pkg/errors"
"github.com/smallstep/certificates/authority/admin"
"github.com/smallstep/certificates/db"
"golang.org/x/crypto/ssh"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials"
"go.step.sm/crypto/jose"
"go.step.sm/crypto/keyutil"
"go.step.sm/crypto/tlsutil"
"go.step.sm/crypto/x509util"
"go.step.sm/linkedca"
"golang.org/x/crypto/ssh"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials"
"github.com/smallstep/certificates/authority/admin"
"github.com/smallstep/certificates/db"
)
const uuidPattern = "^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}$"

1
authority/policy/options.go
View file

@ -183,6 +183,7 @@ func (o *SSHUserCertificateOptions) GetDeniedNameOptions() *SSHNameOptions {
// names configured.
func (o *SSHNameOptions) HasNames() bool {
return len(o.DNSDomains) > 0 ||
len(o.IPRanges) > 0 ||
len(o.EmailAddresses) > 0 ||
len(o.Principals) > 0
}

13
authority/provisioner/jwk.go
View file

@ -210,23 +210,10 @@ func (p *JWK) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
// revocation status. Just confirms that the provisioner that created the
// certificate was configured to allow renewals.
func (p *JWK) AuthorizeRenew(ctx context.Context, cert *x509.Certificate) error {
// if p.claimer.IsDisableRenewal() {
// return errs.Unauthorized("jwk.AuthorizeRenew; renew is disabled for jwk provisioner '%s'", p.GetName())
// }
// TODO(hs): authorize the SANs using x509 name policy allow/deny rules (also for other provisioners with AuthorizeRewew and AuthorizeSSHRenew)
//return p.authorizeRenew(cert)
// return nil
return p.ctl.AuthorizeRenew(ctx, cert)
}
// func (p *JWK) authorizeRenew(cert *x509.Certificate) error {
// if p.x509PolicyEngine == nil {
// return nil
// }
// _, err := p.x509PolicyEngine.AreCertificateNamesAllowed(cert)
// return err
// }
// AuthorizeSSHSign returns the list of SignOption for a SignSSH request.
func (p *JWK) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption, error) {
if !p.ctl.Claimer.IsSSHCAEnabled() {

8
authority/provisioner/k8sSA.go
View file

@ -56,7 +56,6 @@ type K8sSA struct {
ctl *Controller
x509Policy policy.X509Policy
sshHostPolicy policy.HostPolicy
sshUserPolicy policy.UserPolicy
}
// GetID returns the provisioner unique identifier. The name and credential id
@ -149,11 +148,6 @@ func (p *K8sSA) Init(config Config) (err error) {
return err
}
// Initialize the SSH allow/deny policy engine for user certificates
if p.sshUserPolicy, err = policy.NewSSHUserPolicyEngine(p.Options.GetSSHOptions()); err != nil {
return err
}
// Initialize the SSH allow/deny policy engine for host certificates
if p.sshHostPolicy, err = policy.NewSSHHostPolicyEngine(p.Options.GetSSHOptions()); err != nil {
return err
@ -304,7 +298,7 @@ func (p *K8sSA) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOptio
// Require and validate all the default fields in the SSH certificate.
&sshCertDefaultValidator{},
// Ensure that all principal names are allowed
newSSHNamePolicyValidator(p.sshHostPolicy, p.sshUserPolicy),
newSSHNamePolicyValidator(p.sshHostPolicy, nil),
), nil
}

8
authority/provisioner/nebula.go
View file

@ -45,7 +45,6 @@ type Nebula struct {
ctl *Controller
x509Policy policy.X509Policy
sshHostPolicy policy.HostPolicy
sshUserPolicy policy.UserPolicy
}
// Init verifies and initializes the Nebula provisioner.
@ -69,11 +68,6 @@ func (p *Nebula) Init(config Config) (err error) {
return err
}
// Initialize the SSH allow/deny policy engine for user certificates
if p.sshUserPolicy, err = policy.NewSSHUserPolicyEngine(p.Options.GetSSHOptions()); err != nil {
return err
}
// Initialize the SSH allow/deny policy engine for host certificates
if p.sshHostPolicy, err = policy.NewSSHHostPolicyEngine(p.Options.GetSSHOptions()); err != nil {
return err
@ -276,7 +270,7 @@ func (p *Nebula) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOpti
// Require all the fields in the SSH certificate
&sshCertDefaultValidator{},
// Ensure that all principal names are allowed
newSSHNamePolicyValidator(p.sshHostPolicy, p.sshUserPolicy),
newSSHNamePolicyValidator(p.sshHostPolicy, nil),
), nil
}

8
authority/provisioner/oidc.go
View file

@ -95,7 +95,6 @@ type OIDC struct {
Options *Options `json:"options,omitempty"`
configuration openIDConfiguration
keyStore *keyStore
getIdentityFunc GetIdentityFunc
ctl *Controller
x509Policy policy.X509Policy
sshHostPolicy policy.HostPolicy
@ -202,13 +201,6 @@ func (o *OIDC) Init(config Config) (err error) {
return err
}
// Set the identity getter if it exists, otherwise use the default.
if config.GetIdentityFunc == nil {
o.getIdentityFunc = DefaultIdentityFunc
} else {
o.getIdentityFunc = config.GetIdentityFunc
}
// Initialize the x509 allow/deny policy engine
if o.x509Policy, err = policy.NewX509PolicyEngine(o.Options.GetX509Options()); err != nil {
return err

2
authority/provisioner/sign_options.go
View file

@ -422,7 +422,7 @@ func (v *x509NamePolicyValidator) Valid(cert *x509.Certificate, _ SignOptions) e
if v.policyEngine == nil {
return nil
}
_, err := v.policyEngine.AreCertificateNamesAllowed(cert)
_, err := v.policyEngine.IsX509CertificateAllowed(cert)
return err
}

4
authority/provisioner/sign_ssh_options.go
View file

@ -480,7 +480,7 @@ func (v *sshNamePolicyValidator) Valid(cert *ssh.Certificate, _ SignSSHOptions)
if v.hostPolicyEngine == nil && v.userPolicyEngine != nil {
return errors.New("SSH host certificate not authorized")
}
_, err := v.hostPolicyEngine.ArePrincipalsAllowed(cert)
_, err := v.hostPolicyEngine.IsSSHCertificateAllowed(cert)
return err
case ssh.UserCert:
// when no user policy engine is configured, but a host policy engine is
@ -488,7 +488,7 @@ func (v *sshNamePolicyValidator) Valid(cert *ssh.Certificate, _ SignSSHOptions)
if v.userPolicyEngine == nil && v.hostPolicyEngine != nil {
return errors.New("SSH user certificate not authorized")
}
_, err := v.userPolicyEngine.ArePrincipalsAllowed(cert)
_, err := v.userPolicyEngine.IsSSHCertificateAllowed(cert)
return err
default:
return fmt.Errorf("unexpected SSH certificate type %d", cert.CertType) // satisfy return; shouldn't happen

2
authority/provisioner/sshpop.go
View file

@ -97,8 +97,6 @@ func (p *SSHPOP) Init(config Config) (err error) {
p.sshPubKeys = config.SSHKeys
config.Audiences = config.Audiences.WithFragment(p.GetIDForToken())
// Update claims with global ones
p.ctl, err = NewController(p, p.Claims, config)
return
}

6
authority/ssh.go
View file

@ -5,11 +5,11 @@ import (
"crypto/rand"
"crypto/x509"
"encoding/binary"
"errors"
"net/http"
"strings"
"time"
"github.com/pkg/errors"
"github.com/smallstep/certificates/authority/config"
"github.com/smallstep/certificates/authority/provisioner"
"github.com/smallstep/certificates/db"
@ -250,7 +250,7 @@ func (a *Authority) SignSSH(ctx context.Context, key ssh.PublicKey, opts provisi
return nil, errs.ForbiddenErr(errors.New("authority not allowed to sign ssh user certificates"), "authority.SignSSH: error creating ssh user certificate")
}
if a.sshUserPolicy != nil {
allowed, err := a.sshUserPolicy.ArePrincipalsAllowed(certTpl)
allowed, err := a.sshUserPolicy.IsSSHCertificateAllowed(certTpl)
if err != nil {
return nil, errs.InternalServerErr(err,
errs.WithMessage("authority.SignSSH: error creating ssh user certificate"),
@ -267,7 +267,7 @@ func (a *Authority) SignSSH(ctx context.Context, key ssh.PublicKey, opts provisi
return nil, errs.ForbiddenErr(errors.New("authority not allowed to sign ssh host certificates"), "authority.SignSSH: error creating ssh user certificate")
}
if a.sshHostPolicy != nil {
allowed, err := a.sshHostPolicy.ArePrincipalsAllowed(certTpl)
allowed, err := a.sshHostPolicy.IsSSHCertificateAllowed(certTpl)
if err != nil {
return nil, errs.InternalServerErr(err,
errs.WithMessage("authority.SignSSH: error creating ssh host certificate"),

2
authority/tls.go
View file

@ -238,7 +238,7 @@ func (a *Authority) isAllowedToSign(cert *x509.Certificate) (bool, error) {
return true, nil
}
return a.x509Policy.AreCertificateNamesAllowed(cert)
return a.x509Policy.IsX509CertificateAllowed(cert)
}
// AreSANsAllowed evaluates the provided sans against the

583
policy/engine.go
View file

@ -1,17 +1,13 @@
package policy
import (
"bytes"
"crypto/x509"
"crypto/x509/pkix"
"fmt"
"net"
"net/url"
"reflect"
"strings"
"golang.org/x/crypto/ssh"
"golang.org/x/net/idna"
"go.step.sm/crypto/x509util"
)
@ -161,8 +157,8 @@ func removeDuplicateIPRanges(ipRanges []*net.IPNet) []*net.IPNet {
return result
}
// AreCertificateNamesAllowed verifies that all SANs in a Certificate are allowed.
func (e *NamePolicyEngine) AreCertificateNamesAllowed(cert *x509.Certificate) (bool, error) {
// IsX509CertificateAllowed verifies that all SANs in a Certificate are allowed.
func (e *NamePolicyEngine) IsX509CertificateAllowed(cert *x509.Certificate) (bool, error) {
dnsNames, ips, emails, uris := cert.DNSNames, cert.IPAddresses, cert.EmailAddresses, cert.URIs
// when Subject Common Name must be verified in addition to the SANs, it is
// added to the appropriate slice of names.
@ -175,8 +171,8 @@ func (e *NamePolicyEngine) AreCertificateNamesAllowed(cert *x509.Certificate) (b
return true, nil
}
// AreCSRNamesAllowed verifies that all names in the CSR are allowed.
func (e *NamePolicyEngine) AreCSRNamesAllowed(csr *x509.CertificateRequest) (bool, error) {
// IsX509CertificateRequestAllowed verifies that all names in the CSR are allowed.
func (e *NamePolicyEngine) IsX509CertificateRequestAllowed(csr *x509.CertificateRequest) (bool, error) {
dnsNames, ips, emails, uris := csr.DNSNames, csr.IPAddresses, csr.EmailAddresses, csr.URIs
// when Subject Common Name must be verified in addition to the SANs, it is
// added to the appropriate slice of names.
@ -215,8 +211,8 @@ func (e *NamePolicyEngine) IsIPAllowed(ip net.IP) (bool, error) {
return true, nil
}
// ArePrincipalsAllowed verifies that all principals in an SSH certificate are allowed.
func (e *NamePolicyEngine) ArePrincipalsAllowed(cert *ssh.Certificate) (bool, error) {
// IsSSHCertificateAllowed verifies that all principals in an SSH certificate are allowed.
func (e *NamePolicyEngine) IsSSHCertificateAllowed(cert *ssh.Certificate) (bool, error) {
dnsNames, ips, emails, principals, err := splitSSHPrincipals(cert)
if err != nil {
return false, err
@ -259,7 +255,7 @@ func splitSSHPrincipals(cert *ssh.Certificate) (dnsNames []string, ips []net.IP,
case ssh.UserCert:
// re-using SplitSANs results in anything that can't be parsed as an IP, URI or email
// 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 URIs, because they can be confusing
// when used in a SSH user certificate.
principals, ips, emails, uris = x509util.SplitSANs(cert.ValidPrincipals)
if len(uris) > 0 {
@ -271,568 +267,3 @@ func splitSSHPrincipals(cert *ssh.Certificate) (dnsNames []string, ips []net.IP,
return
}
// validateNames verifies that all names are allowed.
// 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
func (e *NamePolicyEngine) validateNames(dnsNames []string, ips []net.IP, emailAddresses []string, uris []*url.URL, principals []string) error {
// nothing to compare against; return early
if e.totalNumberOfConstraints == 0 {
return nil
}
// TODO: implement check that requires at least a single name in all of the SANs + subject?
// TODO: set limit on total of all names validated? In x509 there's a limit on the number of comparisons
// that protects the CA from a DoS (i.e. many heavy comparisons). The x509 implementation takes
// this number as a total of all checks and keeps a (pointer to a) counter of the number of checks
// executed so far.
// TODO: gather all errors, or return early? Currently we return early on the first wrong name; check might fail for multiple names.
// Perhaps make that an option?
for _, dns := range dnsNames {
// if there are DNS names to check, no DNS constraints set, but there are other permitted constraints,
// then return error, because DNS should be explicitly configured to be allowed in that case. In case there are
// (other) excluded constraints, we'll allow a DNS (implicit allow; currently).
if e.numberOfDNSDomainConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 {
return &NamePolicyError{
Reason: NotAuthorizedForThisName,
Detail: fmt.Sprintf("dns %q is not explicitly permitted by any constraint", dns),
}
}
didCutWildcard := false
if strings.HasPrefix(dns, "*.") {
dns = dns[1:]
didCutWildcard = true
}
parsedDNS, err := idna.Lookup.ToASCII(dns)
if err != nil {
return &NamePolicyError{
Reason: CannotParseDomain,
Detail: fmt.Sprintf("dns %q cannot be converted to ASCII", dns),
}
}
if didCutWildcard {
parsedDNS = "*" + parsedDNS
}
if _, ok := domainToReverseLabels(parsedDNS); !ok {
return &NamePolicyError{
Reason: CannotParseDomain,
Detail: fmt.Sprintf("cannot parse dns %q", dns),
}
}
if err := checkNameConstraints("dns", dns, parsedDNS,
func(parsedName, constraint interface{}) (bool, error) {
return e.matchDomainConstraint(parsedName.(string), constraint.(string))
}, e.permittedDNSDomains, e.excludedDNSDomains); err != nil {
return err
}
}
for _, ip := range ips {
if e.numberOfIPRangeConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 {
return &NamePolicyError{
Reason: NotAuthorizedForThisName,
Detail: fmt.Sprintf("ip %q is not explicitly permitted by any constraint", ip.String()),
}
}
if err := checkNameConstraints("ip", ip.String(), ip,
func(parsedName, constraint interface{}) (bool, error) {
return matchIPConstraint(parsedName.(net.IP), constraint.(*net.IPNet))
}, e.permittedIPRanges, e.excludedIPRanges); err != nil {
return err
}
}
for _, email := range emailAddresses {
if e.numberOfEmailAddressConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 {
return &NamePolicyError{
Reason: NotAuthorizedForThisName,
Detail: fmt.Sprintf("email %q is not explicitly permitted by any constraint", email),
}
}
mailbox, ok := parseRFC2821Mailbox(email)
if !ok {
return &NamePolicyError{
Reason: CannotParseRFC822Name,
Detail: fmt.Sprintf("invalid rfc822Name %q", mailbox),
}
}
// According to RFC 5280, section 7.5, emails are considered to match if the local part is
// an exact match and the host (domain) part matches the ASCII representation (case-insensitive):
// https://datatracker.ietf.org/doc/html/rfc5280#section-7.5
domainASCII, err := idna.ToASCII(mailbox.domain)
if err != nil {
return &NamePolicyError{
Reason: CannotParseDomain,
Detail: fmt.Sprintf("cannot parse email domain %q", email),
}
}
mailbox.domain = domainASCII
if err := checkNameConstraints("email", email, mailbox,
func(parsedName, constraint interface{}) (bool, error) {
return e.matchEmailConstraint(parsedName.(rfc2821Mailbox), constraint.(string))
}, e.permittedEmailAddresses, e.excludedEmailAddresses); err != nil {
return err
}
}
// TODO(hs): fix internationalization for URIs (IRIs)
for _, uri := range uris {
if e.numberOfURIDomainConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 {
return &NamePolicyError{
Reason: NotAuthorizedForThisName,
Detail: fmt.Sprintf("uri %q is not explicitly permitted by any constraint", uri.String()),
}
}
if err := checkNameConstraints("uri", uri.String(), uri,
func(parsedName, constraint interface{}) (bool, error) {
return e.matchURIConstraint(parsedName.(*url.URL), constraint.(string))
}, e.permittedURIDomains, e.excludedURIDomains); err != nil {
return err
}
}
for _, principal := range principals {
if e.numberOfPrincipalConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 {
return &NamePolicyError{
Reason: NotAuthorizedForThisName,
Detail: fmt.Sprintf("username principal %q is not explicitly permitted by any constraint", principal),
}
}
// TODO: some validation? I.e. allowed characters?
if err := checkNameConstraints("principal", principal, principal,
func(parsedName, constraint interface{}) (bool, error) {
return matchUsernameConstraint(parsedName.(string), constraint.(string))
}, e.permittedPrincipals, e.excludedPrincipals); err != nil {
return err
}
}
// if all checks out, all SANs are allowed
return nil
}
// checkNameConstraints checks that a name, of type nameType is permitted.
// The argument parsedName contains the parsed form of name, suitable for passing
// to the match function.
// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go
func checkNameConstraints(
nameType string,
name string,
parsedName interface{},
match func(parsedName, constraint interface{}) (match bool, err error),
permitted, excluded interface{}) error {
excludedValue := reflect.ValueOf(excluded)
for i := 0; i < excludedValue.Len(); i++ {
constraint := excludedValue.Index(i).Interface()
match, err := match(parsedName, constraint)
if err != nil {
return &NamePolicyError{
Reason: CannotMatchNameToConstraint,
Detail: err.Error(),
}
}
if match {
return &NamePolicyError{
Reason: NotAuthorizedForThisName,
Detail: fmt.Sprintf("%s %q is excluded by constraint %q", nameType, name, constraint),
}
}
}
permittedValue := reflect.ValueOf(permitted)
ok := true
for i := 0; i < permittedValue.Len(); i++ {
constraint := permittedValue.Index(i).Interface()
var err error
if ok, err = match(parsedName, constraint); err != nil {
return &NamePolicyError{
Reason: CannotMatchNameToConstraint,
Detail: err.Error(),
}
}
if ok {
break
}
}
if !ok {
return &NamePolicyError{
Reason: NotAuthorizedForThisName,
Detail: fmt.Sprintf("%s %q is not permitted by any constraint", nameType, name),
}
}
return nil
}
// domainToReverseLabels converts a textual domain name like foo.example.com to
// the list of labels in reverse order, e.g. ["com", "example", "foo"].
// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go
func domainToReverseLabels(domain string) (reverseLabels []string, ok bool) {
for len(domain) > 0 {
if i := strings.LastIndexByte(domain, '.'); i == -1 {
reverseLabels = append(reverseLabels, domain)
domain = ""
} else {
reverseLabels = append(reverseLabels, domain[i+1:])
domain = domain[:i]
}
}
if len(reverseLabels) > 0 && reverseLabels[0] == "" {
// An empty label at the end indicates an absolute value.
return nil, false
}
for _, label := range reverseLabels {
if label == "" {
// Empty labels are otherwise invalid.
return nil, false
}
for _, c := range label {
if c < 33 || c > 126 {
// Invalid character.
return nil, false
}
}
}
return reverseLabels, true
}
// rfc2821Mailbox represents a “mailbox” (which is an email address to most
// people) by breaking it into the “local” (i.e. before the '@') and “domain”
// parts.
// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go
type rfc2821Mailbox struct {
local, domain string
}
// parseRFC2821Mailbox parses an email address into local and domain parts,
// based on the ABNF for a “Mailbox” from RFC 2821. According to RFC 5280,
// Section 4.2.1.6 that's correct for an rfc822Name from a certificate: “The
// format of an rfc822Name is a "Mailbox" as defined in RFC 2821, Section 4.1.2”.
// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go
func parseRFC2821Mailbox(in string) (mailbox rfc2821Mailbox, ok bool) {
if in == "" {
return mailbox, false
}
localPartBytes := make([]byte, 0, len(in)/2)
if in[0] == '"' {
// Quoted-string = DQUOTE *qcontent DQUOTE
// non-whitespace-control = %d1-8 / %d11 / %d12 / %d14-31 / %d127
// qcontent = qtext / quoted-pair
// qtext = non-whitespace-control /
// %d33 / %d35-91 / %d93-126
// quoted-pair = ("\" text) / obs-qp
// text = %d1-9 / %d11 / %d12 / %d14-127 / obs-text
//
// (Names beginning with “obs-” are the obsolete syntax from RFC 2822,
// Section 4. Since it has been 16 years, we no longer accept that.)
in = in[1:]
QuotedString:
for {
if in == "" {
return mailbox, false
}
c := in[0]
in = in[1:]
switch {
case c == '"':
break QuotedString
case c == '\\':
// quoted-pair
if in == "" {
return mailbox, false
}
if in[0] == 11 ||
in[0] == 12 ||
(1 <= in[0] && in[0] <= 9) ||
(14 <= in[0] && in[0] <= 127) {
localPartBytes = append(localPartBytes, in[0])
in = in[1:]
} else {
return mailbox, false
}
case c == 11 ||
c == 12 ||
// Space (char 32) is not allowed based on the
// BNF, but RFC 3696 gives an example that
// assumes that it is. Several “verified”
// errata continue to argue about this point.
// We choose to accept it.
c == 32 ||
c == 33 ||
c == 127 ||
(1 <= c && c <= 8) ||
(14 <= c && c <= 31) ||
(35 <= c && c <= 91) ||
(93 <= c && c <= 126):
// qtext
localPartBytes = append(localPartBytes, c)
default:
return mailbox, false
}
}
} else {
// Atom ("." Atom)*
NextChar:
for len(in) > 0 {
// atext from RFC 2822, Section 3.2.4
c := in[0]
switch {
case c == '\\':
// Examples given in RFC 3696 suggest that
// escaped characters can appear outside of a
// quoted string. Several “verified” errata
// continue to argue the point. We choose to
// accept it.
in = in[1:]
if in == "" {
return mailbox, false
}
fallthrough
case ('0' <= c && c <= '9') ||
('a' <= c && c <= 'z') ||
('A' <= c && c <= 'Z') ||
c == '!' || c == '#' || c == '$' || c == '%' ||
c == '&' || c == '\'' || c == '*' || c == '+' ||
c == '-' || c == '/' || c == '=' || c == '?' ||
c == '^' || c == '_' || c == '`' || c == '{' ||
c == '|' || c == '}' || c == '~' || c == '.':
localPartBytes = append(localPartBytes, in[0])
in = in[1:]
default:
break NextChar
}
}
if len(localPartBytes) == 0 {
return mailbox, false
}
// From RFC 3696, Section 3:
// “period (".") may also appear, but may not be used to start
// or end the local part, nor may two or more consecutive
// periods appear.”
twoDots := []byte{'.', '.'}
if localPartBytes[0] == '.' ||
localPartBytes[len(localPartBytes)-1] == '.' ||
bytes.Contains(localPartBytes, twoDots) {
return mailbox, false
}
}
if in == "" || in[0] != '@' {
return mailbox, false
}
in = in[1:]
// The RFC species a format for domains, but that's known to be
// violated in practice so we accept that anything after an '@' is the
// domain part.
if _, ok := domainToReverseLabels(in); !ok {
return mailbox, false
}
mailbox.local = string(localPartBytes)
mailbox.domain = in
return mailbox, true
}
// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go
func (e *NamePolicyEngine) matchDomainConstraint(domain, constraint string) (bool, error) {
// The meaning of zero length constraints is not specified, but this
// code follows NSS and accepts them as matching everything.
if constraint == "" {
return true, nil
}
// A single whitespace seems to be considered a valid domain, but we don't allow it.
if domain == " " {
return false, nil
}
// Block domains that start with just a period
if domain[0] == '.' {
return false, nil
}
// Block wildcard domains that don't start with exactly "*." (i.e. double wildcards and such)
if domain[0] == '*' && domain[1] != '.' {
return false, nil
}
// Check if the domain starts with a wildcard and return early if not allowed
if strings.HasPrefix(domain, "*.") && !e.allowLiteralWildcardNames {
return false, nil
}
// Only allow asterisk at the start of the domain; we don't allow them as part of a domain label or as a (sub)domain label (currently)
if strings.LastIndex(domain, "*") > 0 {
return false, nil
}
// Don't allow constraints with empty labels in any position
if strings.Contains(constraint, "..") {
return false, nil
}
domainLabels, ok := domainToReverseLabels(domain)
if !ok {
return false, fmt.Errorf("cannot parse domain %q", domain)
}
// RFC 5280 says that a leading period in a domain name means that at
// least one label must be prepended, but only for URI and email
// constraints, not DNS constraints. The code also supports that
// behavior for DNS constraints. In our adaptation of the original
// Go stdlib x509 Name Constraint implementation we look for exactly
// one subdomain, currently.
mustHaveSubdomains := false
if constraint[0] == '.' {
mustHaveSubdomains = true
constraint = constraint[1:]
}
constraintLabels, ok := domainToReverseLabels(constraint)
if !ok {
return false, fmt.Errorf("cannot parse domain constraint %q", constraint)
}
// fmt.Println(mustHaveSubdomains)
// fmt.Println(constraintLabels)
// fmt.Println(domainLabels)
expectedNumberOfLabels := len(constraintLabels)
if mustHaveSubdomains {
// we expect exactly one more label if it starts with the "canonical" x509 "wildcard": "."
// in the future we could extend this to support multiple additional labels and/or more
// complex matching.
expectedNumberOfLabels++
}
if len(domainLabels) != expectedNumberOfLabels {
return false, nil
}
for i, constraintLabel := range constraintLabels {
if !strings.EqualFold(constraintLabel, domainLabels[i]) {
return false, nil
}
}
return true, nil
}
// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go
func matchIPConstraint(ip net.IP, constraint *net.IPNet) (bool, error) {
// TODO(hs): this is code from Go library, but I got some unexpected result:
// with permitted net 127.0.0.0/24, 127.0.0.1 is NOT allowed. When parsing 127.0.0.1 as net.IP
// which is in the IPAddresses slice, the underlying length is 16. The contraint.IP has a length
// of 4 instead. I currently don't believe that this is a bug in Go now, but why is it like that?
// Is there a difference because we're not operating on a sans []string slice? Or is the Go
// implementation stricter regarding IPv4 vs. IPv6? I've been bitten by some unfortunate differences
// between the two before (i.e. IPv4 in IPv6; IP SANS in ACME)
// if len(ip) != len(constraint.IP) {
// return false, nil
// }
// for i := range ip {
// if mask := constraint.Mask[i]; ip[i]&mask != constraint.IP[i]&mask {
// return false, nil
// }
// }
contained := constraint.Contains(ip) // TODO(hs): validate that this is the correct behavior; also check IPv4-in-IPv6 (again)
return contained, nil
}
// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go
func (e *NamePolicyEngine) matchEmailConstraint(mailbox rfc2821Mailbox, constraint string) (bool, error) {
// TODO(hs): handle literal wildcard case for emails? Does that even make sense?
// If the constraint contains an @, then it specifies an exact mailbox name (currently)
if strings.Contains(constraint, "*") {
return false, fmt.Errorf("email constraint %q cannot contain asterisk", constraint)
}
if strings.Contains(constraint, "@") {
constraintMailbox, ok := parseRFC2821Mailbox(constraint)
if !ok {
return false, fmt.Errorf("cannot parse constraint %q", constraint)
}
return mailbox.local == constraintMailbox.local && strings.EqualFold(mailbox.domain, constraintMailbox.domain), nil
}
// Otherwise the constraint is like a DNS constraint of the domain part
// of the mailbox.
return e.matchDomainConstraint(mailbox.domain, constraint)
}
// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go
func (e *NamePolicyEngine) matchURIConstraint(uri *url.URL, constraint string) (bool, error) {
// From RFC 5280, Section 4.2.1.10:
// “a uniformResourceIdentifier that does not include an authority
// component with a host name specified as a fully qualified domain
// name (e.g., if the URI either does not include an authority
// component or includes an authority component in which the host name
// is specified as an IP address), then the application MUST reject the
// certificate.”
host := uri.Host
if host == "" {
return false, fmt.Errorf("URI with empty host (%q) cannot be matched against constraints", uri.String())
}
// Block hosts with the wildcard character; no exceptions, also not when wildcards allowed.
if strings.Contains(host, "*") {
return false, fmt.Errorf("URI host %q cannot contain asterisk", uri.String())
}
if strings.Contains(host, ":") && !strings.HasSuffix(host, "]") {
var err error
host, _, err = net.SplitHostPort(uri.Host)
if err != nil {
return false, err
}
}
if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") ||
net.ParseIP(host) != nil {
return false, fmt.Errorf("URI with IP %q cannot be matched against constraints", uri.String())
}
// TODO(hs): add checks for scheme, path, etc.; either here, or in a different constraint matcher (to keep this one simple)
return e.matchDomainConstraint(host, constraint)
}
// matchUsernameConstraint performs a string literal match against a constraint.
func matchUsernameConstraint(username, constraint string) (bool, error) {
// allow any plain principal username
if constraint == "*" {
return true, nil
}
return strings.EqualFold(username, constraint), nil
}

14
policy/engine_test.go
View file

@ -2223,16 +2223,16 @@ func TestNamePolicyEngine_X509_AllAllowed(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
engine, err := New(tt.options...)
assert.FatalError(t, err)
got, err := engine.AreCertificateNamesAllowed(tt.cert)
got, err := engine.IsX509CertificateAllowed(tt.cert)
if (err != nil) != tt.wantErr {
t.Errorf("NamePolicyEngine.AreCertificateNamesAllowed() error = %v, wantErr %v", err, tt.wantErr)
t.Errorf("NamePolicyEngine.IsX509CertificateAllowed() error = %v, wantErr %v", err, tt.wantErr)
return
}
if err != nil {
assert.NotEquals(t, "", err.Error()) // TODO(hs): implement a more specific error comparison?
}
if got != tt.want {
t.Errorf("NamePolicyEngine.AreCertificateNamesAllowed() = %v, want %v", got, tt.want)
t.Errorf("NamePolicyEngine.IsX509CertificateAllowed() = %v, want %v", got, tt.want)
}
// Perform the same tests for a CSR, which are similar to Certificates
@ -2243,7 +2243,7 @@ func TestNamePolicyEngine_X509_AllAllowed(t *testing.T) {
IPAddresses: tt.cert.IPAddresses,
URIs: tt.cert.URIs,
}
got, err = engine.AreCSRNamesAllowed(csr)
got, err = engine.IsX509CertificateRequestAllowed(csr)
if (err != nil) != tt.wantErr {
t.Errorf("NamePolicyEngine.AreCSRNamesAllowed() error = %v, wantErr %v", err, tt.wantErr)
return
@ -2705,13 +2705,13 @@ func TestNamePolicyEngine_SSH_ArePrincipalsAllowed(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
engine, err := New(tt.options...)
assert.FatalError(t, err)
got, err := engine.ArePrincipalsAllowed(tt.cert)
got, err := engine.IsSSHCertificateAllowed(tt.cert)
if (err != nil) != tt.wantErr {
t.Errorf("NamePolicyEngine.ArePrincipalsAllowed() error = %v, wantErr %v", err, tt.wantErr)
t.Errorf("NamePolicyEngine.IsSSHCertificateAllowed() error = %v, wantErr %v", err, tt.wantErr)
return
}
if got != tt.want {
t.Errorf("NamePolicyEngine.ArePrincipalsAllowed() = %v, want %v", got, tt.want)
t.Errorf("NamePolicyEngine.IsSSHCertificateAllowed() = %v, want %v", got, tt.want)
}
})
}

2
policy/ssh.go
View file

@ -5,5 +5,5 @@ import (
)
type SSHNamePolicyEngine interface {
ArePrincipalsAllowed(cert *ssh.Certificate) (bool, error)
IsSSHCertificateAllowed(cert *ssh.Certificate) (bool, error)
}

580
policy/validate.go Normal file
View file

@ -0,0 +1,580 @@
// Copyright 2011 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
package policy
import (
"bytes"
"fmt"
"net"
"net/url"
"reflect"
"strings"
"golang.org/x/net/idna"
)
// validateNames verifies that all names are allowed.
// 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
func (e *NamePolicyEngine) validateNames(dnsNames []string, ips []net.IP, emailAddresses []string, uris []*url.URL, principals []string) error {
// nothing to compare against; return early
if e.totalNumberOfConstraints == 0 {
return nil
}
// TODO: implement check that requires at least a single name in all of the SANs + subject?
// TODO: set limit on total of all names validated? In x509 there's a limit on the number of comparisons
// that protects the CA from a DoS (i.e. many heavy comparisons). The x509 implementation takes
// this number as a total of all checks and keeps a (pointer to a) counter of the number of checks
// executed so far.
// TODO: gather all errors, or return early? Currently we return early on the first wrong name; check might fail for multiple names.
// Perhaps make that an option?
for _, dns := range dnsNames {
// if there are DNS names to check, no DNS constraints set, but there are other permitted constraints,
// then return error, because DNS should be explicitly configured to be allowed in that case. In case there are
// (other) excluded constraints, we'll allow a DNS (implicit allow; currently).
if e.numberOfDNSDomainConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 {
return &NamePolicyError{
Reason: NotAuthorizedForThisName,
Detail: fmt.Sprintf("dns %q is not explicitly permitted by any constraint", dns),
}
}
didCutWildcard := false
if strings.HasPrefix(dns, "*.") {
dns = dns[1:]
didCutWildcard = true
}
parsedDNS, err := idna.Lookup.ToASCII(dns)
if err != nil {
return &NamePolicyError{
Reason: CannotParseDomain,
Detail: fmt.Sprintf("dns %q cannot be converted to ASCII", dns),
}
}
if didCutWildcard {
parsedDNS = "*" + parsedDNS
}
if _, ok := domainToReverseLabels(parsedDNS); !ok {
return &NamePolicyError{
Reason: CannotParseDomain,
Detail: fmt.Sprintf("cannot parse dns %q", dns),
}
}
if err := checkNameConstraints("dns", dns, parsedDNS,
func(parsedName, constraint interface{}) (bool, error) {
return e.matchDomainConstraint(parsedName.(string), constraint.(string))
}, e.permittedDNSDomains, e.excludedDNSDomains); err != nil {
return err
}
}
for _, ip := range ips {
if e.numberOfIPRangeConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 {
return &NamePolicyError{
Reason: NotAuthorizedForThisName,
Detail: fmt.Sprintf("ip %q is not explicitly permitted by any constraint", ip.String()),
}
}
if err := checkNameConstraints("ip", ip.String(), ip,
func(parsedName, constraint interface{}) (bool, error) {
return matchIPConstraint(parsedName.(net.IP), constraint.(*net.IPNet))
}, e.permittedIPRanges, e.excludedIPRanges); err != nil {
return err
}
}
for _, email := range emailAddresses {
if e.numberOfEmailAddressConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 {
return &NamePolicyError{
Reason: NotAuthorizedForThisName,
Detail: fmt.Sprintf("email %q is not explicitly permitted by any constraint", email),
}
}
mailbox, ok := parseRFC2821Mailbox(email)
if !ok {
return &NamePolicyError{
Reason: CannotParseRFC822Name,
Detail: fmt.Sprintf("invalid rfc822Name %q", mailbox),
}
}
// According to RFC 5280, section 7.5, emails are considered to match if the local part is
// an exact match and the host (domain) part matches the ASCII representation (case-insensitive):
// https://datatracker.ietf.org/doc/html/rfc5280#section-7.5
domainASCII, err := idna.ToASCII(mailbox.domain)
if err != nil {
return &NamePolicyError{
Reason: CannotParseDomain,
Detail: fmt.Sprintf("cannot parse email domain %q", email),
}
}
mailbox.domain = domainASCII
if err := checkNameConstraints("email", email, mailbox,
func(parsedName, constraint interface{}) (bool, error) {
return e.matchEmailConstraint(parsedName.(rfc2821Mailbox), constraint.(string))
}, e.permittedEmailAddresses, e.excludedEmailAddresses); err != nil {
return err
}
}
// TODO(hs): fix internationalization for URIs (IRIs)
for _, uri := range uris {
if e.numberOfURIDomainConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 {
return &NamePolicyError{
Reason: NotAuthorizedForThisName,
Detail: fmt.Sprintf("uri %q is not explicitly permitted by any constraint", uri.String()),
}
}
if err := checkNameConstraints("uri", uri.String(), uri,
func(parsedName, constraint interface{}) (bool, error) {
return e.matchURIConstraint(parsedName.(*url.URL), constraint.(string))
}, e.permittedURIDomains, e.excludedURIDomains); err != nil {
return err
}
}
for _, principal := range principals {
if e.numberOfPrincipalConstraints == 0 && e.totalNumberOfPermittedConstraints > 0 {
return &NamePolicyError{
Reason: NotAuthorizedForThisName,
Detail: fmt.Sprintf("username principal %q is not explicitly permitted by any constraint", principal),
}
}
// TODO: some validation? I.e. allowed characters?
if err := checkNameConstraints("principal", principal, principal,
func(parsedName, constraint interface{}) (bool, error) {
return matchUsernameConstraint(parsedName.(string), constraint.(string))
}, e.permittedPrincipals, e.excludedPrincipals); err != nil {
return err
}
}
// if all checks out, all SANs are allowed
return nil
}
// checkNameConstraints checks that a name, of type nameType is permitted.
// The argument parsedName contains the parsed form of name, suitable for passing
// to the match function.
// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go
func checkNameConstraints(
nameType string,
name string,
parsedName interface{},
match func(parsedName, constraint interface{}) (match bool, err error),
permitted, excluded interface{}) error {
excludedValue := reflect.ValueOf(excluded)
for i := 0; i < excludedValue.Len(); i++ {
constraint := excludedValue.Index(i).Interface()
match, err := match(parsedName, constraint)
if err != nil {
return &NamePolicyError{
Reason: CannotMatchNameToConstraint,
Detail: err.Error(),
}
}
if match {
return &NamePolicyError{
Reason: NotAuthorizedForThisName,
Detail: fmt.Sprintf("%s %q is excluded by constraint %q", nameType, name, constraint),
}
}
}
permittedValue := reflect.ValueOf(permitted)
ok := true
for i := 0; i < permittedValue.Len(); i++ {
constraint := permittedValue.Index(i).Interface()
var err error
if ok, err = match(parsedName, constraint); err != nil {
return &NamePolicyError{
Reason: CannotMatchNameToConstraint,
Detail: err.Error(),
}
}
if ok {
break
}
}
if !ok {
return &NamePolicyError{
Reason: NotAuthorizedForThisName,
Detail: fmt.Sprintf("%s %q is not permitted by any constraint", nameType, name),
}
}
return nil
}
// domainToReverseLabels converts a textual domain name like foo.example.com to
// the list of labels in reverse order, e.g. ["com", "example", "foo"].
// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go
func domainToReverseLabels(domain string) (reverseLabels []string, ok bool) {
for len(domain) > 0 {
if i := strings.LastIndexByte(domain, '.'); i == -1 {
reverseLabels = append(reverseLabels, domain)
domain = ""
} else {
reverseLabels = append(reverseLabels, domain[i+1:])
domain = domain[:i]
}
}
if len(reverseLabels) > 0 && reverseLabels[0] == "" {
// An empty label at the end indicates an absolute value.
return nil, false
}
for _, label := range reverseLabels {
if label == "" {
// Empty labels are otherwise invalid.
return nil, false
}
for _, c := range label {
if c < 33 || c > 126 {
// Invalid character.
return nil, false
}
}
}
return reverseLabels, true
}
// rfc2821Mailbox represents a “mailbox” (which is an email address to most
// people) by breaking it into the “local” (i.e. before the '@') and “domain”
// parts.
// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go
type rfc2821Mailbox struct {
local, domain string
}
// parseRFC2821Mailbox parses an email address into local and domain parts,
// based on the ABNF for a “Mailbox” from RFC 2821. According to RFC 5280,
// Section 4.2.1.6 that's correct for an rfc822Name from a certificate: “The
// format of an rfc822Name is a "Mailbox" as defined in RFC 2821, Section 4.1.2”.
// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go
func parseRFC2821Mailbox(in string) (mailbox rfc2821Mailbox, ok bool) {
if in == "" {
return mailbox, false
}
localPartBytes := make([]byte, 0, len(in)/2)
if in[0] == '"' {
// Quoted-string = DQUOTE *qcontent DQUOTE
// non-whitespace-control = %d1-8 / %d11 / %d12 / %d14-31 / %d127
// qcontent = qtext / quoted-pair
// qtext = non-whitespace-control /
// %d33 / %d35-91 / %d93-126
// quoted-pair = ("\" text) / obs-qp
// text = %d1-9 / %d11 / %d12 / %d14-127 / obs-text
//
// (Names beginning with “obs-” are the obsolete syntax from RFC 2822,
// Section 4. Since it has been 16 years, we no longer accept that.)
in = in[1:]
QuotedString:
for {
if in == "" {
return mailbox, false
}
c := in[0]
in = in[1:]
switch {
case c == '"':
break QuotedString
case c == '\\':
// quoted-pair
if in == "" {
return mailbox, false
}
if in[0] == 11 ||
in[0] == 12 ||
(1 <= in[0] && in[0] <= 9) ||
(14 <= in[0] && in[0] <= 127) {
localPartBytes = append(localPartBytes, in[0])
in = in[1:]
} else {
return mailbox, false
}
case c == 11 ||
c == 12 ||
// Space (char 32) is not allowed based on the
// BNF, but RFC 3696 gives an example that
// assumes that it is. Several “verified”
// errata continue to argue about this point.
// We choose to accept it.
c == 32 ||
c == 33 ||
c == 127 ||
(1 <= c && c <= 8) ||
(14 <= c && c <= 31) ||
(35 <= c && c <= 91) ||
(93 <= c && c <= 126):
// qtext
localPartBytes = append(localPartBytes, c)
default:
return mailbox, false
}
}
} else {
// Atom ("." Atom)*
NextChar:
for len(in) > 0 {
// atext from RFC 2822, Section 3.2.4
c := in[0]
switch {
case c == '\\':
// Examples given in RFC 3696 suggest that
// escaped characters can appear outside of a
// quoted string. Several “verified” errata
// continue to argue the point. We choose to
// accept it.
in = in[1:]
if in == "" {
return mailbox, false
}
fallthrough
case ('0' <= c && c <= '9') ||
('a' <= c && c <= 'z') ||
('A' <= c && c <= 'Z') ||
c == '!' || c == '#' || c == '$' || c == '%' ||
c == '&' || c == '\'' || c == '*' || c == '+' ||
c == '-' || c == '/' || c == '=' || c == '?' ||
c == '^' || c == '_' || c == '`' || c == '{' ||
c == '|' || c == '}' || c == '~' || c == '.':
localPartBytes = append(localPartBytes, in[0])
in = in[1:]
default:
break NextChar
}
}
if len(localPartBytes) == 0 {
return mailbox, false
}
// From RFC 3696, Section 3:
// “period (".") may also appear, but may not be used to start
// or end the local part, nor may two or more consecutive
// periods appear.”
twoDots := []byte{'.', '.'}
if localPartBytes[0] == '.' ||
localPartBytes[len(localPartBytes)-1] == '.' ||
bytes.Contains(localPartBytes, twoDots) {
return mailbox, false
}
}
if in == "" || in[0] != '@' {
return mailbox, false
}
in = in[1:]
// The RFC species a format for domains, but that's known to be
// violated in practice so we accept that anything after an '@' is the
// domain part.
if _, ok := domainToReverseLabels(in); !ok {
return mailbox, false
}
mailbox.local = string(localPartBytes)
mailbox.domain = in
return mailbox, true
}
// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go
func (e *NamePolicyEngine) matchDomainConstraint(domain, constraint string) (bool, error) {
// The meaning of zero length constraints is not specified, but this
// code follows NSS and accepts them as matching everything.
if constraint == "" {
return true, nil
}
// A single whitespace seems to be considered a valid domain, but we don't allow it.
if domain == " " {
return false, nil
}
// Block domains that start with just a period
if domain[0] == '.' {
return false, nil
}
// Block wildcard domains that don't start with exactly "*." (i.e. double wildcards and such)
if domain[0] == '*' && domain[1] != '.' {
return false, nil
}
// Check if the domain starts with a wildcard and return early if not allowed
if strings.HasPrefix(domain, "*.") && !e.allowLiteralWildcardNames {
return false, nil
}
// Only allow asterisk at the start of the domain; we don't allow them as part of a domain label or as a (sub)domain label (currently)
if strings.LastIndex(domain, "*") > 0 {
return false, nil
}
// Don't allow constraints with empty labels in any position
if strings.Contains(constraint, "..") {
return false, nil
}
domainLabels, ok := domainToReverseLabels(domain)
if !ok {
return false, fmt.Errorf("cannot parse domain %q", domain)
}
// RFC 5280 says that a leading period in a domain name means that at
// least one label must be prepended, but only for URI and email
// constraints, not DNS constraints. The code also supports that
// behavior for DNS constraints. In our adaptation of the original
// Go stdlib x509 Name Constraint implementation we look for exactly
// one subdomain, currently.
mustHaveSubdomains := false
if constraint[0] == '.' {
mustHaveSubdomains = true
constraint = constraint[1:]
}
constraintLabels, ok := domainToReverseLabels(constraint)
if !ok {
return false, fmt.Errorf("cannot parse domain constraint %q", constraint)
}
// fmt.Println(mustHaveSubdomains)
// fmt.Println(constraintLabels)
// fmt.Println(domainLabels)
expectedNumberOfLabels := len(constraintLabels)
if mustHaveSubdomains {
// we expect exactly one more label if it starts with the "canonical" x509 "wildcard": "."
// in the future we could extend this to support multiple additional labels and/or more
// complex matching.
expectedNumberOfLabels++
}
if len(domainLabels) != expectedNumberOfLabels {
return false, nil
}
for i, constraintLabel := range constraintLabels {
if !strings.EqualFold(constraintLabel, domainLabels[i]) {
return false, nil
}
}
return true, nil
}
// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go
func matchIPConstraint(ip net.IP, constraint *net.IPNet) (bool, error) {
// TODO(hs): this is code from Go library, but I got some unexpected result:
// with permitted net 127.0.0.0/24, 127.0.0.1 is NOT allowed. When parsing 127.0.0.1 as net.IP
// which is in the IPAddresses slice, the underlying length is 16. The contraint.IP has a length
// of 4 instead. I currently don't believe that this is a bug in Go now, but why is it like that?
// Is there a difference because we're not operating on a sans []string slice? Or is the Go
// implementation stricter regarding IPv4 vs. IPv6? I've been bitten by some unfortunate differences
// between the two before (i.e. IPv4 in IPv6; IP SANS in ACME)
// if len(ip) != len(constraint.IP) {
// return false, nil
// }
// for i := range ip {
// if mask := constraint.Mask[i]; ip[i]&mask != constraint.IP[i]&mask {
// return false, nil
// }
// }
contained := constraint.Contains(ip) // TODO(hs): validate that this is the correct behavior; also check IPv4-in-IPv6 (again)
return contained, nil
}
// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go
func (e *NamePolicyEngine) matchEmailConstraint(mailbox rfc2821Mailbox, constraint string) (bool, error) {
// TODO(hs): handle literal wildcard case for emails? Does that even make sense?
// If the constraint contains an @, then it specifies an exact mailbox name (currently)
if strings.Contains(constraint, "*") {
return false, fmt.Errorf("email constraint %q cannot contain asterisk", constraint)
}
if strings.Contains(constraint, "@") {
constraintMailbox, ok := parseRFC2821Mailbox(constraint)
if !ok {
return false, fmt.Errorf("cannot parse constraint %q", constraint)
}
return mailbox.local == constraintMailbox.local && strings.EqualFold(mailbox.domain, constraintMailbox.domain), nil
}
// Otherwise the constraint is like a DNS constraint of the domain part
// of the mailbox.
return e.matchDomainConstraint(mailbox.domain, constraint)
}
// SOURCE: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/crypto/x509/verify.go
func (e *NamePolicyEngine) matchURIConstraint(uri *url.URL, constraint string) (bool, error) {
// From RFC 5280, Section 4.2.1.10:
// “a uniformResourceIdentifier that does not include an authority
// component with a host name specified as a fully qualified domain
// name (e.g., if the URI either does not include an authority
// component or includes an authority component in which the host name
// is specified as an IP address), then the application MUST reject the
// certificate.”
host := uri.Host
if host == "" {
return false, fmt.Errorf("URI with empty host (%q) cannot be matched against constraints", uri.String())
}
// Block hosts with the wildcard character; no exceptions, also not when wildcards allowed.
if strings.Contains(host, "*") {
return false, fmt.Errorf("URI host %q cannot contain asterisk", uri.String())
}
if strings.Contains(host, ":") && !strings.HasSuffix(host, "]") {
var err error
host, _, err = net.SplitHostPort(uri.Host)
if err != nil {
return false, err
}
}
if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") ||
net.ParseIP(host) != nil {
return false, fmt.Errorf("URI with IP %q cannot be matched against constraints", uri.String())
}
// TODO(hs): add checks for scheme, path, etc.; either here, or in a different constraint matcher (to keep this one simple)
return e.matchDomainConstraint(host, constraint)
}
// matchUsernameConstraint performs a string literal match against a constraint.
func matchUsernameConstraint(username, constraint string) (bool, error) {
// allow any plain principal username
if constraint == "*" {
return true, nil
}
return strings.EqualFold(username, constraint), nil
}

4
policy/x509.go
View file

@ -6,8 +6,8 @@ import (
)
type X509NamePolicyEngine interface {
AreCertificateNamesAllowed(cert *x509.Certificate) (bool, error)
AreCSRNamesAllowed(csr *x509.CertificateRequest) (bool, error)
IsX509CertificateAllowed(cert *x509.Certificate) (bool, error)
IsX509CertificateRequestAllowed(csr *x509.CertificateRequest) (bool, error)
AreSANsAllowed(sans []string) (bool, error)
IsDNSAllowed(dns string) (bool, error)
IsIPAllowed(ip net.IP) (bool, error)