From 9c6580ccd2365c3681d35af9e30e79d077a8676c Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Fri, 14 Jan 2022 10:48:23 +0100 Subject: [PATCH 1/6] Fix macOS SCEP client issues Fixes #746 --- authority/authority.go | 14 +++++----- authority/provisioner/scep.go | 38 ++++++++++++++++++++++++--- ca/ca.go | 22 ++++++++++++---- scep/api/api.go | 19 ++++++++++---- scep/authority.go | 48 +++++++++++++++++++++++------------ scep/common.go | 4 +-- scep/provisioner.go | 2 ++ 7 files changed, 108 insertions(+), 39 deletions(-) diff --git a/authority/authority.go b/authority/authority.go index b6fcdf23..42e21149 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -437,13 +437,6 @@ func (a *Authority) init() error { } } - // Check if a KMS with decryption capability is required and available - if a.requiresDecrypter() { - if _, ok := a.keyManager.(kmsapi.Decrypter); !ok { - return errors.New("keymanager doesn't provide crypto.Decrypter") - } - } - // TODO: decide if this is a good approach for providing the SCEP functionality // It currently mirrors the logic for the x509CAService if a.requiresSCEPService() && a.scepService == nil { @@ -454,6 +447,7 @@ func (a *Authority) init() error { if err != nil { return err } + options.CertificateChain = append(options.CertificateChain, a.rootX509Certs...) options.Signer, err = a.keyManager.CreateSigner(&kmsapi.CreateSignerRequest{ SigningKey: a.config.IntermediateKey, Password: []byte(a.password), @@ -601,6 +595,12 @@ func (a *Authority) IsRevoked(sn string) (bool, error) { return a.db.IsRevoked(sn) } +// GetIntermediateCertificate returns the x509 CA intermediate +// certificate. +func (a *Authority) GetIntermediateCertificate() (*x509.Certificate, error) { + return pemutil.ReadCertificate(a.config.IntermediateCert) +} + // requiresDecrypter returns whether the Authority // requires a KMS that provides a crypto.Decrypter // Currently this is only required when SCEP is diff --git a/authority/provisioner/scep.go b/authority/provisioner/scep.go index 145a1920..0531bd10 100644 --- a/authority/provisioner/scep.go +++ b/authority/provisioner/scep.go @@ -18,13 +18,20 @@ type SCEP struct { ForceCN bool `json:"forceCN,omitempty"` ChallengePassword string `json:"challenge,omitempty"` Capabilities []string `json:"capabilities,omitempty"` + // IncludeRoots makes the provisioner return the CA root(s) in the GetCACerts response + IncludeRoots bool `json:"includeRoots,omitempty"` // MinimumPublicKeyLength is the minimum length for public keys in CSRs - MinimumPublicKeyLength int `json:"minimumPublicKeyLength,omitempty"` - Options *Options `json:"options,omitempty"` - Claims *Claims `json:"claims,omitempty"` - claimer *Claimer + MinimumPublicKeyLength int `json:"minimumPublicKeyLength,omitempty"` + // Numerical identifier for the ContentEncryptionAlgorithm as defined in github.com/mozilla-services/pkcs7 + // at https://github.com/mozilla-services/pkcs7/blob/33d05740a3526e382af6395d3513e73d4e66d1cb/encrypt.go#L63 + // Defaults to 2, being AES-256-CBC + EncryptionAlgorithmIdentifier *int `json:"encryptionAlgorithmIdentifier,omitempty"` + Options *Options `json:"options,omitempty"` + Claims *Claims `json:"claims,omitempty"` + claimer *Claimer secretChallengePassword string + encryptionAlgorithm int } // GetID returns the provisioner unique identifier. @@ -100,6 +107,15 @@ func (s *SCEP) Init(config Config) (err error) { return errors.Errorf("only minimum public keys exactly divisible by 8 are supported; %d is not exactly divisible by 8", s.MinimumPublicKeyLength) } + s.encryptionAlgorithm = 2 // default to AES-256-CBC + if s.EncryptionAlgorithmIdentifier != nil { + value := *s.EncryptionAlgorithmIdentifier + if value < 0 || value > 4 { + return errors.Errorf("only encryption algorithm identifiers from 0 to 4 are valid") + } + s.encryptionAlgorithm = value + } + // TODO: add other, SCEP specific, options? return err @@ -129,3 +145,17 @@ func (s *SCEP) GetChallengePassword() string { func (s *SCEP) GetCapabilities() []string { return s.Capabilities } + +// ShouldIncludeRootsInChain indicates if the CA should +// return its intermediate, which is currently used for +// both signing and decryption, as well as the other certs +// in its chain (usually a single root certificate). +func (s *SCEP) ShouldIncludeRootsInChain() bool { + return s.IncludeRoots +} + +// GetContentEncryptionAlgorithm returns the numeric identifier +// for the pkcs7 package encryption algorithm to use. +func (s *SCEP) GetContentEncryptionAlgorithm() int { + return s.encryptionAlgorithm +} diff --git a/ca/ca.go b/ca/ca.go index da0fb874..a07ae406 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -417,11 +417,6 @@ func (ca *CA) getTLSConfig(auth *authority.Authority) (*tls.Config, error) { } } - certPool := x509.NewCertPool() - for _, crt := range auth.GetRootCertificates() { - certPool.AddCert(crt) - } - // GetCertificate will only be called if the client supplies SNI // information or if tlsConfig.Certificates is empty. // When client requests are made using an IP address (as opposed to a domain @@ -432,6 +427,23 @@ func (ca *CA) getTLSConfig(auth *authority.Authority) (*tls.Config, error) { tlsConfig.Certificates = []tls.Certificate{} tlsConfig.GetCertificate = ca.renewer.GetCertificateForCA + certPool := x509.NewCertPool() + for _, crt := range auth.GetRootCertificates() { + certPool.AddCert(crt) + } + + // adding the intermediate CA to the pool will allow clients that + // fail to send the intermediate for chain building to connect to the CA + // successfully. + shouldAddIntermediateToClientCAPool := true // TODO(hs): make this into a configuration + if shouldAddIntermediateToClientCAPool { + cert, err := auth.GetIntermediateCertificate() + if err != nil { + return nil, err + } + certPool.AddCert(cert) + } + // Add support for mutual tls to renew certificates tlsConfig.ClientAuth = tls.VerifyClientCertIfGiven tlsConfig.ClientCAs = certPool diff --git a/scep/api/api.go b/scep/api/api.go index 0c8c469b..d1ade2aa 100644 --- a/scep/api/api.go +++ b/scep/api/api.go @@ -212,7 +212,7 @@ func (h *Handler) lookupProvisioner(next nextHTTP) nextHTTP { // GetCACert returns the CA certificates in a SCEP response func (h *Handler) GetCACert(ctx context.Context) (SCEPResponse, error) { - certs, err := h.Auth.GetCACertificates() + certs, err := h.Auth.GetCACertificates(ctx) if err != nil { return SCEPResponse{}, err } @@ -289,20 +289,29 @@ func (h *Handler) PKIOperation(ctx context.Context, request SCEPRequest) (SCEPRe // NOTE: at this point we have sufficient information for returning nicely signed CertReps csr := msg.CSRReqMessage.CSR - if msg.MessageType == microscep.PKCSReq { - + // NOTE: we're blocking the RenewalReq if the challenge does not match, because otherwise we don't have any authentication. + // The macOS SCEP client performs renewals using PKCSreq. The CertNanny SCEP client will use PKCSreq with challenge too, it seems, + // even if using the renewal flow as described in the README.md. MicroMDM SCEP client also only does PKCSreq by default, unless + // a certificate exists; then it will use RenewalReq. Adding the challenge check here may be a small breaking change for clients. + // We'll have to see how it works out. + if msg.MessageType == microscep.PKCSReq || msg.MessageType == microscep.RenewalReq { challengeMatches, err := h.Auth.MatchChallengePassword(ctx, msg.CSRReqMessage.ChallengePassword) if err != nil { return h.createFailureResponse(ctx, csr, msg, microscep.BadRequest, errors.New("error when checking password")) } - if !challengeMatches { // TODO: can this be returned safely to the client? In the end, if the password was correct, that gains a bit of info too. return h.createFailureResponse(ctx, csr, msg, microscep.BadRequest, errors.New("wrong password provided")) } } - // TODO: check if CN already exists, if renewal is allowed and if existing should be revoked; fail if not + // TODO: authorize renewal: we can authorize renewals with the challenge password (if reusable secrets are used). + // Renewals OPTIONALLY include the challenge if the existing cert is used as authentication, but client SHOULD omit the challenge. + // This means that for renewal requests we should check the certificate provided to be signed before by the CA. We could + // enforce use of the challenge if we want too. That way we could be more flexible in terms of authentication scheme (i.e. reusing + // tokens from other provisioners, calling a webhook, storing multiple secrets, allowing them to be multi-use, etc). + // Authentication by the (self-signed) certificate with an optional challenge is required; supporting renewals incl. verification + // of the client cert is not. certRep, err := h.Auth.SignCSR(ctx, csr, msg) if err != nil { diff --git a/scep/authority.go b/scep/authority.go index 47d1d41c..04e77160 100644 --- a/scep/authority.go +++ b/scep/authority.go @@ -23,7 +23,7 @@ type Interface interface { LoadProvisionerByID(string) (provisioner.Interface, error) GetLinkExplicit(provName string, absoluteLink bool, baseURL *url.URL, inputs ...string) string - GetCACertificates() ([]*x509.Certificate, error) + GetCACertificates(ctx context.Context) ([]*x509.Certificate, error) DecryptPKIEnvelope(ctx context.Context, msg *PKIMessage) error SignCSR(ctx context.Context, csr *x509.CertificateRequest, msg *PKIMessage) (*PKIMessage, error) CreateFailureResponse(ctx context.Context, csr *x509.CertificateRequest, msg *PKIMessage, info FailInfoName, infoText string) (*PKIMessage, error) @@ -36,6 +36,7 @@ type Authority struct { prefix string dns string intermediateCertificate *x509.Certificate + caCerts []*x509.Certificate // TODO(hs): change to use these instead of root and intermediate service *Service signAuth SignAuthority } @@ -72,6 +73,8 @@ func New(signAuth SignAuthority, ops AuthorityOptions) (*Authority, error) { // in its entirety to make this more interoperable with the rest of // step-ca, I think. if ops.Service != nil { + authority.caCerts = ops.Service.certificateChain + // TODO(hs): look into refactoring SCEP into using just caCerts everywhere, if it makes sense for more elaborate SCEP configuration. Keeping it like this for clarity (for now). authority.intermediateCertificate = ops.Service.certificateChain[0] authority.service = ops.Service } @@ -82,7 +85,7 @@ func New(signAuth SignAuthority, ops AuthorityOptions) (*Authority, error) { var ( // TODO: check the default capabilities; https://tools.ietf.org/html/rfc8894#section-3.5.2 defaultCapabilities = []string{ - "Renewal", + "Renewal", // NOTE: removing this will result in macOS SCEP client stating the server doesn't support renewal, but it uses PKCSreq to do so. "SHA-1", "SHA-256", "AES", @@ -100,18 +103,13 @@ func (a *Authority) LoadProvisionerByID(id string) (provisioner.Interface, error // GetLinkExplicit returns the requested link from the directory. func (a *Authority) GetLinkExplicit(provName string, abs bool, baseURL *url.URL, inputs ...string) string { - // TODO: taken from ACME; move it to directory (if we need a directory in SCEP)? return a.getLinkExplicit(provName, abs, baseURL, inputs...) } // getLinkExplicit returns an absolute or partial path to the given resource and a base // URL dynamically obtained from the request for which the link is being calculated. func (a *Authority) getLinkExplicit(provisionerName string, abs bool, baseURL *url.URL, inputs ...string) string { - - // TODO: do we need to provide a way to provide a different suffix? - // Like "/cgi-bin/pkiclient.exe"? Or would it be enough to have that as the name? link := "/" + provisionerName - if abs { // Copy the baseURL value from the pointer. https://github.com/golang/go/issues/38351 u := url.URL{} @@ -137,7 +135,7 @@ func (a *Authority) getLinkExplicit(provisionerName string, abs bool, baseURL *u } // GetCACertificates returns the certificate (chain) for the CA -func (a *Authority) GetCACertificates() ([]*x509.Certificate, error) { +func (a *Authority) GetCACertificates(ctx context.Context) ([]*x509.Certificate, error) { // TODO: this should return: the "SCEP Server (RA)" certificate, the issuing CA up to and excl. the root // Some clients do need the root certificate however; also see: https://github.com/openxpki/openxpki/issues/73 @@ -153,14 +151,27 @@ func (a *Authority) GetCACertificates() ([]*x509.Certificate, error) { // Using an RA does not seem to exist in https://tools.ietf.org/html/rfc8894, but is mentioned in // https://tools.ietf.org/id/draft-nourse-scep-21.html. Will continue using the CA directly for now. // - // The certificate to use should probably depend on the (configured) Provisioner and may + // The certificate to use should probably depend on the (configured) provisioner and may // use a distinct certificate, apart from the intermediate. - if a.intermediateCertificate == nil { + p, err := provisionerFromContext(ctx) + if err != nil { + return nil, err + } + + if len(a.caCerts) == 0 { return nil, errors.New("no intermediate certificate available in SCEP authority") } - return []*x509.Certificate{a.intermediateCertificate}, nil + certs := []*x509.Certificate{} + certs = append(certs, a.caCerts[0]) + + // TODO(hs): we're adding the roots here, but they may be something different than what the RFC means. Clients are responsible to select the right cert(s) to use, though. + if p.ShouldIncludeRootsInChain() && len(a.caCerts) >= 2 { + certs = append(certs, a.caCerts[1:]...) + } + + return certs, nil } // DecryptPKIEnvelope decrypts an enveloped message @@ -211,8 +222,6 @@ func (a *Authority) DecryptPKIEnvelope(ctx context.Context, msg *PKIMessage) err // SignCSR creates an x509.Certificate based on a CSR template and Cert Authority credentials // returns a new PKIMessage with CertRep data -//func (msg *PKIMessage) SignCSR(crtAuth *x509.Certificate, keyAuth *rsa.PrivateKey, template *x509.Certificate) (*PKIMessage, error) { -//func (a *Authority) SignCSR(ctx context.Context, msg *PKIMessage, template *x509.Certificate) (*PKIMessage, error) { func (a *Authority) SignCSR(ctx context.Context, csr *x509.CertificateRequest, msg *PKIMessage) (*PKIMessage, error) { // TODO: intermediate storage of the request? In SCEP it's possible to request a csr/certificate @@ -220,7 +229,7 @@ func (a *Authority) SignCSR(ctx context.Context, csr *x509.CertificateRequest, m // poll for the status. It seems to be similar as what can happen in ACME, so might want to model // the implementation after the one in the ACME authority. Requires storage, etc. - p, err := ProvisionerFromContext(ctx) + p, err := provisionerFromContext(ctx) if err != nil { return nil, err } @@ -292,10 +301,17 @@ func (a *Authority) SignCSR(ctx context.Context, csr *x509.CertificateRequest, m return nil, err } + // apparently the pkcs7 library uses a global default setting for the content encryption + // algorithm to use when en- or decrypting data. We need to restore the current setting after + // the cryptographic operation, so that other usages of the library are not influenced by + // this call to Encrypt(). + encryptionAlgorithmToRestore := pkcs7.ContentEncryptionAlgorithm + pkcs7.ContentEncryptionAlgorithm = p.GetContentEncryptionAlgorithm() e7, err := pkcs7.Encrypt(deg, msg.P7.Certificates) if err != nil { return nil, err } + pkcs7.ContentEncryptionAlgorithm = encryptionAlgorithmToRestore // PKIMessageAttributes to be signed config := pkcs7.SignerInfoConfig{ @@ -434,7 +450,7 @@ func (a *Authority) CreateFailureResponse(ctx context.Context, csr *x509.Certifi // MatchChallengePassword verifies a SCEP challenge password func (a *Authority) MatchChallengePassword(ctx context.Context, password string) (bool, error) { - p, err := ProvisionerFromContext(ctx) + p, err := provisionerFromContext(ctx) if err != nil { return false, err } @@ -453,7 +469,7 @@ func (a *Authority) MatchChallengePassword(ctx context.Context, password string) // GetCACaps returns the CA capabilities func (a *Authority) GetCACaps(ctx context.Context) []string { - p, err := ProvisionerFromContext(ctx) + p, err := provisionerFromContext(ctx) if err != nil { return defaultCapabilities } diff --git a/scep/common.go b/scep/common.go index ca87841f..73b16ed4 100644 --- a/scep/common.go +++ b/scep/common.go @@ -14,9 +14,9 @@ const ( ProvisionerContextKey = ContextKey("provisioner") ) -// ProvisionerFromContext searches the context for a SCEP provisioner. +// provisionerFromContext searches the context for a SCEP provisioner. // Returns the provisioner or an error. -func ProvisionerFromContext(ctx context.Context) (Provisioner, error) { +func provisionerFromContext(ctx context.Context) (Provisioner, error) { val := ctx.Value(ProvisionerContextKey) if val == nil { return nil, errors.New("provisioner expected in request context") diff --git a/scep/provisioner.go b/scep/provisioner.go index e1b7f8b1..75820f56 100644 --- a/scep/provisioner.go +++ b/scep/provisioner.go @@ -16,4 +16,6 @@ type Provisioner interface { GetOptions() *provisioner.Options GetChallengePassword() string GetCapabilities() []string + ShouldIncludeRootsInChain() bool + GetContentEncryptionAlgorithm() int } From 3612eefc318587ca0d585a173f4c48b3a39f02b2 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Tue, 18 Jan 2022 15:54:18 +0100 Subject: [PATCH 2/6] Cleanup --- authority/authority.go | 2 +- authority/authority_test.go | 52 +---------------------------------- authority/provisioner/scep.go | 16 ++++------- ca/ca.go | 16 ++++------- scep/authority.go | 7 +++-- 5 files changed, 18 insertions(+), 75 deletions(-) diff --git a/authority/authority.go b/authority/authority.go index 42e21149..da0a6f6f 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -595,7 +595,7 @@ func (a *Authority) IsRevoked(sn string) (bool, error) { return a.db.IsRevoked(sn) } -// GetIntermediateCertificate returns the x509 CA intermediate +// GetIntermediateCertificate returns the x509 intermediate CA // certificate. func (a *Authority) GetIntermediateCertificate() (*x509.Certificate, error) { return pemutil.ReadCertificate(a.config.IntermediateCert) diff --git a/authority/authority_test.go b/authority/authority_test.go index abb06cf4..1f63333d 100644 --- a/authority/authority_test.go +++ b/authority/authority_test.go @@ -6,7 +6,6 @@ import ( "crypto/sha256" "crypto/x509" "encoding/hex" - "fmt" "net" "os" "reflect" @@ -328,7 +327,6 @@ func TestAuthority_CloseForReload(t *testing.T) { } func testScepAuthority(t *testing.T, opts ...Option) *Authority { - p := provisioner.List{ &provisioner.SCEP{ Name: "scep1", @@ -353,39 +351,15 @@ func testScepAuthority(t *testing.T, opts ...Option) *Authority { } func TestAuthority_GetSCEPService(t *testing.T) { - auth := testScepAuthority(t) - fmt.Println(auth) - + _ = testScepAuthority(t) p := provisioner.List{ &provisioner.SCEP{ Name: "scep1", Type: "SCEP", }, } - type fields struct { config *Config - // keyManager kms.KeyManager - // provisioners *provisioner.Collection - // db db.AuthDB - // templates *templates.Templates - // x509CAService cas.CertificateAuthorityService - // rootX509Certs []*x509.Certificate - // federatedX509Certs []*x509.Certificate - // certificates *sync.Map - // scepService *scep.Service - // sshCAUserCertSignKey ssh.Signer - // sshCAHostCertSignKey ssh.Signer - // sshCAUserCerts []ssh.PublicKey - // sshCAHostCerts []ssh.PublicKey - // sshCAUserFederatedCerts []ssh.PublicKey - // sshCAHostFederatedCerts []ssh.PublicKey - // initOnce bool - // startTime time.Time - // sshBastionFunc func(ctx context.Context, user, hostname string) (*Bastion, error) - // sshCheckHostFunc func(ctx context.Context, principal string, tok string, roots []*x509.Certificate) (bool, error) - // sshGetHostsFunc func(ctx context.Context, cert *x509.Certificate) ([]Host, error) - // getIdentityFunc provisioner.GetIdentityFunc } tests := []struct { name string @@ -434,30 +408,6 @@ func TestAuthority_GetSCEPService(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // a := &Authority{ - // config: tt.fields.config, - // keyManager: tt.fields.keyManager, - // provisioners: tt.fields.provisioners, - // db: tt.fields.db, - // templates: tt.fields.templates, - // x509CAService: tt.fields.x509CAService, - // rootX509Certs: tt.fields.rootX509Certs, - // federatedX509Certs: tt.fields.federatedX509Certs, - // certificates: tt.fields.certificates, - // scepService: tt.fields.scepService, - // sshCAUserCertSignKey: tt.fields.sshCAUserCertSignKey, - // sshCAHostCertSignKey: tt.fields.sshCAHostCertSignKey, - // sshCAUserCerts: tt.fields.sshCAUserCerts, - // sshCAHostCerts: tt.fields.sshCAHostCerts, - // sshCAUserFederatedCerts: tt.fields.sshCAUserFederatedCerts, - // sshCAHostFederatedCerts: tt.fields.sshCAHostFederatedCerts, - // initOnce: tt.fields.initOnce, - // startTime: tt.fields.startTime, - // sshBastionFunc: tt.fields.sshBastionFunc, - // sshCheckHostFunc: tt.fields.sshCheckHostFunc, - // sshGetHostsFunc: tt.fields.sshGetHostsFunc, - // getIdentityFunc: tt.fields.getIdentityFunc, - // } a, err := New(tt.fields.config) if (err != nil) != tt.wantErr { t.Errorf("Authority.New(), error = %v, wantErr %v", err, tt.wantErr) diff --git a/authority/provisioner/scep.go b/authority/provisioner/scep.go index 0531bd10..9daf2e8b 100644 --- a/authority/provisioner/scep.go +++ b/authority/provisioner/scep.go @@ -24,8 +24,8 @@ type SCEP struct { MinimumPublicKeyLength int `json:"minimumPublicKeyLength,omitempty"` // Numerical identifier for the ContentEncryptionAlgorithm as defined in github.com/mozilla-services/pkcs7 // at https://github.com/mozilla-services/pkcs7/blob/33d05740a3526e382af6395d3513e73d4e66d1cb/encrypt.go#L63 - // Defaults to 2, being AES-256-CBC - EncryptionAlgorithmIdentifier *int `json:"encryptionAlgorithmIdentifier,omitempty"` + // Defaults to 0, being DES-CBC + EncryptionAlgorithmIdentifier int `json:"encryptionAlgorithmIdentifier,omitempty"` Options *Options `json:"options,omitempty"` Claims *Claims `json:"claims,omitempty"` claimer *Claimer @@ -104,16 +104,12 @@ func (s *SCEP) Init(config Config) (err error) { } if s.MinimumPublicKeyLength%8 != 0 { - return errors.Errorf("only minimum public keys exactly divisible by 8 are supported; %d is not exactly divisible by 8", s.MinimumPublicKeyLength) + return errors.Errorf("%d bits is not exactly divisible by 8", s.MinimumPublicKeyLength) } - s.encryptionAlgorithm = 2 // default to AES-256-CBC - if s.EncryptionAlgorithmIdentifier != nil { - value := *s.EncryptionAlgorithmIdentifier - if value < 0 || value > 4 { - return errors.Errorf("only encryption algorithm identifiers from 0 to 4 are valid") - } - s.encryptionAlgorithm = value + s.encryptionAlgorithm = s.EncryptionAlgorithmIdentifier + if s.encryptionAlgorithm < 0 || s.encryptionAlgorithm > 4 { + return errors.New("only encryption algorithm identifiers from 0 to 4 are valid") } // TODO: add other, SCEP specific, options? diff --git a/ca/ca.go b/ca/ca.go index a07ae406..eab49b50 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -432,17 +432,13 @@ func (ca *CA) getTLSConfig(auth *authority.Authority) (*tls.Config, error) { certPool.AddCert(crt) } - // adding the intermediate CA to the pool will allow clients that - // fail to send the intermediate for chain building to connect to the CA - // successfully. - shouldAddIntermediateToClientCAPool := true // TODO(hs): make this into a configuration - if shouldAddIntermediateToClientCAPool { - cert, err := auth.GetIntermediateCertificate() - if err != nil { - return nil, err - } - certPool.AddCert(cert) + // adding the intermediate CA to the pool will allow clients that do not + // send the intermediate for chain building to connect to the CA successfully. + cert, err := auth.GetIntermediateCertificate() + if err != nil { + return nil, err } + certPool.AddCert(cert) // Add support for mutual tls to renew certificates tlsConfig.ClientAuth = tls.VerifyClientCertIfGiven diff --git a/scep/authority.go b/scep/authority.go index 04e77160..0003fd0e 100644 --- a/scep/authority.go +++ b/scep/authority.go @@ -166,8 +166,9 @@ func (a *Authority) GetCACertificates(ctx context.Context) ([]*x509.Certificate, certs := []*x509.Certificate{} certs = append(certs, a.caCerts[0]) - // TODO(hs): we're adding the roots here, but they may be something different than what the RFC means. Clients are responsible to select the right cert(s) to use, though. - if p.ShouldIncludeRootsInChain() && len(a.caCerts) >= 2 { + // NOTE: we're adding the CA roots here, but they are (highly likely) different than what the RFC means. + // Clients are responsible to select the right cert(s) to use, though. + if p.ShouldIncludeRootsInChain() && len(a.caCerts) > 1 { certs = append(certs, a.caCerts[1:]...) } @@ -304,7 +305,7 @@ func (a *Authority) SignCSR(ctx context.Context, csr *x509.CertificateRequest, m // apparently the pkcs7 library uses a global default setting for the content encryption // algorithm to use when en- or decrypting data. We need to restore the current setting after // the cryptographic operation, so that other usages of the library are not influenced by - // this call to Encrypt(). + // this call to Encrypt(). We are not required to use the same algorithm the SCEP client uses. encryptionAlgorithmToRestore := pkcs7.ContentEncryptionAlgorithm pkcs7.ContentEncryptionAlgorithm = p.GetContentEncryptionAlgorithm() e7, err := pkcs7.Encrypt(deg, msg.P7.Certificates) From 64680bb16dd076a33e3c1ad53b71b507825c6560 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Wed, 19 Jan 2022 11:31:33 +0100 Subject: [PATCH 3/6] Fix PR comments --- authority/authority.go | 6 ------ authority/provisioner/scep.go | 17 +++++++++-------- ca/ca.go | 17 +++++++++++------ scep/authority.go | 4 ++-- scep/provisioner.go | 2 +- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/authority/authority.go b/authority/authority.go index da0a6f6f..6624f11d 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -595,12 +595,6 @@ func (a *Authority) IsRevoked(sn string) (bool, error) { return a.db.IsRevoked(sn) } -// GetIntermediateCertificate returns the x509 intermediate CA -// certificate. -func (a *Authority) GetIntermediateCertificate() (*x509.Certificate, error) { - return pemutil.ReadCertificate(a.config.IntermediateCert) -} - // requiresDecrypter returns whether the Authority // requires a KMS that provides a crypto.Decrypter // Currently this is only required when SCEP is diff --git a/authority/provisioner/scep.go b/authority/provisioner/scep.go index 9daf2e8b..5d67762c 100644 --- a/authority/provisioner/scep.go +++ b/authority/provisioner/scep.go @@ -18,8 +18,9 @@ type SCEP struct { ForceCN bool `json:"forceCN,omitempty"` ChallengePassword string `json:"challenge,omitempty"` Capabilities []string `json:"capabilities,omitempty"` - // IncludeRoots makes the provisioner return the CA root(s) in the GetCACerts response - IncludeRoots bool `json:"includeRoots,omitempty"` + // IncludeRoot makes the provisioner return the CA root in addition to the + // intermediate in the GetCACerts response + IncludeRoot bool `json:"includeRoot,omitempty"` // MinimumPublicKeyLength is the minimum length for public keys in CSRs MinimumPublicKeyLength int `json:"minimumPublicKeyLength,omitempty"` // Numerical identifier for the ContentEncryptionAlgorithm as defined in github.com/mozilla-services/pkcs7 @@ -107,7 +108,7 @@ func (s *SCEP) Init(config Config) (err error) { return errors.Errorf("%d bits is not exactly divisible by 8", s.MinimumPublicKeyLength) } - s.encryptionAlgorithm = s.EncryptionAlgorithmIdentifier + s.encryptionAlgorithm = s.EncryptionAlgorithmIdentifier // TODO(hs): we might want to upgrade the default security to AES-CBC? if s.encryptionAlgorithm < 0 || s.encryptionAlgorithm > 4 { return errors.New("only encryption algorithm identifiers from 0 to 4 are valid") } @@ -142,12 +143,12 @@ func (s *SCEP) GetCapabilities() []string { return s.Capabilities } -// ShouldIncludeRootsInChain indicates if the CA should +// ShouldIncludeRootInChain indicates if the CA should // return its intermediate, which is currently used for -// both signing and decryption, as well as the other certs -// in its chain (usually a single root certificate). -func (s *SCEP) ShouldIncludeRootsInChain() bool { - return s.IncludeRoots +// both signing and decryption, as well as the root in +// its chain. +func (s *SCEP) ShouldIncludeRootInChain() bool { + return s.IncludeRoot } // GetContentEncryptionAlgorithm returns the numeric identifier diff --git a/ca/ca.go b/ca/ca.go index eab49b50..216acb97 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -427,18 +427,23 @@ func (ca *CA) getTLSConfig(auth *authority.Authority) (*tls.Config, error) { tlsConfig.Certificates = []tls.Certificate{} tlsConfig.GetCertificate = ca.renewer.GetCertificateForCA + // initialize a certificate pool with root CA certificates to trust when doing mTLS. certPool := x509.NewCertPool() for _, crt := range auth.GetRootCertificates() { certPool.AddCert(crt) } - // adding the intermediate CA to the pool will allow clients that do not - // send the intermediate for chain building to connect to the CA successfully. - cert, err := auth.GetIntermediateCertificate() - if err != nil { - return nil, err + // adding the intermediate CA certificates to the pool will allow clients that + // do mTLS but don't send an intermediate to successfully connect. The intermediates + // added here are used when building a certificate chain. + intermediates := tlsCrt.Certificate[1:] + for _, certBytes := range intermediates { + cert, err := x509.ParseCertificate(certBytes) + if err != nil { + return nil, err + } + certPool.AddCert(cert) } - certPool.AddCert(cert) // Add support for mutual tls to renew certificates tlsConfig.ClientAuth = tls.VerifyClientCertIfGiven diff --git a/scep/authority.go b/scep/authority.go index 0003fd0e..f97bd55e 100644 --- a/scep/authority.go +++ b/scep/authority.go @@ -168,8 +168,8 @@ func (a *Authority) GetCACertificates(ctx context.Context) ([]*x509.Certificate, // NOTE: we're adding the CA roots here, but they are (highly likely) different than what the RFC means. // Clients are responsible to select the right cert(s) to use, though. - if p.ShouldIncludeRootsInChain() && len(a.caCerts) > 1 { - certs = append(certs, a.caCerts[1:]...) + if p.ShouldIncludeRootInChain() && len(a.caCerts) > 1 { + certs = append(certs, a.caCerts[1]) } return certs, nil diff --git a/scep/provisioner.go b/scep/provisioner.go index 75820f56..679c6353 100644 --- a/scep/provisioner.go +++ b/scep/provisioner.go @@ -16,6 +16,6 @@ type Provisioner interface { GetOptions() *provisioner.Options GetChallengePassword() string GetCapabilities() []string - ShouldIncludeRootsInChain() bool + ShouldIncludeRootInChain() bool GetContentEncryptionAlgorithm() int } From 3b72d241e0384f4fdbd1a3282bddb8e091c14d90 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Fri, 21 Jan 2022 16:07:31 +0100 Subject: [PATCH 4/6] Add LinkedCA integration for improved SCEP provisioner --- authority/provisioners.go | 25 +++++++++++++++++++++---- go.mod | 9 +++++---- go.sum | 27 ++++++++++++++++++--------- scep/api/api.go | 12 ++++++------ scep/authority.go | 8 ++++++++ 5 files changed, 58 insertions(+), 23 deletions(-) diff --git a/authority/provisioners.go b/authority/provisioners.go index a98b78a6..ae2ebb76 100644 --- a/authority/provisioners.go +++ b/authority/provisioners.go @@ -711,6 +711,21 @@ func ProvisionerToCertificates(p *linkedca.Provisioner) (provisioner.Interface, Claims: claims, Options: options, }, nil + case *linkedca.ProvisionerDetails_SCEP: + cfg := d.SCEP + return &provisioner.SCEP{ + ID: p.Id, + Type: p.Type.String(), + Name: p.Name, + ForceCN: cfg.ForceCn, + ChallengePassword: cfg.Challenge, + Capabilities: cfg.Capabilities, + IncludeRoot: cfg.IncludeRoot, + MinimumPublicKeyLength: int(cfg.MinimumPublicKeyLength), + EncryptionAlgorithmIdentifier: int(cfg.EncryptionAlgorithmIdentifier), + Claims: claims, + Options: options, + }, nil case *linkedca.ProvisionerDetails_Nebula: var roots []byte for i, root := range d.Nebula.GetRoots() { @@ -943,10 +958,12 @@ func ProvisionerToLinkedca(p provisioner.Interface) (*linkedca.Provisioner, erro Details: &linkedca.ProvisionerDetails{ Data: &linkedca.ProvisionerDetails_SCEP{ SCEP: &linkedca.SCEPProvisioner{ - ForceCn: p.ForceCN, - Challenge: p.GetChallengePassword(), - Capabilities: p.Capabilities, - MinimumPublicKeyLength: int32(p.MinimumPublicKeyLength), + ForceCn: p.ForceCN, + Challenge: p.GetChallengePassword(), + Capabilities: p.Capabilities, + MinimumPublicKeyLength: int32(p.MinimumPublicKeyLength), + IncludeRoot: p.IncludeRoot, + EncryptionAlgorithmIdentifier: int32(p.EncryptionAlgorithmIdentifier), }, }, }, diff --git a/go.mod b/go.mod index 16633d1c..0f2231c1 100644 --- a/go.mod +++ b/go.mod @@ -35,12 +35,13 @@ require ( go.mozilla.org/pkcs7 v0.0.0-20210826202110-33d05740a352 go.step.sm/cli-utils v0.7.0 go.step.sm/crypto v0.14.0 - go.step.sm/linkedca v0.9.0 + go.step.sm/linkedca v0.9.2 golang.org/x/crypto v0.0.0-20211215153901-e495a2d5b3d3 - golang.org/x/net v0.0.0-20211216030914-fe4d6282115f + golang.org/x/net v0.0.0-20220114011407-0dd24b26b47d + golang.org/x/sys v0.0.0-20220114195835-da31bd327af9 // indirect google.golang.org/api v0.47.0 - google.golang.org/genproto v0.0.0-20210719143636-1d5a45f8e492 - google.golang.org/grpc v1.39.0 + google.golang.org/genproto v0.0.0-20220118154757-00ab72f36ad5 + google.golang.org/grpc v1.43.0 google.golang.org/protobuf v1.27.1 gopkg.in/square/go-jose.v2 v2.6.0 ) diff --git a/go.sum b/go.sum index f66f7934..d207a9a3 100644 --- a/go.sum +++ b/go.sum @@ -148,7 +148,11 @@ github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDk github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc= github.com/cncf/udpa/go v0.0.0-20200629203442-efcf912fb354/go.mod h1:WmhPx2Nbnhtbo57+VJT5O0JRkEi1Wbu0z5j0R8u5Hbk= github.com/cncf/udpa/go v0.0.0-20201120205902-5459f2c99403/go.mod h1:WmhPx2Nbnhtbo57+VJT5O0JRkEi1Wbu0z5j0R8u5Hbk= +github.com/cncf/udpa/go v0.0.0-20210930031921-04548b0d99d4/go.mod h1:6pvJx4me5XPnfI9Z40ddWsdw2W/uZgQLFXToKeRcDiI= github.com/cncf/xds/go v0.0.0-20210312221358-fbca930ec8ed/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs= +github.com/cncf/xds/go v0.0.0-20210805033703-aa0b78936158/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs= +github.com/cncf/xds/go v0.0.0-20210922020428-25de7278fc84/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs= +github.com/cncf/xds/go v0.0.0-20211011173535-cb28da3451f1/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs= github.com/cockroachdb/datadriven v0.0.0-20190809214429-80d97fb3cbaa/go.mod h1:zn76sxSg3SzpJ0PPJaLDCu+Bu0Lg3sKTORVIj19EIF8= github.com/codahale/hdrhistogram v0.0.0-20161010025455-3a0bb77429bd/go.mod h1:sE/e/2PUdi/liOCUjSTXgM1o87ZssimdTWN964YiIeI= github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE= @@ -196,6 +200,7 @@ github.com/envoyproxy/go-control-plane v0.9.7/go.mod h1:cwu0lG7PUMfa9snN8LXBig5y github.com/envoyproxy/go-control-plane v0.9.9-0.20201210154907-fd9021fe5dad/go.mod h1:cXg6YxExXjJnVBQHBLXeUAgxn2UodCpnH306RInaBQk= github.com/envoyproxy/go-control-plane v0.9.9-0.20210217033140-668b12f5399d/go.mod h1:cXg6YxExXjJnVBQHBLXeUAgxn2UodCpnH306RInaBQk= github.com/envoyproxy/go-control-plane v0.9.9-0.20210512163311-63b5d3c536b0/go.mod h1:hliV/p42l8fGbc6Y9bQ70uLwIvmJyVE5k4iMKlh8wCQ= +github.com/envoyproxy/go-control-plane v0.9.10-0.20210907150352-cf90f659a021/go.mod h1:AFq3mo9L8Lqqiid3OhADV3RfLJnjiw63cSpi+fDTRC0= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= github.com/flynn/noise v1.0.0/go.mod h1:xbMo+0i6+IGbYdJhF31t2eR1BIU0CYc12+BNAKwUTag= @@ -607,8 +612,10 @@ go.step.sm/cli-utils v0.7.0/go.mod h1:Ur6bqA/yl636kCUJbp30J7Unv5JJ226eW2KqXPDwF/ go.step.sm/crypto v0.9.0/go.mod h1:+CYG05Mek1YDqi5WK0ERc6cOpKly2i/a5aZmU1sfGj0= go.step.sm/crypto v0.14.0 h1:HzSkUDwqKhODKpsTxevJz956U2xVDZ3sDdGQVwR6Ttw= go.step.sm/crypto v0.14.0/go.mod h1:3G0yQr5lQqfEG0CMYz8apC/qMtjLRQlzflL2AxkcN+g= -go.step.sm/linkedca v0.9.0 h1:xKXZoRXy4B7LeGBZozq62IQ0p3v8dT33O9UOMpVtRtI= -go.step.sm/linkedca v0.9.0/go.mod h1:5uTRjozEGSPAZal9xJqlaD38cvJcLe3o1VAFVjqcORo= +go.step.sm/linkedca v0.9.1 h1:FQ2A4vSo9a7l74dnMVJRl4DC+Hu9FNUzTGUD0kEqlSM= +go.step.sm/linkedca v0.9.1/go.mod h1:5uTRjozEGSPAZal9xJqlaD38cvJcLe3o1VAFVjqcORo= +go.step.sm/linkedca v0.9.2 h1:CpAkd174sLXFfrOZrbPEiTzik91QRj3+L0omsiwsiok= +go.step.sm/linkedca v0.9.2/go.mod h1:5uTRjozEGSPAZal9xJqlaD38cvJcLe3o1VAFVjqcORo= go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= go.uber.org/atomic v1.5.0/go.mod h1:sABNBOSYdrvTF6hTgEIbc7YasKWGhgEQZyfxyTvoXHQ= go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0= @@ -717,8 +724,9 @@ golang.org/x/net v0.0.0-20210525063256-abc453219eb5/go.mod h1:9nx3DQGgdP8bBQD5qx golang.org/x/net v0.0.0-20210805182204-aaa1db679c0d/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20211020060615-d418f374d309/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= -golang.org/x/net v0.0.0-20211216030914-fe4d6282115f h1:hEYJvxw1lSnWIl8X9ofsYMklzaDs90JI2az5YMd4fPM= golang.org/x/net v0.0.0-20211216030914-fe4d6282115f/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= +golang.org/x/net v0.0.0-20220114011407-0dd24b26b47d h1:1n1fc535VhN8SYtD4cDUyNlfpAF2ROMM9+11equK3hs= +golang.org/x/net v0.0.0-20220114011407-0dd24b26b47d/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -813,8 +821,9 @@ golang.org/x/sys v0.0.0-20210809222454-d867a43fc93e/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20210915083310-ed5796bab164/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20211025201205-69cdffdb9359/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20211031064116-611d5d643895/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20211103235746-7861aae1554b h1:1VkfZQv42XQlA/jchYumAnv1UPo6RgF9rJFkTgZIxO4= golang.org/x/sys v0.0.0-20211103235746-7861aae1554b/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220114195835-da31bd327af9 h1:XfKQ4OlFl8okEOr5UvAqFRVj8pY/4yfcXrddB8qAbU0= +golang.org/x/sys v0.0.0-20220114195835-da31bd327af9/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 h1:JGgROgKl9N8DuW20oFS5gxc+lE67/N3FcwmBPMe7ArY= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= @@ -889,7 +898,6 @@ golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4f golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0= golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= -golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.7/go.mod h1:LGqMHiF4EqQNHR1JncWGqT5BVaXmza+X+BDGol+dOxo= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= @@ -974,8 +982,8 @@ google.golang.org/genproto v0.0.0-20210319143718-93e7006c17a6/go.mod h1:FWY/as6D google.golang.org/genproto v0.0.0-20210402141018-6c239bbf2bb1/go.mod h1:9lPAdzaEmUacj36I+k7YKbEc5CXzPIeORRgDAUOu28A= google.golang.org/genproto v0.0.0-20210513213006-bf773b8c8384/go.mod h1:P3QM42oQyzQSnHPnZ/vqoCdDmzH28fzWByN9asMeM8A= google.golang.org/genproto v0.0.0-20210602131652-f16073e35f0c/go.mod h1:UODoCrxHCcBojKKwX1terBiRUaqAsFqJiF615XL43r0= -google.golang.org/genproto v0.0.0-20210719143636-1d5a45f8e492 h1:7yQQsvnwjfEahbNNEKcBHv3mR+HnB1ctGY/z1JXzx8M= -google.golang.org/genproto v0.0.0-20210719143636-1d5a45f8e492/go.mod h1:ob2IJxKrgPT52GcgX759i1sleT07tiKowYBGbczaW48= +google.golang.org/genproto v0.0.0-20220118154757-00ab72f36ad5 h1:zzNejm+EgrbLfDZ6lu9Uud2IVvHySPl8vQzf04laR5Q= +google.golang.org/genproto v0.0.0-20220118154757-00ab72f36ad5/go.mod h1:5CzLGKJ67TSI2B9POpiiyGha0AjJvZIUgRMt1dSmuhc= google.golang.org/grpc v1.17.0/go.mod h1:6QZJwpn2B+Zp71q/5VxRsJ6NXXVCE5NRUHRo+f3cWCs= google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= google.golang.org/grpc v1.20.0/go.mod h1:chYK+tFQF0nDUGJgXMSgLCQk3phJEuONr2DCgLDdAQM= @@ -1003,8 +1011,9 @@ google.golang.org/grpc v1.36.1/go.mod h1:qjiiYl8FncCW8feJPdyg3v6XW24KsRHe+dy9BAG google.golang.org/grpc v1.37.0/go.mod h1:NREThFqKR1f3iQ6oBuvc5LadQuXVGo9rkm5ZGrQdJfM= google.golang.org/grpc v1.37.1/go.mod h1:NREThFqKR1f3iQ6oBuvc5LadQuXVGo9rkm5ZGrQdJfM= google.golang.org/grpc v1.38.0/go.mod h1:NREThFqKR1f3iQ6oBuvc5LadQuXVGo9rkm5ZGrQdJfM= -google.golang.org/grpc v1.39.0 h1:Klz8I9kdtkIN6EpHHUOMLCYhTn/2WAe5a0s1hcBkdTI= -google.golang.org/grpc v1.39.0/go.mod h1:PImNr+rS9TWYb2O4/emRugxiyHZ5JyHW5F+RPnDzfrE= +google.golang.org/grpc v1.40.0/go.mod h1:ogyxbiOoUXAkP+4+xa6PZSE9DZgIHtSpzjDTB9KAK34= +google.golang.org/grpc v1.43.0 h1:Eeu7bZtDZ2DpRCsLhUlcrLnvYaMK1Gz86a+hMVvELmM= +google.golang.org/grpc v1.43.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ590SU= google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.1.0/go.mod h1:6Kw0yEErY5E/yWrBtf03jp27GLLJujG4z/JK95pnjjw= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= diff --git a/scep/api/api.go b/scep/api/api.go index d1ade2aa..4f8d897b 100644 --- a/scep/api/api.go +++ b/scep/api/api.go @@ -66,8 +66,8 @@ func New(scepAuth scep.Interface) api.RouterHandler { // Route traffic and implement the Router interface. func (h *Handler) Route(r api.Router) { getLink := h.Auth.GetLinkExplicit - r.MethodFunc(http.MethodGet, getLink("{provisionerID}", false, nil), h.lookupProvisioner(h.Get)) - r.MethodFunc(http.MethodPost, getLink("{provisionerID}", false, nil), h.lookupProvisioner(h.Post)) + r.MethodFunc(http.MethodGet, getLink("{provisionerName}", false, nil), h.lookupProvisioner(h.Get)) + r.MethodFunc(http.MethodPost, getLink("{provisionerName}", false, nil), h.lookupProvisioner(h.Post)) } // Get handles all SCEP GET requests @@ -184,14 +184,14 @@ func decodeSCEPRequest(r *http.Request) (SCEPRequest, error) { func (h *Handler) lookupProvisioner(next nextHTTP) nextHTTP { return func(w http.ResponseWriter, r *http.Request) { - name := chi.URLParam(r, "provisionerID") - provisionerID, err := url.PathUnescape(name) + name := chi.URLParam(r, "provisionerName") + provisionerName, err := url.PathUnescape(name) if err != nil { - api.WriteError(w, errors.Errorf("error url unescaping provisioner id '%s'", name)) + api.WriteError(w, errors.Errorf("error url unescaping provisioner name '%s'", name)) return } - p, err := h.Auth.LoadProvisionerByID("scep/" + provisionerID) + p, err := h.Auth.LoadProvisionerByName(provisionerName) if err != nil { api.WriteError(w, err) return diff --git a/scep/authority.go b/scep/authority.go index f97bd55e..1bdd1e0a 100644 --- a/scep/authority.go +++ b/scep/authority.go @@ -20,6 +20,7 @@ import ( // Interface is the SCEP authority interface. type Interface interface { + LoadProvisionerByName(string) (provisioner.Interface, error) LoadProvisionerByID(string) (provisioner.Interface, error) GetLinkExplicit(provName string, absoluteLink bool, baseURL *url.URL, inputs ...string) string @@ -58,6 +59,7 @@ type AuthorityOptions struct { type SignAuthority interface { Sign(cr *x509.CertificateRequest, opts provisioner.SignOptions, signOpts ...provisioner.SignOption) ([]*x509.Certificate, error) LoadProvisionerByID(string) (provisioner.Interface, error) + LoadProvisionerByName(string) (provisioner.Interface, error) } // New returns a new Authority that implements the SCEP interface. @@ -101,6 +103,12 @@ func (a *Authority) LoadProvisionerByID(id string) (provisioner.Interface, error return a.signAuth.LoadProvisionerByID(id) } +// LoadProvisionerByName calls out to the SignAuthority interface to load a +// provisioner by name. +func (a *Authority) LoadProvisionerByName(name string) (provisioner.Interface, error) { + return a.signAuth.LoadProvisionerByName(name) +} + // GetLinkExplicit returns the requested link from the directory. func (a *Authority) GetLinkExplicit(provName string, abs bool, baseURL *url.URL, inputs ...string) string { return a.getLinkExplicit(provName, abs, baseURL, inputs...) From ad041d6bb774b72f7ec261f81200004a22903cd0 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Fri, 21 Jan 2022 16:17:40 +0100 Subject: [PATCH 5/6] Fix deprecation of grpc.WithInsecure option With the update of go.step.sm/linkedca grpc.WithInsecure was deprecated. This commit fixes this by setting up the (insecure) connection using the new method. --- cas/cloudcas/cloudcas_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cas/cloudcas/cloudcas_test.go b/cas/cloudcas/cloudcas_test.go index 7f996c15..1a16ef1e 100644 --- a/cas/cloudcas/cloudcas_test.go +++ b/cas/cloudcas/cloudcas_test.go @@ -31,6 +31,7 @@ import ( longrunningpb "google.golang.org/genproto/googleapis/longrunning" "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/status" "google.golang.org/grpc/test/bufconn" "google.golang.org/protobuf/types/known/anypb" @@ -852,7 +853,7 @@ func TestCloudCAS_CreateCertificateAuthority(t *testing.T) { defer srv.Stop() // Create fake privateca client - conn, err := grpc.DialContext(context.Background(), "", grpc.WithInsecure(), + conn, err := grpc.DialContext(context.Background(), "", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithContextDialer(func(context.Context, string) (net.Conn, error) { return lis.Dial() })) From 5f42ae0bce8c7ba16ca4c83240b7f8f25a07f97b Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Thu, 27 Jan 2022 21:06:55 +0100 Subject: [PATCH 6/6] Remove unused function LoadProvisionerByID from SCEP --- scep/authority.go | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/scep/authority.go b/scep/authority.go index 1bdd1e0a..269e3ae1 100644 --- a/scep/authority.go +++ b/scep/authority.go @@ -21,7 +21,6 @@ import ( // Interface is the SCEP authority interface. type Interface interface { LoadProvisionerByName(string) (provisioner.Interface, error) - LoadProvisionerByID(string) (provisioner.Interface, error) GetLinkExplicit(provName string, absoluteLink bool, baseURL *url.URL, inputs ...string) string GetCACertificates(ctx context.Context) ([]*x509.Certificate, error) @@ -58,7 +57,6 @@ type AuthorityOptions struct { // SignAuthority is the interface for a signing authority type SignAuthority interface { Sign(cr *x509.CertificateRequest, opts provisioner.SignOptions, signOpts ...provisioner.SignOption) ([]*x509.Certificate, error) - LoadProvisionerByID(string) (provisioner.Interface, error) LoadProvisionerByName(string) (provisioner.Interface, error) } @@ -97,12 +95,6 @@ var ( } ) -// LoadProvisionerByID calls out to the SignAuthority interface to load a -// provisioner by ID. -func (a *Authority) LoadProvisionerByID(id string) (provisioner.Interface, error) { - return a.signAuth.LoadProvisionerByID(id) -} - // LoadProvisionerByName calls out to the SignAuthority interface to load a // provisioner by name. func (a *Authority) LoadProvisionerByName(name string) (provisioner.Interface, error) { @@ -285,11 +277,7 @@ func (a *Authority) SignCSR(ctx context.Context, csr *x509.CertificateRequest, m return nil, errors.Wrap(err, "error retrieving authorization options from SCEP provisioner") } - opts := provisioner.SignOptions{ - // NotBefore: provisioner.NewTimeDuration(o.NotBefore), - // NotAfter: provisioner.NewTimeDuration(o.NotAfter), - } - + opts := provisioner.SignOptions{} templateOptions, err := provisioner.TemplateOptions(p.GetOptions(), data) if err != nil { return nil, errors.Wrap(err, "error creating template options from SCEP provisioner")