Skip to content

serverconn: sign TLS leaf binding for mailbox auth (#448) - #456

Closed
ellemouton wants to merge 1 commit into
mainfrom
fix/448-tls-binding-sig
Closed

serverconn: sign TLS leaf binding for mailbox auth (#448)#456
ellemouton wants to merge 1 commit into
mainfrom
fix/448-tls-binding-sig

Conversation

@ellemouton

Copy link
Copy Markdown
Member

Closes lightninglabs/darepo#448.

Summary

Adds the client-side half of the #448 TLS-binding mTLS gap fix.

  • Introduces SignMailboxTLSBind / VerifyMailboxTLSBind in
    client/serverconn. The signature is BIP-340 over a tagged
    (mailbox-tls-bind) digest of senderPubKey || tlsLeafSPKI, using
    the mailbox secp256k1 key.
  • darepod.dialServer stamps the binding signature as a gRPC metadata
    header (x-mailbox-tls-bind-sig) on outbound mailbox calls so the
    server can verify, on first-contact Send, that the mailbox-key
    holder chose the TLS leaf the server observes.

Without this header an attacker who captured a victim's signed first
Send could replay the bytes over a TLS connection backed by an
attacker-controlled leaf and have their fingerprint bound to the
victim's mailbox ID, defeating the post-registration fingerprint
defense added in lightninglabs/darepo#443.

Server side

Server-side enforcement lives in lightninglabs/darepo#443, which also
folds in the #448 fix to keep the rollout atomic (a soft-rollout flag
MailboxConfig.RequireTLSBindingSig gates strict enforcement).

Test plan

  • new unit tests for SignMailboxTLSBind / VerifyMailboxTLSBind
    (wrong-key, wrong-leaf, malformed, replay across leaves)
  • darepod.dialServer stamps the header end-to-end

Add a secp256k1 → TLS-leaf binding signature carried in a new
x-mailbox-tls-bind-sig envelope header so the server can verify, on
first-contact Send, that the secp256k1 mailbox key holder chose the
TLS leaf the server observes on the connection.

Without this binding, the existing Schnorr auth signature (which
covers only the mailbox identity + envelope contents) does not
constrain which TLS session the envelope is replayed across. An
attacker who captured a victim's signed first Send could replay the
same bytes over a TLS connection backed by an attacker-controlled
leaf and have their fingerprint bound to the victim's mailbox ID,
defeating the post-registration fingerprint defense added in #443.

The new digest uses a dedicated BIP-340 tag (mailbox-tls-bind) so
neither signature can be reinterpreted as the other. The leaf is
identified by its SubjectPublicKeyInfo DER bytes, which commits to
the curve and algorithm identifier alongside the raw key — TLS
certs in this deployment use P-256, distinct from the secp256k1
mailbox key, so the binding has to come from a signature rather
than key identity.

darepod stamps the binding header on every outbound envelope from
the connector and on the inline response-send path so the server
can complete first-contact registration regardless of which Send is
the one that triggers it.

Closes #448.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements a mechanism to bind a client's secp256k1 mailbox identity to its TLS leaf certificate, mitigating registration-time replay attacks. Changes include adding fields to the Server struct to store the TLS leaf's SubjectPublicKeyInfo and the corresponding Schnorr signature, as well as updating the serverconn package with logic for signature generation and verification. Feedback was provided regarding a potential issue where the TLS leaf certificate might not be automatically populated, which could lead to the binding being silently skipped; a code suggestion was offered to explicitly parse the certificate if necessary.

Comment thread darepod/server.go
Comment on lines +2062 to +2064
if clientCert.Leaf != nil {
s.tlsLeafSPKI = clientCert.Leaf.RawSubjectPublicKeyInfo
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The tls.Certificate.Leaf field is not automatically populated by standard library functions like tls.X509KeyPair or tls.LoadX509KeyPair. If serverconn.GenerateClientTLSCert follows this pattern, clientCert.Leaf will be nil, causing the TLS binding to be silently skipped even when TLS is enabled. This would leave the registration-time replay window open.

It is safer to explicitly parse the leaf certificate from the raw bytes if it is missing.

if clientCert.Leaf == nil && len(clientCert.Certificate) > 0 {
	var err error
	clientCert.Leaf, err = x509.ParseCertificate(clientCert.Certificate[0])
	if err != nil {
		return nil, fmt.Errorf("parse client TLS cert: %w", err)
	}
}
if clientCert.Leaf != nil {
	s.tlsLeafSPKI = clientCert.Leaf.RawSubjectPublicKeyInfo
}

@ellemouton

Copy link
Copy Markdown
Member Author

Superseded by consolidated PR #459. Closing to reduce CI load.

@ellemouton ellemouton closed this May 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant