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

security: allow separate CA to verify client certificates. #27636

Merged
merged 1 commit into from
Jul 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
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
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