Allow specs to configure what key usage we validate against (#103)

* Allow specs to specify key_usage, instead of always assuming certs are for server auth

Previously if a cert was generated for any other usage, certmgr would consider the cert invalid
and refresh it on every check.

Default behavior of assuming certificates are for server auth has been retained, but if you want to use a cert for other usages you can now specify that in the spec as "key_usages". Note that this must be an array, for example "key_usages": ["client auth"]

README.md

@@ -190,6 +190,9 @@ A certificate spec has the following fields:
 * `take_actions_only_if_running`: boolean, if true, only fire a spec's action if the service is actually running.
   If this is set to false (the default for historical reasons), this can lead to certmgr starting a downed service
   when PKI expiry occurs.
+* `key_usages`: optional: An array of strings defining what this key should be used for. Certmgr will consider a cert invalid
+   if it does not contain these key usages. Possible values are from cfssl's [ExtKeyUsage map](https://github.com/cloudflare/cfssl/blob/1.3.3/config/config.go#L568)
+
 
 
 **Note**: `certmgr` will throw a warning if `svcmgr` is `dummy` _AND_ `action` is "nop" or undefined. This is because such a setup will not properly restart or reload a service upon certificate renewal, which will likely cause your service to crash. Running `certmgr` with the `--strict` flag will not even load a certificate spec with a `dummy svcmgr` and undefined/nop `action` configuration.

cert/spec.go

@@ -50,9 +50,13 @@ type SpecOptions struct {
 	// This is Primarily useful to force an initial randomization if many nodes with certmgr are restarted all
 	// at the same time.
 	InitialSplay time.Duration
+
+	// KeyUsages specifies what this key will be used for, so that certmgr can verify it is valid for that usage.
+	// It is optional and by default we assume keys will be used for "TLS Web Server Authentication"
+	KeyUsages []x509.ExtKeyUsage
 }
 
-// NewSpecOptions creates a new SpecOptions structu and populates it with defaults.
+// NewSpecOptions creates a new SpecOptions struct and populates it with defaults.
 func NewSpecOptions() *SpecOptions {
 	return &SpecOptions{
 		Before:   DefaultBefore,
@@ -160,7 +164,7 @@ func (spec *Spec) validateStoredPKI(currentCA *x509.Certificate) error {
 	// update internal metrics
 	spec.updateCertExpiry(keyPair.Leaf.NotAfter)
 
-	err = CertificateChainVerify(currentCA, keyPair.Leaf)
+	err = CertificateChainVerify(currentCA, keyPair.Leaf, spec.KeyUsages)
 	if err != nil {
 		return errors.WithMessage(err, "stored cert failed CA check")
 	}

cert/verification.go

@@ -41,11 +41,12 @@ func CertificateMatchesHostname(hosts []string, cert *x509.Certificate) bool {
 }
 
 // CertificateChainVerify validates if a given cert is derived from the given CA
-func CertificateChainVerify(ca *x509.Certificate, cert *x509.Certificate) error {
+func CertificateChainVerify(ca *x509.Certificate, cert *x509.Certificate, keyUsages []x509.ExtKeyUsage) error {
 	roots := x509.NewCertPool()
 	roots.AddCert(ca)
 	_, err := cert.Verify(x509.VerifyOptions{
-		Roots: roots,
+		Roots:     roots,
+		KeyUsages: keyUsages,
 	})
 	return err
 }

certmgr/mgr/file.go

@@ -1,10 +1,12 @@
 package mgr
 
 import (
+	"crypto/x509"
 	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 	"time"
 
@@ -15,10 +17,21 @@ import (
 	"github.com/cloudflare/certmgr/cert"
 	"github.com/cloudflare/certmgr/cert/storage"
 	"github.com/cloudflare/certmgr/cert/storage/util"
+	"github.com/cloudflare/cfssl/config"
 	"github.com/cloudflare/cfssl/csr"
 	log "github.com/sirupsen/logrus"
 )
 
+//validExtUsage extracts the valid values from cfssl's ExtKeyUsage map, which we print when someone specifies an invalid value
+var validExtUsage = func() []string {
+	usages := make([]string, 0, len(config.ExtKeyUsage))
+	for usage := range config.ExtKeyUsage {
+		usages = append(usages, usage)
+	}
+	sort.Strings(usages)
+	return usages
+}
+
 // ParsableAuthority is an authority struct that can load the Authkey from content on disk.  This is used internally
 // by Authority for unmarshal- this shouldn't be used for anything but on disk certmgr spec's.
 type ParsableAuthority struct {
@@ -98,6 +111,9 @@ type ParsableSpecOptions struct {
 	// ParsedInitialSplay is used to update the SpecOptions.InitialSplay field.
 	ParsedInitialSplay ParsableDuration `json:"initial_splay" yaml:"initial_splay"`
 
+	// ParsedKeyUsages is used to update the SpecOptions.KeyUsages field.
+	ParsedKeyUsages []string `json:"key_usages" yaml:"key_usages"`
+
 	// Remote is shorthand for updating CA.Remote for instantiation.
 	// This specifies the remote upstream to talk to.
 	Remote string `json:"remote" yaml:"remote"`
@@ -201,6 +217,19 @@ func ReadSpecFile(path string, defaults *ParsableSpecOptions) (*cert.Spec, error
 	// transfer the parsed durations into their final resting spot.
 	spec.FinalizeSpecOptionParsing()
 
+	if len(spec.ParsedKeyUsages) > 0 {
+		for _, KeyUsageRaw := range spec.ParsedKeyUsages {
+			keyUsage, ok := config.ExtKeyUsage[strings.ToLower(KeyUsageRaw)]
+			if !ok {
+				return nil, fmt.Errorf("spec %s specifies unknown key usage '%s'. Valid values are: %q", path, KeyUsageRaw, validExtUsage())
+			}
+			spec.KeyUsages = append(spec.KeyUsages, keyUsage)
+		}
+	} else { // Key usage not defined, default to server auth as that is both most common and was our previous behavior.
+		log.Warnf("spec %s does not specify key usage, defaulting to \"server auth\"", path)
+		spec.KeyUsages = []x509.ExtKeyUsage{config.ExtKeyUsage["server auth"]}
+	}
+
 	if spec.Authority.Remote == "" {
 		spec.Authority.Remote = spec.Remote
 	}