Checks do not modify services when not needed

CHANGELOG.md

@@ -1,5 +1,9 @@
 ## UNRELEASED
 
+BUG FIXES:
+
+* agent: (Consul Enterprise) Fixed an issue where the `non_voting_server` setting could be ignored when bootstrapping the cluster. [[GH-4699](https://github.com/hashicorp/consul/pull/4699)]
+
 ## 1.2.3 (September 13, 2018)
 
 FEATURES:

GNUmakefile

@@ -162,7 +162,7 @@ test-internal:
 	@# hide it from travis as it exceeds their log limits and causes job to be
 	@# terminated (over 4MB and over 10k lines in the UI). We need to output
 	@# _something_ to stop them terminating us due to inactivity...
-	{ go test $(GOTEST_FLAGS) -tags '$(GOTAGS)' $(GOTEST_PKGS) 2>&1 ; echo $$? > exit-code ; } | tee test.log | egrep '^(ok|FAIL|panic:|--- FAIL)'
+	{ go test -v $(GOTEST_FLAGS) -tags '$(GOTAGS)' $(GOTEST_PKGS) 2>&1 ; echo $$? > exit-code ; } | tee test.log | egrep '^(ok|FAIL|panic:|--- FAIL|--- PASS)'
 	@echo "Exit code: $$(cat exit-code)"
 	@# This prints all the race report between ====== lines
 	@awk '/^WARNING: DATA RACE/ {do_print=1; print "=================="} do_print==1 {print} /^={10,}/ {do_print=0}' test.log || true

agent/connect/ca/provider.go

@@ -22,6 +22,16 @@ type Provider interface {
 	// ActiveIntermediate()
 	ActiveRoot() (string, error)
 
+	// GenerateIntermediateCSR generates a CSR for an intermediate CA
+	// certificate, to be signed by the root of another datacenter. If isRoot was
+	// set to true with Configure(), calling this is an error.
+	GenerateIntermediateCSR() (string, error)
+
+	// SetIntermediate sets the provider to use the given intermediate certificate
+	// as well as the root it was signed by. This completes the initialization for
+	// a provider where isRoot was set to false in Configure().
+	SetIntermediate(intermediatePEM, rootPEM string) error
+
 	// ActiveIntermediate returns the current signing cert used by this provider
 	// for generating SPIFFE leaf certs. Note that this must not change except
 	// when Consul requests the change via GenerateIntermediate. Changing the
@@ -41,6 +51,12 @@ type Provider interface {
 	// intemediate and any cross-signed intermediates managed by Consul.
 	Sign(*x509.CertificateRequest) (string, error)
 
+	// SignIntermediate will validate the CSR to ensure the trust domain in the
+	// URI SAN matches the local one and that basic constraints for a CA certificate
+	// are met. It should return a signed CA certificate with a path length constraint
+	// of 0 to ensure that the certificate cannot be used to generate further CA certs.
+	SignIntermediate(*x509.CertificateRequest) (string, error)
+
 	// CrossSignCA must accept a CA certificate from another CA provider
 	// and cross sign it exactly as it is such that it forms a chain back the the
 	// CAProvider's current root. Specifically, the Distinguished Name, Subject

agent/connect/ca/provider_consul.go

@@ -25,9 +25,12 @@ var ErrNotInitialized = errors.New("provider not initialized")
 type ConsulProvider struct {
 	Delegate ConsulProviderStateDelegate
 
-	config *structs.ConsulCAProviderConfig
-	id     string
-	isRoot bool
+	config    *structs.ConsulCAProviderConfig
+	id        string
+	clusterID string
+	isRoot    bool
+	spiffeID  *connect.SpiffeIDSigning
+
 	sync.RWMutex
 }
 
@@ -44,9 +47,11 @@ func (c *ConsulProvider) Configure(clusterID string, isRoot bool, rawConfig map[
 		return err
 	}
 	c.config = config
-	c.isRoot = isRoot
 	hash := sha256.Sum256([]byte(fmt.Sprintf("%s,%s,%v", config.PrivateKey, config.RootCert, isRoot)))
 	c.id = strings.Replace(fmt.Sprintf("% x", hash), " ", ":", -1)
+	c.clusterID = clusterID
+	c.isRoot = isRoot
+	c.spiffeID = connect.SpiffeIDSigningForCluster(&structs.CAConfiguration{ClusterID: clusterID})
 
 	// Exit early if the state store has an entry for this provider's config.
 	_, providerState, err := c.Delegate.State().CAProviderState(c.id)
@@ -107,8 +112,7 @@ func (c *ConsulProvider) Configure(clusterID string, isRoot bool, rawConfig map[
 
 // ActiveRoot returns the active root CA certificate.
 func (c *ConsulProvider) ActiveRoot() (string, error) {
-	state := c.Delegate.State()
-	_, providerState, err := state.CAProviderState(c.id)
+	_, providerState, err := c.getState()
 	if err != nil {
 		return "", err
 	}
@@ -119,15 +123,11 @@ func (c *ConsulProvider) ActiveRoot() (string, error) {
 // GenerateRoot initializes a new root certificate and private key
 // if needed.
 func (c *ConsulProvider) GenerateRoot() error {
-	state := c.Delegate.State()
-	idx, providerState, err := state.CAProviderState(c.id)
+	idx, providerState, err := c.getState()
 	if err != nil {
 		return err
 	}
 
-	if providerState == nil {
-		return ErrNotInitialized
-	}
 	if !c.isRoot {
 		return fmt.Errorf("provider is not the root certificate authority")
 	}
@@ -170,10 +170,129 @@ func (c *ConsulProvider) GenerateRoot() error {
 	return nil
 }
 
+// GenerateIntermediateCSR creates a private key and generates a CSR
+// for another datacenter's root to sign.
+func (c *ConsulProvider) GenerateIntermediateCSR() (string, error) {
+	_, providerState, err := c.getState()
+	if err != nil {
+		return "", err
+	}
+
+	if c.isRoot {
+		return "", fmt.Errorf("provider is the root certificate authority, " +
+			"cannot generate an intermediate CSR")
+	}
+
+	// Create a new private key and CSR.
+	signer, pk, err := connect.GeneratePrivateKey()
+	if err != nil {
+		return "", err
+	}
+
+	csr, err := connect.CreateCACSR(c.spiffeID, signer)
+	if err != nil {
+		return "", err
+	}
+
+	// Write the new provider state to the store.
+	newState := *providerState
+	newState.PrivateKey = pk
+	args := &structs.CARequest{
+		Op:            structs.CAOpSetProviderState,
+		ProviderState: &newState,
+	}
+	if err := c.Delegate.ApplyCARequest(args); err != nil {
+		return "", err
+	}
+
+	return csr, nil
+}
+
+// SetIntermediate validates that the given intermediate is for the right private key
+// and writes the given intermediate and root certificates to the state.
+func (c *ConsulProvider) SetIntermediate(intermediatePEM, rootPEM string) error {
+	_, providerState, err := c.getState()
+	if err != nil {
+		return err
+	}
+
+	if c.isRoot {
+		return fmt.Errorf("cannot set an intermediate using another root in the primary datacenter")
+	}
+
+	// Get the key from the incoming intermediate cert so we can compare it
+	// to the currently stored key.
+	intermediate, err := connect.ParseCert(intermediatePEM)
+	if err != nil {
+		return fmt.Errorf("error parsing intermediate PEM: %v", err)
+	}
+	privKey, err := connect.ParseSigner(providerState.PrivateKey)
+	if err != nil {
+		return err
+	}
+
+	// Compare the two keys to make sure they match.
+	b1, err := x509.MarshalPKIXPublicKey(intermediate.PublicKey)
+	if err != nil {
+		return err
+	}
+	b2, err := x509.MarshalPKIXPublicKey(privKey.Public())
+	if err != nil {
+		return err
+	}
+	if !bytes.Equal(b1, b2) {
+		return fmt.Errorf("intermediate cert is for a different private key")
+	}
+
+	// Validate the remaining fields and make sure the intermediate validates against
+	// the given root cert.
+	if !intermediate.IsCA {
+		return fmt.Errorf("intermediate is not a CA certificate")
+	}
+	if uriCount := len(intermediate.URIs); uriCount != 1 {
+		return fmt.Errorf("incoming intermediate cert has unexpected number of URIs: %d", uriCount)
+	}
+	if got, want := intermediate.URIs[0].String(), c.spiffeID.URI().String(); got != want {
+		return fmt.Errorf("incoming cert URI %q does not match current URI: %q", got, want)
+	}
+
+	pool := x509.NewCertPool()
+	pool.AppendCertsFromPEM([]byte(rootPEM))
+	_, err = intermediate.Verify(x509.VerifyOptions{
+		Roots: pool,
+	})
+	if err != nil {
+		return fmt.Errorf("could not verify intermediate cert against root: %v", err)
+	}
+
+	// Update the state
+	newState := *providerState
+	newState.IntermediateCert = intermediatePEM
+	newState.RootCert = rootPEM
+	args := &structs.CARequest{
+		Op:            structs.CAOpSetProviderState,
+		ProviderState: &newState,
+	}
+	if err := c.Delegate.ApplyCARequest(args); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 // We aren't maintaining separate root/intermediate CAs for the builtin
 // provider, so just return the root.
 func (c *ConsulProvider) ActiveIntermediate() (string, error) {
-	return c.ActiveRoot()
+	if c.isRoot {
+		return c.ActiveRoot()
+	}
+
+	_, providerState, err := c.getState()
+	if err != nil {
+		return "", err
+	}
+
+	return providerState.IntermediateCert, nil
 }
 
 // We aren't maintaining separate root/intermediate CAs for the builtin
@@ -216,7 +335,7 @@ func (c *ConsulProvider) Sign(csr *x509.CertificateRequest) (string, error) {
 		return "", err
 	}
 	if signer == nil {
-		return "", fmt.Errorf("error signing cert: Consul CA not initialized yet")
+		return "", ErrNotInitialized
 	}
 	keyId, err := connect.KeyId(signer.Public())
 	if err != nil {
@@ -234,7 +353,11 @@ func (c *ConsulProvider) Sign(csr *x509.CertificateRequest) (string, error) {
 	}
 
 	// Parse the CA cert
-	caCert, err := connect.ParseCert(providerState.RootCert)
+	certPEM, err := c.ActiveIntermediate()
+	if err != nil {
+		return "", err
+	}
+	caCert, err := connect.ParseCert(certPEM)
 	if err != nil {
 		return "", fmt.Errorf("error parsing CA cert: %s", err)
 	}
@@ -290,6 +413,93 @@ func (c *ConsulProvider) Sign(csr *x509.CertificateRequest) (string, error) {
 	return buf.String(), nil
 }
 
+// SignIntermediate will validate the CSR to ensure the trust domain in the
+// URI SAN matches the local one and that basic constraints for a CA certificate
+// are met. It should return a signed CA certificate with a path length constraint
+// of 0 to ensure that the certificate cannot be used to generate further CA certs.
+func (c *ConsulProvider) SignIntermediate(csr *x509.CertificateRequest) (string, error) {
+	idx, providerState, err := c.getState()
+	if err != nil {
+		return "", err
+	}
+
+	if uriCount := len(csr.URIs); uriCount != 1 {
+		return "", fmt.Errorf("incoming CSR has unexpected number of URIs: %d", uriCount)
+	}
+	certURI, err := connect.ParseCertURI(csr.URIs[0])
+	if err != nil {
+		return "", err
+	}
+
+	// Verify that the trust domain is valid.
+	if !c.spiffeID.CanSign(certURI) {
+		return "", fmt.Errorf("incoming CSR domain %q is not valid for our domain %q",
+			certURI.URI().String(), c.spiffeID.URI().String())
+	}
+
+	// Get the signing private key.
+	signer, err := connect.ParseSigner(providerState.PrivateKey)
+	if err != nil {
+		return "", err
+	}
+	subjectKeyId, err := connect.KeyId(csr.PublicKey)
+	if err != nil {
+		return "", err
+	}
+
+	// Parse the CA cert
+	caCert, err := connect.ParseCert(providerState.RootCert)
+	if err != nil {
+		return "", fmt.Errorf("error parsing CA cert: %s", err)
+	}
+
+	// Cert template for generation
+	sn := &big.Int{}
+	sn.SetUint64(idx + 1)
+	// Sign the certificate valid from 1 minute in the past, this helps it be
+	// accepted right away even when nodes are not in close time sync accross the
+	// cluster. A minute is more than enough for typical DC clock drift.
+	effectiveNow := time.Now().Add(-1 * time.Minute)
+	template := x509.Certificate{
+		SerialNumber:          sn,
+		Subject:               csr.Subject,
+		URIs:                  csr.URIs,
+		Signature:             csr.Signature,
+		SignatureAlgorithm:    csr.SignatureAlgorithm,
+		PublicKeyAlgorithm:    csr.PublicKeyAlgorithm,
+		PublicKey:             csr.PublicKey,
+		BasicConstraintsValid: true,
+		KeyUsage: x509.KeyUsageCertSign |
+			x509.KeyUsageCRLSign |
+			x509.KeyUsageDigitalSignature,
+		IsCA:           true,
+		MaxPathLenZero: true,
+		NotAfter:       effectiveNow.Add(365 * 24 * time.Hour),
+		NotBefore:      effectiveNow,
+		SubjectKeyId:   subjectKeyId,
+	}
+
+	// Create the certificate, PEM encode it and return that value.
+	var buf bytes.Buffer
+	bs, err := x509.CreateCertificate(
+		rand.Reader, &template, caCert, csr.PublicKey, signer)
+	if err != nil {
+		return "", fmt.Errorf("error generating certificate: %s", err)
+	}
+	err = pem.Encode(&buf, &pem.Block{Type: "CERTIFICATE", Bytes: bs})
+	if err != nil {
+		return "", fmt.Errorf("error encoding certificate: %s", err)
+	}
+
+	err = c.incrementProviderIndex(providerState)
+	if err != nil {
+		return "", err
+	}
+
+	// Set the response
+	return buf.String(), nil
+}
+
 // CrossSignCA returns the given CA cert signed by the current active root.
 func (c *ConsulProvider) CrossSignCA(cert *x509.Certificate) (string, error) {
 	c.Lock()
@@ -356,6 +566,22 @@ func (c *ConsulProvider) CrossSignCA(cert *x509.Certificate) (string, error) {
 	return buf.String(), nil
 }
 
+// getState returns the current provider state from the state delegate, and returns
+// ErrNotInitialized if no entry is found.
+func (c *ConsulProvider) getState() (uint64, *structs.CAConsulProviderState, error) {
+	state := c.Delegate.State()
+	idx, providerState, err := state.CAProviderState(c.id)
+	if err != nil {
+		return 0, nil, err
+	}
+
+	if providerState == nil {
+		return 0, nil, ErrNotInitialized
+	}
+
+	return idx, providerState, nil
+}
+
 // incrementProviderIndex does a write to increment the provider state store table index
 // used for serial numbers when generating certificates.
 func (c *ConsulProvider) incrementProviderIndex(providerState *structs.CAConsulProviderState) error {