m/pkg/pki: refactor, allow for external certificates
The pki library supported managing certificates in two modes:
- default, when name != ""
- volatile/ephemeral, when name == ""
The difference between the two being that default certificates were
fully stored in etcd (key and x509 certificate), while volatile
certificates weren't stored at all. However, both kinds needed private
keys passed to the pki library.
We want to be able to emit certificates without having private keys for
that certificate, so we end up a third mode of operation: 'external
certificates'. These are still stored in etcd, but without any
corresponding private key.
In the future we might actually get rid of ephemeral certificates by
expanding the logic of external certificates to provide a full audit log
and revocation system, instead of matching by Certificate Name. But this
will do for now.
We also use this opportunity to write some simple tests for this
package.
Change-Id: I193f4b147273b0a3981c38d749b43362d3c1b69a
Reviewed-on: https://review.monogon.dev/c/monogon/+/263
Reviewed-by: Mateusz Zalega <mateusz@monogon.tech>
diff --git a/metropolis/pkg/pki/BUILD.bazel b/metropolis/pkg/pki/BUILD.bazel
index 243abf9..c215ce2 100644
--- a/metropolis/pkg/pki/BUILD.bazel
+++ b/metropolis/pkg/pki/BUILD.bazel
@@ -1,11 +1,10 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_library")
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
go_library(
name = "go_default_library",
srcs = [
"ca.go",
"certificate.go",
- "doc.go",
"x509.go",
],
importpath = "source.monogon.dev/metropolis/pkg/pki",
@@ -15,3 +14,10 @@
"@io_etcd_go_etcd//clientv3:go_default_library",
],
)
+
+go_test(
+ name = "go_default_test",
+ srcs = ["certificate_test.go"],
+ embed = [":go_default_library"],
+ deps = ["@io_etcd_go_etcd//integration:go_default_library"],
+)
diff --git a/metropolis/pkg/pki/ca.go b/metropolis/pkg/pki/ca.go
index 5ab1089..4931b5e 100644
--- a/metropolis/pkg/pki/ca.go
+++ b/metropolis/pkg/pki/ca.go
@@ -37,29 +37,15 @@
// will sign certificates when Issue is called, or nil if this is
// self-signing issuer.
CACertificate(ctx context.Context, kv clientv3.KV) ([]byte, error)
- // Issue will generate a key and certificate signed by the Issuer. The
- // returned certificate is x509 DER-encoded, while the key is a bare
- // ed25519 key.
- Issue(ctx context.Context, req *Certificate, kv clientv3.KV) (cert, key []byte, err error)
+ // Issue will generate a certificate signed by the Issuer. The returned
+ // certificate is x509 DER-encoded.
+ Issue(ctx context.Context, req *Certificate, kv clientv3.KV) (cert []byte, err error)
}
// issueCertificate is a generic low level certificate-and-key issuance
-// function. If ca or cakey is null, the certificate will be self-signed. The
-// returned certificate is DER-encoded, while the returned key is internal.
-func issueCertificate(req *Certificate, ca *x509.Certificate, caKey interface{}) (cert, key []byte, err error) {
- var privKey ed25519.PrivateKey
- var pubKey ed25519.PublicKey
- if req.key != nil {
- privKey = req.key
- pubKey = privKey.Public().(ed25519.PublicKey)
- } else {
- var err error
- pubKey, privKey, err = ed25519.GenerateKey(rand.Reader)
- if err != nil {
- panic(err)
- }
- }
-
+// function. If ca is null, the certificate will be self-signed. The returned
+// certificate is DER-encoded
+func issueCertificate(req *Certificate, ca *x509.Certificate, caKey ed25519.PrivateKey) (cert []byte, err error) {
serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 127)
serialNumber, err := rand.Int(rand.Reader, serialNumberLimit)
if err != nil {
@@ -67,32 +53,27 @@
return
}
- skid, err := calculateSKID(pubKey)
+ skid, err := calculateSKID(req.PublicKey)
if err != nil {
- return []byte{}, privKey, err
+ return nil, err
}
- req.template.SerialNumber = serialNumber
- req.template.NotBefore = time.Now()
- req.template.NotAfter = unknownNotAfter
- req.template.BasicConstraintsValid = true
- req.template.SubjectKeyId = skid
+ req.Template.SerialNumber = serialNumber
+ req.Template.NotBefore = time.Now()
+ req.Template.NotAfter = unknownNotAfter
+ req.Template.BasicConstraintsValid = true
+ req.Template.SubjectKeyId = skid
// Set the AuthorityKeyID to the SKID of the signing certificate (or self,
// if self-signing).
- if ca != nil && caKey != nil {
- req.template.AuthorityKeyId = ca.AuthorityKeyId
+ if ca != nil {
+ req.Template.AuthorityKeyId = ca.AuthorityKeyId
} else {
- req.template.AuthorityKeyId = req.template.SubjectKeyId
+ req.Template.AuthorityKeyId = req.Template.SubjectKeyId
+ ca = &req.Template
}
- if ca == nil || caKey == nil {
- ca = &req.template
- caKey = privKey
- }
-
- caCertRaw, err := x509.CreateCertificate(rand.Reader, &req.template, ca, pubKey, caKey)
- return caCertRaw, privKey, err
+ return x509.CreateCertificate(rand.Reader, &req.Template, ca, req.PublicKey, caKey)
}
type selfSigned struct{}
@@ -103,8 +84,14 @@
)
// Issue will generate a key and certificate that is self-signed.
-func (s *selfSigned) Issue(ctx context.Context, req *Certificate, kv clientv3.KV) (cert, key []byte, err error) {
- return issueCertificate(req, nil, nil)
+func (s *selfSigned) Issue(ctx context.Context, req *Certificate, kv clientv3.KV) (cert []byte, err error) {
+ if err := req.ensureKey(ctx, kv); err != nil {
+ return nil, err
+ }
+ if req.PrivateKey == nil {
+ return nil, fmt.Errorf("cannot issue self-signed certificate without a private key")
+ }
+ return issueCertificate(req, nil, req.PrivateKey)
}
// CACertificate returns nil for self-signed issuers.
@@ -114,24 +101,33 @@
// Issue will generate a key and certificate that is signed by this
// Certificate, if the Certificate is a CA.
-func (c *Certificate) Issue(ctx context.Context, req *Certificate, kv clientv3.KV) (cert, key []byte, err error) {
- caCert, caKey, err := c.ensure(ctx, kv)
+func (c *Certificate) Issue(ctx context.Context, req *Certificate, kv clientv3.KV) (cert []byte, err error) {
+ if err := c.ensureKey(ctx, kv); err != nil {
+ return nil, fmt.Errorf("could not ensure CA %q key exists: %w", c.Name, err)
+ }
+ if err := req.ensureKey(ctx, kv); err != nil {
+ return nil, fmt.Errorf("could not subject %q key exists: %w", req.Name, err)
+ }
+ if c.PrivateKey == nil {
+ return nil, fmt.Errorf("cannot use certificate without private key as CA")
+ }
+
+ caCert, err := c.ensure(ctx, kv)
if err != nil {
- return nil, nil, fmt.Errorf("could not ensure CA certificate %q exists: %w", c.name, err)
+ return nil, fmt.Errorf("could not ensure CA %q certificate exists: %w", c.Name, err)
}
ca, err := x509.ParseCertificate(caCert)
if err != nil {
- return nil, nil, fmt.Errorf("could not parse CA certificate: %w", err)
+ return nil, fmt.Errorf("could not parse CA certificate: %w", err)
}
// Ensure only one level of CAs exist, and that they are created explicitly.
- req.template.IsCA = false
- return issueCertificate(req, ca, ed25519.PrivateKey(caKey))
+ req.Template.IsCA = false
+ return issueCertificate(req, ca, c.PrivateKey)
}
// CACertificate returns the DER encoded x509 form of this Certificate that
// will be the used to issue child certificates.
func (c *Certificate) CACertificate(ctx context.Context, kv clientv3.KV) ([]byte, error) {
- cert, _, err := c.ensure(ctx, kv)
- return cert, err
+ return c.ensure(ctx, kv)
}
diff --git a/metropolis/pkg/pki/certificate.go b/metropolis/pkg/pki/certificate.go
index c0a1f53..4ec3bf0 100644
--- a/metropolis/pkg/pki/certificate.go
+++ b/metropolis/pkg/pki/certificate.go
@@ -14,11 +14,15 @@
// See the License for the specific language governing permissions and
// limitations under the License.
+// package pki implements an x509 PKI (Public Key Infrastructure) system backed
+// on etcd.
package pki
import (
+ "bytes"
"context"
"crypto/ed25519"
+ "crypto/rand"
"crypto/x509"
"crypto/x509/pkix"
"encoding/pem"
@@ -45,51 +49,70 @@
}
}
+type CertificateMode int
+
+const (
+ // CertificateManaged is a certificate whose key material is fully managed by
+ // the Certificate code. When set, PublicKey and PrivateKey must not be set by
+ // the user, and instead will be populated by the Ensure call. Name must be set,
+ // and will be used to store this Certificate and its keys within etcd. After
+ // the initial generation during Ensure, other Certificates with the same Name
+ // will be retrieved (including key material) from etcd.
+ CertificateManaged CertificateMode = iota
+
+ // CertificateExternal is a certificate whose key material is not managed by
+ // Certificate or stored in etcd, but the X509 certificate itself is. PublicKey
+ // must be set while PrivateKey must not be set. Name must be set, and will be
+ // used to store the emitted X509 certificate in etcd on Ensure. After the
+ // initial generation during Ensure, other Certificates with the same Name will
+ // be retrieved (without key material) from etcd.
+ CertificateExternal
+
+ // CertificateEphemeral is a certificate whose data (X509 certificate and
+ // possibly key material) is generated on demand each time Ensure is called.
+ // Nothing is stored in etcd or loaded from etcd. PrivateKey or PublicKey can be
+ // set, if both are nil then a new keypair will be generated. Name is ignored.
+ CertificateEphemeral
+)
+
// Certificate is the promise of a Certificate being available to the caller.
// In this case, Certificate refers to a pair of x509 certificate and
// corresponding private key. Certificates can be stored in etcd, and their
// issuers might also be store on etcd. As such, this type's methods contain
-// references to an etcd KV client. This Certificate type is agnostic to
-// usage, but mostly geared towards Kubernetes certificates.
+// references to an etcd KV client.
type Certificate struct {
- namespace *Namespace
+ Namespace *Namespace
- // issuer is the Issuer that will generate this certificate if one doesn't
- // yet exist or etcd, or the requested certificate is volatile (not to be
+ // Issuer is the Issuer that will generate this certificate if one doesn't
+ // yet exist or etcd, or the requested certificate is ephemeral (not to be
// stored on etcd).
Issuer Issuer
- // name is a unique key for storing the certificate in etcd. If empty,
- // certificate is 'volatile', will not be stored on etcd, and every
- // .Ensure() call will generate a new pair.
- name string
- // template is an x509 certificate definition that will be used to generate
+ // Name is a unique key for storing the certificate in etcd (if the requested
+ // certificate is not ephemeral).
+ Name string
+ // Template is an x509 certificate definition that will be used to generate
// the certificate when issuing it.
- template x509.Certificate
- // key is the private key for which the certificate should emitted, or nil
- // if the key should be generated. The private key is required (vs. the
- // private one) because the Certificate might be attempted to be issued via
- // self-signing.
- key ed25519.PrivateKey
+ Template x509.Certificate
+
+ // Mode in which this Certificate will operate. This influences the behaviour of
+ // the Ensure call.
+ Mode CertificateMode
+
+ // PrivateKey is the private key for this Certificate. It should never be set by
+ // the user, and instead will be populated by the Ensure call for Managed
+ // Certificates.
+ PrivateKey ed25519.PrivateKey
+
+ // PublicKey is the public key for this Certificate. It should only be set by
+ // the user for External or Ephemeral certificates, and will be populated by the
+ // next Ensure call if missing.
+ PublicKey ed25519.PublicKey
}
func (n *Namespace) etcdPath(f string, args ...interface{}) string {
return n.prefix + fmt.Sprintf(f, args...)
}
-// New creates a new Certificate, or to be more precise, a promise that a
-// certificate will exist once Ensure is called. Issuer must be a valid
-// certificate issuer (SelfSigned or another Certificate). Name must be unique
-// among all certificates, or empty (which will cause the certificate to be
-// volatile, ie. not stored in etcd).
-func (n *Namespace) New(issuer Issuer, name string, template x509.Certificate) *Certificate {
- return &Certificate{
- namespace: n,
- Issuer: issuer,
- name: name,
- template: template,
- }
-}
-
// Client makes a Kubernetes PKI-compatible client certificate template.
// Directly derived from Kubernetes PKI requirements documented at
// https://kubernetes.io/docs/setup/best-practices/certificates/#configure-certificates-manually
@@ -127,81 +150,70 @@
}
}
-func (c *Certificate) etcdPaths() (cert, key string) {
- return c.namespace.etcdPath("%s-cert.der", c.name), c.namespace.etcdPath("%s-key.der", c.name)
-}
-
-func (c *Certificate) UseExistingKey(key ed25519.PrivateKey) {
- c.key = key
-}
-
// ensure returns a DER-encoded x509 certificate and internally encoded bare
-// ed25519 key for a given Certificate, in memory (if volatile), loading it
+// ed25519 key for a given Certificate, in memory (if ephemeral), loading it
// from etcd, or creating and saving it on etcd if needed.
// This function is safe to call in parallel from multiple etcd clients
// (including across machines), but it will error in case a concurrent
// certificate generation happens. These errors are, however, safe to retry -
// as long as all the certificate creators (ie., Metropolis nodes) run the same
// version of this code.
-//
-// TODO(q3k): in the future, this should be handled better - especially as we
-// introduce new certificates, or worse, change the issuance chain. As a
-// stopgap measure, an explicit per-certificate or even global lock can be
-// implemented. And, even before that, we can handle concurrency errors in a
-// smarter way.
-func (c *Certificate) ensure(ctx context.Context, kv clientv3.KV) (cert, key []byte, err error) {
- if c.name == "" {
- // Volatile certificate - generate.
- // TODO(q3k): cache internally?
- cert, key, err = c.Issuer.Issue(ctx, c, kv)
- if err != nil {
- err = fmt.Errorf("failed to issue: %w", err)
- return
- }
- return
+func (c *Certificate) ensure(ctx context.Context, kv clientv3.KV) (cert []byte, err error) {
+ // Ensure key is available.
+ if err := c.ensureKey(ctx, kv); err != nil {
+ return nil, err
}
- certPath, keyPath := c.etcdPaths()
+ switch c.Mode {
+ case CertificateEphemeral:
+ // TODO(q3k): cache internally?
+ cert, err = c.Issuer.Issue(ctx, c, kv)
+ if err != nil {
+ return nil, fmt.Errorf("failed to issue: %w", err)
+ }
+ return cert, nil
+ case CertificateManaged, CertificateExternal:
+ default:
+ return nil, fmt.Errorf("invalid certificate mode %v", c.Mode)
+ }
- // Try loading certificate and key from etcd.
+ certPath := c.Namespace.etcdPath("%s-cert.der", c.Name)
+
+ // Try loading certificate from etcd.
certRes, err := kv.Get(ctx, certPath)
if err != nil {
- err = fmt.Errorf("failed to get certificate from etcd: %w", err)
- return
+ return nil, fmt.Errorf("failed to get certificate from etcd: %w", err)
}
- keyRes, err := kv.Get(ctx, keyPath)
+
+ if len(certRes.Kvs) == 1 {
+ certBytes := certRes.Kvs[0].Value
+ cert, err := x509.ParseCertificate(certBytes)
+ if err != nil {
+ return nil, fmt.Errorf("failed to parse certificate retrieved from etcd: %w", err)
+ }
+ pk, ok := cert.PublicKey.(ed25519.PublicKey)
+ if !ok {
+ return nil, fmt.Errorf("unexpected non-ed25519 certificate found in etcd")
+ }
+ if !bytes.Equal(pk, c.PublicKey) {
+ return nil, fmt.Errorf("certificate stored in etcd emitted for different public key")
+ }
+ // TODO(q3k): ensure issuer and template haven't changed
+ return certBytes, nil
+ }
+
+ // No certificate found - issue one and save to etcd.
+ cert, err = c.Issuer.Issue(ctx, c, kv)
if err != nil {
- err = fmt.Errorf("failed to get key from etcd: %w", err)
- return
+ return nil, fmt.Errorf("failed to issue: %w", err)
}
- if len(certRes.Kvs) == 1 && len(keyRes.Kvs) == 1 {
- // Certificate and key exists in etcd, return that.
- cert = certRes.Kvs[0].Value
- key = keyRes.Kvs[0].Value
-
- err = nil
- // TODO(q3k): check for expiration
- return
- }
-
- // No certificate found - issue one.
- cert, key, err = c.Issuer.Issue(ctx, c, kv)
- if err != nil {
- err = fmt.Errorf("failed to issue: %w", err)
- return
- }
-
- // Save to etcd in transaction. This ensures that no partial writes happen,
- // and that we haven't been raced to the save.
res, err := kv.Txn(ctx).
If(
clientv3.Compare(clientv3.CreateRevision(certPath), "=", 0),
- clientv3.Compare(clientv3.CreateRevision(keyPath), "=", 0),
).
Then(
clientv3.OpPut(certPath, string(cert)),
- clientv3.OpPut(keyPath, string(key)),
).Commit()
if err != nil {
err = fmt.Errorf("failed to write newly issued certificate: %w", err)
@@ -212,21 +224,105 @@
return
}
-// Ensure returns an x509 DER-encoded (but not PEM-encoded) certificate and key
-// for a given Certificate. If the certificate is volatile, each call to
-// Ensure will cause a new certificate to be generated. Otherwise, it will be
-// retrieved from etcd, or generated and stored there if needed.
-func (c *Certificate) Ensure(ctx context.Context, kv clientv3.KV) (cert, key []byte, err error) {
- cert, key, err = c.ensure(ctx, kv)
- if err != nil {
- return nil, nil, err
+// ensureKey retrieves or creates PublicKey as needed based on the Certificate
+// Mode. For Managed Certificates and Ephemeral Certificates with no PrivateKey
+// it will also populate PrivateKay.
+func (c *Certificate) ensureKey(ctx context.Context, kv clientv3.KV) error {
+ // If we have a public key then we're all set.
+ if c.PublicKey != nil {
+ return nil
}
- key, err = x509.MarshalPKCS8PrivateKey(ed25519.PrivateKey(key))
- if err != nil {
- err = fmt.Errorf("could not marshal private key (data corruption?): %w", err)
- return
+
+ // For ephemeral keys, we just generate them.
+ // For external keys, we can't do anything - not having the keys set means
+ // a programming error.
+
+ switch c.Mode {
+ case CertificateEphemeral:
+ pub, priv, err := ed25519.GenerateKey(rand.Reader)
+ if err != nil {
+ return fmt.Errorf("when generating ephemeral key: %w", err)
+ }
+ c.PublicKey = pub
+ c.PrivateKey = priv
+ return nil
+ case CertificateExternal:
+ if c.PrivateKey != nil {
+ // We prohibit having PrivateKey set in External Certificates to simplify the
+ // different logic paths this library implements. Being able to assume External
+ // == PublicKey only makes things easier elsewhere.
+ return fmt.Errorf("external certificate must not have PrivateKey set")
+ }
+ return fmt.Errorf("external certificate must have PublicKey set")
+ case CertificateManaged:
+ default:
+ return fmt.Errorf("invalid certificate mode %v", c.Mode)
}
- return cert, key, err
+
+ // For managed keys, synchronize with etcd.
+ if c.Name == "" {
+ return fmt.Errorf("managed certificate must have Name set")
+ }
+
+ // First, try loading.
+ privPath := c.Namespace.etcdPath("%s-privkey.bin", c.Name)
+ privRes, err := kv.Get(ctx, privPath)
+ if err != nil {
+ return fmt.Errorf("failed to get private key from etcd: %w", err)
+ }
+ if len(privRes.Kvs) == 1 {
+ privBytes := privRes.Kvs[0].Value
+ if len(privBytes) != ed25519.PrivateKeySize {
+ return fmt.Errorf("stored private key has invalid size")
+ }
+ c.PrivateKey = privBytes
+ c.PublicKey = c.PrivateKey.Public().(ed25519.PublicKey)
+ return nil
+ }
+
+ // No key in etcd? Generate and save.
+ pub, priv, err := ed25519.GenerateKey(rand.Reader)
+ if err != nil {
+ return fmt.Errorf("while generating keypair: %w", err)
+ }
+
+ res, err := kv.Txn(ctx).
+ If(
+ clientv3.Compare(clientv3.CreateRevision(privPath), "=", 0),
+ ).
+ Then(
+ clientv3.OpPut(privPath, string(priv)),
+ ).Commit()
+ if err != nil {
+ return fmt.Errorf("failed to write newly generated keypair: %w", err)
+ } else if !res.Succeeded {
+ return fmt.Errorf("key generation transaction failed: concurrent write")
+ }
+
+ c.PrivateKey = priv
+ c.PublicKey = pub
+ return nil
+}
+
+// Ensure returns an x509 DER-encoded (but not PEM-encoded) certificate for a
+// given Certificate.
+//
+// If the Certificate is ephemeral, each call to Ensure will cause a new
+// certificate to be generated. Otherwise, it will be retrieved from etcd, or
+// generated and stored there if needed.
+func (c *Certificate) Ensure(ctx context.Context, kv clientv3.KV) (cert []byte, err error) {
+ return c.ensure(ctx, kv)
+}
+
+func (c *Certificate) PrivateKeyX509() ([]byte, error) {
+ if c.PrivateKey == nil {
+ return nil, fmt.Errorf("certificate has no private key")
+ }
+ key, err := x509.MarshalPKCS8PrivateKey(c.PrivateKey)
+ if err != nil {
+ return nil, fmt.Errorf("could not marshal private key (data corruption?): %w", err)
+ }
+ return key, nil
}
// FilesystemCertificate is a fileargs.FileArgs wrapper which will contain PEM
@@ -244,7 +340,8 @@
// CertPath is the full path at which the certificate is available. Read
// only.
CertPath string
- // KeyPath is the full path at which the key is available. Read only.
+ // KeyPath is the full path at which the private key is available, or an empty
+ // string if the Certificate was created without a private key. Read only.
KeyPath string
}
@@ -262,7 +359,7 @@
}
fs := &FilesystemCertificate{FileArgs: fa}
- cert, key, err := c.Ensure(ctx, kv)
+ cert, err := c.Ensure(ctx, kv)
if err != nil {
return nil, fmt.Errorf("when issuing certificate: %w", err)
}
@@ -278,7 +375,13 @@
fs.CACertPath = fs.ArgPath("ca.crt", pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: cacert}))
fs.CertPath = fs.ArgPath("tls.crt", pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: cert}))
- fs.KeyPath = fs.ArgPath("tls.key", pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: key}))
+ if c.PrivateKey != nil {
+ key, err := c.PrivateKeyX509()
+ if err != nil {
+ return nil, err
+ }
+ fs.KeyPath = fs.ArgPath("tls.key", pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: key}))
+ }
return fs, nil
}
diff --git a/metropolis/pkg/pki/certificate_test.go b/metropolis/pkg/pki/certificate_test.go
new file mode 100644
index 0000000..da8dee9
--- /dev/null
+++ b/metropolis/pkg/pki/certificate_test.go
@@ -0,0 +1,172 @@
+package pki
+
+import (
+ "bytes"
+ "context"
+ "crypto/ed25519"
+ "crypto/rand"
+ "crypto/x509"
+ "testing"
+
+ "go.etcd.io/etcd/integration"
+)
+
+// TestManaged ensures Managed Certificates work, including re-ensuring
+// certificates with the same data and issuing subordinate certificates.
+func TestManaged(t *testing.T) {
+ cluster := integration.NewClusterV3(nil, &integration.ClusterConfig{
+ Size: 1,
+ })
+ cl := cluster.Client(0)
+ defer cluster.Terminate(nil)
+ ctx, ctxC := context.WithCancel(context.Background())
+ defer ctxC()
+ ns := Namespaced("/test-managed/")
+
+ // Test CA certificate issuance.
+ ca := &Certificate{
+ Namespace: &ns,
+ Issuer: SelfSigned,
+ Name: "ca",
+ Template: CA("Test CA"),
+ }
+ caBytes, err := ca.Ensure(ctx, cl)
+ if err != nil {
+ t.Fatalf("Failed to Ensure CA: %v", err)
+ }
+ caCert, err := x509.ParseCertificate(caBytes)
+ if err != nil {
+ t.Fatalf("Failed to parse newly emited CA cert: %v", err)
+ }
+ if !caCert.IsCA {
+ t.Errorf("Newly emitted CA cert is not CA")
+ }
+ if ca.PublicKey == nil {
+ t.Errorf("Newly emitted CA cert has no public key")
+ }
+ if ca.PrivateKey == nil {
+ t.Errorf("Newly emitted CA cert has no public key")
+ }
+
+ // Re-emitting CA certificate with same parameters should return exact same
+ // data.
+ ca2 := &Certificate{
+ Namespace: &ns,
+ Issuer: SelfSigned,
+ Name: "ca",
+ Template: CA("Test CA"),
+ }
+ caBytes2, err := ca2.Ensure(ctx, cl)
+ if err != nil {
+ t.Fatalf("Failed to re-Ensure CA: %v", err)
+ }
+ if !bytes.Equal(caBytes, caBytes2) {
+ t.Errorf("New CA has different x509 certificate")
+ }
+ if !bytes.Equal(ca.PublicKey, ca2.PublicKey) {
+ t.Errorf("New CA has different public key")
+ }
+ if !bytes.Equal(ca.PrivateKey, ca2.PrivateKey) {
+ t.Errorf("New CA has different private key")
+ }
+
+ // Emitting a subordinate certificate should work.
+ client := &Certificate{
+ Namespace: &ns,
+ Issuer: ca2,
+ Name: "client",
+ Template: Client("foo", nil),
+ }
+ clientBytes, err := client.Ensure(ctx, cl)
+ if err != nil {
+ t.Fatalf("Failed to ensure client certificate: %v", err)
+ }
+ clientCert, err := x509.ParseCertificate(clientBytes)
+ if err != nil {
+ t.Fatalf("Failed to parse newly emitted client certificate: %v", err)
+ }
+ if clientCert.IsCA {
+ t.Errorf("New client cert is CA")
+ }
+ if want, got := "foo", clientCert.Subject.CommonName; want != got {
+ t.Errorf("New client CN should be %q, got %q", want, got)
+ }
+ if want, got := caCert.Subject.String(), clientCert.Issuer.String(); want != got {
+ t.Errorf("New client issuer should be %q, got %q", want, got)
+ }
+}
+
+// TestExternal ensures External certificates work correctly, including
+// re-Ensuring certificates with the same public key, and attempting to re-issue
+// the same certificate with a different public key (which should fail).
+func TestExternal(t *testing.T) {
+ cluster := integration.NewClusterV3(nil, &integration.ClusterConfig{
+ Size: 1,
+ })
+ cl := cluster.Client(0)
+ defer cluster.Terminate(nil)
+ ctx, ctxC := context.WithCancel(context.Background())
+ defer ctxC()
+ ns := Namespaced("/test-external/")
+
+ ca := &Certificate{
+ Namespace: &ns,
+ Issuer: SelfSigned,
+ Name: "ca",
+ Template: CA("Test CA"),
+ }
+
+ // Issuing an external certificate should work.
+ pk, _, err := ed25519.GenerateKey(rand.Reader)
+ if err != nil {
+ t.Fatalf("GenerateKey: %v", err)
+ }
+ server := &Certificate{
+ Namespace: &ns,
+ Issuer: ca,
+ Name: "server",
+ Template: Server([]string{"server"}, nil),
+ Mode: CertificateExternal,
+ PublicKey: pk,
+ }
+ serverBytes, err := server.Ensure(ctx, cl)
+ if err != nil {
+ t.Fatalf("Failed to Ensure server certificate: %v", err)
+ }
+
+ // Issuing an external certificate with the same name but different public key
+ // should fail.
+ pk2, _, err := ed25519.GenerateKey(rand.Reader)
+ if err != nil {
+ t.Fatalf("GenerateKey: %v", err)
+ }
+ server2 := &Certificate{
+ Namespace: &ns,
+ Issuer: ca,
+ Name: "server",
+ Template: Server([]string{"server"}, nil),
+ Mode: CertificateExternal,
+ PublicKey: pk2,
+ }
+ if _, err := server2.Ensure(ctx, cl); err == nil {
+ t.Fatalf("Issuing server certificate with different public key should have failed")
+ }
+
+ // Issuing the external certificate with the same name and same public key
+ // should work and yield the same x509 bytes.
+ server3 := &Certificate{
+ Namespace: &ns,
+ Issuer: ca,
+ Name: "server",
+ Template: Server([]string{"server"}, nil),
+ Mode: CertificateExternal,
+ PublicKey: pk,
+ }
+ serverBytes3, err := server3.Ensure(ctx, cl)
+ if err != nil {
+ t.Fatalf("Failed to re-Ensure server certificate: %v", err)
+ }
+ if !bytes.Equal(serverBytes, serverBytes3) {
+ t.Errorf("New server certificate has different x509 certificate")
+ }
+}
diff --git a/metropolis/pkg/pki/doc.go b/metropolis/pkg/pki/doc.go
deleted file mode 100644
index 9174b0f..0000000
--- a/metropolis/pkg/pki/doc.go
+++ /dev/null
@@ -1,52 +0,0 @@
-// Copyright 2020 The Monogon Project Authors.
-//
-// SPDX-License-Identifier: Apache-2.0
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-// package pki implements an x509 PKI (Public Key Infrastructure) system backed
-// on etcd.
-//
-// The PKI is made of Certificates, all constrained within a Namespace. The
-// Namespace allows for multiple users of this library to co-exist on a single
-// etcd server.
-//
-// Any time a Certificate object is created, it describes the promise (or
-// intent) of an x509 certificate to exist. For every created Certifiacte an
-// Issuer must be specified - either another Certificate (which will act as a
-// CA and sign that Certificate), or SelfSigned (which will cause the
-// Certificate to be self-signed when generated).
-//
-// Once a Certificate object is created, a call to Ensure() must be placed to
-// turn the intent of a certificate into physical bytes that can then be
-// accessed by the appliaction.
-//
-// Two kinds of Certificates can be created:
-// - Named certificates are stored in etcd, and an Ensure call will either
-// create them, or return a Certificate already stored in etcd. Multiple
-// concurrent calls to Ensure for a Certificate with the same name are
-// permitted, even across machines, as long as the Certificate intent data
-// is the same. If not, it is still safe to perform this action
-// concurrently, but the first transaction will win, causing the losing
-// transaction to return the Ensure call with a certificate that was not
-// based on the same intent.
-// It is the responsibility of the caller to ensure these cases are handled
-// gracefully.
-// - Volatile certificates are stored in memory, and have an empty ("") name.
-// Any time Ensure is called, the certificate already present in memory is
-// returned, or one is created if it does not yet exist.
-// Currently, these certificates live fully in memory, but in the future we
-// will likely perform audit logging (and revocation) of these certificate
-// within etcd, too.
-//
-package pki