Skip to content

Commit

Permalink
Add support for arbitrary length onion errors
Browse files Browse the repository at this point in the history
The specification recommends using a length of 256 for onion errors, but
it doesn't say that we should reject errors that use a different length.

We may want to start creating errors with a bigger length than 256 if we
need to transmit more data to the sender. In order to prepare for this,
we keep creating 256-bytes onion errors, but allow receiving errors of
arbitrary length.

See the specification here: lightning/bolts#1021

Fixes #2438
  • Loading branch information
t-bast committed Sep 29, 2022
1 parent 6e381d4 commit 06267d0
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ case class Hmac256(key: ByteVector) extends Mac32 {

override def mac(message: ByteVector): ByteVector32 = Mac32.hmac256(key, message)

override def verify(mac: ByteVector32, message: ByteVector): Boolean = this.mac(message) === mac
override def verify(mac: ByteVector32, message: ByteVector): Boolean = this.mac(message) == mac

}

Expand Down
17 changes: 3 additions & 14 deletions eclair-core/src/main/scala/fr/acinq/eclair/crypto/Sphinx.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package fr.acinq.eclair.crypto

import fr.acinq.bitcoin.scalacompat.Crypto.{PrivateKey, PublicKey}
import fr.acinq.bitcoin.scalacompat.{ByteVector32, Crypto}
import fr.acinq.eclair.crypto.Monitoring.{Metrics, Tags}
import fr.acinq.eclair.wire.protocol._
import grizzled.slf4j.Logging
import scodec.Attempt
Expand Down Expand Up @@ -272,9 +271,6 @@ object Sphinx extends Logging {

object FailurePacket {

val MaxPayloadLength = 256
val PacketLength = MacLength + MaxPayloadLength + 2 + 2

/**
* Create a failure packet that will be returned to the sender.
* Each intermediate hop will add a layer of encryption and forward to the previous hop.
Expand All @@ -301,16 +297,11 @@ object Sphinx extends Logging {
* @return an encrypted failure packet that can be sent to the destination node.
*/
def wrap(packet: ByteVector, sharedSecret: ByteVector32): ByteVector = {
if (packet.length != PacketLength) {
logger.warn(s"invalid error packet length ${packet.length}, must be $PacketLength (malicious or buggy downstream node)")
}
val key = generateKey("ammag", sharedSecret)
val stream = generateStream(key, PacketLength)
val stream = generateStream(key, packet.length.toInt)
logger.debug(s"ammag key: $key")
logger.debug(s"error stream: $stream")
// If we received a packet with an invalid length, we trim and pad to forward a packet with a normal length upstream.
// This is a poor man's attempt at increasing the likelihood of the sender receiving the error.
packet.take(PacketLength).padLeft(PacketLength) xor stream
packet xor stream
}

/**
Expand All @@ -324,8 +315,6 @@ object Sphinx extends Logging {
* decrypted, Failure otherwise.
*/
def decrypt(packet: ByteVector, sharedSecrets: Seq[(ByteVector32, PublicKey)]): Try[DecryptedFailurePacket] = Try {
require(packet.length == PacketLength, s"invalid error packet length ${packet.length}, must be $PacketLength")

@tailrec
def loop(packet: ByteVector, secrets: Seq[(ByteVector32, PublicKey)]): DecryptedFailurePacket = secrets match {
case Nil => throw new RuntimeException(s"couldn't parse error packet=$packet with sharedSecrets=$sharedSecrets")
Expand Down Expand Up @@ -381,7 +370,7 @@ object Sphinx extends Logging {
}

/**
* @param route blinded route.
* @param route blinded route.
* @param lastBlinding blinding point for the last node, which can be used to derive the blinded private key.
*/
case class BlindedRouteDetails(route: BlindedRoute, lastBlinding: PublicKey)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import fr.acinq.eclair.wire.protocol.CommonCodecs._
import fr.acinq.eclair.wire.protocol.FailureMessageCodecs.failureMessageCodec
import fr.acinq.eclair.wire.protocol.LightningMessageCodecs.{channelFlagsCodec, channelUpdateCodec, meteredLightningMessageCodec}
import fr.acinq.eclair.{BlockHeight, CltvExpiry, MilliSatoshi, MilliSatoshiLong, UInt64}
import scodec.bits.ByteVector
import scodec.codecs._
import scodec.{Attempt, Codec}

Expand Down Expand Up @@ -139,19 +140,27 @@ object FailureMessageCodecs {
}, (_: FailureMessage).code)
)

private def failureOnionPayload(payloadAndPadLength: Int): Codec[FailureMessage] = Codec(
// We create payloads of the requested size: by always using the same size we ensure error messages are indistinguishable.
encoder = f => variableSizeBytes(uint16, failureMessageCodec).encode(f).flatMap(bits => {
val payloadLength = bits.bytes.length - 2
val padLen = payloadAndPadLength - payloadLength
variableSizeBytes(uint16, bytes).encode(ByteVector.fill(padLen)(0)).map(pad => bits ++ pad)
}),
// We accept payloads of any size in case more space is needed for future failure messages.
decoder = bits => (
("failure" | variableSizeBytes(uint16, failureMessageCodec))
:: ("padding" | variableSizeBytes(uint16, bytes))
).map(_.head).decode(bits)
)

/**
* An onion-encrypted failure from an intermediate node:
* {{{
* +----------------+----------------------------------+-----------------+----------------------+-----+
* | HMAC(32 bytes) | failure message length (2 bytes) | failure message | pad length (2 bytes) | pad |
* +----------------+----------------------------------+-----------------+----------------------+-----+
* }}}
* with failure message length + pad length = 256
* Bolt 4: SHOULD set pad such that the failure_len plus pad_len is equal to 256
*/
def failureOnionCodec(mac: Mac32): Codec[FailureMessage] = CommonCodecs.prependmac(
paddedFixedSizeBytesDependent(
260,
"failureMessage" | variableSizeBytes(uint16, FailureMessageCodecs.failureMessageCodec),
nBits => "padding" | variableSizeBytes(uint16, ignore(nBits - 2 * 8)) // two bytes are used to encode the padding length
).as[FailureMessage], mac)
def failureOnionCodec(mac: Mac32, payloadAndPadLength: Int = 256): Codec[FailureMessage] = CommonCodecs.prependmac(failureOnionPayload(payloadAndPadLength).complete, mac)

}
Original file line number Diff line number Diff line change
Expand Up @@ -1925,16 +1925,16 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with
alice2blockchain.expectMsgType[WatchTxConfirmed]
}

test("recv UpdateFailHtlc (invalid onion error length)") { f =>
test("recv UpdateFailHtlc (onion error bigger than recommended value)") { f =>
import f._
val (_, htlc) = addHtlc(50000000 msat, alice, bob, alice2bob, bob2alice)
crossSign(alice, bob, alice2bob, bob2alice)
// Bob receives a failure with a completely invalid onion error (missing mac)
bob ! CMD_FAIL_HTLC(htlc.id, Left(ByteVector.fill(260)(42)))
bob ! CMD_FAIL_HTLC(htlc.id, Left(ByteVector.fill(561)(42)))
val fail = bob2alice.expectMsgType[UpdateFailHtlc]
assert(fail.id == htlc.id)
// We should rectify the packet length before forwarding upstream.
assert(fail.reason.length == Sphinx.FailurePacket.PacketLength)
// We propagate failure upstream (hopefully the sender knows how to unwrap them).
assert(fail.reason.length == 561)
}

private def testCmdUpdateFee(f: FixtureParam): Unit = {
Expand Down
Loading

0 comments on commit 06267d0

Please sign in to comment.