Skip to content

Commit

Permalink
security: allow separate CA to verify client certificates.
Browse files Browse the repository at this point in the history
Part of #26630.

This PR adds the following optional files in the certs directory:
- `client.node.crt` (and associated `.key`): client certificate for the
  node user
- `ca-client.crt`: certificate to verify client certificates

This allows for split server/client certificates signed by different
CAs.

If `ca-client.crt` exists, it is used in the node's server-side TLS.Config
CertPool for client certificate verification. Otherwise, we fall back on
`ca.crt`.

If `client.node.crt` exists, it is used in the node's client-side
TLS.Config as the client certificate. Otherwise, we call back on
`node.crt`. At load-time, we verify that the certificate to use contains
`CN=node` and `ExtendedKeyUsage=ClientAuth`.

Other bits in this PR:
- add `cockroach cert create-client-ca` command
- use client CA to sign client certs if present
- show client CA on `cockroach cert list`
- show all certs in debug page
- metric for client CA and node client expiration times

Release note (general change): allow separate CA for client certificates
  • Loading branch information
marc committed Jul 20, 2018
1 parent f38e4c7 commit ff877b1
Show file tree
Hide file tree
Showing 16 changed files with 1,097 additions and 463 deletions.
50 changes: 49 additions & 1 deletion pkg/cli/cert.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,44 @@ func runCreateCACert(cmd *cobra.Command, args []string) error {
"failed to generate CA cert and key")
}

// A createClientCACert command generates a client CA certificate and stores it
// in the cert directory.
var createClientCACertCmd = &cobra.Command{
Use: "create-client-ca --certs-dir=<path to cockroach certs dir> --ca-key=<path-to-client-ca-key>",
Short: "create client CA certificate and key",
Long: `
Generate a client CA certificate "<certs-dir>/ca-client.crt" and CA key "<client-ca-key>".
The certs directory is created if it does not exist.
If the CA key exists and --allow-ca-key-reuse is true, the key is used.
If the CA certificate exists and --overwrite is true, the new CA certificate is prepended to it.
The client CA is optional and should only be used when separate CAs are desired for server certificates
and client certificates.
If the client CA exists, a client.node.crt client certificate must be created using:
cockroach cert create-client node
Once the client.node.crt exists, all client certificates will be verified using the client CA.
`,
Args: cobra.NoArgs,
RunE: MaybeDecorateGRPCError(runCreateClientCACert),
}

// runCreateClientCACert generates a key and CA certificate and writes them
// to their corresponding files.
func runCreateClientCACert(cmd *cobra.Command, args []string) error {
return errors.Wrap(
security.CreateClientCAPair(
baseCfg.SSLCertsDir,
baseCfg.SSLCAKey,
keySize,
caCertificateLifetime,
allowCAKeyReuse,
overwriteFiles),
"failed to generate client CA cert and key")
}

// A createNodeCert command generates a node certificate and stores it
// in the cert directory.
var createNodeCertCmd = &cobra.Command{
Expand Down Expand Up @@ -137,7 +175,8 @@ Creation fails if the CA expiration time is before the desired certificate expir
func runCreateClientCert(cmd *cobra.Command, args []string) error {
var err error
var username string
if username, err = sql.NormalizeAndValidateUsername(args[0]); err != nil {
// We intentionally allow the `node` user to have a cert.
if username, err = sql.NormalizeAndValidateUsernameNoBlacklist(args[0]); err != nil {
return errors.Wrap(err, "failed to generate client certificate and key")
}

Expand Down Expand Up @@ -200,6 +239,14 @@ func runListCerts(cmd *cobra.Command, args []string) error {
addRow(cert, notes)
}

if cert := cm.ClientCACert(); cert != nil {
var notes string
if cert.Error == nil && len(cert.ParsedCertificates) > 0 {
notes = fmt.Sprintf("num certs: %d", len(cert.ParsedCertificates))
}
addRow(cert, notes)
}

if cert := cm.NodeCert(); cert != nil {
var addresses []string
if cert.Error == nil && len(cert.ParsedCertificates) > 0 {
Expand Down Expand Up @@ -230,6 +277,7 @@ func runListCerts(cmd *cobra.Command, args []string) error {

var certCmds = []*cobra.Command{
createCACertCmd,
createClientCACertCmd,
createNodeCertCmd,
createClientCertCmd,
listCertsCmd,
Expand Down
4 changes: 2 additions & 2 deletions pkg/cli/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ func init() {
StringFlag(f, &baseCfg.SSLCertsDir, cliflags.CertsDir, baseCfg.SSLCertsDir)
}

for _, cmd := range []*cobra.Command{createCACertCmd} {
for _, cmd := range []*cobra.Command{createCACertCmd, createClientCACertCmd} {
f := cmd.Flags()
// CA certificates have a longer expiration time.
DurationFlag(f, &caCertificateLifetime, cliflags.CertificateLifetime, defaultCALifetime)
Expand All @@ -259,7 +259,7 @@ func init() {
}

// The remaining flags are shared between all cert-generating functions.
for _, cmd := range []*cobra.Command{createCACertCmd, createNodeCertCmd, createClientCertCmd} {
for _, cmd := range []*cobra.Command{createCACertCmd, createClientCACertCmd, createNodeCertCmd, createClientCertCmd} {
f := cmd.Flags()
StringFlag(f, &baseCfg.SSLCAKey, cliflags.CAKey, baseCfg.SSLCAKey)
IntFlag(f, &keySize, cliflags.KeySize, defaultKeySize)
Expand Down
209 changes: 122 additions & 87 deletions pkg/security/certificate_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,15 @@ func ResetAssetLoader() {
assetLoaderImpl = defaultAssetLoader
}

type pemUsage uint32
// PemUsage indicates the purpose of a given certificate.
type PemUsage uint32

const (
_ pemUsage = iota
// CAPem describes a CA certificate.
_ PemUsage = iota
// CAPem describes the main CA certificate.
CAPem
// ClientCAPem describes the CA certificate used to verify client certificates.
ClientCAPem
// NodePem describes a combined server/client certificate for user Node.
NodePem
// ClientPem describes a client certificate.
Expand All @@ -89,10 +92,16 @@ const (
defaultCertsDirPerm = 0700
)

func (p pemUsage) String() string {
func isCA(usage PemUsage) bool {
return usage == CAPem || usage == ClientCAPem
}

func (p PemUsage) String() string {
switch p {
case CAPem:
return "Certificate Authority"
return "CA"
case ClientCAPem:
return "Client CA"
case NodePem:
return "Node"
case ClientPem:
Expand All @@ -109,7 +118,7 @@ func (p pemUsage) String() string {
// If Err != nil, the CertInfo must NOT be used.
type CertInfo struct {
// FileUsage describes the use of this certificate.
FileUsage pemUsage
FileUsage PemUsage

// Filename is the base filename of the certificate.
Filename string
Expand Down Expand Up @@ -147,6 +156,55 @@ func isCertificateFile(filename string) bool {
return strings.HasSuffix(filename, certExtension)
}

// CertInfoFromFilename takes a filename and attempts to determine the
// certificate usage (ca, node, etc..).
func CertInfoFromFilename(filename string) (*CertInfo, error) {
parts := strings.Split(filename, `.`)
numParts := len(parts)

if numParts < 2 {
return nil, errors.New("not enough parts found")
}

var fileUsage PemUsage
var name string
prefix := parts[0]
switch parts[0] {
case `ca`:
// This is main CA certificate.
fileUsage = CAPem
if numParts != 2 {
return nil, errors.Errorf("CA certificate filename should match ca%s", certExtension)
}
case `ca-client`:
// This is client CA certificate.
fileUsage = ClientCAPem
if numParts != 2 {
return nil, errors.Errorf("client CA certificate filename should match ca-client%s", certExtension)
}
case `node`:
fileUsage = NodePem
if numParts != 2 {
return nil, errors.Errorf("node certificate filename should match node%s", certExtension)
}
case `client`:
fileUsage = ClientPem
// strip prefix and suffix and re-join middle parts.
name = strings.Join(parts[1:numParts-1], `.`)
if len(name) == 0 {
return nil, errors.Errorf("client certificate filename should match client.<user>%s", certExtension)
}
default:
return nil, errors.Errorf("unknown prefix %q", prefix)
}

return &CertInfo{
FileUsage: fileUsage,
Filename: filename,
Name: name,
}, nil
}

// CertificateLoader searches for certificates and keys in the certs directory.
type CertificateLoader struct {
certsDir string
Expand Down Expand Up @@ -236,12 +294,20 @@ func (cl *CertificateLoader) Load() error {
}

// Build the info struct from the filename.
ci, err := cl.certInfoFromFilename(filename)
ci, err := CertInfoFromFilename(filename)
if err != nil {
log.Warningf(context.Background(), "bad filename %s: %v", fullPath, err)
continue
}

// Read the cert file contents.
fullCertPath := filepath.Join(cl.certsDir, filename)
certPEMBlock, err := assetLoaderImpl.ReadFile(fullCertPath)
if err != nil {
log.Warningf(context.Background(), "could not read certificate file %s: %v", fullPath, err)
}
ci.FileContents = certPEMBlock

// Parse certificate, then look for the private key.
// Errors are persisted for better visibility later.
if err := parseCertificate(ci); err != nil {
Expand All @@ -260,61 +326,11 @@ func (cl *CertificateLoader) Load() error {
return nil
}

// certInfoFromFilename takes a filename and attempts to determine the
// certificate usage (ca, node, etc..).
func (cl *CertificateLoader) certInfoFromFilename(filename string) (*CertInfo, error) {
parts := strings.Split(filename, `.`)
numParts := len(parts)

if numParts < 2 {
return nil, errors.New("not enough parts found")
}

var fileUsage pemUsage
var name string
prefix := parts[0]
switch parts[0] {
case `ca`:
fileUsage = CAPem
if numParts != 2 {
return nil, errors.Errorf("CA certificate filename should match ca%s", certExtension)
}
case `node`:
fileUsage = NodePem
if numParts != 2 {
return nil, errors.Errorf("node certificate filename should match node%s", certExtension)
}
case `client`:
fileUsage = ClientPem
// strip prefix and suffix and re-join middle parts.
name = strings.Join(parts[1:numParts-1], `.`)
if len(name) == 0 {
return nil, errors.Errorf("client certificate filename should match client.<user>%s", certExtension)
}
default:
return nil, errors.Errorf("unknown prefix %q", prefix)
}

// Read cert file contents.
fullCertPath := filepath.Join(cl.certsDir, filename)
certPEMBlock, err := assetLoaderImpl.ReadFile(fullCertPath)
if err != nil {
return nil, errors.Errorf("could not read certificate file: %v", err)
}

return &CertInfo{
FileUsage: fileUsage,
Filename: filename,
FileContents: certPEMBlock,
Name: name,
}, nil
}

// findKey takes a CertInfo and looks for the corresponding key file.
// If found, sets the 'keyFilename' and returns nil, returns error otherwise.
// Does not load CA keys.
func (cl *CertificateLoader) findKey(ci *CertInfo) error {
if ci.FileUsage == CAPem {
if isCA(ci.FileUsage) {
return nil
}

Expand Down Expand Up @@ -397,46 +413,65 @@ func parseCertificate(ci *CertInfo) error {
return nil
}

// validateCockroachCertificate takes a CertInfo and a parsed certificate and checks the
// values of certain fields.
func validateCockroachCertificate(ci *CertInfo, cert *x509.Certificate) error {
func hasKeyUsage(cert *x509.Certificate, usage x509.KeyUsage) bool {
return cert.KeyUsage&usage != 0
}

hasKeyUsage := func(usage x509.KeyUsage) bool {
return cert.KeyUsage&usage != 0
func hasExtendedKeyUsage(cert *x509.Certificate, usage x509.ExtKeyUsage) bool {
if cert.ExtKeyUsage == nil {
return false
}

hasExtendedKeyUsage := func(usage x509.ExtKeyUsage) bool {
if cert.ExtKeyUsage == nil {
return false
}
for _, u := range cert.ExtKeyUsage {
if u == usage {
return true
}
for _, u := range cert.ExtKeyUsage {
if u == usage {
return true
}
return false
}
return false
}

// validateDualPurposeNodeCert takes a CertInfo and a parsed certificate and checks the
// values of certain fields.
// This should only be called on the NodePem CertInfo when there is no specific
// client certificate for the 'node' user.
// Fields required for a valid server certificate are already checked.
func validateDualPurposeNodeCert(ci *CertInfo) error {
// The first certificate is used in client auth.
cert := ci.ParsedCertificates[0]

// Check Subject Common Name.
if a, e := cert.Subject.CommonName, NodeUser; a != e {
return errors.Errorf("client/server node certificate has Subject \"CN=%s\", expected \"CN=%s\"", a, e)
}

hasServer := hasExtendedKeyUsage(cert, x509.ExtKeyUsageServerAuth)
hasClient := hasExtendedKeyUsage(cert, x509.ExtKeyUsageClientAuth)
if !hasServer || !hasClient {
return errors.Errorf("client/server node certificate extended key usages: ServerAuth=%t, ClientAuth=%t, but both are needed",
hasServer, hasClient)
}

return nil
}

// validateCockroachCertificate takes a CertInfo and a parsed certificate and checks the
// values of certain fields.
func validateCockroachCertificate(ci *CertInfo, cert *x509.Certificate) error {

switch ci.FileUsage {
case NodePem:
// Check Subject Common Name.
if a, e := cert.Subject.CommonName, NodeUser; a != e {
return errors.Errorf("node certificate has Subject \"CN=%s\", expected \"CN=%s\"", a, e)
}
// Common Name and ExtendedKeyUsage are checked only if there is no client certificate for 'node'.
// This is done in validateDualPurposeNodeCert.

// Check key usages.
hasEncipherment := hasKeyUsage(x509.KeyUsageKeyEncipherment)
hasSignature := hasKeyUsage(x509.KeyUsageDigitalSignature)
hasEncipherment := hasKeyUsage(cert, x509.KeyUsageKeyEncipherment)
hasSignature := hasKeyUsage(cert, x509.KeyUsageDigitalSignature)
if !hasEncipherment || !hasSignature {
return errors.Errorf("node certificate key usages: KeyEncipherment=%t, DigitalSignature=%t, but both are needed",
hasEncipherment, hasSignature)
}

hasServer := hasExtendedKeyUsage(x509.ExtKeyUsageServerAuth)
hasClient := hasExtendedKeyUsage(x509.ExtKeyUsageClientAuth)
if !hasServer || !hasClient {
return errors.Errorf("node certificate extended key usages: ServerAuth=%t, ClientAuth=%t, but both are needed",
hasServer, hasClient)
if !hasExtendedKeyUsage(cert, x509.ExtKeyUsageServerAuth) {
return errors.Errorf("node certificate extended key usage missing ServerAuth")
}
case ClientPem:
// Check that CommonName matches the username extracted from the filename.
Expand All @@ -445,14 +480,14 @@ func validateCockroachCertificate(ci *CertInfo, cert *x509.Certificate) error {
}

// Check key usages.
hasEncipherment := hasKeyUsage(x509.KeyUsageKeyEncipherment)
hasSignature := hasKeyUsage(x509.KeyUsageDigitalSignature)
hasEncipherment := hasKeyUsage(cert, x509.KeyUsageKeyEncipherment)
hasSignature := hasKeyUsage(cert, x509.KeyUsageDigitalSignature)
if !hasEncipherment || !hasSignature {
return errors.Errorf("client certificate key usages: KeyEncipherment=%t, DigitalSignature=%t, but both are needed",
hasEncipherment, hasSignature)
}

if !hasExtendedKeyUsage(x509.ExtKeyUsageClientAuth) {
if !hasExtendedKeyUsage(cert, x509.ExtKeyUsageClientAuth) {
return errors.Errorf("client certificate does not have ClientAuth extended key usage")
}

Expand Down
Loading

0 comments on commit ff877b1

Please sign in to comment.