Skip to content

[ca] Separate RootCA signing cert from root certs #2038

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aaronlehmann merged 2 commits into from
Mar 18, 2017
Merged

[ca] Separate RootCA signing cert from root certs #2038

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1c34390
Provide a separate field in which to specify the signing certificate,…
cyli Mar 13, 2017
5454ec0
Provide an accessor for the RootCA signer that raises an error if no …
cyli Mar 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
274 changes: 164 additions & 110 deletions ca/certificates.go
View file
Open in desktop

Large diffs are not rendered by default.

307 changes: 201 additions & 106 deletions ca/certificates_test.go
View file
Open in desktop

Large diffs are not rendered by default.

51 changes: 26 additions & 25 deletions ca/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,13 @@ func (s *SecurityConfig) UpdateRootCA(cert, key []byte, certExpiry time.Duration
s.mu.Lock()
defer s.mu.Unlock()

rootCA, err := NewRootCA(cert, key, certExpiry, nil)
// If we have no signing key, then we shouldn't pass a signing cert either (because we don't want a local
// signer at all)
signingCert := cert
if len(key) == 0 {
signingCert = nil
}
rootCA, err := NewRootCA(cert, signingCert, key, certExpiry, nil)
if err != nil {
return err
}
Expand Down Expand Up @@ -359,39 +365,34 @@ type CertificateRequestConfig struct {
func (rootCA RootCA) CreateSecurityConfig(ctx context.Context, krw *KeyReadWriter, config CertificateRequestConfig) (*SecurityConfig, error) {
ctx = log.WithModule(ctx, "tls")

var (
tlsKeyPair *tls.Certificate
err error
)

if rootCA.CanSign() {
// Create a new random ID for this certificate
cn := identity.NewID()
org := identity.NewID()

proposedRole := ManagerRole
tlsKeyPair, err = rootCA.IssueAndSaveNewCertificates(krw, cn, proposedRole, org)
if err != nil {
log.G(ctx).WithFields(logrus.Fields{
"node.id": cn,
"node.role": proposedRole,
}).WithError(err).Errorf("failed to issue and save new certificate")
return nil, err
}
// Create a new random ID for this certificate
cn := identity.NewID()
org := identity.NewID()

log.G(ctx).WithFields(logrus.Fields{
"node.id": cn,
"node.role": proposedRole,
}).Debug("issued new TLS certificate")
} else {
proposedRole := ManagerRole
tlsKeyPair, err := rootCA.IssueAndSaveNewCertificates(krw, cn, proposedRole, org)
switch errors.Cause(err) {
case ErrNoValidSigner:
// Request certificate issuance from a remote CA.
// Last argument is nil because at this point we don't have any valid TLS creds
tlsKeyPair, err = rootCA.RequestAndSaveNewCertificates(ctx, krw, config)
if err != nil {
log.G(ctx).WithError(err).Error("failed to request save new certificate")
return nil, err
}
case nil:
log.G(ctx).WithFields(logrus.Fields{
"node.id": cn,
"node.role": proposedRole,
}).Debug("issued new TLS certificate")
default:
log.G(ctx).WithFields(logrus.Fields{
"node.id": cn,
"node.role": proposedRole,
}).WithError(err).Errorf("failed to issue and save new certificate")
return nil, err
}

// Create the Server TLS Credentials for this node. These will not be used by workers.
serverTLSCreds, err := rootCA.NewServerTLSCredentials(tlsKeyPair)
if err != nil {
Expand Down
66 changes: 42 additions & 24 deletions ca/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ func TestDownloadRootCASuccess(t *testing.T) {
rootCA, err := ca.DownloadRootCA(tc.Context, tc.Paths.RootCA, tc.WorkerToken, tc.ConnBroker)
require.NoError(t, err)
require.NotNil(t, rootCA.Pool)
require.NotNil(t, rootCA.Cert)
require.Nil(t, rootCA.Signer)
require.False(t, rootCA.CanSign())
require.Equal(t, tc.RootCA.Cert, rootCA.Cert)
require.NotNil(t, rootCA.Certs)
_, err = rootCA.Signer()
require.Equal(t, err, ca.ErrNoValidSigner)
require.Equal(t, tc.RootCA.Certs, rootCA.Certs)

// Remove the CA cert
os.RemoveAll(tc.Paths.RootCA.Cert)
Expand All @@ -47,10 +47,10 @@ func TestDownloadRootCASuccess(t *testing.T) {
rootCA, err = ca.DownloadRootCA(tc.Context, tc.Paths.RootCA, "", tc.ConnBroker)
require.NoError(t, err)
require.NotNil(t, rootCA.Pool)
require.NotNil(t, rootCA.Cert)
require.Nil(t, rootCA.Signer)
require.False(t, rootCA.CanSign())
require.Equal(t, tc.RootCA.Cert, rootCA.Cert)
require.NotNil(t, rootCA.Certs)
_, err = rootCA.Signer()
require.Equal(t, err, ca.ErrNoValidSigner)
require.Equal(t, tc.RootCA.Certs, rootCA.Certs)
}

func TestDownloadRootCAWrongCAHash(t *testing.T) {
Expand Down Expand Up @@ -140,17 +140,19 @@ func TestCreateSecurityConfigNoCerts(t *testing.T) {
func TestLoadSecurityConfigExpiredCert(t *testing.T) {
tc := testutils.NewTestCA(t)
defer tc.Stop()
s, err := tc.RootCA.Signer()
require.NoError(t, err)

krw := ca.NewKeyReadWriter(tc.Paths.Node, nil, nil)
now := time.Now()

_, err := tc.RootCA.IssueAndSaveNewCertificates(krw, "cn", "ou", "org")
_, err = tc.RootCA.IssueAndSaveNewCertificates(krw, "cn", "ou", "org")
require.NoError(t, err)
certBytes, _, err := krw.Read()
require.NoError(t, err)

// A cert that is not yet valid is not valid even if expiry is allowed
invalidCert := testutils.ReDateCert(t, certBytes, tc.RootCA.Cert, tc.RootCA.Signer.Key, now.Add(time.Hour), now.Add(time.Hour*2))
invalidCert := testutils.ReDateCert(t, certBytes, tc.RootCA.Certs, s.Key, now.Add(time.Hour), now.Add(time.Hour*2))
require.NoError(t, ioutil.WriteFile(tc.Paths.Node.Cert, invalidCert, 0700))

_, err = ca.LoadSecurityConfig(tc.Context, tc.RootCA, krw, false)
Expand All @@ -162,7 +164,7 @@ func TestLoadSecurityConfigExpiredCert(t *testing.T) {
require.IsType(t, x509.CertificateInvalidError{}, errors.Cause(err))

// a cert that is expired is not valid if expiry is not allowed
invalidCert = testutils.ReDateCert(t, certBytes, tc.RootCA.Cert, tc.RootCA.Signer.Key, now.Add(-2*time.Minute), now.Add(-1*time.Minute))
invalidCert = testutils.ReDateCert(t, certBytes, tc.RootCA.Certs, s.Key, now.Add(-2*time.Minute), now.Add(-1*time.Minute))
require.NoError(t, ioutil.WriteFile(tc.Paths.Node.Cert, invalidCert, 0700))

_, err = ca.LoadSecurityConfig(tc.Context, tc.RootCA, krw, false)
Expand Down Expand Up @@ -245,7 +247,7 @@ func TestLoadSecurityConfigIntermediates(t *testing.T) {
paths := ca.NewConfigPaths(tempdir)
krw := ca.NewKeyReadWriter(paths.Node, nil, nil)

rootCA, err := ca.NewRootCA(testutils.ECDSACertChain[2], nil, ca.DefaultNodeCertExpiration, nil)
rootCA, err := ca.NewRootCA(testutils.ECDSACertChain[2], nil, nil, ca.DefaultNodeCertExpiration, nil)
require.NoError(t, err)

// loading the incomplete chain fails
Expand Down Expand Up @@ -288,7 +290,7 @@ func TestSecurityConfigUpdateRootCA(t *testing.T) {
// create the "original" security config, and we'll update it to trust the test server's
cert, key, err := testutils.CreateRootCertAndKey("root1")
require.NoError(t, err)
rootCA, err := ca.NewRootCA(cert, key, ca.DefaultNodeCertExpiration, nil)
rootCA, err := ca.NewRootCA(cert, cert, key, ca.DefaultNodeCertExpiration, nil)
require.NoError(t, err)

tempdir, err := ioutil.TempDir("", "test-security-config-update")
Expand Down Expand Up @@ -333,9 +335,11 @@ func TestSecurityConfigUpdateRootCA(t *testing.T) {
req := ca.PrepareCSR(csr, "cn", ca.ManagerRole, secConfig.ClientTLSCreds.Organization())

externalServer := tc.ExternalSigningServer
tcSigner, err := tc.RootCA.Signer()
require.NoError(t, err)
if testutils.External {
// stop the external server and create a new one because the external server actually has to trust our client certs as well.
updatedRoot, err := ca.NewRootCA(append(tc.RootCA.Cert, cert...), tc.RootCA.Signer.Key, ca.DefaultNodeCertExpiration, nil)
updatedRoot, err := ca.NewRootCA(append(tc.RootCA.Certs, cert...), tcSigner.Cert, tcSigner.Key, ca.DefaultNodeCertExpiration, nil)
require.NoError(t, err)
externalServer, err = testutils.NewExternalSigningServer(updatedRoot, tc.TempDir)
require.NoError(t, err)
Expand All @@ -350,7 +354,9 @@ func TestSecurityConfigUpdateRootCA(t *testing.T) {

// update the root CA on the "original"" security config to support both the old root
// and the "new root" (the testing CA root)
err = secConfig.UpdateRootCA(append(rootCA.Cert, tc.RootCA.Cert...), rootCA.Signer.Key, ca.DefaultNodeCertExpiration)
rSigner, err := rootCA.Signer()
require.NoError(t, err)
err = secConfig.UpdateRootCA(append(rootCA.Certs, tc.RootCA.Certs...), rSigner.Key, ca.DefaultNodeCertExpiration)
require.NoError(t, err)

// can now connect to the test CA using our modified security config, and can cannect to our server using
Expand Down Expand Up @@ -397,10 +403,10 @@ func TestRenewTLSConfigUpdateRootCARace(t *testing.T) {
go func() {
defer close(done1)
var key []byte
if rootCA.Signer != nil {
key = rootCA.Signer.Key
if signer, err := rootCA.Signer(); err == nil {
key = signer.Key
}
require.NoError(t, secConfig.UpdateRootCA(append(rootCA.Cert, cert...), key, ca.DefaultNodeCertExpiration))
require.NoError(t, secConfig.UpdateRootCA(append(rootCA.Certs, cert...), key, ca.DefaultNodeCertExpiration))
}()

go func() {
Expand Down Expand Up @@ -428,6 +434,8 @@ func TestRenewTLSConfigWorker(t *testing.T) {

tc := testutils.NewTestCA(t)
defer tc.Stop()
s, err := tc.RootCA.Signer()
require.NoError(t, err)

ctx, cancel := context.WithCancel(context.Background())
defer cancel()
Expand All @@ -439,9 +447,11 @@ func TestRenewTLSConfigWorker(t *testing.T) {
// Create a new RootCA, and change the policy to issue 6 minute certificates
// Because of the default backdate of 5 minutes, this issues certificates
// valid for 1 minute.
newRootCA, err := ca.NewRootCA(tc.RootCA.Cert, tc.RootCA.Signer.Key, ca.DefaultNodeCertExpiration, nil)
newRootCA, err := ca.NewRootCA(tc.RootCA.Certs, s.Cert, s.Key, ca.DefaultNodeCertExpiration, nil)
assert.NoError(t, err)
newRootCA.Signer.SetPolicy(&cfconfig.Signing{
newSigner, err := newRootCA.Signer()
require.NoError(t, err)
newSigner.SetPolicy(&cfconfig.Signing{
Default: &cfconfig.SigningProfile{
Usage: []string{"signing", "key encipherment", "server auth", "client auth"},
Expiry: 6 * time.Minute,
Expand Down Expand Up @@ -482,6 +492,8 @@ func TestRenewTLSConfigManager(t *testing.T) {

tc := testutils.NewTestCA(t)
defer tc.Stop()
s, err := tc.RootCA.Signer()
assert.NoError(t, err)

ctx, cancel := context.WithCancel(context.Background())
defer cancel()
Expand All @@ -493,9 +505,11 @@ func TestRenewTLSConfigManager(t *testing.T) {
// Create a new RootCA, and change the policy to issue 6 minute certificates
// Because of the default backdate of 5 minutes, this issues certificates
// valid for 1 minute.
newRootCA, err := ca.NewRootCA(tc.RootCA.Cert, tc.RootCA.Signer.Key, ca.DefaultNodeCertExpiration, nil)
newRootCA, err := ca.NewRootCA(tc.RootCA.Certs, s.Cert, s.Key, ca.DefaultNodeCertExpiration, nil)
assert.NoError(t, err)
newSigner, err := newRootCA.Signer()
assert.NoError(t, err)
newRootCA.Signer.SetPolicy(&cfconfig.Signing{
newSigner.SetPolicy(&cfconfig.Signing{
Default: &cfconfig.SigningProfile{
Usage: []string{"signing", "key encipherment", "server auth", "client auth"},
Expiry: 6 * time.Minute,
Expand Down Expand Up @@ -538,6 +552,8 @@ func TestRenewTLSConfigWithNoNode(t *testing.T) {

tc := testutils.NewTestCA(t)
defer tc.Stop()
s, err := tc.RootCA.Signer()
require.NoError(t, err)

ctx, cancel := context.WithCancel(context.Background())
defer cancel()
Expand All @@ -549,9 +565,11 @@ func TestRenewTLSConfigWithNoNode(t *testing.T) {
// Create a new RootCA, and change the policy to issue 6 minute certificates.
// Because of the default backdate of 5 minutes, this issues certificates
// valid for 1 minute.
newRootCA, err := ca.NewRootCA(tc.RootCA.Cert, tc.RootCA.Signer.Key, ca.DefaultNodeCertExpiration, nil)
newRootCA, err := ca.NewRootCA(tc.RootCA.Certs, s.Cert, s.Key, ca.DefaultNodeCertExpiration, nil)
assert.NoError(t, err)
newSigner, err := newRootCA.Signer()
assert.NoError(t, err)
newRootCA.Signer.SetPolicy(&cfconfig.Signing{
newSigner.SetPolicy(&cfconfig.Signing{
Default: &cfconfig.SigningProfile{
Usage: []string{"signing", "key encipherment", "server auth", "client auth"},
Expiry: 6 * time.Minute,
Expand Down
19 changes: 6 additions & 13 deletions ca/external.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"github.com/cloudflare/cfssl/api"
"github.com/cloudflare/cfssl/config"
"github.com/cloudflare/cfssl/csr"
"github.com/cloudflare/cfssl/helpers"
"github.com/cloudflare/cfssl/signer"
"github.com/pkg/errors"
"golang.org/x/net/context"
Expand Down Expand Up @@ -107,20 +106,14 @@ func (eca *ExternalCA) Sign(ctx context.Context, req signer.SignRequest) (cert [
// CrossSignRootCA takes a RootCA object, generates a CA CSR, sends a signing request with the CA CSR to the external
// CFSSL API server in order to obtain a cross-signed root
func (eca *ExternalCA) CrossSignRootCA(ctx context.Context, rca RootCA) ([]byte, error) {
if !rca.CanSign() {
return nil, errors.Wrap(ErrNoValidSigner, "cannot generate CSR for a cross-signed root")
}
rootCert, err := helpers.ParseCertificatePEM(rca.Cert)
if err != nil {
return nil, errors.Wrap(err, "could not parse CA certificate")
}
rootSigner, err := helpers.ParsePrivateKeyPEM(rca.Signer.Key)
if err != nil {
return nil, errors.Wrap(err, "could not parse old CA key")
}
// ExtractCertificateRequest generates a new key request, and we want to continue to use the old
// key. However, ExtractCertificateRequest will also convert the pkix.Name to csr.Name, which we
// need in order to generate a signing request
rcaSigner, err := rca.Signer()
if err != nil {
return nil, err
}
rootCert := rcaSigner.parsedCert
cfCSRObj := csr.ExtractCertificateRequest(rootCert)

der, err := x509.CreateCertificateRequest(cryptorand.Reader, &x509.CertificateRequest{
Expand All @@ -132,7 +125,7 @@ func (eca *ExternalCA) CrossSignRootCA(ctx context.Context, rca RootCA) ([]byte,
DNSNames: rootCert.DNSNames,
EmailAddresses: rootCert.EmailAddresses,
IPAddresses: rootCert.IPAddresses,
}, rootSigner)
}, rcaSigner.cryptoSigner)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion ca/external_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestExternalCACrossSign(t *testing.T) {
cert2, key2, err := testutils.CreateRootCertAndKey("rootCN2")
require.NoError(t, err)

rootCA2, err := ca.NewRootCA(cert2, key2, ca.DefaultNodeCertExpiration, nil)
rootCA2, err := ca.NewRootCA(cert2, cert2, key2, ca.DefaultNodeCertExpiration, nil)
require.NoError(t, err)

krw := ca.NewKeyReadWriter(paths.Node, nil, nil)
Expand Down
2 changes: 1 addition & 1 deletion ca/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ func (s *Server) GetRootCACertificate(ctx context.Context, request *api.GetRootC
})

return &api.GetRootCACertificateResponse{
Certificate: s.securityConfig.RootCA().Cert,
Certificate: s.securityConfig.RootCA().Certs,
}, nil
}

Expand Down
Loading