Skip to content
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.

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

connect: Enable renewing the intermediate cert in the primary DC #8784

Merged
merged 6 commits into from
Oct 9, 2020
Merged
Changes from 1 commit
Commits
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
7 changes: 3 additions & 4 deletions agent/connect/ca/provider_consul.go
Original file line number Diff line number Diff line change
@@ -21,14 +21,13 @@ import (
"github.com/hashicorp/go-hclog"
)

const (

var (
// NotBefore will be CertificateTimeDriftBuffer in the past to account for
// time drift between different servers.
CertificateTimeDriftBuffer = time.Minute
)

var ErrNotInitialized = errors.New("provider not initialized")
ErrNotInitialized = errors.New("provider not initialized")
)

type ConsulProvider struct {
Delegate ConsulProviderStateDelegate
15 changes: 14 additions & 1 deletion agent/connect/ca/provider_vault.go
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@ import (
"io/ioutil"
"net/http"
"strings"
"time"

"github.com/hashicorp/consul/agent/connect"
"github.com/hashicorp/consul/agent/structs"
@@ -476,8 +477,20 @@ func (v *VaultProvider) SignIntermediate(csr *x509.CertificateRequest) (string,
// CrossSignCA takes a CA certificate and cross-signs it to form a trust chain
// back to our active root.
func (v *VaultProvider) CrossSignCA(cert *x509.Certificate) (string, error) {
rootPEM, err := v.ActiveRoot()
if err != nil {
return "", err
}
rootCert, err := connect.ParseCert(rootPEM)
if err != nil {
return "", fmt.Errorf("error parsing root cert: %v", err)
}
if rootCert.NotAfter.Before(time.Now()) {
return "", fmt.Errorf("root certificate is expired")
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that means that Vault would happily sign a cert with an expired root CA?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, it's because the sign-self-issued endpoint does minimal validation (only checks for CA cert and self-issued) so it was possible for the cert to expire and the provider would still cross-sign with it.


var pemBuf bytes.Buffer
err := pem.Encode(&pemBuf, &pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw})
err = pem.Encode(&pemBuf, &pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw})
if err != nil {
return "", err
}
4 changes: 2 additions & 2 deletions agent/consul/leader_connect.go
Original file line number Diff line number Diff line change
@@ -516,6 +516,7 @@ func (s *Server) persistNewRoot(provider ca.Provider, newActiveRoot *structs.CAR
// which means it must never take that lock itself or call anything that does.
func (s *Server) getIntermediateCAPrimary(provider ca.Provider, newActiveRoot *structs.CARoot) error {
connectLogger := s.loggers.Named(logging.Connect)
// Generate and sign an intermediate cert using the root CA.
intermediatePEM, err := provider.GenerateIntermediate()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This generates and signs an intermediate cert. I had to look that up which is why I am leaving a note here for others.

if err != nil {
return fmt.Errorf("error generating new intermediate cert: %v", err)
@@ -531,7 +532,7 @@ func (s *Server) getIntermediateCAPrimary(provider ca.Provider, newActiveRoot *s
newActiveRoot.IntermediateCerts = append(newActiveRoot.IntermediateCerts, intermediatePEM)
newActiveRoot.SigningKeyID = connect.EncodeSigningKeyID(intermediateCert.SubjectKeyId)

connectLogger.Info("generated new intermediate certificate in primary datacenter")
connectLogger.Info("generated new intermediate certificate for primary datacenter")
return nil
}

@@ -738,7 +739,6 @@ func (s *Server) intermediateCertRenewalWatch(ctx context.Context) error {

if lessThanHalfTimePassed(time.Now(), intermediateCert.NotBefore.Add(ca.CertificateTimeDriftBuffer),
intermediateCert.NotAfter) {
//connectLogger.Info("checked time passed", intermediateCert.NotBefore.Add(ca.CertificateTimeDriftBuffer), intermediateCert.NotAfter)
return nil
}

7 changes: 5 additions & 2 deletions agent/consul/leader_connect_test.go
Original file line number Diff line number Diff line change
@@ -185,11 +185,14 @@ func TestLeader_PrimaryCA_IntermediateRenew(t *testing.T) {
// no parallel execution because we change globals
origInterval := structs.IntermediateCertRenewInterval
origMinTTL := structs.MinLeafCertTTL
origDriftBuffer := ca.CertificateTimeDriftBuffer
defer func() {
structs.IntermediateCertRenewInterval = origInterval
structs.MinLeafCertTTL = origMinTTL
ca.CertificateTimeDriftBuffer = origDriftBuffer
}()

ca.CertificateTimeDriftBuffer = 30 * time.Second
structs.IntermediateCertRenewInterval = time.Millisecond
structs.MinLeafCertTTL = time.Second
require := require.New(t)
@@ -207,15 +210,15 @@ func TestLeader_PrimaryCA_IntermediateRenew(t *testing.T) {
"Token": testVault.RootToken,
"RootPKIPath": "pki-root/",
"IntermediatePKIPath": "pki-intermediate/",
"LeafCertTTL": "5s",
"LeafCertTTL": "1s",
// The retry loop only retries for 7sec max and
// the ttl needs to be below so that it
// triggers definitely.
// Since certs are created so that they are
// valid from 1minute in the past, we need to
// account for that, otherwise it will be
// expired immediately.
"IntermediateCertTTL": "15s",
"IntermediateCertTTL": "5s",
},
}
})