Bolt4: remove final_expiry_too_soon error message

It allowed probing attacks and the spec deprecated it in favor of
IncorrectOrUnknownPaymentDetails.

eclair-core/src/main/scala/fr/acinq/eclair/payment/Autoprobe.scala

@@ -62,7 +62,7 @@ class Autoprobe(nodeParams: NodeParams, router: ActorRef, paymentInitiator: Acto
 
     case paymentResult: PaymentResult =>
       paymentResult match {
-        case PaymentFailed(_, _, _ :+ RemoteFailure(_, DecryptedFailurePacket(targetNodeId, IncorrectOrUnknownPaymentDetails(_)))) =>
+        case PaymentFailed(_, _, _ :+ RemoteFailure(_, DecryptedFailurePacket(targetNodeId, IncorrectOrUnknownPaymentDetails(_, _)))) =>
           log.info(s"payment probe successful to node=$targetNodeId")
         case _ =>
           log.info(s"payment probe failed with paymentResult=$paymentResult")

eclair-core/src/main/scala/fr/acinq/eclair/payment/LocalPaymentHandler.scala

@@ -66,15 +66,15 @@ class LocalPaymentHandler(nodeParams: NodeParams) extends Actor with ActorLoggin
           // see https://github.com/lightningnetwork/lightning-rfc/blob/master/04-onion-routing.md#failure-messages
           paymentRequest.amount match {
             case _ if paymentRequest.isExpired =>
-              sender ! CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(htlc.amountMsat)), commit = true)
+              sender ! CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(htlc.amountMsat, Globals.blockCount.get())), commit = true)
             case _ if htlc.cltvExpiry < minFinalExpiry =>
-              sender ! CMD_FAIL_HTLC(htlc.id, Right(FinalExpiryTooSoon), commit = true)
+              sender ! CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(htlc.amountMsat, Globals.blockCount.get())), commit = true)
             case Some(amount) if htlc.amountMsat < amount =>
               log.warning(s"received payment with amount too small for paymentHash=${htlc.paymentHash} amount=${htlc.amountMsat}")
-              sender ! CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(htlc.amountMsat)), commit = true)
+              sender ! CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(htlc.amountMsat, Globals.blockCount.get())), commit = true)
             case Some(amount) if htlc.amountMsat > amount * 2 =>
               log.warning(s"received payment with amount too large for paymentHash=${htlc.paymentHash} amount=${htlc.amountMsat}")
-              sender ! CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(htlc.amountMsat)), commit = true)
+              sender ! CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(htlc.amountMsat, Globals.blockCount.get())), commit = true)
             case _ =>
               log.info(s"received payment for paymentHash=${htlc.paymentHash} amount=${htlc.amountMsat}")
               // amount is correct or was not specified in the payment request
@@ -83,7 +83,7 @@ class LocalPaymentHandler(nodeParams: NodeParams) extends Actor with ActorLoggin
               context.system.eventStream.publish(PaymentReceived(htlc.amountMsat, htlc.paymentHash, htlc.channelId))
           }
         case None =>
-          sender ! CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(htlc.amountMsat)), commit = true)
+          sender ! CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(htlc.amountMsat, Globals.blockCount.get())), commit = true)
       }
   }
 

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

@@ -19,15 +19,15 @@ package fr.acinq.eclair.wire
 import fr.acinq.bitcoin.ByteVector32
 import fr.acinq.eclair.MilliSatoshi
 import fr.acinq.eclair.crypto.Mac32
-import fr.acinq.eclair.wire.CommonCodecs.{sha256, millisatoshi}
+import fr.acinq.eclair.wire.CommonCodecs.{millisatoshi, sha256}
 import fr.acinq.eclair.wire.LightningMessageCodecs.{channelUpdateCodec, lightningMessageCodec}
 import scodec.codecs._
 import scodec.{Attempt, Codec}
 
 /**
-  * see https://github.com/lightningnetwork/lightning-rfc/blob/master/04-onion-routing.md
-  * Created by fabrice on 14/03/17.
-  */
+ * see https://github.com/lightningnetwork/lightning-rfc/blob/master/04-onion-routing.md
+ * Created by fabrice on 14/03/17.
+ */
 
 // @formatter:off
 sealed trait FailureMessage { def message: String }
@@ -52,9 +52,11 @@ case class AmountBelowMinimum(amount: MilliSatoshi, update: ChannelUpdate) exten
 case class FeeInsufficient(amount: MilliSatoshi, update: ChannelUpdate) extends Update { def message = s"payment fee was below the minimum required by the channel" }
 case class ChannelDisabled(messageFlags: Byte, channelFlags: Byte, update: ChannelUpdate) extends Update { def message = "channel is currently disabled" }
 case class IncorrectCltvExpiry(expiry: Long, update: ChannelUpdate) extends Update { def message = "payment expiry doesn't match the value in the onion" }
-case class IncorrectOrUnknownPaymentDetails(amount: MilliSatoshi) extends Perm { def message = "incorrect payment amount or unknown payment hash" }
+case class IncorrectOrUnknownPaymentDetails(amount: MilliSatoshi, height: Long) extends Perm { def message = "incorrect payment details or unknown payment hash" }
+/** Deprecated: this failure code allows probing attacks: IncorrectOrUnknownPaymentDetails should be used instead. */
 case object IncorrectPaymentAmount extends Perm { def message = "payment amount is incorrect" }
 case class ExpiryTooSoon(update: ChannelUpdate) extends Update { def message = "payment expiry is too close to the current block height for safe handling by the relaying node" }
+/** Deprecated: this failure code allows probing attacks: IncorrectOrUnknownPaymentDetails should be used instead. */
 case object FinalExpiryTooSoon extends FailureMessage { def message = "payment expiry is too close to the current block height for safe handling by the final node" }
 case class FinalIncorrectCltvExpiry(expiry: Long) extends FailureMessage { def message = "payment expiry doesn't match the value in the onion" }
 case class FinalIncorrectHtlcAmount(amount: MilliSatoshi) extends FailureMessage { def message = "payment amount is incorrect in the final htlc" }
@@ -91,26 +93,26 @@ object FailureMessageCodecs {
     .typecase(UPDATE | 13, (("expiry" | uint32) :: ("channelUpdate" | channelUpdateWithLengthCodec)).as[IncorrectCltvExpiry])
     .typecase(UPDATE | 14, ("channelUpdate" | channelUpdateWithLengthCodec).as[ExpiryTooSoon])
     .typecase(UPDATE | 20, (("messageFlags" | byte) :: ("channelFlags" | byte) :: ("channelUpdate" | channelUpdateWithLengthCodec)).as[ChannelDisabled])
-    .typecase(PERM | 15, ("amountMsat" | withDefaultValue(optional(bitsRemaining, millisatoshi), MilliSatoshi(0))).as[IncorrectOrUnknownPaymentDetails])
+    .typecase(PERM | 15, (("amountMsat" | withDefaultValue(optional(bitsRemaining, millisatoshi), MilliSatoshi(0))) :: ("height" | withDefaultValue(optional(bitsRemaining, uint32), 0L))).as[IncorrectOrUnknownPaymentDetails])
     .typecase(PERM | 16, provide(IncorrectPaymentAmount))
     .typecase(17, provide(FinalExpiryTooSoon))
     .typecase(18, ("expiry" | uint32).as[FinalIncorrectCltvExpiry])
     .typecase(19, ("amountMsat" | millisatoshi).as[FinalIncorrectHtlcAmount])
     .typecase(21, provide(ExpiryTooFar))
 
   /**
-    * Return the failure code for a given failure message. This method actually encodes the failure message, which is a
-    * bit clunky and not particularly efficient. It shouldn't be used on the application's hot path.
-    */
+   * Return the failure code for a given failure message. This method actually encodes the failure message, which is a
+   * bit clunky and not particularly efficient. It shouldn't be used on the application's hot path.
+   */
   def failureCode(failure: FailureMessage): Int = failureMessageCodec.encode(failure).flatMap(uint16.decode).require.value
 
   /**
-    * 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
-    */
+   * 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
+   */
   def failureOnionCodec(mac: Mac32): Codec[FailureMessage] = CommonCodecs.prependmac(
     paddedFixedSizeBytesDependent(
       260,

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

@@ -452,7 +452,7 @@ class OfflineStateSpec extends TestkitBaseClass with StateTestsHelperMethods {
 
     // We simulate a pending failure on that HTLC.
     // Even if we get close to expiring upstream we shouldn't close the channel, because we have nothing to lose.
-    sender.send(commandBuffer, CommandSend(htlc.channelId, htlc.id, CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(MilliSatoshi(0))))))
+    sender.send(commandBuffer, CommandSend(htlc.channelId, htlc.id, CMD_FAIL_HTLC(htlc.id, Right(IncorrectOrUnknownPaymentDetails(MilliSatoshi(0), 0)))))
     sender.send(bob, CurrentBlockCount(htlc.cltvExpiry - bob.underlyingActor.nodeParams.fulfillSafetyBeforeTimeoutBlocks))
 
     bob2blockchain.expectNoMsg(250 millis)

eclair-core/src/test/scala/fr/acinq/eclair/integration/IntegrationSpec.scala

@@ -344,7 +344,7 @@ class IntegrationSpec extends TestKit(ActorSystem("test")) with BitcoindService
     assert(failed.id == paymentId)
     assert(failed.paymentHash === pr.paymentHash)
     assert(failed.failures.size === 1)
-    assert(failed.failures.head.asInstanceOf[RemoteFailure].e === DecryptedFailurePacket(nodes("D").nodeParams.nodeId, IncorrectOrUnknownPaymentDetails(MilliSatoshi(100000000L))))
+    assert(failed.failures.head.asInstanceOf[RemoteFailure].e === DecryptedFailurePacket(nodes("D").nodeParams.nodeId, IncorrectOrUnknownPaymentDetails(MilliSatoshi(100000000L), Globals.blockCount.get())))
   }
 
   test("send an HTLC A->D with a lower amount than requested") {
@@ -364,7 +364,7 @@ class IntegrationSpec extends TestKit(ActorSystem("test")) with BitcoindService
     assert(failed.id == paymentId)
     assert(failed.paymentHash === pr.paymentHash)
     assert(failed.failures.size === 1)
-    assert(failed.failures.head.asInstanceOf[RemoteFailure].e === DecryptedFailurePacket(nodes("D").nodeParams.nodeId, IncorrectOrUnknownPaymentDetails(MilliSatoshi(100000000L))))
+    assert(failed.failures.head.asInstanceOf[RemoteFailure].e === DecryptedFailurePacket(nodes("D").nodeParams.nodeId, IncorrectOrUnknownPaymentDetails(MilliSatoshi(100000000L), Globals.blockCount.get())))
   }
 
   test("send an HTLC A->D with too much overpayment") {
@@ -384,7 +384,7 @@ class IntegrationSpec extends TestKit(ActorSystem("test")) with BitcoindService
     assert(paymentId == failed.id)
     assert(failed.paymentHash === pr.paymentHash)
     assert(failed.failures.size === 1)
-    assert(failed.failures.head.asInstanceOf[RemoteFailure].e === DecryptedFailurePacket(nodes("D").nodeParams.nodeId, IncorrectOrUnknownPaymentDetails(MilliSatoshi(600000000L))))
+    assert(failed.failures.head.asInstanceOf[RemoteFailure].e === DecryptedFailurePacket(nodes("D").nodeParams.nodeId, IncorrectOrUnknownPaymentDetails(MilliSatoshi(600000000L), Globals.blockCount.get())))
   }
 
   test("send an HTLC A->D with a reasonable overpayment") {

eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentHandlerSpec.scala

@@ -24,7 +24,7 @@ import fr.acinq.eclair.TestConstants.Alice
 import fr.acinq.eclair.channel.{CMD_FAIL_HTLC, CMD_FULFILL_HTLC}
 import fr.acinq.eclair.payment.PaymentLifecycle.ReceivePayment
 import fr.acinq.eclair.payment.PaymentRequest.ExtraHop
-import fr.acinq.eclair.wire.{FinalExpiryTooSoon, UpdateAddHtlc}
+import fr.acinq.eclair.wire.{FinalExpiryTooSoon, IncorrectOrUnknownPaymentDetails, UpdateAddHtlc}
 import fr.acinq.eclair.{Globals, MilliSatoshi, ShortChannelId, TestConstants, randomKey}
 import org.scalatest.FunSuiteLike
 import scodec.bits.ByteVector
@@ -83,7 +83,7 @@ class PaymentHandlerSpec extends TestKit(ActorSystem("test")) with FunSuiteLike
 
       val add = UpdateAddHtlc(ByteVector32(ByteVector.fill(32)(1)), 0, amountMsat, pr.paymentHash, cltvExpiry = Globals.blockCount.get() + 3, TestConstants.emptyOnionPacket)
       sender.send(handler, add)
-      assert(sender.expectMsgType[CMD_FAIL_HTLC].reason == Right(FinalExpiryTooSoon))
+      assert(sender.expectMsgType[CMD_FAIL_HTLC].reason == Right(IncorrectOrUnknownPaymentDetails(amountMsat, Globals.blockCount.get())))
       eventListener.expectNoMsg(300 milliseconds)
       assert(nodeParams.db.payments.getIncomingPayment(pr.paymentHash).isEmpty)
     }

eclair-core/src/test/scala/fr/acinq/eclair/wire/FailureMessageCodecsSpec.scala

@@ -18,14 +18,14 @@ package fr.acinq.eclair.wire
 
 import fr.acinq.bitcoin.{Block, ByteVector32, ByteVector64}
 import fr.acinq.eclair.crypto.Hmac256
-import fr.acinq.eclair.{MilliSatoshi, ShortChannelId, randomBytes32, randomBytes64}
 import fr.acinq.eclair.wire.FailureMessageCodecs._
+import fr.acinq.eclair.{MilliSatoshi, ShortChannelId, randomBytes32, randomBytes64}
 import org.scalatest.FunSuite
 import scodec.bits._
 
 /**
-  * Created by PM on 31/05/2016.
-  */
+ * Created by PM on 31/05/2016.
+ */
 
 class FailureMessageCodecsSpec extends FunSuite {
   val channelUpdate = ChannelUpdate(
@@ -47,7 +47,7 @@ class FailureMessageCodecsSpec extends FunSuite {
         InvalidOnionVersion(randomBytes32) :: InvalidOnionHmac(randomBytes32) :: InvalidOnionKey(randomBytes32) :: InvalidOnionPayload(randomBytes32) ::
         TemporaryChannelFailure(channelUpdate) :: PermanentChannelFailure :: RequiredChannelFeatureMissing :: UnknownNextPeer ::
         AmountBelowMinimum(MilliSatoshi(123456), channelUpdate) :: FeeInsufficient(MilliSatoshi(546463), channelUpdate) :: IncorrectCltvExpiry(1211, channelUpdate) :: ExpiryTooSoon(channelUpdate) ::
-        IncorrectOrUnknownPaymentDetails(MilliSatoshi(123456L)) :: IncorrectPaymentAmount :: FinalExpiryTooSoon :: FinalIncorrectCltvExpiry(1234) :: ChannelDisabled(0, 1, channelUpdate) :: ExpiryTooFar :: Nil
+        IncorrectOrUnknownPaymentDetails(MilliSatoshi(123456L), 1105) :: IncorrectPaymentAmount :: FinalExpiryTooSoon :: FinalIncorrectCltvExpiry(1234) :: ChannelDisabled(0, 1, channelUpdate) :: ExpiryTooFar :: Nil
 
     msgs.foreach {
       msg => {
@@ -75,7 +75,7 @@ class FailureMessageCodecsSpec extends FunSuite {
     val codec = failureOnionCodec(Hmac256(ByteVector32.Zeroes))
     val testCases = Map(
       InvalidOnionKey(ByteVector32(hex"2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a")) -> hex"41a824e2d630111669fa3e52b600a518f369691909b4e89205dc624ee17ed2c1 0022 c006 2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a 00de 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
-      IncorrectOrUnknownPaymentDetails(MilliSatoshi(42)) -> hex"ba6e122b2941619e2106e8437bf525356ffc8439ac3b2245f68546e298a08cc6 000a 400f 000000000000002a 00f6 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"
+      IncorrectOrUnknownPaymentDetails(MilliSatoshi(42), 1105) -> hex"5eb766da1b2f45b4182e064dacd8da9eca2c9a33f0dce363ff308e9bdb3ee4e3 000e 400f 000000000000002a 00000451 00f2 0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"
     )
 
     for ((expected, bin) <- testCases) {
@@ -87,6 +87,22 @@ class FailureMessageCodecsSpec extends FunSuite {
     }
   }
 
+  test("decode backwards-compatible IncorrectOrUnknownPaymentDetails") {
+    val codec = failureOnionCodec(Hmac256(ByteVector32.Zeroes))
+    val testCases = Map(
+      // Without any data.
+      IncorrectOrUnknownPaymentDetails(MilliSatoshi(0), 0) -> hex"0d83b55dd5a6086e4033c3659125ed1ff436964ce0e67ed5a03bddb16a9a1041 0002 400f 00fe 0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
+      // With an amount but no height.
+      IncorrectOrUnknownPaymentDetails(MilliSatoshi(42), 0) -> hex"ba6e122b2941619e2106e8437bf525356ffc8439ac3b2245f68546e298a08cc6 000a 400f 000000000000002a 00f6 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
+      // With amount and height.
+      IncorrectOrUnknownPaymentDetails(MilliSatoshi(42), 1105) -> hex"5eb766da1b2f45b4182e064dacd8da9eca2c9a33f0dce363ff308e9bdb3ee4e3 000e 400f 000000000000002a 00000451 00f2 0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"
+    )
+
+    for ((expected, bin) <- testCases) {
+      assert(codec.decode(bin.bits).require.value === expected)
+    }
+  }
+
   test("decode invalid failure onion packet") {
     val codec = failureOnionCodec(Hmac256(ByteVector32.Zeroes))
     val testCases = Seq(