From 7be40406fa217c4b8783d30fc6f2cbfbc550c2b1 Mon Sep 17 00:00:00 2001
From: Dan Upton <daniel@floppy.co>
Date: Tue, 5 Apr 2022 19:16:20 +0100
Subject: [PATCH] ca: move ConnectCA.Sign authorization logic to CAManager
 (#12609)

OSS sync of enterprise changes at 8d6fd125
---
 agent/consul/connect_ca_endpoint.go | 40 +--------------------------
 agent/consul/leader_connect_ca.go   | 43 +++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 39 deletions(-)

diff --git a/agent/consul/connect_ca_endpoint.go b/agent/consul/connect_ca_endpoint.go
index bf68611fca..c325ff1237 100644
--- a/agent/consul/connect_ca_endpoint.go
+++ b/agent/consul/connect_ca_endpoint.go
@@ -8,7 +8,6 @@ import (
 	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/go-memdb"
 
-	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/consul/state"
 	"github.com/hashicorp/consul/agent/structs"
@@ -145,54 +144,17 @@ func (s *ConnectCA) Sign(
 		return err
 	}
 
-	// Parse the CSR
 	csr, err := connect.ParseCSR(args.CSR)
 	if err != nil {
 		return err
 	}
 
-	// Parse the SPIFFE ID
-	spiffeID, err := connect.ParseCertURI(csr.URIs[0])
-	if err != nil {
-		return err
-	}
-
-	// Verify that the ACL token provided has permission to act as this service
 	authz, err := s.srv.ResolveToken(args.Token)
 	if err != nil {
 		return err
 	}
 
-	var authzContext acl.AuthorizerContext
-	var entMeta structs.EnterpriseMeta
-
-	serviceID, isService := spiffeID.(*connect.SpiffeIDService)
-	agentID, isAgent := spiffeID.(*connect.SpiffeIDAgent)
-	if !isService && !isAgent {
-		return fmt.Errorf("SPIFFE ID in CSR must be a service or agent ID")
-	}
-
-	if isService {
-		entMeta.Merge(serviceID.GetEnterpriseMeta())
-		entMeta.FillAuthzContext(&authzContext)
-		if err := authz.ToAllowAuthorizer().ServiceWriteAllowed(serviceID.Service, &authzContext); err != nil {
-			return err
-		}
-
-		// Verify that the DC in the service URI matches us. We might relax this
-		// requirement later but being restrictive for now is safer.
-		if serviceID.Datacenter != s.srv.config.Datacenter {
-			return fmt.Errorf("SPIFFE ID in CSR from a different datacenter: %s, "+
-				"we are %s", serviceID.Datacenter, s.srv.config.Datacenter)
-		}
-	} else if isAgent {
-		agentID.GetEnterpriseMeta().FillAuthzContext(&authzContext)
-		if err := authz.ToAllowAuthorizer().NodeWriteAllowed(agentID.Agent, &authzContext); err != nil {
-			return err
-		}
-	}
-
-	cert, err := s.srv.caManager.SignCertificate(csr, spiffeID)
+	cert, err := s.srv.caManager.AuthorizeAndSignCertificate(csr, authz)
 	if err != nil {
 		return err
 	}
diff --git a/agent/consul/leader_connect_ca.go b/agent/consul/leader_connect_ca.go
index ed5587ff57..899ff494a0 100644
--- a/agent/consul/leader_connect_ca.go
+++ b/agent/consul/leader_connect_ca.go
@@ -15,6 +15,7 @@ import (
 	uuid "github.com/hashicorp/go-uuid"
 	"golang.org/x/time/rate"
 
+	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/lib/semaphore"
 
 	"github.com/hashicorp/consul/agent/connect"
@@ -1375,6 +1376,48 @@ func (l *connectSignRateLimiter) getCSRRateLimiterWithLimit(limit rate.Limit) *r
 	return l.csrRateLimiter
 }
 
+// AuthorizeAndSignCertificate signs a leaf certificate for the service or agent
+// identified by the SPIFFE ID in the given CSR's SAN. It performs authorization
+// using the given acl.Authorizer.
+func (c *CAManager) AuthorizeAndSignCertificate(csr *x509.CertificateRequest, authz acl.Authorizer) (*structs.IssuedCert, error) {
+	// Parse the SPIFFE ID from the CSR SAN.
+	if len(csr.URIs) == 0 {
+		return nil, errors.New("CSR SAN does not contain a SPIFFE ID")
+	}
+	spiffeID, err := connect.ParseCertURI(csr.URIs[0])
+	if err != nil {
+		return nil, err
+	}
+
+	// Perform authorization.
+	var authzContext acl.AuthorizerContext
+	allow := authz.ToAllowAuthorizer()
+	switch v := spiffeID.(type) {
+	case *connect.SpiffeIDService:
+		v.GetEnterpriseMeta().FillAuthzContext(&authzContext)
+		if err := allow.ServiceWriteAllowed(v.Service, &authzContext); err != nil {
+			return nil, err
+		}
+
+		// Verify that the DC in the service URI matches us. We might relax this
+		// requirement later but being restrictive for now is safer.
+		dc := c.serverConf.Datacenter
+		if v.Datacenter != dc {
+			return nil, fmt.Errorf("SPIFFE ID in CSR from a different datacenter: %s, "+
+				"we are %s", v.Datacenter, dc)
+		}
+	case *connect.SpiffeIDAgent:
+		v.GetEnterpriseMeta().FillAuthzContext(&authzContext)
+		if err := allow.NodeWriteAllowed(v.Agent, &authzContext); err != nil {
+			return nil, err
+		}
+	default:
+		return nil, errors.New("SPIFFE ID in CSR must be a service or agent ID")
+	}
+
+	return c.SignCertificate(csr, spiffeID)
+}
+
 func (c *CAManager) SignCertificate(csr *x509.CertificateRequest, spiffeID connect.CertURI) (*structs.IssuedCert, error) {
 	provider, caRoot := c.getCAProvider()
 	if provider == nil {