From a3441383f09ffd5a42ccfb0acb7a4506e84fd1fe Mon Sep 17 00:00:00 2001 From: Mark Mennell Date: Thu, 17 Sep 2026 09:57:20 +0800 Subject: [PATCH 1/2] Avoid retaining failed TLS connections in sender --- cmd/fmsgd/sender.go | 26 ++++--- cmd/fmsgd/sender_tls_test.go | 129 +++++++++++++++++++++++++++++++++++ 2 files changed, 145 insertions(+), 10 deletions(-) create mode 100644 cmd/fmsgd/sender_tls_test.go diff --git a/cmd/fmsgd/sender.go b/cmd/fmsgd/sender.go index f0872a0..092f14d 100644 --- a/cmd/fmsgd/sender.go +++ b/cmd/fmsgd/sender.go @@ -508,6 +508,21 @@ func markLocalDelivered(target pendingTarget) { } } +// dialTargetIPs returns only a successfully connected TLS connection. Failed +// dials return a nil *tls.Conn, which must not be retained in a net.Conn interface. +func dialTargetIPs(targetIPs []net.IP, port int, tlsConf *tls.Config) net.Conn { + dialer := &net.Dialer{Timeout: 10 * time.Second} + for _, ip := range targetIPs { + addr := net.JoinHostPort(ip.String(), fmt.Sprintf("%d", port)) + conn, err := tls.DialWithDialer(dialer, "tcp", addr, tlsConf) + if err == nil { + return conn + } + log.Printf("WARN: sender: connect to %s failed: %s", addr, err) + } + return nil +} + // deliverUnit sends one wire message — the original message or a single add-to // batch — to target.Domain over its own connection, recording per-recipient // outcomes. It owns its transaction: it locks this unit's pending recipients in @@ -592,17 +607,8 @@ func deliverUnit(db *sql.DB, target pendingTarget, h *FMsgHeader, table string, return } - var conn net.Conn - dialer := &net.Dialer{Timeout: 10 * time.Second} tlsConf := buildClientTLSConfig("fmsg." + target.Domain) - for _, ip := range targetIPs { - addr := net.JoinHostPort(ip.String(), fmt.Sprintf("%d", RemotePort)) - conn, err = tls.DialWithDialer(dialer, "tcp", addr, tlsConf) - if err == nil { - break - } - log.Printf("WARN: sender: connect to %s failed: %s", addr, err) - } + conn := dialTargetIPs(targetIPs, RemotePort, tlsConf) if conn == nil { log.Printf("ERROR: sender: could not connect to any IP for fmsg.%s", target.Domain) return diff --git a/cmd/fmsgd/sender_tls_test.go b/cmd/fmsgd/sender_tls_test.go new file mode 100644 index 0000000..c9020c1 --- /dev/null +++ b/cmd/fmsgd/sender_tls_test.go @@ -0,0 +1,129 @@ +package main + +import ( + "crypto/ed25519" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "errors" + "io" + "math/big" + "net" + "testing" + "time" +) + +func senderTestCertificate(t *testing.T, notAfter time.Time) (tls.Certificate, *tls.Config) { + t.Helper() + publicKey, privateKey, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatal(err) + } + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + DNSNames: []string{"fmsg.example.com"}, + NotBefore: time.Now().Add(-2 * time.Hour), + NotAfter: notAfter, + KeyUsage: x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + } + der, err := x509.CreateCertificate(rand.Reader, template, template, publicKey, privateKey) + if err != nil { + t.Fatal(err) + } + leaf, err := x509.ParseCertificate(der) + if err != nil { + t.Fatal(err) + } + config := buildClientTLSConfig("fmsg.example.com") + if config.InsecureSkipVerify { + t.Fatal("sender must verify the server certificate") + } + config.RootCAs = x509.NewCertPool() + config.RootCAs.AddCert(leaf) + config.ClientSessionCache = tls.NewLRUClientSessionCache(1) + return tls.Certificate{Certificate: [][]byte{der}, PrivateKey: privateKey, Leaf: leaf}, config +} + +func startSenderTLSServer(t *testing.T, cert tls.Certificate) (int, <-chan error) { + t.Helper() + listener, err := tls.Listen("tcp4", "127.0.0.1:0", &tls.Config{ + Certificates: []tls.Certificate{cert}, + MinVersion: tls.VersionTLS13, + MaxVersion: tls.VersionTLS13, + NextProtos: []string{"fmsg/1"}, + }) + if err != nil { + t.Fatal(err) + } + result := make(chan error, 1) + go func() { + defer close(result) + conn, err := listener.Accept() + if err == nil { + defer conn.Close() + err = conn.SetDeadline(time.Now().Add(5 * time.Second)) + if err == nil { + err = conn.(*tls.Conn).Handshake() + } + if err == nil { + _, err = conn.Write([]byte{AcceptCodeContinue}) + } + } + result <- err + }() + t.Cleanup(func() { + listener.Close() + <-result + }) + return listener.Addr().(*net.TCPAddr).Port, result +} + +func TestDialTargetIPsExpiredCertificate(t *testing.T) { + cert, config := senderTestCertificate(t, time.Now().Add(-time.Hour)) + // The fixture is trusted and has the correct hostname; expiry is the failure. + _, err := cert.Leaf.Verify(x509.VerifyOptions{DNSName: config.ServerName, Roots: config.RootCAs}) + var invalid x509.CertificateInvalidError + if !errors.As(err, &invalid) || invalid.Reason != x509.Expired { + t.Fatalf("fixture verification = %v, want expired certificate", err) + } + port, serverResult := startSenderTLSServer(t, cert) + conn := dialTargetIPs([]net.IP{net.ParseIP("127.0.0.1")}, port, config) + if conn != nil { + t.Fatalf("failed TLS dial retained a non-nil net.Conn (%T); caller must return for retry", conn) + } + if err := <-serverResult; err == nil { + t.Fatal("expired certificate unexpectedly completed a TLS handshake") + } +} + +func TestDialTargetIPsFallbackAfterUnreachableIP(t *testing.T) { + cert, config := senderTestCertificate(t, time.Now().Add(time.Hour)) + port, serverResult := startSenderTLSServer(t, cert) + // The server listens only on 127.0.0.1, so the first loopback IP refuses TCP. + conn := dialTargetIPs([]net.IP{net.ParseIP("127.0.0.2"), net.ParseIP("127.0.0.1")}, port, config) + if conn == nil { + t.Fatal("did not fall back to the reachable IP") + } + defer conn.Close() + if addr := conn.RemoteAddr().(*net.TCPAddr); !addr.IP.Equal(net.ParseIP("127.0.0.1")) || addr.Port != port { + t.Fatalf("connected to %s, want the fallback server on port %d", addr, port) + } + state := conn.(*tls.Conn).ConnectionState() + if !state.HandshakeComplete || state.Version != tls.VersionTLS13 || state.NegotiatedProtocol != "fmsg/1" || len(state.VerifiedChains) == 0 { + t.Fatalf("fallback did not establish verified TLS 1.3 with fmsg/1: %+v", state) + } + if err := conn.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil { + t.Fatal(err) + } + var response [1]byte + if _, err := io.ReadFull(conn, response[:]); err != nil { + t.Fatalf("fallback connection is not usable: %v", err) + } + if response[0] != AcceptCodeContinue { + t.Fatalf("response = %d, want %d", response[0], AcceptCodeContinue) + } + if err := <-serverResult; err != nil { + t.Fatalf("fallback server: %v", err) + } +} From 583a689eebb47f6b97278ca9dde340f45c6804e0 Mon Sep 17 00:00:00 2001 From: Mark Mennell Date: Thu, 17 Sep 2026 10:34:31 +0800 Subject: [PATCH 2/2] Remove immutable finalization and upgrade section from README --- README.md | 60 ------------------------------------------------------- 1 file changed, 60 deletions(-) diff --git a/README.md b/README.md index 9eca337..c58dfb0 100644 --- a/README.md +++ b/README.md @@ -141,63 +141,3 @@ sudo systemctl daemon-reload sudo systemctl enable fmsgd sudo systemctl start fmsgd ``` -## Immutable message finalization and upgrades - -`fmsg-webapi` finalizes local messages with `pkg/message`: the timestamp, SHA-256, -exact header, and durable wire payloads are committed together, including local-only -messages and reactions. The hash covers the encoded wire header and expanded body -and attachment bytes. Compression and common media type encoding are chosen before -hashing. Add-to exchanges retain independent hashes and reuse the finalized payload. -The daemon reuses these representations for federation and challenge responses; -it refuses a representation that differs from an established hash. - -The `wire_message` JSONB columns are versioned internal snapshots containing payload -paths; they are not API objects. `.fmsg-wire-*` directories beside message content -must be retained with the message database and data directory. Both services need -access to the shared files (normally the same service user/group). The API keeps its -expanded downloadable content separately. New received messages also preserve their -wire payloads before expanding the downloadable copies. - -`dd.sql` bootstraps a new, empty database. The daemon and API require finalized -sent messages and do not repair old rows during normal operation. - -For an existing installation, build the single standalone migration binary: - -```sh -CGO_ENABLED=0 go build -o fmsg-backfill ./cmd/fmsg-backfill -``` - -The binary embeds the schema changes; no SQL scripts, source checkout or running -services are needed on the target host. It upgrades the pre-finalization schema -(with `wire_header` and add-to batch hashes) and can also verify a completed migration. -It uses the standard `PG*` connection variables. Run it as the service account with -write access to the database and every stored payload path, including shared volumes. - -Stop both services and back up the message database and data directory together. -Then validate and apply the conversion before starting the matching daemon and API: - -```sh -./fmsg-backfill -domain example.com -./fmsg-backfill -domain example.com -apply -``` - -The default is a full dry run: it reconstructs and verifies every sent message and -batch, then rolls back schema/data changes and removes staged files. `-apply` commits -the schema and data together in one transaction. It preserves timestamps, message IDs -and all published hashes, finalizes local-only parents before replies, and prepares -existing federated messages for later delivery. A successful run installs the strict -schema; **do not rerun `dd.sql` against the existing database**. - -Missing files, inconsistent reply identities or representations that cannot reproduce -a published hash fail the entire migration. Old received compression must be -reconstructible with the exact declared wire size; otherwise recover the original wire -payload before upgrading. Resolve reported records and rerun while services remain -stopped. The command is separate from the daemon and is not bundled in its image. -A process crash before commit may leave an unreferenced `.fmsg-wire-*` directory; -only remove such directories after checking both snapshot columns for references. - -PostgreSQL tests use an isolated temporary schema in the supplied test database: - -```sh -FMSG_TEST_DATABASE_URL=postgres://postgres@localhost/fmsg_test?sslmode=disable go test ./... -```