Channel state splitbrain issue with VLS integration

Issue: Channel State Splitbrain, And VLS `policy-commitment-retry-same` ======================================================================= There is a race condition where the rust-lightning node can ask VLS to sign a new counterparty commitment (to which VLS then signs and responds), then rust-lightning crashes before it can write the new state to disk. On restart, rust-lightning will forget about the new state, and may attempt to ask VLS to ask to sign old state. However, VLS has already signed new state before the crash, and by its policy `policy-commitment-retry-same`, it will reject the signing request after restart, and there is no way to recover other than to close the channel. Fortunately, this state is not a funds-loss event; as far as VLS is concerned, both the old state and new state are valid, and as far as rust-lightning (and presumably its counterparty) is concerned, there is only the old state. In VLS, `policy-commitment-retry-same` is the policy which requires that every `sign_counterparty_commitment` is either the same as the previous call, ***or*** is at the next higher state index. In this issue, VLS thinks we are alreaady at the state `N`, but rust-lightning attempts the previous state `N - 1`. This is related to a similar splitbrain issue with CLN. In CLN, the same problem manifests as an attempt to create a ***different*** new commitment transaction, but the root cause is the same: there is a splitbrain condition where VLS believes the new state before the crash was made valid, while the rust-lightning / CLN node was unable to write that new state to disk. * rust-lightning: - The symptom is that rust-lightning looks like it is retrying state `N - 1`, when VLS is already at state `N` (or in other cases, may also have the same symptom as CLN) * CLN: - The symptom is that CLN looks like it is attempting to create a different state `N`, which VLS rejects because the state `N` has a different set of HTLCs or different feerate. Incorrect Solution =================== CLN proposed a solution of "VLS should always just accept signing any alternate new state `N` as long as state `N - 1` is not invalidated." For rust-lightning, the equivalent would be "VLS should accept signing old state `N - 1` as long as state `N - 1` is not invalidated, ***in addition to*** the CLN proposal". This violates the VLS policy `policy-commitment-retry-same`. The issue here is that VLS has the goal of "the node can be compromised, but this will not lead to a funds loss event". The problem is that VLS signing ***any*** alternate new state `N` (the solution proposed by CLN) leads to a ***funds-loss vulnerability*** that is exploitable if the counterparty has compromised the node. The following user story shows the funds-loss vulnerability: * Actors: - `R`, the counterparty. - secretly malicious. - `A`, the node (CLN or rust-lightning). - secretly compromised by `R`. - `V`, the Validating Lightning Signer. * Start with `R` commitment at state index `N - 1`. * `R` sends `update_add_htlc` with `H1` with the tiniest amount. * `A` decides to sign the next `R` state `N`, and ultimately calls into `EcdsaChannelSigner::sign_counterparty_commitment`, which causes a RPC into `V`. * `V` signs the new state and sends back the signature in response. Let us call this state `<R.N> H1`. * `A` receives the signature for `<R.N> H1` and gives it to `R`. - `R` is now in possession of `<R.N> H1`. * `A` ***PRETENDS TO CRASH***. * On "restart", `A` ***PRETENDS*** to perform `channel_reestablish` with `R` at state `N - 1`. - This calls `EcdsaChannelSigner::sign_counterparty_commitment` at state index `N - 1`. - Without `policy-commitment-retry-same`, `V` will accept this signing attempt. * `R` sends `update_add_htlc` with `H2` with the largest possible amount, to be forwarded elsewhere (to another node owned by `R`). * `A` requests, to `V`, a signature for the next `R` state `N` with `H2` as the HTLC state. * `V` signs the new state due to lack of `policy-commitment-retry-same`. Call this signature `<R.N> H2`. * `R` sends the revocation for state `R.N - 1`. * `V` now believes that `H2` is "irrevocably committed" and authorizes its forwarding onwards. * `R` completes the signature for `<R.N> H1`. - This can confirm, as it is now signed by both `V` and `R`. - `V` cannot punish it it, because only state `T.N - 1` is revoked. - `R` loses only the tiny amount `H1` (and it really only gets locked until timeout) but gains the amount of `H2`. - `R` WINS. Due to this proposed solution allowing funds loss (and the issue itself ***not*** leading to funds loss, other than the "cost of doing business" for fees for closing and re-opening channels), VLS rejects this solution as worse than the problem. Correct Solution ================ The correct solution is to write an ***intent*** to sign to disk, ***before*** performing the signing. This is valid as a solution: writing the ***intent*** to a log (called the "write-ahead log" or "***intent*** log") is how all modern databases implement ACID transactions. Thus, writing the "intent to sign this new state" to the log (in rust-lightning terms, the sequence of `ChannelMonitorUpdate`s) ***is*** semantically equivalent to "I signed this new state". Once it has been written in the on-disk log, and durably saved with `fsync`, it is equivalent to atomically having signed the new state. The advantage is that this is effectively atomic from the point of view of both VLS and rust-lightning! Thus, it makes atomic the two operations "make signature" and "write new state to disk", even if the two operations are done on different machines: * Suppose rust-lightning and VLS crash before it can write the intent-to-sign on disk. On restart, the view is: - rust-lightning: `N - 1` - VLS: `N - 1` - Both are in synchrony. * Suppose rust-lightning writes intent-to-sign to disk, but crashes before it can make the request to VLS. On restart, the view is: - rust-lightning: `N` - VLS: `N - 1` - On restart, rust-lightning sees the intent-to-sign and calls VLS to make the request to sign `N`. This is accepted by VLS because it is a state advancement from `N - 1` to `N`. * Suppose rust-lightning writes intent-to-sign to disk and is able to make the request to VLS, but VLS crashes before it can sign and updates its own disk. On VLS restart, the view is: - rust-lightning: `N` - VLS: `N - 1` - The rust-lightning-VLS integration code will repeat the last request that has not been responde to on reconnection. This is the same request that signs `N`, which is accepted by VLS as a state advancement. * Suppose VLS signs and writes to disk, but crashes before it can send the response to rust-lightning. On VLS restart, the view is: - rust-lightning: `N` - VLS: `N` - As noted, the rust-lightning-VLS integration code will repeat the last request on reconnection of VLS. As this request is the same as the request VLS received before crashing, this passes `policy-commitment-retry-same`. * Suppose VLS is able to respond to rust-lightning, but rust-lightning crashes before it can send the signature in a `commitment_signed`. On restart, the view is: - rust-lightning: `N` - VLS: `N` - On restart, rust-lightning will see the intent-to-sign on disk, and re-do the signing. This is the same request as what VLS received before, which passes `policy-commitment-retry-same`. Then rust-lightning will use the signature for `connection_reestablish`. In rust-lightning terms, we need to write a `ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo` to disk ***before*** calling into `EcdsaChannelSigner::sign_counterparty_commitment`. (I am assuming here that `ChannelContext::build_commitment_transaction` can be used to recover the inbound and outbound preimages on restart; if not, you actually need to create a new update-step that includes the inbound and outbound preimages, which could cause problems with compatibility.) Then, we can completely deprecate the use of `ChannelMonitorUpdateStep::LatestCounterpartyCommitment` (i.e. the signed version of the commitment transaction). Instead, rust-lightning should cache the `Vec<CommitmentTransaction>` in-memory only; on restart, it will re-create the commitment transactions by calling into the signer again, and again cache the in-memory version. In that case, it will never use `LatestCounterpartyCommitment`. Expanding View ============== There are four different things that a signer has to handle for the channel state advancement: * Our commitment: - Validate signature from counterparty for `N`. - Provide revocation to counterparty for `N - 1`. * Counterparty commitment: - Provide signature to counterpatry for `N`. - Validate revocaation from counterparty for `N - 1`. Uniquely, this splitbrain phenomoenon only affects the "provide signature to counterparty for `N`". * Validate signature from counterparty for `N`. - If the counterparty makes signatures for different `N` states instead of just a single `N` state, that means the counterparty is opening itself to the vulnerability described here. - This means that this is not a funds loss for *us* at least. - VLS can safely replace its own state with the latest signature and latest state from the counterparty or node. * Provide revocation to counterparty for `N - 1`. - The revocation key is independent of any HTLCs or other state. - The only validation VLS makes here is to ensure that "validate signature from counterparty" has been done. * Validate revocation from counterparty for `N - 1`. - The revocation key is independent of any HTLCs or other state. - rust-lightning can repeat this request to VLS any number of times and the result will be the same each time (i.e. fully idempotent). Thus, changes are unnnecessary in rust-lightning for the other parts.