From 60515d92c5d88d6ef74e787a33af08522716b340 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 16 Sep 2020 13:31:26 -0700 Subject: [PATCH] Remove unnecessary properties. --- authority/authority.go | 45 ++++++++++-------------------- authority/authority_test.go | 6 ++-- authority/options.go | 13 +++++++-- authority/tls_test.go | 55 +++++++++++++++++++++---------------- 4 files changed, 60 insertions(+), 59 deletions(-) diff --git a/authority/authority.go b/authority/authority.go index a51e986f..5d8f9b04 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -2,7 +2,6 @@ package authority import ( "context" - "crypto" "crypto/sha256" "crypto/x509" "encoding/hex" @@ -39,8 +38,6 @@ type Authority struct { x509CAService cas.CertificateAuthorityService rootX509Certs []*x509.Certificate federatedX509Certs []*x509.Certificate - x509Signer crypto.Signer - x509Issuer *x509.Certificate certificates *sync.Map // SSH CA @@ -110,9 +107,9 @@ func NewEmbedded(opts ...Option) (*Authority, error) { return nil, errors.New("cannot create an authority without a configuration") case len(a.rootX509Certs) == 0 && a.config.Root.HasEmpties(): return nil, errors.New("cannot create an authority without a root certificate") - case a.x509Issuer == nil && a.config.IntermediateCert == "": + case a.x509CAService == nil && a.config.IntermediateCert == "": return nil, errors.New("cannot create an authority without an issuer certificate") - case a.x509Signer == nil && a.config.IntermediateKey == "": + case a.x509CAService == nil && a.config.IntermediateKey == "": return nil, errors.New("cannot create an authority without an issuer signer") } @@ -188,38 +185,26 @@ func (a *Authority) init() error { a.certificates.Store(hex.EncodeToString(sum[:]), crt) } - // Read intermediate and create X509 signer. - if a.x509Signer == nil { - crt, err := pemutil.ReadCertificate(a.config.IntermediateCert) - if err != nil { - return err - } - a.x509Issuer = crt - - // Read signer only is the CAS is the default one. - if a.config.CAS.HasType(casapi.SoftCAS) { - signer, err := a.keyManager.CreateSigner(&kmsapi.CreateSignerRequest{ - SigningKey: a.config.IntermediateKey, - Password: []byte(a.config.Password), - }) - if err != nil { - return err - } - a.x509Signer = signer - } - } - - // Initialize the X.509 CA Service if it has not been set in the options + // Initialize the X.509 CA Service if it has not been set in the options. if a.x509CAService == nil { var options casapi.Options if a.config.CAS != nil { options = *a.config.CAS } - // Set issuer and signer for default CAS. + // Read intermediate and create X509 signer for default CAS. if options.HasType(casapi.SoftCAS) { - options.Issuer = a.x509Issuer - options.Signer = a.x509Signer + options.Issuer, err = pemutil.ReadCertificate(a.config.IntermediateCert) + if err != nil { + return err + } + options.Signer, err = a.keyManager.CreateSigner(&kmsapi.CreateSignerRequest{ + SigningKey: a.config.IntermediateKey, + Password: []byte(a.config.Password), + }) + if err != nil { + return err + } } a.x509CAService, err = cas.New(context.Background(), options) diff --git a/authority/authority_test.go b/authority/authority_test.go index 54de0040..8b003572 100644 --- a/authority/authority_test.go +++ b/authority/authority_test.go @@ -143,8 +143,7 @@ func TestAuthorityNew(t *testing.T) { assert.Equals(t, auth.rootX509Certs[0], root) assert.True(t, auth.initOnce) - assert.NotNil(t, auth.x509Signer) - assert.NotNil(t, auth.x509Issuer) + assert.NotNil(t, auth.x509CAService) for _, p := range tc.config.AuthorityConfig.Provisioners { var _p provisioner.Interface _p, ok = auth.provisioners.Load(p.GetID()) @@ -256,8 +255,7 @@ func TestNewEmbedded(t *testing.T) { if err == nil { assert.True(t, got.initOnce) assert.NotNil(t, got.rootX509Certs) - assert.NotNil(t, got.x509Signer) - assert.NotNil(t, got.x509Issuer) + assert.NotNil(t, got.x509CAService) } }) } diff --git a/authority/options.go b/authority/options.go index 9457f276..da5a8f88 100644 --- a/authority/options.go +++ b/authority/options.go @@ -8,6 +8,8 @@ import ( "github.com/pkg/errors" "github.com/smallstep/certificates/authority/provisioner" + "github.com/smallstep/certificates/cas" + casapi "github.com/smallstep/certificates/cas/apiv1" "github.com/smallstep/certificates/db" "github.com/smallstep/certificates/kms" "golang.org/x/crypto/ssh" @@ -92,8 +94,15 @@ func WithKeyManager(k kms.KeyManager) Option { // WithX509Signer defines the signer used to sign X509 certificates. func WithX509Signer(crt *x509.Certificate, s crypto.Signer) Option { return func(a *Authority) error { - a.x509Issuer = crt - a.x509Signer = s + srv, err := cas.New(context.Background(), casapi.Options{ + Type: casapi.SoftCAS, + Issuer: crt, + Signer: s, + }) + if err != nil { + return err + } + a.x509CAService = srv return nil } } diff --git a/authority/tls_test.go b/authority/tls_test.go index 9d8c6226..75f9e234 100644 --- a/authority/tls_test.go +++ b/authority/tls_test.go @@ -55,6 +55,14 @@ func (m *certificateDurationEnforcer) Enforce(cert *x509.Certificate) error { return nil } +func getDefaultIssuer(a *Authority) *x509.Certificate { + return a.x509CAService.(*softcas.SoftCAS).Issuer +} + +func getDefaultSigner(a *Authority) crypto.Signer { + return a.x509CAService.(*softcas.SoftCAS).Signer +} + func generateCertificate(t *testing.T, commonName string, sans []string, opts ...interface{}) *x509.Certificate { t.Helper() @@ -541,17 +549,15 @@ ZYtQ9Ot36qc= assert.Equals(t, leaf.DNSNames, []string{"test.smallstep.com"}) } assert.Equals(t, leaf.Issuer, intermediate.Subject) - assert.Equals(t, leaf.SignatureAlgorithm, x509.ECDSAWithSHA256) assert.Equals(t, leaf.PublicKeyAlgorithm, x509.ECDSA) - assert.Equals(t, leaf.ExtKeyUsage, - []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}) + assert.Equals(t, leaf.ExtKeyUsage, []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}) + issuer := getDefaultIssuer(a) subjectKeyID, err := generateSubjectKeyID(pub) assert.FatalError(t, err) assert.Equals(t, leaf.SubjectKeyId, subjectKeyID) - - assert.Equals(t, leaf.AuthorityKeyId, a.x509Issuer.SubjectKeyId) + assert.Equals(t, leaf.AuthorityKeyId, issuer.SubjectKeyId) // Verify Provisioner OID found := 0 @@ -587,8 +593,7 @@ ZYtQ9Ot36qc= } } assert.Equals(t, found, 1) - - realIntermediate, err := x509.ParseCertificate(a.x509Issuer.Raw) + realIntermediate, err := x509.ParseCertificate(issuer.Raw) assert.FatalError(t, err) assert.Equals(t, intermediate, realIntermediate) } @@ -616,17 +621,20 @@ func TestAuthority_Renew(t *testing.T) { NotAfter: provisioner.NewTimeDuration(na1), } + issuer := getDefaultIssuer(a) + signer := getDefaultSigner(a) + cert := generateCertificate(t, "renew", []string{"test.smallstep.com", "test"}, withNotBeforeNotAfter(so.NotBefore.Time(), so.NotAfter.Time()), withDefaultASN1DN(a.config.AuthorityConfig.Template), withProvisionerOID("Max", a.config.AuthorityConfig.Provisioners[0].(*provisioner.JWK).Key.KeyID), - withSigner(a.x509Issuer, a.x509Signer)) + withSigner(issuer, signer)) certNoRenew := generateCertificate(t, "renew", []string{"test.smallstep.com", "test"}, withNotBeforeNotAfter(so.NotBefore.Time(), so.NotAfter.Time()), withDefaultASN1DN(a.config.AuthorityConfig.Template), withProvisionerOID("dev", a.config.AuthorityConfig.Provisioners[2].(*provisioner.JWK).Key.KeyID), - withSigner(a.x509Issuer, a.x509Signer)) + withSigner(issuer, signer)) type renewTest struct { auth *Authority @@ -665,8 +673,6 @@ func TestAuthority_Renew(t *testing.T) { _a := testAuthority(t) _a.x509CAService.(*softcas.SoftCAS).Issuer = intCert _a.x509CAService.(*softcas.SoftCAS).Signer = intSigner - _a.x509Signer = intSigner - _a.x509Issuer = intCert return &renewTest{ auth: _a, cert: cert, @@ -733,8 +739,9 @@ func TestAuthority_Renew(t *testing.T) { assert.Equals(t, leaf.SubjectKeyId, subjectKeyID) // We did not change the intermediate before renewing. - if a.x509Issuer.SerialNumber == tc.auth.x509Issuer.SerialNumber { - assert.Equals(t, leaf.AuthorityKeyId, a.x509Issuer.SubjectKeyId) + authIssuer := getDefaultIssuer(tc.auth) + if issuer.SerialNumber == authIssuer.SerialNumber { + assert.Equals(t, leaf.AuthorityKeyId, issuer.SubjectKeyId) // Compare extensions: they can be in a different order for _, ext1 := range tc.cert.Extensions { //skip SubjectKeyIdentifier @@ -754,7 +761,7 @@ func TestAuthority_Renew(t *testing.T) { } } else { // We did change the intermediate before renewing. - assert.Equals(t, leaf.AuthorityKeyId, tc.auth.x509Issuer.SubjectKeyId) + assert.Equals(t, leaf.AuthorityKeyId, authIssuer.SubjectKeyId) // Compare extensions: they can be in a different order for _, ext1 := range tc.cert.Extensions { //skip SubjectKeyIdentifier @@ -782,7 +789,7 @@ func TestAuthority_Renew(t *testing.T) { } } - realIntermediate, err := x509.ParseCertificate(tc.auth.x509Issuer.Raw) + realIntermediate, err := x509.ParseCertificate(authIssuer.Raw) assert.FatalError(t, err) assert.Equals(t, intermediate, realIntermediate) } @@ -813,17 +820,20 @@ func TestAuthority_Rekey(t *testing.T) { NotAfter: provisioner.NewTimeDuration(na1), } + issuer := getDefaultIssuer(a) + signer := getDefaultSigner(a) + cert := generateCertificate(t, "renew", []string{"test.smallstep.com", "test"}, withNotBeforeNotAfter(so.NotBefore.Time(), so.NotAfter.Time()), withDefaultASN1DN(a.config.AuthorityConfig.Template), withProvisionerOID("Max", a.config.AuthorityConfig.Provisioners[0].(*provisioner.JWK).Key.KeyID), - withSigner(a.x509Issuer, a.x509Signer)) + withSigner(issuer, signer)) certNoRenew := generateCertificate(t, "renew", []string{"test.smallstep.com", "test"}, withNotBeforeNotAfter(so.NotBefore.Time(), so.NotAfter.Time()), withDefaultASN1DN(a.config.AuthorityConfig.Template), withProvisionerOID("dev", a.config.AuthorityConfig.Provisioners[2].(*provisioner.JWK).Key.KeyID), - withSigner(a.x509Issuer, a.x509Signer)) + withSigner(issuer, signer)) type renewTest struct { auth *Authority @@ -870,8 +880,6 @@ func TestAuthority_Rekey(t *testing.T) { _a := testAuthority(t) _a.x509CAService.(*softcas.SoftCAS).Issuer = intCert _a.x509CAService.(*softcas.SoftCAS).Signer = intSigner - _a.x509Signer = intSigner - _a.x509Issuer = intCert return &renewTest{ auth: _a, cert: cert, @@ -948,8 +956,9 @@ func TestAuthority_Rekey(t *testing.T) { } // We did not change the intermediate before renewing. - if a.x509Issuer.SerialNumber == tc.auth.x509Issuer.SerialNumber { - assert.Equals(t, leaf.AuthorityKeyId, a.x509Issuer.SubjectKeyId) + authIssuer := getDefaultIssuer(tc.auth) + if issuer.SerialNumber == authIssuer.SerialNumber { + assert.Equals(t, leaf.AuthorityKeyId, issuer.SubjectKeyId) // Compare extensions: they can be in a different order for _, ext1 := range tc.cert.Extensions { //skip SubjectKeyIdentifier @@ -969,7 +978,7 @@ func TestAuthority_Rekey(t *testing.T) { } } else { // We did change the intermediate before renewing. - assert.Equals(t, leaf.AuthorityKeyId, tc.auth.x509Issuer.SubjectKeyId) + assert.Equals(t, leaf.AuthorityKeyId, authIssuer.SubjectKeyId) // Compare extensions: they can be in a different order for _, ext1 := range tc.cert.Extensions { //skip SubjectKeyIdentifier @@ -997,7 +1006,7 @@ func TestAuthority_Rekey(t *testing.T) { } } - realIntermediate, err := x509.ParseCertificate(tc.auth.x509Issuer.Raw) + realIntermediate, err := x509.ParseCertificate(authIssuer.Raw) assert.FatalError(t, err) assert.Equals(t, intermediate, realIntermediate) }