A token with all four parts

Given the broker's address and its certificate, mesh-control now issues a token carrying everything ADR 0004 asks for: where to connect, what to expect there, whose signature to believe afterwards, and a one-time right to join. Verified by decoding one and checking the fingerprint against `openssl x509 | sha256sum` -- they match. The fingerprint is derived from the certificate on disk and never configured. A configured pin can drift from the certificate it describes, and a drifted pin is worse than none: every node issued a token during the drift refuses to connect, and the failure looks like an attack rather than a mistake. Computed over DER, which is what a client sees on the wire. Hashing the PEM text instead would mean the same certificate, re-wrapped with different line endings, produced a different pin -- there is a test for exactly that, and one for pointing this at tls.key by mistake, which would otherwise produce a confident pin over the wrong file. Having no broker stays a state rather than a failure: a control plane holds records and a signing key without one. Having half a broker is refused, because a token with an address and nothing to check it against invites a node to trust whatever answers. Fault injection caught the same weak test I wrote earlier in the day -- asking whether something failed rather than why, so deleting the guard changed nothing because it failed one line later anyway. Both are now asserted on the reason.
2026-08-29 15:13:54 +02:00
parent 7553af6c5a
commit ea6569277d
4 changed files with 320 additions and 5 deletions
@@ -0,0 +1,180 @@
+package broker
+
+import (
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
+	"crypto/sha256"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/hex"
+	"encoding/pem"
+	"errors"
+	"math/big"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+)
+
+// writeCertificate puts a real self-signed certificate on disk and returns its path and the
+// DER bytes, which is what a TLS client would see on the wire.
+func writeCertificate(t *testing.T) (string, []byte) {
+	t.Helper()
+	key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	template := x509.Certificate{
+		SerialNumber: big.NewInt(1),
+		Subject:      pkix.Name{CommonName: "mesh-broker"},
+		NotBefore:    time.Now().Add(-time.Hour),
+		NotAfter:     time.Now().Add(24 * time.Hour),
+	}
+	der, err := x509.CreateCertificate(rand.Reader, &template, &template, &key.PublicKey, key)
+	if err != nil {
+		t.Fatal(err)
+	}
+	path := filepath.Join(t.TempDir(), "tls.crt")
+	if err := os.WriteFile(path, pem.EncodeToMemory(
+		&pem.Block{Type: "CERTIFICATE", Bytes: der}), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	return path, der
+}
+
+func TestTheFingerprintIsOverWhatAClientSees(t *testing.T) {
+	// A node computes this from the certificate the broker presents, which is DER on the wire.
+	// Hashing the PEM text instead would mean the same certificate, re-wrapped with different
+	// line endings, produced a different pin — and every token issued around that moment would
+	// send a node to something it refuses to talk to.
+	path, der := writeCertificate(t)
+
+	got, err := FingerprintOf(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	sum := sha256.Sum256(der)
+	want := "sha256:" + hex.EncodeToString(sum[:])
+	if got != want {
+		t.Errorf("fingerprint is %s, and a client computing it from the wire gets %s", got, want)
+	}
+}
+
+func TestReWrappingTheSameCertificateDoesNotChangeThePin(t *testing.T) {
+	// The property the test above protects, stated directly: same certificate, different file
+	// formatting, same pin.
+	path, der := writeCertificate(t)
+	first, err := FingerprintOf(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	rewrapped := filepath.Join(t.TempDir(), "same.crt")
+	body := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der})
+	if err := os.WriteFile(rewrapped, append([]byte("\n\n"), body...), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	second, err := FingerprintOf(rewrapped)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if first != second {
+		t.Errorf("the same certificate produced two pins:\n  %s\n  %s", first, second)
+	}
+}
+
+func TestAPrivateKeyIsNotACertificate(t *testing.T) {
+	// The mistake somebody makes once: pointing this at tls.key. Left unchecked it would produce
+	// a confident pin over the wrong file, and every node would refuse the broker.
+	key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	der, err := x509.MarshalECPrivateKey(key)
+	if err != nil {
+		t.Fatal(err)
+	}
+	path := filepath.Join(t.TempDir(), "tls.key")
+	if err := os.WriteFile(path, pem.EncodeToMemory(
+		&pem.Block{Type: "EC PRIVATE KEY", Bytes: der}), 0o600); err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = FingerprintOf(path)
+	if err == nil {
+		t.Fatal("a private key was fingerprinted as if it were a certificate")
+	}
+	if !strings.Contains(err.Error(), "private key") {
+		t.Errorf("the error does not say what the file actually is: %v", err)
+	}
+}
+
+func TestAMalformedCertificateIsRefusedRatherThanHashed(t *testing.T) {
+	// Bytes wrapped in the right PEM header are not a certificate. Hashing them would produce a
+	// pin that matches nothing, and the failure would arrive on a node instead of here.
+	path := filepath.Join(t.TempDir(), "broken.crt")
+	if err := os.WriteFile(path, pem.EncodeToMemory(
+		&pem.Block{Type: "CERTIFICATE", Bytes: []byte("not a certificate")}), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := FingerprintOf(path); err == nil {
+		t.Fatal("malformed bytes were accepted as a certificate")
+	}
+}
+
+func TestNoBrokerIsAStateAndNotAFailure(t *testing.T) {
+	t.Setenv(AddressVar, "")
+	t.Setenv(CertificateVar, "")
+	if _, err := FromEnvironment(); !errors.Is(err, ErrNotConfigured) {
+		t.Fatalf("expected ErrNotConfigured, got %v", err)
+	}
+}
+
+func TestAnAddressWithoutACertificateIsRefused(t *testing.T) {
+	// Worse than neither: a token with somewhere to connect and nothing to check would have a
+	// node trust whatever answers at that address.
+	//
+	// Asserted on which refusal fired. Without the check this still fails a line later, trying to
+	// read a certificate at the empty path — so a test asking only "was there an error" passes
+	// with the guard deleted. It was written that way first and confirmed to defend nothing.
+	t.Setenv(AddressVar, "192.0.2.10:5671")
+	t.Setenv(CertificateVar, "")
+
+	_, err := FromEnvironment()
+	if err == nil || errors.Is(err, ErrNotConfigured) {
+		t.Fatalf("an address with no certificate was accepted: %v", err)
+	}
+	if !strings.Contains(err.Error(), "must be set together") {
+		t.Errorf("refused for the wrong reason: %v", err)
+	}
+}
+
+func TestACertificateWithoutAnAddressIsRefused(t *testing.T) {
+	path, _ := writeCertificate(t)
+	t.Setenv(AddressVar, "")
+	t.Setenv(CertificateVar, path)
+
+	_, err := FromEnvironment()
+	if err == nil || errors.Is(err, ErrNotConfigured) {
+		t.Fatalf("a certificate with no address was accepted: %v", err)
+	}
+	if !strings.Contains(err.Error(), "must be set together") {
+		t.Errorf("refused for the wrong reason: %v", err)
+	}
+}
+
+func TestBothTogetherGiveABroker(t *testing.T) {
+	path, _ := writeCertificate(t)
+	t.Setenv(AddressVar, "192.0.2.10:5671")
+	t.Setenv(CertificateVar, path)
+
+	known, err := FromEnvironment()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if known.Address != "192.0.2.10:5671" || !strings.HasPrefix(known.Fingerprint, "sha256:") {
+		t.Errorf("got %+v", known)
+	}
+}