From 3612eefc318587ca0d585a173f4c48b3a39f02b2 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Tue, 18 Jan 2022 15:54:18 +0100 Subject: [PATCH] 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)