Avoid retaining failed TLS connections in sender

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 ./...
-```

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

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)
+	}
+}