Cleanup

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)

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)

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?

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

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)