Add support for arbitrary length onion errors

eclair-core/src/main/scala/fr/acinq/eclair/crypto/Mac.scala

@@ -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
 
 }
 

eclair-core/src/main/scala/fr/acinq/eclair/crypto/Sphinx.scala

@@ -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
@@ -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.
@@ -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
     }
 
     /**
@@ -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")
@@ -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)

eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/FailureMessage.scala

@@ -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}
 
@@ -139,19 +140,27 @@ object FailureMessageCodecs {
     }, (_: FailureMessage).code)
   )
 
+  private def failureOnionPayload(payloadAndPadLength: Int): Codec[FailureMessage] = Codec(
+    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: by always using the same size we
+   * ensure error messages are indistinguishable.
    */
-  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)
+
 }

eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala

@@ -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 = {