diff --git a/cmd/mesh-builder/main.go b/cmd/mesh-builder/main.go index c19a508..5e9b7f8 100644 --- a/cmd/mesh-builder/main.go +++ b/cmd/mesh-builder/main.go @@ -324,22 +324,37 @@ func dial(held Credential) (*amqp.Connection, error) { if held.Fingerprint == "" { return amqp.Dial(held.URL) } - // InsecureSkipVerify with a VerifyPeerCertificate is **pinning, not skipping**: the standard - // chain check is replaced, not removed, and what replaces it is stricter — one certificate is - // accepted rather than every certificate a public authority would sign. - return amqp.DialTLS(held.URL, &tls.Config{ + return amqp.DialTLS(held.URL, pinning(held.Fingerprint)) +} + +// pinning is a TLS configuration that trusts exactly one certificate. +// +// InsecureSkipVerify with a VerifyPeerCertificate is **pinning, not skipping**: the standard chain +// check is replaced, not removed, and what replaces it is stricter — one certificate is accepted +// rather than every certificate a public authority would sign. +// +// Its own function so a test can drive it against a real handshake. A pin check that is only ever +// exercised through a broker is a pin check nothing tests. +func pinning(fingerprint string) *tls.Config { + return &tls.Config{ InsecureSkipVerify: true, VerifyPeerCertificate: func(raw [][]byte, _ [][]*x509.Certificate) error { if len(raw) == 0 { return errors.New("the broker presented no certificate") } - got := sha256.Sum256(raw[0]) - if hex.EncodeToString(got[:]) != held.Fingerprint { + // The leaf, and in the same spelling the mesh writes it — `sha256:` and 64 hex + // characters. Comparing a bare digest against a written fingerprint never matches, + // and the failure is indistinguishable from being pointed at the wrong broker. + sum := sha256.Sum256(raw[0]) + got := "sha256:" + hex.EncodeToString(sum[:]) + if got != fingerprint { return fmt.Errorf( - "this is not the broker this builder was told about: it presented a "+ - "certificate with fingerprint %s", hex.EncodeToString(got[:])) + "this is not the broker this builder was told about\n expected %s\n "+ + "got %s\nEither this mesh's broker was replaced, or this builder is "+ + "being pointed at something else. Retrying will not help", + fingerprint, got) } return nil }, - }) + } } diff --git a/cmd/mesh-builder/where_test.go b/cmd/mesh-builder/where_test.go index 800ca35..5571493 100644 --- a/cmd/mesh-builder/where_test.go +++ b/cmd/mesh-builder/where_test.go @@ -1,10 +1,20 @@ package main import ( + "crypto/ed25519" + "crypto/rand" + "crypto/sha256" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/hex" + "math/big" + "net" "os" "path/filepath" "strings" "testing" + "time" ) // Where a builder publishes. @@ -141,3 +151,73 @@ func TestABuilderWithNoCredentialAtAllSaysSo(t *testing.T) { t.Fatal("a builder with no broker reported one") } } + +// The pin is compared in the spelling the mesh writes it. +// +// A bare digest against a written fingerprint never matches, and the failure is indistinguishable +// from being pointed at the wrong broker — which is the one thing this check exists to report +// truthfully. It cost a lab run. +func TestThePinIsComparedInTheSpellingTheMeshWritesIt(t *testing.T) { + certificate, key := aServerCertificate(t) + der := certificate.Certificate[0] + sum := sha256.Sum256(der) + written := "sha256:" + hex.EncodeToString(sum[:]) + + listener, err := tls.Listen("tcp", "127.0.0.1:0", &tls.Config{ + Certificates: []tls.Certificate{certificate}, MinVersion: tls.VersionTLS12, + }) + if err != nil { + t.Fatal(err) + } + defer listener.Close() + go func() { + for { + conn, err := listener.Accept() + if err != nil { + return + } + _ = conn.(*tls.Conn).Handshake() + conn.Close() + } + }() + _ = key + + // The pin the mesh wrote must be accepted. + if err := handshakeWith(listener.Addr().String(), written); err != nil { + t.Fatalf("the broker this builder was told about was refused: %v", err) + } + // And a different one refused, or the check reports nothing. + other := "sha256:" + strings.Repeat("ab", 32) + if err := handshakeWith(listener.Addr().String(), other); err == nil { + t.Fatal("a broker this builder was not told about was accepted") + } +} + +// handshakeWith runs the builder's own pin check against an address. +func handshakeWith(address, pin string) error { + conn, err := tls.Dial("tcp", address, pinning(pin)) + if err != nil { + return err + } + return conn.Close() +} + +func aServerCertificate(t *testing.T) (tls.Certificate, ed25519.PrivateKey) { + t.Helper() + public, private, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatal(err) + } + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "a broker"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + IPAddresses: []net.IP{net.ParseIP("127.0.0.1")}, + } + der, err := x509.CreateCertificate(rand.Reader, template, template, public, private) + if err != nil { + t.Fatal(err) + } + return tls.Certificate{Certificate: [][]byte{der}, PrivateKey: private}, private +}