m/n/core: factor out gRPC/TLS into rpc and identity libraries
This is an annoying large change, which started its life as me pulling
the 'let's add tests for authentication' thread, and ended up in
unifying a whole bunch of dispersed logic under two new libraries.
Notable changes:
- m/n/core/identity now contains the NodeCertificate (now called Node)
and NodeCredentials types. These used to exist in the cluster code,
but were factored out to prevent loops between the curator, the
cluster enrolment logic, and other code. They can now be shared by
nearly all of the node code, removing the need for some conversions
between subsystems/packages.
- Alongside Node{,Credentials} types, the identity package contains
code that creates x509 certificate templates and verifies x509
certificates, and has functions specific to nodes and users - not
clients and servers. This allows moving most of the rest of
certificate checking code into a single set of functions, and allows
us to test this logic thoroughly.
- pki.{Client,Server,CA} are not used by the node core code anymore,
and can now be moved to kubernetes-specific code (as that was their
original purpose and that's their only current use).
- m/n/core/rpc has been refactored to deduplicate code between the
local/external gRPC servers and unary/stream interceptors for these
servers, also allowing for more thorough testing and unified
behaviour between all.
- A PeerInfo structure is now injected into all gRPC handlers, and is
unified to contain information both about nodes, users, and possibly
unauthenticated callers.
- The AAA.Escrow implementation now makes use of PeerInfo in order to
retrieve the client's certificate, instead of rolling its own logic.
- The EphemeralClusterCredentials test helper has been moved to the rpc
library, and now returns identity objects, allowing for simplified
test code (less juggling of bare public keys and
{x509,tls}.Certificate objects).
Change-Id: I9284966b4f18c0d7628167ca3168b4b4037808c1
Reviewed-on: https://review.monogon.dev/c/monogon/+/325
Reviewed-by: Lorenz Brun <lorenz@monogon.tech>
diff --git a/metropolis/node/core/cluster/BUILD.bazel b/metropolis/node/core/cluster/BUILD.bazel
index 322a337..2d3e813 100644
--- a/metropolis/node/core/cluster/BUILD.bazel
+++ b/metropolis/node/core/cluster/BUILD.bazel
@@ -1,11 +1,10 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
go_library(
name = "go_default_library",
srcs = [
"cluster.go",
"cluster_bootstrap.go",
- "node.go",
"status.go",
"watcher.go",
],
@@ -15,6 +14,7 @@
"//metropolis/node/core/consensus:go_default_library",
"//metropolis/node/core/consensus/client:go_default_library",
"//metropolis/node/core/curator:go_default_library",
+ "//metropolis/node/core/identity:go_default_library",
"//metropolis/node/core/localstorage:go_default_library",
"//metropolis/node/core/network:go_default_library",
"//metropolis/pkg/event:go_default_library",
@@ -26,10 +26,3 @@
"@org_golang_google_protobuf//proto:go_default_library",
],
)
-
-go_test(
- name = "go_default_test",
- srcs = ["node_test.go"],
- embed = [":go_default_library"],
- deps = ["//metropolis/node/core/curator:go_default_library"],
-)
diff --git a/metropolis/node/core/cluster/cluster_bootstrap.go b/metropolis/node/core/cluster/cluster_bootstrap.go
index 2905d17..2b3b333 100644
--- a/metropolis/node/core/cluster/cluster_bootstrap.go
+++ b/metropolis/node/core/cluster/cluster_bootstrap.go
@@ -26,6 +26,7 @@
"source.monogon.dev/metropolis/node/core/consensus"
"source.monogon.dev/metropolis/node/core/curator"
+ "source.monogon.dev/metropolis/node/core/identity"
"source.monogon.dev/metropolis/pkg/supervisor"
apb "source.monogon.dev/metropolis/proto/api"
cpb "source.monogon.dev/metropolis/proto/common"
@@ -127,7 +128,7 @@
// Using the short-circuited credentials from the curator, build our
// NodeCredentials. That, and the public part of the credentials
// (NodeCertificate) are the primary output of the cluster manager.
- creds, err := NewNodeCredentials(priv, nodeCert, caCert)
+ creds, err := identity.NewNodeCredentials(priv, nodeCert, caCert)
if err != nil {
return fmt.Errorf("failed to use newly bootstrapped node credentials: %w", err)
}
diff --git a/metropolis/node/core/cluster/node.go b/metropolis/node/core/cluster/node.go
deleted file mode 100644
index af5b654..0000000
--- a/metropolis/node/core/cluster/node.go
+++ /dev/null
@@ -1,154 +0,0 @@
-package cluster
-
-import (
- "crypto/ed25519"
- "crypto/subtle"
- "crypto/tls"
- "crypto/x509"
- "fmt"
-
- "source.monogon.dev/metropolis/node/core/curator"
- "source.monogon.dev/metropolis/node/core/localstorage"
-)
-
-// NodeCertificate is the public part of the credentials of a node. They are
-// emitted for a node by the cluster CA contained within the curator.
-type NodeCertificate struct {
- node *x509.Certificate
- ca *x509.Certificate
-}
-
-// ClusterCA returns the CA certificate of the cluster for which this
-// NodeCertificate is emitted.
-func (n *NodeCertificate) ClusterCA() *x509.Certificate {
- return n.ca
-}
-
-// NodeCredentials are the public and private part of the credentials of a node.
-//
-// It represents all the data necessary for a node to authenticate over mTLS to
-// other nodes and the rest of the cluster.
-//
-// It must never be made available to any node other than the node it has been
-// emitted for.
-type NodeCredentials struct {
- NodeCertificate
- private ed25519.PrivateKey
-}
-
-// NewNodeCertificate wraps a pair CA and node DER-encoded certificates into
-// NodeCertificate, ensuring the given certificate data is valid and compatible
-// Metropolis assumptions.
-//
-// It does _not_ verify that the given CA is a known/trusted Metropolis CA for a
-// running cluster.
-func NewNodeCertificate(cert, ca []byte) (*NodeCertificate, error) {
- certParsed, err := x509.ParseCertificate(cert)
- if err != nil {
- return nil, fmt.Errorf("could not parse node certificate: %w", err)
- }
- caCertParsed, err := x509.ParseCertificate(ca)
- if err != nil {
- return nil, fmt.Errorf("could not parse ca certificate: %w", err)
- }
-
- // Ensure both CA and node certs use ED25519.
- if certParsed.PublicKeyAlgorithm != x509.Ed25519 {
- return nil, fmt.Errorf("node certificate must use ED25519, is %s", certParsed.PublicKeyAlgorithm.String())
- }
- if pub, ok := certParsed.PublicKey.(ed25519.PublicKey); !ok || len(pub) != ed25519.PublicKeySize {
- return nil, fmt.Errorf("node certificate ED25519 key invalid")
- }
- if caCertParsed.PublicKeyAlgorithm != x509.Ed25519 {
- return nil, fmt.Errorf("CA certificate must use ED25519, is %s", caCertParsed.PublicKeyAlgorithm.String())
- }
- if pub, ok := caCertParsed.PublicKey.(ed25519.PublicKey); !ok || len(pub) != ed25519.PublicKeySize {
- return nil, fmt.Errorf("CA certificate ED25519 key invalid")
- }
-
- // Ensure that the certificate is signed by the CA certificate.
- if err := certParsed.CheckSignatureFrom(caCertParsed); err != nil {
- return nil, fmt.Errorf("certificate not signed by given CA: %w", err)
- }
-
- // Ensure that the certificate has the node's calculated ID in its DNS names.
- found := false
- nid := curator.NodeID(certParsed.PublicKey.(ed25519.PublicKey))
- for _, n := range certParsed.DNSNames {
- if n == nid {
- found = true
- break
- }
- }
- if !found {
- return nil, fmt.Errorf("calculated node ID %q not found in node certificate's DNS names (%v)", nid, certParsed.DNSNames)
- }
-
- return &NodeCertificate{
- node: certParsed,
- ca: caCertParsed,
- }, nil
-}
-
-// NewNodeCredentials wraps a pair of CA and node DER-encoded certificates plus
-// a private key into NodeCredentials, ensuring that the given data is valid and
-// compatible with Metropolis assumptions.
-//
-// It does _not_ verify that the given CA is a known/trusted Metropolis CA for a
-// running cluster.
-func NewNodeCredentials(priv, cert, ca []byte) (*NodeCredentials, error) {
- nc, err := NewNodeCertificate(cert, ca)
- if err != nil {
- return nil, err
- }
-
- // Ensure that the private key is a valid length.
- if want, got := ed25519.PrivateKeySize, len(priv); want != got {
- return nil, fmt.Errorf("private key is not the correct length, wanted %d, got %d", want, got)
- }
-
- // Ensure that the given private key matches the given public key.
- if want, got := ed25519.PrivateKey(priv).Public().(ed25519.PublicKey), nc.PublicKey(); subtle.ConstantTimeCompare(want, got) != 1 {
- return nil, fmt.Errorf("public key does not match private key")
- }
-
- return &NodeCredentials{
- NodeCertificate: *nc,
- private: ed25519.PrivateKey(priv),
- }, nil
-}
-
-// Save stores the given node credentials in local storage.
-func (c *NodeCredentials) Save(d *localstorage.PKIDirectory) error {
- if err := d.CACertificate.Write(c.ca.Raw, 0400); err != nil {
- return fmt.Errorf("when writing CA certificate: %w", err)
- }
- if err := d.Certificate.Write(c.node.Raw, 0400); err != nil {
- return fmt.Errorf("when writing node certificate: %w", err)
- }
- if err := d.Key.Write(c.private, 0400); err != nil {
- return fmt.Errorf("when writing node private key: %w", err)
- }
- return nil
-}
-
-// PublicKey returns the ED25519 public key corresponding to this node's
-// certificate/credentials.
-func (nc *NodeCertificate) PublicKey() ed25519.PublicKey {
- // Safe: we have ensured that the given certificate has an ed25519 public key on
- // NewNodeCertificate.
- return nc.node.PublicKey.(ed25519.PublicKey)
-}
-
-// ID returns the canonical ID/name of the node for which this
-// certificate/credentials were emitted.
-func (nc *NodeCertificate) ID() string {
- return curator.NodeID(nc.PublicKey())
-}
-
-func (nc *NodeCredentials) TLSCredentials() tls.Certificate {
- return tls.Certificate{
- Certificate: [][]byte{nc.node.Raw},
- PrivateKey: nc.private,
- }
-}
diff --git a/metropolis/node/core/cluster/node_test.go b/metropolis/node/core/cluster/node_test.go
deleted file mode 100644
index 079d4dc..0000000
--- a/metropolis/node/core/cluster/node_test.go
+++ /dev/null
@@ -1,105 +0,0 @@
-package cluster
-
-import (
- "crypto/ed25519"
- "crypto/rand"
- "crypto/x509"
- "crypto/x509/pkix"
- "math/big"
- "testing"
- "time"
-
- "source.monogon.dev/metropolis/node/core/curator"
-)
-
-type alterCert func(t *x509.Certificate)
-
-func createPKI(t *testing.T, fca, fnode alterCert) (caCertBytes, nodeCertBytes, nodePriv []byte) {
- t.Helper()
-
- caPub, caPriv, err := ed25519.GenerateKey(rand.Reader)
- if err != nil {
- t.Fatalf("GenerateKey: %v", err)
- }
- var nodePub ed25519.PublicKey
- nodePub, nodePriv, err = ed25519.GenerateKey(rand.Reader)
- if err != nil {
- t.Fatalf("GenerateKey: %v", err)
- }
-
- caTemplate := &x509.Certificate{
- SerialNumber: big.NewInt(1),
- Subject: pkix.Name{CommonName: "CA"},
- IsCA: true,
- KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageDigitalSignature,
- ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageOCSPSigning},
- NotBefore: time.Now(),
- NotAfter: time.Unix(253402300799, 0),
- BasicConstraintsValid: true,
- }
- fca(caTemplate)
-
- caCertBytes, err = x509.CreateCertificate(rand.Reader, caTemplate, caTemplate, caPub, caPriv)
- if err != nil {
- t.Fatalf("CreateCertificate (CA): %v", err)
- }
- caCert, err := x509.ParseCertificate(caCertBytes)
- if err != nil {
- t.Fatalf("ParseCertificate (CA): %v", err)
- }
-
- nodeTemplate := &x509.Certificate{
- SerialNumber: big.NewInt(2),
- Subject: pkix.Name{},
- KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment,
- NotBefore: time.Now(),
- NotAfter: time.Unix(253402300799, 0),
- DNSNames: []string{curator.NodeID(nodePub)},
- }
- fnode(nodeTemplate)
-
- nodeCertBytes, err = x509.CreateCertificate(rand.Reader, nodeTemplate, caCert, nodePub, caPriv)
- if err != nil {
- t.Fatalf("CreateCertificate (node): %v", err)
- }
-
- return
-}
-
-// TestNodeCertificateX509 exercises X509 validity checks performed by
-// NewNodeCertificate.
-func TestNodeCertificateX509(t *testing.T) {
- for i, te := range []struct {
- fca alterCert
- fnode alterCert
- success bool
- }{
- // Case 0: everything should work.
- {
- func(ca *x509.Certificate) {},
- func(n *x509.Certificate) {},
- true,
- },
- // Case 1: CA must be IsCA
- {
- func(ca *x509.Certificate) { ca.IsCA = false },
- func(n *x509.Certificate) {},
- false,
- },
- // Case 2: node must have its ID as a DNS name.
- {
- func(ca *x509.Certificate) {},
- func(n *x509.Certificate) { n.DNSNames = []string{"node"} },
- false,
- },
- } {
- caCert, nodeCert, nodePriv := createPKI(t, te.fca, te.fnode)
- _, err := NewNodeCredentials(nodePriv, nodeCert, caCert)
- if te.success && err != nil {
- t.Fatalf("Case %d: NewNodeCredentials failed: %v", i, err)
- }
- if !te.success && err == nil {
- t.Fatalf("Case %d: NewNodeCredentials succeeded, wanted failure", i)
- }
- }
-}
diff --git a/metropolis/node/core/cluster/status.go b/metropolis/node/core/cluster/status.go
index 3f99567..3dbfb56 100644
--- a/metropolis/node/core/cluster/status.go
+++ b/metropolis/node/core/cluster/status.go
@@ -5,6 +5,7 @@
"fmt"
"source.monogon.dev/metropolis/node/core/consensus/client"
+ "source.monogon.dev/metropolis/node/core/identity"
cpb "source.monogon.dev/metropolis/proto/common"
)
@@ -26,7 +27,7 @@
// Credentials used for the node to authenticate to the Curator and other
// cluster services.
- Credentials *NodeCredentials
+ Credentials *identity.NodeCredentials
}
// ConsensusUser is the to-level user of an etcd client in Metropolis node