diff --git a/authority/authority.go b/authority/authority.go index 3bc88c0a..a51b2162 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -18,6 +18,7 @@ import ( casapi "github.com/smallstep/certificates/cas/apiv1" "github.com/smallstep/certificates/db" "github.com/smallstep/certificates/kms" + "github.com/smallstep/certificates/kms/apiv1" kmsapi "github.com/smallstep/certificates/kms/apiv1" "github.com/smallstep/certificates/kms/sshagentkms" "github.com/smallstep/certificates/templates" @@ -201,13 +202,14 @@ func (a *Authority) init() error { } // TODO: decide if this is a good approach for providing the SCEP functionality + // It currently mirrors the logic for the x509CAServer if a.scepService == nil { var options casapi.Options if a.config.AuthorityConfig.Options != nil { options = *a.config.AuthorityConfig.Options } - // Read intermediate and create X509 signer for default CAS. + // Read intermediate and create X509 signer and decrypter for default CAS. if options.Is(casapi.SoftCAS) { options.CertificateChain, err = pemutil.ReadCertificateBundle(a.config.IntermediateCert) if err != nil { @@ -220,12 +222,15 @@ func (a *Authority) init() error { if err != nil { return err } - options.Decrypter, err = a.keyManager.CreateDecrypter(&kmsapi.CreateDecrypterRequest{ - DecryptionKey: a.config.IntermediateKey, - Password: []byte(a.config.Password), - }) - if err != nil { - return err + + if km, ok := a.keyManager.(apiv1.Decrypter); ok { + options.Decrypter, err = km.CreateDecrypter(&kmsapi.CreateDecrypterRequest{ + DecryptionKey: a.config.IntermediateKey, + Password: []byte(a.config.Password), + }) + if err != nil { + return err + } } } @@ -368,11 +373,6 @@ func (a *Authority) init() error { audiences := a.config.getAudiences() a.provisioners = provisioner.NewCollection(audiences) config := provisioner.Config{ - // TODO: I'm not sure if extending this configuration is a good way to integrate - // It's powerful, but leaks quite some seemingly internal stuff to the provisioner. - // IntermediateCert: a.config.IntermediateCert, - // SigningKey: a.config.IntermediateKey, - // CACertificates: a.rootX509Certs, Claims: claimer.Claims(), Audiences: audiences, DB: a.db, @@ -382,6 +382,14 @@ func (a *Authority) init() error { }, GetIdentityFunc: a.getIdentityFunc, } + + // Check if a KMS with decryption capability is required and available + if a.requiresDecrypter() { + if _, ok := a.keyManager.(apiv1.Decrypter); !ok { + return errors.New("keymanager doesn't provide crypto.Decrypter") + } + } + // Store all the provisioners for _, p := range a.config.AuthorityConfig.Provisioners { if err := p.Init(config); err != nil { @@ -434,6 +442,20 @@ func (a *Authority) CloseForReload() { } } +// requiresDecrypter iterates over the configured provisioners +// and determines if the Authority requires a KMS that provides +// a crypto.Decrypter by implementing the apiv1.Decrypter +// interface. Currently only the SCEP provider requires this, +// but others may be added in the future. +func (a *Authority) requiresDecrypter() bool { + for _, p := range a.config.AuthorityConfig.Provisioners { + if p.GetType() == provisioner.TypeSCEP { + return true + } + } + return false +} + // GetSCEPService returns the configured SCEP Service // TODO: this function is intended to exist temporarily // in order to make SCEP work more easily. It can be diff --git a/authority/config.go b/authority/config.go index 9d79ce9a..3151578d 100644 --- a/authority/config.go +++ b/authority/config.go @@ -53,6 +53,7 @@ type Config struct { IntermediateCert string `json:"crt"` IntermediateKey string `json:"key"` Address string `json:"address"` + InsecureAddress string `json:"insecureAddress"` DNSNames []string `json:"dnsNames"` KMS *kms.Options `json:"kms,omitempty"` SSH *SSHConfig `json:"ssh,omitempty"` @@ -207,6 +208,13 @@ func (c *Config) Validate() error { return errors.Errorf("invalid address %s", c.Address) } + // Validate insecure address if it is configured + if c.InsecureAddress != "" { + if _, _, err := net.SplitHostPort(c.InsecureAddress); err != nil { + return errors.Errorf("invalid address %s", c.InsecureAddress) + } + } + if c.TLS == nil { c.TLS = &DefaultTLSOptions } else { diff --git a/ca/ca.go b/ca/ca.go index edcc9bba..c6af8a28 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -65,11 +65,12 @@ func WithDatabase(db db.AuthDB) Option { // CA is the type used to build the complete certificate authority. It builds // the HTTP server, set ups the middlewares and the HTTP handlers. type CA struct { - auth *authority.Authority - config *authority.Config - srv *server.Server - opts *options - renewer *TLSRenewer + auth *authority.Authority + config *authority.Config + srv *server.Server + insecureSrv *server.Server + opts *options + renewer *TLSRenewer } // New creates and initializes the CA with the given configuration and options. @@ -107,6 +108,9 @@ func (ca *CA) Init(config *authority.Config) (*CA, error) { mux := chi.NewRouter() handler := http.Handler(mux) + insecureMux := chi.NewRouter() + insecureHandler := http.Handler(insecureMux) + // Add regular CA api endpoints in / and /1.0 routerHandler := api.New(auth) routerHandler.Route(mux) @@ -145,17 +149,6 @@ func (ca *CA) Init(config *authority.Config) (*CA, error) { acmeRouterHandler.Route(r) }) - // TODO: THIS SHOULDN'T HAPPEN (or should become configurable) - // Current SCEP client I'm testing with doesn't seem to easily trust untrusted certs. - // Idea: provide a second mux/handler that runs without TLS. It probably should only - // have routes that are intended to be ran without TLS, like the SCEP ones. Look into - // option to not enable it in case no SCEP providers are configured. It might - // be nice to still include the SCEP routes in the secure handler too, for - // client that do understand HTTPS. The RFC does not seem to explicitly exclude HTTPS - // usage, but it mentions some caveats related to managing web PKI certificates as - // well as certificates via SCEP. - tlsConfig = nil - scepPrefix := "scep" scepAuthority, err := scep.New(auth, scep.AuthorityOptions{ IntermediateCertificatePath: config.IntermediateCert, @@ -173,6 +166,16 @@ func (ca *CA) Init(config *authority.Config) (*CA, error) { scepRouterHandler.Route(r) }) + // According to the RFC (https://tools.ietf.org/html/rfc8894#section-7.10), + // SCEP operations are performed using HTTP, so that's why the API is mounted + // to the insecure mux. To my current understanding there's no strong reason + // to not use HTTPS also, so that's why I've kept the API endpoints in both + // muxes and both HTTP as well as HTTPS can be used to request certificates + // using SCEP. + insecureMux.Route("/"+scepPrefix, func(r chi.Router) { + scepRouterHandler.Route(r) + }) + // helpful routine for logging all routes //dumpRoutes(mux) @@ -183,6 +186,7 @@ func (ca *CA) Init(config *authority.Config) (*CA, error) { return nil, err } handler = m.Middleware(handler) + insecureHandler = m.Middleware(insecureHandler) } // Add logger if configured @@ -192,16 +196,37 @@ func (ca *CA) Init(config *authority.Config) (*CA, error) { return nil, err } handler = logger.Middleware(handler) + insecureHandler = logger.Middleware(insecureHandler) } ca.auth = auth ca.srv = server.New(config.Address, handler, tlsConfig) + + // TODO: instead opt for having a single server.Server but two http.Servers + // handling the HTTP vs. HTTPS handler? + if config.InsecureAddress != "" { + ca.insecureSrv = server.New(config.InsecureAddress, insecureHandler, nil) + } + return ca, nil } // Run starts the CA calling to the server ListenAndServe method. func (ca *CA) Run() error { - return ca.srv.ListenAndServe() + + errors := make(chan error, 1) + go func() { + if ca.insecureSrv != nil { + errors <- ca.insecureSrv.ListenAndServe() + } + }() + go func() { + errors <- ca.srv.ListenAndServe() + }() + + // wait till error occurs; ensures the servers keep listening + err := <-errors + return err } // Stop stops the CA calling to the server Shutdown method. @@ -210,7 +235,17 @@ func (ca *CA) Stop() error { if err := ca.auth.Shutdown(); err != nil { log.Printf("error stopping ca.Authority: %+v\n", err) } - return ca.srv.Shutdown() + var insecureShutdownErr error + if ca.insecureSrv != nil { + insecureShutdownErr = ca.insecureSrv.Shutdown() + } + + secureErr := ca.srv.Shutdown() + + if insecureShutdownErr != nil { + return insecureShutdownErr + } + return secureErr } // Reload reloads the configuration of the CA and calls to the server Reload @@ -243,6 +278,13 @@ func (ca *CA) Reload() error { return errors.Wrap(err, "error reloading ca") } + if ca.insecureSrv != nil { + if err = ca.insecureSrv.Reload(newCA.insecureSrv); err != nil { + logContinue("Reload failed because insecure server could not be replaced.") + return errors.Wrap(err, "error reloading insecure server") + } + } + if err = ca.srv.Reload(newCA.srv); err != nil { logContinue("Reload failed because server could not be replaced.") return errors.Wrap(err, "error reloading server") diff --git a/kms/apiv1/options.go b/kms/apiv1/options.go index 0e6f32df..1574a426 100644 --- a/kms/apiv1/options.go +++ b/kms/apiv1/options.go @@ -13,10 +13,13 @@ type KeyManager interface { GetPublicKey(req *GetPublicKeyRequest) (crypto.PublicKey, error) CreateKey(req *CreateKeyRequest) (*CreateKeyResponse, error) CreateSigner(req *CreateSignerRequest) (crypto.Signer, error) - CreateDecrypter(req *CreateDecrypterRequest) (crypto.Decrypter, error) // TODO: split into separate interface? Close() error } +type Decrypter interface { + CreateDecrypter(req *CreateDecrypterRequest) (crypto.Decrypter, error) +} + // CertificateManager is the interface implemented by the KMS that can load and // store x509.Certificates. type CertificateManager interface { diff --git a/kms/awskms/awskms.go b/kms/awskms/awskms.go index 00d7d0b3..da392989 100644 --- a/kms/awskms/awskms.go +++ b/kms/awskms/awskms.go @@ -3,7 +3,6 @@ package awskms import ( "context" "crypto" - "fmt" "net/url" "strings" "time" @@ -222,11 +221,6 @@ func (k *KMS) CreateSigner(req *apiv1.CreateSignerRequest) (crypto.Signer, error return NewSigner(k.service, req.SigningKey) } -// CreateDecrypter creates a new crypto.decrypter backed by AWS KMS -func (k *KMS) CreateDecrypter(req *apiv1.CreateDecrypterRequest) (crypto.Decrypter, error) { - return nil, fmt.Errorf("not implemented yet") -} - // Close closes the connection of the KMS client. func (k *KMS) Close() error { return nil diff --git a/kms/cloudkms/cloudkms.go b/kms/cloudkms/cloudkms.go index ace12a1a..cfbf8235 100644 --- a/kms/cloudkms/cloudkms.go +++ b/kms/cloudkms/cloudkms.go @@ -3,7 +3,6 @@ package cloudkms import ( "context" "crypto" - "fmt" "log" "strings" "time" @@ -285,11 +284,6 @@ func (k *CloudKMS) GetPublicKey(req *apiv1.GetPublicKeyRequest) (crypto.PublicKe return pk, nil } -// CreateDecrypter creates a new crypto.Decrypter backed by Google Cloud KMS -func (k *CloudKMS) CreateDecrypter(req *apiv1.CreateDecrypterRequest) (crypto.Decrypter, error) { - return nil, fmt.Errorf("not implemented yet") -} - // getPublicKeyWithRetries retries the request if the error is // FailedPrecondition, caused because the key is in the PENDING_GENERATION // status. diff --git a/kms/pkcs11/pkcs11.go b/kms/pkcs11/pkcs11.go index 8c1497e8..47c298a5 100644 --- a/kms/pkcs11/pkcs11.go +++ b/kms/pkcs11/pkcs11.go @@ -352,8 +352,3 @@ func findCertificate(ctx P11, rawuri string) (*x509.Certificate, error) { } return cert, nil } - -// CreateDecrypter creates a new crypto.Decrypter backed by PKCS11 -func (k *PKCS11) CreateDecrypter(req *apiv1.CreateDecrypterRequest) (crypto.Decrypter, error) { - return nil, fmt.Errorf("not implemented yet") -} diff --git a/kms/sshagentkms/sshagentkms.go b/kms/sshagentkms/sshagentkms.go index 9c8a7866..b3627a08 100644 --- a/kms/sshagentkms/sshagentkms.go +++ b/kms/sshagentkms/sshagentkms.go @@ -7,7 +7,6 @@ import ( "crypto/ed25519" "crypto/rsa" "crypto/x509" - "fmt" "io" "net" "os" @@ -205,8 +204,3 @@ func (k *SSHAgentKMS) GetPublicKey(req *apiv1.GetPublicKeyRequest) (crypto.Publi return nil, errors.Errorf("unsupported public key type %T", v) } } - -// CreateDecrypter creates a crypto.Decrypter backed by ssh-agent -func (k *SSHAgentKMS) CreateDecrypter(req *apiv1.CreateDecrypterRequest) (crypto.Decrypter, error) { - return nil, fmt.Errorf("not implemented yet") -} diff --git a/kms/yubikey/yubikey.go b/kms/yubikey/yubikey.go index 45ee3d87..19cef55e 100644 --- a/kms/yubikey/yubikey.go +++ b/kms/yubikey/yubikey.go @@ -7,7 +7,6 @@ import ( "crypto" "crypto/x509" "encoding/hex" - "fmt" "net/url" "strings" @@ -190,11 +189,6 @@ func (k *YubiKey) CreateSigner(req *apiv1.CreateSignerRequest) (crypto.Signer, e return signer, nil } -// CreateDecrypter creates a new crypto.Decrypter backed by a YubiKey -func (k *YubiKey) CreateDecrypter(req *apiv1.CreateDecrypterRequest) (crypto.Decrypter, error) { - return nil, fmt.Errorf("not implemented yet") -} - // Close releases the connection to the YubiKey. func (k *YubiKey) Close() error { return errors.Wrap(k.yk.Close(), "error closing yubikey")