diff --git a/cmd/mesh-host/main.go b/cmd/mesh-host/main.go index a1ae9a2..792b520 100644 --- a/cmd/mesh-host/main.go +++ b/cmd/mesh-host/main.go @@ -706,15 +706,18 @@ func enrol(ctx context.Context, opts options) error { fmt.Printf(" signing key %s\n", base64.StdEncoding.EncodeToString(token.Signer)[:16]+"...") - // The check that has to happen before this machine says anything. - conn, err := link.Dial(token.Broker, token.Fingerprint, opts.timeout) - if err != nil { - return err - } - defer conn.Close() - fmt.Println("\nthe broker presented the certificate this token pins") - - conn.Close() + // **The pin is checked by the connection that presents this token, not by a dial of our own** + // (novox/hq 04-ISSUES/146). This opened a raw TLS connection to the bus first, which worked + // against the broker the mesh used to run and cannot work against the one it runs now: NATS + // speaks its own protocol before it upgrades to TLS, so an immediate handshake is answered + // with a plaintext line and the enrolment failed with "first record does not look like a TLS + // handshake" — on every node that has tried to join since the bus changed, which is why this + // went unnoticed: none had. + // + // What ADR 0004 requires still holds, and holds better: the client that presents the token + // carries the same pinned configuration, reads the server's greeting, upgrades, and the + // verification runs inside that handshake — so the one-time secret is sent only after the + // certificate has been checked, and nothing of this node's reaches an impostor. mine, err := identity.Generate(*name) if err != nil { @@ -788,10 +791,16 @@ func enrol(ctx context.Context, opts options) error { proof := mine.Sign(link.EnrolProof(token.Secret, mine.Public, mine.Overlay.Public, sealing.Public, serving.Public)) // The token says where to go and which certificate that address must present. It says nothing - // about which bus is there, and does not need to: every token names the one the mesh runs on - // today until the rollout (novox/hq ADR 0116 step 5), and that is what an empty Transport is. + // about which bus is there, and does not need to: there is one, and this host knows which + // (novox/hq ADR 0131 — the mesh speaks to one seat and the old transport is gone). + // + // **It used to leave this empty** and mean "whatever the mesh runs today", which was true + // while two buses existed and became a refusal the moment one did: an empty transport is not + // the bus's name, so every enrolment ended at "this token is for the \"\" bus" + // (novox/hq 04-ISSUES/146). Nothing caught it because nothing had enrolled since the bus + // changed. reply, err := link.Enrol(ctx, - link.Approach{Address: token.Broker, Fingerprint: token.Fingerprint}, + link.Approach{Address: token.Broker, Fingerprint: token.Fingerprint, Transport: link.OnNATS}, *name, token.Secret, mine.Public, mine.Overlay.Public, sealing.Public, serving.Public, reported, proof, found, opts.timeout) diff --git a/examples/foundation-first-node-nats.lock b/examples/foundation-first-node-nats.lock index 282182f..4f36d73 100644 --- a/examples/foundation-first-node-nats.lock +++ b/examples/foundation-first-node-nats.lock @@ -127,12 +127,21 @@ { "id": "bus-certificate", "type": "action", - "command": ["docker", "run", "--rm", "--entrypoint", "sh", "-v", "mesh-broker-tls:/tls", - "192.0.2.250:5000/nats@sha256:b83efabe3e7def1e0a4a31ec6e078999bb17c80363f881df35edc70fcb6bb927", - "-c", "test -f /tls/tls.crt || (openssl req -x509 -newkey rsa:2048 -nodes -keyout /tls/tls.key -out /tls/tls.crt -days 3650 -subj '/CN=mesh-broker' -addext 'subjectAltName=DNS:mesh-broker,IP:127.0.0.1' >/dev/null 2>&1 && chmod 644 /tls/tls.crt && chmod 600 /tls/tls.key)"], - "verify": ["docker", "run", "--rm", "--entrypoint", "sh", "-v", "mesh-broker-tls:/tls", - "192.0.2.250:5000/nats@sha256:b83efabe3e7def1e0a4a31ec6e078999bb17c80363f881df35edc70fcb6bb927", - "-c", "test -s /tls/tls.crt && openssl x509 -in /tls/tls.crt -noout"] + // **The mesh makes its own** (novox/hq 04-ISSUES/146). This ran `openssl` inside the + // broker's image while the broker was one that carried it; the bus that replaced it has a + // shell and no openssl, and no other image the bundle names has one either. So the program + // that needs the certificate writes it — already on this machine, since the schema step ran + // it, and asking nothing of the image it writes into. Self-signed on purpose: a host pins + // this server's exact certificate (novox/hq ADR 0004), and at this moment there is no mesh + // to ask an authority of. + // `--user 0:0` because the volume is root's and this image runs as nobody, which is right + // for the long-running control plane and wrong for a one-shot writing into a fresh volume. + "command": ["docker", "run", "--rm", "--user", "0:0", "-v", "mesh-broker-tls:/tls", + "192.0.2.250:5000/mesh-controller@sha256:c67db38439ff0aee242b467486765467bb95801f52175fc5727cc4e437338ace", + "broker", "certificate", "--into", "/tls"], + "verify": ["docker", "run", "--rm", "--user", "0:0", "-v", "mesh-broker-tls:/tls", + "192.0.2.250:5000/mesh-controller@sha256:c67db38439ff0aee242b467486765467bb95801f52175fc5727cc4e437338ace", + "broker", "certificate", "--check", "--into", "/tls"] }, { "id": "bus-conf-dir", diff --git a/internal/link/pinned.go b/internal/link/pinned.go index 4e12f10..aae5cce 100644 --- a/internal/link/pinned.go +++ b/internal/link/pinned.go @@ -73,8 +73,19 @@ func PinnedConfig(pin string) (*tls.Config, error) { }, nil } -// Dial opens a TLS connection to the broker, refusing anything but the pinned certificate. -func Dial(address, pin string, timeout time.Duration) (*tls.Conn, error) { +// dialPinned completes a TLS handshake against an address, refusing anything but the pinned +// certificate. +// +// **Not how the bus is reached, and it used to be** (novox/hq 04-ISSUES/146). Enrolment opened one +// of these before it said anything, which was right while the broker answered TLS immediately and +// wrong the moment the mesh moved to a bus that speaks its own protocol first. The pin itself was +// never the problem — PinnedConfig is what the NATS client is given, and the verification runs +// inside the handshake that client performs. +// +// It stays here because this is where the pin is proven: the tests beside it run a real TLS server +// and assert that a wrong certificate is refused before a byte of application data is sent. What it +// must not become again is something a caller uses to reach the bus. +func dialPinned(address, pin string, timeout time.Duration) (*tls.Conn, error) { config, err := PinnedConfig(pin) if err != nil { return nil, err @@ -86,7 +97,7 @@ func Dial(address, pin string, timeout time.Duration) (*tls.Conn, error) { if errors.Is(err, ErrWrongCertificate) { return nil, err } - return nil, fmt.Errorf("cannot reach the broker at %s: %w", address, err) + return nil, fmt.Errorf("cannot reach %s: %w", address, err) } return conn, nil } diff --git a/internal/link/pinned_test.go b/internal/link/pinned_test.go index 792b8ec..05d6ec8 100644 --- a/internal/link/pinned_test.go +++ b/internal/link/pinned_test.go @@ -63,7 +63,7 @@ func server(t *testing.T) (address string, fingerprint string) { func TestTheRightBrokerIsAccepted(t *testing.T) { address, pin := server(t) - conn, err := Dial(address, pin, 5*time.Second) + conn, err := dialPinned(address, pin, 5*time.Second) if err != nil { t.Fatalf("the broker its token describes was refused: %v", err) } @@ -76,7 +76,7 @@ func TestADifferentBrokerIsRefused(t *testing.T) { address, _ := server(t) _, other := server(t) - _, err := Dial(address, other, 5*time.Second) + _, err := dialPinned(address, other, 5*time.Second) if err == nil { t.Fatal("a broker presenting a different certificate was accepted") } @@ -130,7 +130,7 @@ func TestNothingIsSentToTheWrongBroker(t *testing.T) { // A pin for a certificate this server does not have. _, elsewhere := server(t) - if _, err := Dial(listener.Addr().String(), elsewhere, 5*time.Second); err == nil { + if _, err := dialPinned(listener.Addr().String(), elsewhere, 5*time.Second); err == nil { t.Fatal("the impostor was accepted") } if n := <-received; n > 0 { @@ -160,7 +160,7 @@ func TestAnUnreachableBrokerIsAnOrdinaryFailure(t *testing.T) { address := listener.Addr().String() listener.Close() - _, err = Dial(address, pin, 2*time.Second) + _, err = dialPinned(address, pin, 2*time.Second) if err == nil { t.Fatal("dialling a closed port succeeded") }