swapruntime: authenticate swap mailbox clients

[sputn1ck]

Summary

• rebase on main now that multi: consolidated security fixes (May 15) #459 has merged
• export shared mailbox gateway auth helpers from serverconn
• provide a configurable server-side gateway mailbox auth interceptor and gateway-token client interceptor
• provide a reusable authenticated MailboxServiceClient wrapper for Send, Pull, and AckUpTo
• switch swapclientserver to the shared authenticated mailbox client wrapper

Used by:

• https://github.com/lightninglabs/darepo/pull/466
• https://github.com/lightninglabs/swapdk-server/pull/36

Validation

• go test ./serverconn ./darepod -count=1
• go test -tags swapruntime ./swapclientserver -count=1
• go test ./swaprpc -count=1

Note: make lint-local still reports existing line-length issues in cmd/darepocli/internal/gen-devrpc/main.go on this base.

[gemini-code-assist]

Code Review

This pull request introduces a mechanism for authenticating mailbox RPCs using the daemon's identity key. It adds a SignMailboxAuth method to the RPC server and implements a signingMailboxClient wrapper that automatically attaches Schnorr signatures to outgoing mailbox requests (Send, Pull, and AckUpTo). The newSwapServerClients function was updated to integrate this signing logic, and corresponding tests were added to verify the inclusion of the authentication header. I have no feedback to provide.

[Copilot]

Pull request overview

This PR adds daemon-backed mailbox authentication signing so the swap client subserver can authenticate swapserver mailbox gateway RPCs (Send/Pull/AckUpTo) using the daemon identity key, and extends tests to verify REST header propagation.

Changes:

• Expose RPCServer.SignMailboxAuth to allow optional subservers to request a mailbox auth signature without direct wallet/key access.
• Wrap swapserver mailbox clients with a signing decorator that injects x-mailbox-auth-sig per mailbox ID for gRPC and REST transports.
• Extend swapclientserver REST transport tests to validate mailbox auth header propagation (currently for Pull).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
swapclientserver/service.go	Introduces a signing mailbox client wrapper and wires it into swapserver client construction.
swapclientserver/service_test.go	Updates REST client test harness to assert mailbox auth header propagation and adapts to new constructor signature.
darepod/rpc_server.go	Adds a public RPCServer helper for producing hex-encoded mailbox auth signatures.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: beed06a11d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Delay mailbox signer wiring until identity key is derived

Wiring rpcServer.SignMailboxAuth at subserver registration time introduces a startup race: Register() calls resumePending immediately, so pending receive-swap workers can issue mailbox Pull/AckUpTo before darepod reaches startWalletReadyServices (connectAndBootstrapMailbox) where s.clientKeyDesc is derived. In that window, signMailboxAuth reads s.clientKeyDesc.PubKey and can panic when building the auth message (or fail the worker), causing restart-time receive swaps to stop unexpectedly in swapruntime builds with pending sessions.

Useful? React with 👍 / 👎.

[sputn1ck]

@claude please review

[claude]

Claude finished @sputn1ck's task in 5m 37s —— View job

---

Code Review

• Gather context and understand request
• Read changed files
• Analyze code for correctness, security, and style
• Post review findings

---

The overall shape of the PR is clean — exporting the auth wrapper from serverconn, providing a MailboxAuthSigner callback type, and wiring it through newSwapServerClients is a sound approach. A few issues need attention before merging.

---

High Priority

1. Startup race: SignMailboxAuth called before clientKeyDesc is set

swapclientserver.Register is called from darepod.Server.run() as part of the RPCServiceRegistrars loop (line 930 of server.go), before startWalletReadyServices fires. connectAndBootstrapMailbox (which calls deriveIdentityKeyEarly to populate s.clientKeyDesc) runs inside startWalletReadyServices. resumePending is called synchronously inside Register, which re-arms background goroutines for persisted swap sessions. Those goroutines will call Pull/AckUpTo → SignMailboxAuth → signMailboxAuth → MailboxAuthMessage(s.clientKeyDesc.PubKey, ...) → SerializeCompressed() on a nil *btcec.PublicKey, which panics.

In LND mode the window is small (wallet is already ready by line 1018, so startWalletReadyServices runs synchronously immediately after registration). In lwwallet/btcwallet mode the wallet may not unlock for an arbitrarily long time — the window is wide.

Codex's P1 comment identifies the same issue. The fix is to guard signMailboxAuth (or MailboxAuthMessage) against a nil pubkey, or to ensure resumePending is not called until connectAndBootstrapMailbox completes (e.g., via the existing SuppressResume / higher-layer resume pattern).

Fix this →

2. ClientTLSCerts() always returns nil certs at connection time

ClientTLSCerts() is called in newSwapClientService (before startWalletReadyServices). At that moment clientKeyDesc.PubKey is always nil (identity key has not been derived yet), so the function silently returns nil, nil — the swap server connection is established without an mTLS client cert for the entire daemon lifetime. The client cert is never retroactively injected into an already-established *grpc.ClientConn or *http.Transport. This means mTLS for swap connections effectively never works with this approach.

A log-level warning when certs are omitted (or an architectural note that the swap connection must be re-established after wallet ready) would at minimum make the degradation observable. If full mTLS support is intended, the connection establishment needs to be deferred until after connectAndBootstrapMailbox.

---

Medium Priority

3. AppendToOutgoingContext vs Set for auth header

serverconn/mailbox_auth_rpc.go:107:

return metadata.AppendToOutgoingContext(ctx, AuthHeaderKey, sig), nil

If the incoming context already carries an x-mailbox-auth-sig value (e.g., a future refactor pre-populates it, or the wrapper is stacked), Append will produce multiple header values. The REST gateway emits a separate HTTP header per metadata value, so a double-set auth header will reach the server. Prefer:

md, ok := metadata.FromOutgoingContext(ctx)
if !ok {
    md = metadata.New(nil)
} else {
    md = md.Copy()
}
md.Set(AuthHeaderKey, sig)
return metadata.NewOutgoingContext(ctx, md), nil

Fix this →

4. Dead code: isLocalSwapServerAddr is no longer called

Both call sites in swapServerDialOptions and swapServerRESTBaseURL were removed in this PR. The function at swapclientserver/service.go:756 has no remaining callers and should be deleted. (The linter may flag this on the CI run.)

Fix this →

5. Unused addr parameter in swapServerDialOptions

swapclientserver/service.go:607:

func swapServerDialOptions(cfg *darepod.SwapConfig, _ string,
    clientCerts []tls.Certificate) ([]grpc.DialOption, error) {

addr (now _) was only used by isLocalSwapServerAddr, which was also removed. The parameter should be dropped from the signature and all call sites updated, rather than left as a blank identifier.

---

Low Priority / Style

6. Test gap in REST transport test

swapclientserver/service_test.go only asserts the auth header for the Pull route. Send and AckUpTo go through the same authenticatedMailboxClient wrapper and should have matching assertions to prevent regressions (as also noted by the Copilot reviewer).

7. Misleading embed in capturingMailboxClient

serverconn/mailbox_auth_rpc_test.go:36 embeds mailboxpb.UnimplementedMailboxServiceServer in a struct used as a MailboxServiceClient. The struct explicitly defines all three client-signature methods (...grpc.CallOption), which shadow the promoted server-side methods, so the code compiles and satisfies the interface correctly. But the embed is semantically confusing: a client test double should not embed a server unimplemented stub. Since all three methods are overridden, the embed provides no value and should be removed.

---

Minor Positive Notes

• Centralizing swapServerTLSConfig is a clean refactor that removes duplicated cert-loading logic across gRPC and REST paths.
• NewAuthenticatedMailboxClient returning next unchanged when sign == nil makes the zero-value case explicit and keeps test setup simple.
• The authContext early-return on empty recipient is a good guard.

[bhandras]

codex: This drops the old loopback/plaintext default, but DefaultConfig still points swapruntime at localhost:10030 with ServerInsecure left false. As a result a stock swapruntime daemon now builds TLS credentials here and the first call to the local dev swap server fails the handshake (first record does not look like a TLS handshake); REST has the same issue because swapServerRESTBaseURL now returns https://localhost:10030 unless the user explicitly sets swap.serverinsecure=true. Either the local default needs to set ServerInsecure: true, or the loopback dev fallback needs to stay in this transport selection so the documented local/regtest default remains usable.

[sputn1ck]

Addressed in f2c20b4: restored the loopback/unix plaintext fallback when no TLS cert path is configured, kept explicit TLS cert paths authoritative, and added tests for local default, remote TLS default, and TLS-cert override behavior.