Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BOLT2, BOLT3: reduce attack surface #243

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
2 participants
@sstone
Copy link
Collaborator

commented Sep 18, 2017

add specific htlc payment keys, let node specify their final scriptpubkey in open/accept

The goal of this PR is to reduce the attack surface for LN nodes by minimizing
the set of private keys that they need to have access to. Currently, nodes
need to know the funding private key and the payment key because it is used
for their main output as well as their HTLC outputs. Using separate keys, only
HTLC outputs (which are typically much smaller than the main output) would be
at risk.

This PR does not address attacks where the attacker is the remote party and
can just steal the funding privatekey, or publish a revoke commit tx...

It also does not prevent attacks where the attacker sends HTLCs to
their own nodes (but there are strategies to mitigate this)

In more details:

Letting nodes specify their final scriptpubkey in their open/accept
messages mitigates an attack where an attacker could trick a node into
closing a channel with their scriptpubkey, in which case they could
spend the closing tx right away.

Having separate keys for pending HTLCs means that nodes don't
need to know the tolocal private key and mitigates an attack
where the attacker would steal the tolocal private key, have the
channel close unilaterally (by sending garbage, make the node unresponsive
...) and then spend the toremote output in the commitment tx published by
the remote party.

@sstone sstone force-pushed the sstone:separate-keys branch 2 times, most recently from 15245ba to d93dcbf Sep 18, 2017

@cdecker cdecker requested review from rustyrussell and Roasbeef and removed request for rustyrussell Oct 2, 2017

@sstone sstone changed the title add specific htlc payement keys, let node specify their final scriptp… add specific htlc payment keys, let node specify their final scriptp… Oct 13, 2017

@sstone sstone force-pushed the sstone:separate-keys branch from d93dcbf to 0099307 Oct 25, 2017

BOLT2, BOLT3: reduce attack surface
add specific htlc payment keys, let node specify their final scriptpubkey in open/accept

The goal of this PR is to reduce the attack surface for LN nodes by minimizing
the set of private keys that they need to have access to. Currently, nodes
need to know the funding private key and the payment key because it is used
for their main output as well as their HTLC outputs. Using separate keys, only
HTLC outputs (which are typically much smaller than the main output) would be
at risk.

This PR does not address attacks where the attacker is the remote party and
can just steal the funding privatekey, or publish a revoked commit tx...

It also does not prevent attacks where the attacker sends HTLCs to
their own nodes (but there are strategies to mitigate this)

In more details:

Letting nodes specify their final scriptpubkey in their open/accept
messages mitigates an attack where an attacker could trick a node into
closing a channel with their scriptpubkey, in which case they could
spend the closing tx right away.

Having separate keys for pending HTLCs means that nodes don't
need to know the `tolocal` private key and mitigates an attack
where the attacker would steal the `tolocal` private key, have the
channel close unilaterally (by sending garbage, make the node unresponsive
...) and then spend the `toremote` output in the commitment tx published by
the remote party.

@sstone sstone force-pushed the sstone:separate-keys branch from 0099307 to ac6fe06 Oct 26, 2017

@sstone sstone changed the title add specific htlc payment keys, let node specify their final scriptp… BOLT2, BOLT3: reduce attack surface Oct 26, 2017

@rustyrussell
Copy link
Collaborator

left a comment

The final_scriptpubkey change is quite useful, but is trivial to append as an optional field: if you understand it and it's not zero length, you fail the channel if the shutdown message doesn't match. We should probably still assign a feature bit so you can later insist that a peer support it.

The htlc_payment_basepoint is much more essential. It could be done as an option (needs an option bit though, since you need to know if the other end will use it), in which case it's a little neater to rename payment_basepoint to htlc_payment_basepoint, and the optional basepoint is the payment_basepoint (== htlc_payment_basepoint if not set).

I am deeply reluctant to break freeze (again!) for this, but it is both important and easy. Let's discuss on call.

@@ -32,6 +32,9 @@ def guess_alignment(message, name, sizestr):
if 'signature' in name:
return 1

if message == 'open_channel' and name == 'len':
return 1

This comment has been minimized.

Copy link
@rustyrussell

rustyrussell Oct 30, 2017

Collaborator

We should just drop the alignment checks altogether, if this is needed.

rustyrussell added a commit to rustyrussell/lightning-rfc that referenced this pull request Oct 31, 2017

option_paymentkey: option to avoid holding the payment secret.
This is stolen from @sstone's lightningnetwork#243 "reduce attack surface", turned
into an option.

It's trivial to support, so there's no reason for all future options
which add fields to the open_channel/accept_channel messages not to support
it (they can just set htlc_basepoint == payment_basepoint).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

rustyrussell added a commit to rustyrussell/lightning-rfc that referenced this pull request Oct 31, 2017

BOLT 2: add precommitment to scriptpubkey for mutual close.
This is Fabrice's lightningnetwork#243 "BOLT2, BOLT3: reduce attack surface", split
out with minor polishing:

- Made it an optional feature (we can insist on it if we choose even bit).
- Rename from "final_scriptpubkey" to "shutdown_scriptpubkey".
- Make requirements the same as shutdown's scriptpubkey, or zero-len.
- Leave shutdown's scriptpubkey, just make sure it's the same or fail.
- Add to accept_channel as well as open_channel.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

rustyrussell added a commit to rustyrussell/lightning-rfc that referenced this pull request Nov 13, 2017

htlckey: new basepoint avoid holding the payment secret.
This is stolen from @sstone's lightningnetwork#243 "reduce attack surface".

This breaks compatibility, as agreed at the 2017-11-13 meeting.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

rustyrussell added a commit to rustyrussell/lightning-rfc that referenced this pull request Nov 13, 2017

htlckey: new basepoint avoid holding the payment secret.
This is stolen from @sstone's lightningnetwork#243 "reduce attack surface".

This breaks compatibility, as agreed at the 2017-11-13 meeting.
Note also that it does not update the test vectors.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

rustyrussell added a commit to rustyrussell/lightning-rfc that referenced this pull request Nov 13, 2017

htlckey: new basepoint avoid holding the payment secret.
This is stolen from @sstone's lightningnetwork#243 "reduce attack surface".

This breaks compatibility, as agreed at the 2017-11-13 meeting.
Note also that it does not update the test vectors.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

rustyrussell added a commit to rustyrussell/lightning-rfc that referenced this pull request Nov 13, 2017

htlckey: new basepoint avoid holding the payment secret.
This is stolen from @sstone's lightningnetwork#243 "reduce attack surface".

This breaks compatibility, as agreed at the 2017-11-13 meeting.
Note also that it does not update the test vectors.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

rustyrussell added a commit to rustyrussell/lightning-rfc that referenced this pull request Nov 14, 2017

BOLT 2: add precommitment to scriptpubkey for mutual close.
This is Fabrice's lightningnetwork#243 "BOLT2, BOLT3: reduce attack surface", split
out with minor polishing:

- Made it an optional feature (we can insist on it if we choose even bit).
- Rename from "final_scriptpubkey" to "shutdown_scriptpubkey".
- Make requirements the same as shutdown's scriptpubkey, or zero-len.
- Leave shutdown's scriptpubkey, just make sure it's the same or fail.
- Add to accept_channel as well as open_channel.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

rustyrussell added a commit that referenced this pull request Nov 14, 2017

htlckey: new basepoint avoid holding the payment secret.
This is stolen from @sstone's #243 "reduce attack surface".

This breaks compatibility, as agreed at the 2017-11-13 meeting.
Note also that it does not update the test vectors.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

rustyrussell added a commit to rustyrussell/lightning-rfc that referenced this pull request Nov 17, 2017

BOLT 2: add precommitment to scriptpubkey for mutual close.
This is Fabrice's lightningnetwork#243 "BOLT2, BOLT3: reduce attack surface", split
out with minor polishing:

- Made it an optional feature (we can insist on it if we choose even bit).
- Rename from "final_scriptpubkey" to "shutdown_scriptpubkey".
- Make requirements the same as shutdown's scriptpubkey, or zero-len.
- Leave shutdown's scriptpubkey, just make sure it's the same or fail.
- Add to accept_channel as well as open_channel.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

rustyrussell added a commit to rustyrussell/lightning-rfc that referenced this pull request Nov 28, 2017

BOLT 2: add precommitment to scriptpubkey for mutual close.
This is Fabrice's lightningnetwork#243 "BOLT2, BOLT3: reduce attack surface", split
out with minor polishing:

- Made it an optional feature (we can insist on it if we choose even bit).
- Rename from "final_scriptpubkey" to "shutdown_scriptpubkey".
- Make requirements the same as shutdown's scriptpubkey, or zero-len.
- Leave shutdown's scriptpubkey, just make sure it's the same or fail.
- Add to accept_channel as well as open_channel.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

rustyrussell added a commit that referenced this pull request Nov 28, 2017

BOLT 2: add precommitment to scriptpubkey for mutual close.
This is Fabrice's #243 "BOLT2, BOLT3: reduce attack surface", split
out with minor polishing:

- Made it an optional feature (we can insist on it if we choose even bit).
- Rename from "final_scriptpubkey" to "shutdown_scriptpubkey".
- Make requirements the same as shutdown's scriptpubkey, or zero-len.
- Leave shutdown's scriptpubkey, just make sure it's the same or fail.
- Add to accept_channel as well as open_channel.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell

This comment has been minimized.

Copy link
Collaborator

commented Nov 28, 2017

Closing, as this was split into compulsory (htlckey, #272) and optional (scriptpubkey lockin, #266) both of which have now been merged.

@sstone sstone deleted the sstone:separate-keys branch May 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.