luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

Make all Connect Cert Common Names valid FQDNs (#6423)

This commit is contained in:
Todd Radel 2019-11-11 12:11:54 -05:00 committed by Paul Banks
parent 562225cddf
commit e100fda218
10 changed files with 201 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
agent/cache-types/connect_ca_leaf.go
View File

@ -507,6 +507,7 @@ func (c *ConnectCALeaf) generateNewLeaf(req *ConnectCALeafRequest,
// Build the cert uri
var id connect.CertURI
var commonName string
if req.Service != "" {
id = &connect.SpiffeIDService{
Host: roots.TrustDomain,
@ -514,12 +515,14 @@ func (c *ConnectCALeaf) generateNewLeaf(req *ConnectCALeafRequest,
Namespace: "default",
Service: req.Service,
}
commonName = connect.ServiceCN(req.Service, roots.TrustDomain)
} else if req.Agent != "" {
id = &connect.SpiffeIDAgent{
Host: roots.TrustDomain,
Datacenter: req.Datacenter,
Agent: req.Agent,
}
commonName = connect.ServiceCN(req.Agent, roots.TrustDomain)
} else {
return result, errors.New("URI must be either service or agent")
}
@ -542,7 +545,7 @@ func (c *ConnectCALeaf) generateNewLeaf(req *ConnectCALeafRequest,
}
// Create a CSR.
csr, err := connect.CreateCSR(id, pk)
csr, err := connect.CreateCSR(id, commonName, pk)
if err != nil {
return result, err
}

31
agent/connect/ca/provider_consul.go
View File

@ -188,7 +188,13 @@ func (c *ConsulProvider) GenerateIntermediateCSR() (string, error) {
return "", err
}
csr, err := connect.CreateCACSR(c.spiffeID, signer)
uid, err := connect.CompactUID()
if err != nil {
return "", err
}
cn := connect.CACN("consul", uid, c.clusterID, c.isRoot)
csr, err := connect.CreateCACSR(c.spiffeID, cn, signer)
if err != nil {
return "", err
}
@ -313,14 +319,12 @@ func (c *ConsulProvider) Sign(csr *x509.CertificateRequest) (string, error) {
return "", err
}
subject := ""
switch id := spiffeId.(type) {
case *connect.SpiffeIDService:
subject = id.Service
case *connect.SpiffeIDAgent:
subject = id.Agent
default:
return "", fmt.Errorf("SPIFFE ID in CSR must be a service ID")
// Even though leafs should be from our own CSRs which should have the same CN
// logic as here, override anyway to account for older version clients that
// didn't include the Common Name in the CSR.
subject, err := connect.CNForCertURI(spiffeId)
if err != nil {
return "", err
}
// Parse the CA cert
@ -583,8 +587,6 @@ func (c *ConsulProvider) generateCA(privateKey string, sn uint64) (string, error
return "", fmt.Errorf("error parsing private key %q: %s", privateKey, err)
}
name := fmt.Sprintf("Consul CA %d", sn)
// The URI (SPIFFE compatible) for the cert
id := connect.SpiffeIDSigningForCluster(config)
keyId, err := connect.KeyId(privKey.Public())
@ -593,11 +595,16 @@ func (c *ConsulProvider) generateCA(privateKey string, sn uint64) (string, error
}
// Create the CA cert
uid, err := connect.CompactUID()
if err != nil {
return "", err
}
cn := connect.CACN("consul", uid, c.clusterID, c.isRoot)
serialNum := &big.Int{}
serialNum.SetUint64(sn)
template := x509.Certificate{
SerialNumber: serialNum,
Subject: pkix.Name{CommonName: name},
Subject: pkix.Name{CommonName: cn},
URIs: []*url.URL{id.URI()},
BasicConstraintsValid: true,
KeyUsage: x509.KeyUsageCertSign |

24
agent/connect/ca/provider_consul_test.go
View File

@ -143,7 +143,7 @@ func TestConsulCAProvider_SignLeaf(t *testing.T) {
require.NoError(provider.GenerateRoot())
spiffeService := &connect.SpiffeIDService{
Host: "node1",
Host: connect.TestClusterID + ".consul",
Namespace: "default",
Datacenter: "dc1",
Service: "foo",
@ -161,8 +161,8 @@ func TestConsulCAProvider_SignLeaf(t *testing.T) {
parsed, err := connect.ParseCert(cert)
require.NoError(err)
require.Equal(parsed.URIs[0], spiffeService.URI())
require.Equal(parsed.Subject.CommonName, "foo")
require.Equal(spiffeService.URI(), parsed.URIs[0])
require.Equal(connect.ServiceCN("foo", connect.TestClusterID), parsed.Subject.CommonName)
require.Equal(uint64(2), parsed.SerialNumber.Uint64())
requireNotEncoded(t, parsed.SubjectKeyId)
requireNotEncoded(t, parsed.AuthorityKeyId)
@ -187,8 +187,8 @@ func TestConsulCAProvider_SignLeaf(t *testing.T) {
parsed, err := connect.ParseCert(cert)
require.NoError(err)
require.Equal(parsed.URIs[0], spiffeService.URI())
require.Equal(parsed.Subject.CommonName, "bar")
require.Equal(spiffeService.URI(), parsed.URIs[0])
require.Equal(connect.ServiceCN("bar", connect.TestClusterID), parsed.Subject.CommonName)
require.Equal(parsed.SerialNumber.Uint64(), uint64(2))
requireNotEncoded(t, parsed.SubjectKeyId)
requireNotEncoded(t, parsed.AuthorityKeyId)
@ -199,7 +199,7 @@ func TestConsulCAProvider_SignLeaf(t *testing.T) {
}
spiffeAgent := &connect.SpiffeIDAgent{
Host: "node1",
Host: connect.TestClusterID + ".consul",
Datacenter: "dc1",
Agent: "uuid",
}
@ -216,7 +216,7 @@ func TestConsulCAProvider_SignLeaf(t *testing.T) {
parsed, err := connect.ParseCert(cert)
require.NoError(err)
require.Equal(spiffeAgent.URI(), parsed.URIs[0])
require.Equal("uuid", parsed.Subject.CommonName)
require.Equal(connect.AgentCN("uuid", connect.TestClusterID), parsed.Subject.CommonName)
require.Equal(uint64(2), parsed.SerialNumber.Uint64())
requireNotEncoded(t, parsed.SubjectKeyId)
requireNotEncoded(t, parsed.AuthorityKeyId)
@ -297,7 +297,11 @@ func testCrossSignProviders(t *testing.T, provider1, provider2 Provider) {
requireNotEncoded(t, oldRoot.AuthorityKeyId)
// AuthorityKeyID should now be the signing root's, SubjectKeyId should be kept.
require.Equal(oldRoot.AuthorityKeyId, xc.AuthorityKeyId)
require.Equal(oldRoot.SubjectKeyId, xc.AuthorityKeyId,
"newSKID=%x\nnewAKID=%x\noldSKID=%x\noldAKID=%x\nxcSKID=%x\nxcAKID=%x",
newRoot.SubjectKeyId, newRoot.AuthorityKeyId,
oldRoot.SubjectKeyId, oldRoot.AuthorityKeyId,
xc.SubjectKeyId, xc.AuthorityKeyId)
require.Equal(newRoot.SubjectKeyId, xc.SubjectKeyId)
// Subject name should not have changed.
@ -308,7 +312,7 @@ func testCrossSignProviders(t *testing.T, provider1, provider2 Provider) {
// Get a leaf cert so we can verify against the cross-signed cert.
spiffeService := &connect.SpiffeIDService{
Host: "node1",
Host: connect.TestClusterID + ".consul",
Namespace: "default",
Datacenter: "dc1",
Service: "foo",
@ -396,7 +400,7 @@ func testSignIntermediateCrossDC(t *testing.T, provider1, provider2 Provider) {
// Have provider2 sign a leaf cert and make sure the chain is correct.
spiffeService := &connect.SpiffeIDService{
Host: "node1",
Host: connect.TestClusterID + ".consul",
Namespace: "default",
Datacenter: "dc1",
Service: "foo",

19
agent/connect/ca/provider_vault.go
View File

@ -11,7 +11,6 @@ import (
"github.com/hashicorp/consul/agent/connect"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/go-uuid"
vaultapi "github.com/hashicorp/vault/api"
"github.com/mitchellh/mapstructure"
)
@ -25,7 +24,7 @@ type VaultProvider struct {
config *structs.VaultCAProviderConfig
client *vaultapi.Client
isRoot bool
clusterId string
clusterID string
spiffeID *connect.SpiffeIDSigning
}
@ -41,7 +40,7 @@ func vaultTLSConfig(config *structs.VaultCAProviderConfig) *vaultapi.TLSConfig {
}
// Configure sets up the provider using the given configuration.
func (v *VaultProvider) Configure(clusterId string, isRoot bool, rawConfig map[string]interface{}) error {
func (v *VaultProvider) Configure(clusterID string, isRoot bool, rawConfig map[string]interface{}) error {
config, err := ParseVaultCAConfig(rawConfig)
if err != nil {
return err
@ -63,8 +62,8 @@ func (v *VaultProvider) Configure(clusterId string, isRoot bool, rawConfig map[s
v.config = config
v.client = client
v.isRoot = isRoot
v.clusterId = clusterId
v.spiffeID = connect.SpiffeIDSigningForCluster(&structs.CAConfiguration{ClusterID: clusterId})
v.clusterID = clusterID
v.spiffeID = connect.SpiffeIDSigningForCluster(&structs.CAConfiguration{ClusterID: clusterID})
return nil
}
@ -98,12 +97,12 @@ func (v *VaultProvider) GenerateRoot() error {
fallthrough
case ErrBackendNotInitialized:
uuid, err := uuid.GenerateUUID()
uid, err := connect.CompactUID()
if err != nil {
return err
}
_, err = v.client.Logical().Write(v.config.RootPKIPath+"root/generate/internal", map[string]interface{}{
"common_name": fmt.Sprintf("Vault CA Root Authority %s", uuid),
"common_name": connect.CACN("vault", uid, v.clusterID, v.isRoot),
"uri_sans": v.spiffeID.URI().String(),
"key_type": v.config.PrivateKeyType,
"key_bits": v.config.PrivateKeyBits,
@ -174,8 +173,12 @@ func (v *VaultProvider) generateIntermediateCSR() (string, error) {
}
// Generate a new intermediate CSR for the root to sign.
uid, err := connect.CompactUID()
if err != nil {
return "", err
}
data, err := v.client.Logical().Write(v.config.IntermediatePKIPath+"intermediate/generate/internal", map[string]interface{}{
"common_name": "Vault CA Intermediate Authority",
"common_name": connect.CACN("vault", uid, v.clusterID, v.isRoot),
"key_type": v.config.PrivateKeyType,
"key_bits": v.config.PrivateKeyBits,
"uri_sans": v.spiffeID.URI().String(),

3
agent/connect/ca/provider_vault_test.go
View File

@ -80,7 +80,7 @@ func TestVaultCAProvider_Bootstrap(t *testing.T) {
require.NoError(err)
require.True(parsed.IsCA)
require.Len(parsed.URIs, 1)
require.Equal(parsed.URIs[0].String(), fmt.Sprintf("spiffe://%s.consul", provider.clusterId))
require.Equal(fmt.Sprintf("spiffe://%s.consul", provider.clusterID), parsed.URIs[0].String())
}
}
@ -349,7 +349,6 @@ func testVaultProviderWithConfig(t *testing.T, isRoot bool, rawConf map[string]i
t.Fatalf("err: %v", err)
}
}
return provider, testVault
}

133
agent/connect/common_names.go Normal file
View File

@ -0,0 +1,133 @@
package connect
import (
"crypto/rand"
"encoding/binary"
"fmt"
"regexp"
"strconv"
"strings"
)
var invalidDNSNameChars = regexp.MustCompile(`[^a-z0-9]`)
// ServiceCN returns the common name for a service's certificate. We can't use
// SPIFFE URIs because some CAs require valid FQDN format. We can't use SNI
// values because they are often too long than the 64 bytes allowed by
// CommonNames. We could attempt to encode more information into this to make
// identifying which instance/node it was issued to in a management tool easier
// but that just introduces more complications around length. It's also strange
// that the Common Name would encode more information than the actual
// identifying URI we use to assert anything does and my lead to bad assumptions
// that the common name is in some way "secure" or verified - there is nothing
// inherently provable here except that the requestor had ACLs for that service
// name in that DC.
//
// Format is:
// <sanitized_service_name>.svc.<trust-domain-first-8>.consul
//
// service name is sanitized by removing any chars that are not legal in a DNS
// name and lower casing. It is truncated to the first X chars to keep the
// total at 64.
//
// trust domain is truncated to keep the whole name short
func ServiceCN(serviceName, trustDomain string) string {
svc := invalidDNSNameChars.ReplaceAllString(strings.ToLower(serviceName), "")
// 20 = 7 bytes for ".consul", 8 bytes for trust domain, 5 bytes for ".svc."
return fmt.Sprintf("%s.svc.%s.consul",
truncateTo(svc, 64-20), truncateTo(trustDomain, 8))
}
// AgentCN returns the common name for an agent certificate. See ServiceCN for
// more details on rationale.
//
// Format is:
// <sanitized_node_name>.agnt.<trust-domain-first-8>.consul
//
// node name is sanitized by removing any chars that are not legal in a DNS
// name and lower casing. It is truncated to the first X chars to keep the
// total at 64.
//
// trust domain is truncated to keep the whole name short
func AgentCN(node, trustDomain string) string {
nodeSan := invalidDNSNameChars.ReplaceAllString(strings.ToLower(node), "")
// 21 = 7 bytes for ".consul", 8 bytes for trust domain, 6 bytes for ".agnt."
return fmt.Sprintf("%s.agnt.%s.consul",
truncateTo(nodeSan, 64-21), truncateTo(trustDomain, 8))
}
// CompactUID returns a crypto random Unique Identifier string consiting of 8
// characters of base36 encoded random value. This has roughly 41 bits of
// entropy so is suitable for infrequently occuring events with low probability
// of collision. It is not suitable for UUIDs for very frequent events. It's
// main purpose is to assign unique values to CA certificate Common Names which
// need to be unique in some providers - see CACN - but without using up large
// amounts of the limited 64 character Common Name. It also makes the values
// more easily digestable by humans considering there are likely to be few of
// them ever in use.
func CompactUID() (string, error) {
// 48 bits (6 bytes) is enough to fill 8 bytes in base36 but it's simpler to
// have a whole uint8 to convert from.
var raw [8]byte
_, err := rand.Read(raw[:])
if err != nil {
return "", err
}
i := binary.LittleEndian.Uint64(raw[:])
return truncateTo(strconv.FormatInt(int64(i), 36), 8), nil
}
// CACN returns the common name for a CA certificate. See ServiceCN for more
// details on rationale. A uniqueID is requires because some providers (e.g.
// Vault) cache by subject and so produce incorrect results - for example they
// won't cross-sign an older CA certificate with the same common name since they
// think they already have a valid cert for than CN and just return the current
// root.
//
// This can be generated by any means but will be truncated to 8 chars and
// sanitised to DNS-safe chars. CompactUID generates suitable UIDs for this
// specific purpose.
//
// Format is:
// {provider}-{uniqueID-8-b36}.{root|int}.ca.<trust-domain-first-8>.consul
//
// trust domain is truncated to keep the whole name short
func CACN(provider, uniqueID, trustDomain string, isRoot bool) string {
providerSan := invalidDNSNameChars.ReplaceAllString(strings.ToLower(provider), "")
typ := "root"
if !isRoot {
typ = "int"
}
// 33 = 7 bytes for ".consul", 8 bytes for trust domain, 9 bytes for
// ".root.ca.", 9 bytes for "-{uniqueID-8-b36}"
uidSAN := invalidDNSNameChars.ReplaceAllString(strings.ToLower(uniqueID), "")
return fmt.Sprintf("%s-%s.%s.ca.%s.consul", typ, truncateTo(uidSAN, 8),
truncateTo(providerSan, 64-33), truncateTo(trustDomain, 8))
}
func truncateTo(s string, n int) string {
if len(s) > n {
return s[:n]
}
return s
}
// CNForCertURI returns the correct common name for a given cert URI type. It
// doesn't work for CA Signing IDs since more context is needed and CA Providers
// always know their CN from their own context.
func CNForCertURI(uri CertURI) (string, error) {
// Even though leafs should be from our own CSRs which should have the same CN
// logic as here, override anyway to account for older version clients that
// didn't include the Common Name in the CSR.
switch id := uri.(type) {
case *SpiffeIDService:
return ServiceCN(id.Service, id.Host), nil
case *SpiffeIDAgent:
return AgentCN(id.Agent, id.Host), nil
case *SpiffeIDSigning:
return "", fmt.Errorf("CertURI is a SpiffeIDSigning, not enough context to generate Common Name")
default:
return "", fmt.Errorf("CertURI type not recognized")
}
}

8
agent/connect/csr.go
View File

@ -43,11 +43,13 @@ func SigAlgoForKeyType(keyType string) x509.SignatureAlgorithm {
// CreateCSR returns a CSR to sign the given service along with the PEM-encoded
// private key for this certificate.
func CreateCSR(uri CertURI, privateKey crypto.Signer, extensions ...pkix.Extension) (string, error) {
func CreateCSR(uri CertURI, commonName string, privateKey crypto.Signer,
extensions ...pkix.Extension) (string, error) {
template := &x509.CertificateRequest{
URIs: []*url.URL{uri.URI()},
SignatureAlgorithm: SigAlgoForKey(privateKey),
ExtraExtensions: extensions,
Subject: pkix.Name{CommonName: commonName},
}
// Create the CSR itself
@ -67,13 +69,13 @@ func CreateCSR(uri CertURI, privateKey crypto.Signer, extensions ...pkix.Extensi
// CreateCSR returns a CA CSR to sign the given service along with the PEM-encoded
// private key for this certificate.
func CreateCACSR(uri CertURI, privateKey crypto.Signer) (string, error) {
func CreateCACSR(uri CertURI, commonName string, privateKey crypto.Signer) (string, error) {
ext, err := CreateCAExtension()
if err != nil {
return "", err
}
return CreateCSR(uri, privateKey, ext)
return CreateCSR(uri, commonName, privateKey, ext)
}
// CreateCAExtension creates a pkix.Extension for the x509 Basic Constraints

7
agent/connect/testing_ca.go
View File

@ -211,7 +211,7 @@ func testLeaf(t testing.T, service string, root *structs.CARoot, keyType string,
// Cert template for generation
template := x509.Certificate{
SerialNumber: sn,
Subject: pkix.Name{CommonName: service},
Subject: pkix.Name{CommonName: ServiceCN(service, TestClusterID)},
URIs: []*url.URL{spiffeId.URI()},
SignatureAlgorithm: SigAlgoForKeyType(rootKeyType),
BasicConstraintsValid: true,
@ -262,7 +262,12 @@ func TestLeaf(t testing.T, service string, root *structs.CARoot) (string, string
// TestCSR returns a CSR to sign the given service along with the PEM-encoded
// private key for this certificate.
func TestCSR(t testing.T, uri CertURI) (string, string) {
cn, err := CNForCertURI(uri)
if err != nil {
t.Fatalf("TestCSR failed to get Common Name: %s", err)
}
template := &x509.CertificateRequest{
Subject: pkix.Name{CommonName: cn},
URIs: []*url.URL{uri.URI()},
SignatureAlgorithm: x509.ECDSAWithSHA256,
}

6
agent/consul/auto_encrypt.go
View File

@ -65,7 +65,11 @@ func (c *Client) RequestAutoEncryptCerts(servers []string, port int, token strin
}
// Create a CSR.
csr, err := connect.CreateCSR(id, pk)
//
// The Common Name includes the dummy trust domain for now but Server will
// override this when it is signed anyway so it's OK.
cn := connect.AgentCN(string(c.config.NodeName), dummyTrustDomain)
csr, err := connect.CreateCSR(id, cn, pk)
if err != nil {
return errFn(err)
}

4
agent/consul/auto_encrypt_endpoint_test.go
View File

@ -75,7 +75,9 @@ func TestAutoEncryptSign(t *testing.T) {
require.NoError(t, err, info)
// Create a CSR.
csr, err := connect.CreateCSR(id, pk)
cn, err := connect.CNForCertURI(id)
require.NoError(t, err)
csr, err := connect.CreateCSR(id, cn, pk)
require.NoError(t, err, info)
require.NotEmpty(t, csr, info)
args := &structs.CASignRequest{