diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentRequest.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentRequest.scala index 7f31434dc..fb4e808b9 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentRequest.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentRequest.scala @@ -18,6 +18,7 @@ package fr.acinq.eclair.payment import fr.acinq.bitcoin.Crypto.{PrivateKey, PublicKey} import fr.acinq.bitcoin.{Base58, Base58Check, Bech32, Block, ByteVector32, ByteVector64, Crypto} +import fr.acinq.eclair.Features._ import fr.acinq.eclair.payment.PaymentRequest._ import fr.acinq.eclair.{CltvExpiryDelta, LongToBtcAmount, MilliSatoshi, ShortChannelId, randomBytes32} import scodec.Codec @@ -43,8 +44,13 @@ case class PaymentRequest(prefix: String, amount: Option[MilliSatoshi], timestam amount.foreach(a => require(a > 0.msat, s"amount is not valid")) require(tags.collect { case _: PaymentRequest.PaymentHash => }.size == 1, "there must be exactly one payment hash tag") - require(tags.collect { case _: PaymentRequest.PaymentSecret => }.size <= 1, "there must be at most one payment secret tag") require(tags.collect { case PaymentRequest.Description(_) | PaymentRequest.DescriptionHash(_) => }.size == 1, "there must be exactly one description tag or one description hash tag") + if (features.allowMultiPart) { + require(features.allowPaymentSecret, "there must be a payment secret when using multi-part") + } + if (features.allowPaymentSecret) { + require(tags.collect { case _: PaymentRequest.PaymentSecret => }.size == 1, "there must be exactly one payment secret tag when feature bit is set") + } /** * @return the payment hash @@ -114,8 +120,6 @@ case class PaymentRequest(prefix: String, amount: Option[MilliSatoshi], timestam object PaymentRequest { - import fr.acinq.eclair.Features.PAYMENT_SECRET_OPTIONAL - val DEFAULT_EXPIRY_SECONDS = 3600 val prefixes = Map( @@ -129,20 +133,24 @@ object PaymentRequest { features: Option[Features] = Some(Features(PAYMENT_SECRET_OPTIONAL))): PaymentRequest = { val prefix = prefixes(chainHash) + val tags = { + val defaultTags = List( + Some(PaymentHash(paymentHash)), + Some(Description(description)), + fallbackAddress.map(FallbackAddress(_)), + expirySeconds.map(Expiry(_)), + features).flatten + val paymentSecretTag = if (features.exists(_.allowPaymentSecret)) PaymentSecret(randomBytes32) :: Nil else Nil + val routingInfoTags = extraHops.map(RoutingInfo) + defaultTags ++ paymentSecretTag ++ routingInfoTags + } PaymentRequest( prefix = prefix, amount = amount, timestamp = timestamp, nodeId = privateKey.publicKey, - tags = List( - Some(PaymentHash(paymentHash)), - Some(PaymentSecret(randomBytes32)), - Some(Description(description)), - fallbackAddress.map(FallbackAddress(_)), - expirySeconds.map(Expiry(_)), - features - ).flatten ++ extraHops.map(RoutingInfo), + tags = tags, signature = ByteVector.empty) .sign(privateKey) } @@ -321,11 +329,9 @@ object PaymentRequest { * Features supported or required for receiving this payment. */ case class Features(bitmask: BitVector) extends TaggedField { - - import fr.acinq.eclair.Features._ - lazy val supported: Boolean = areSupported(bitmask) lazy val allowMultiPart: Boolean = hasFeature(bitmask, BASIC_MULTI_PART_PAYMENT_MANDATORY) || hasFeature(bitmask, BASIC_MULTI_PART_PAYMENT_OPTIONAL) + lazy val allowPaymentSecret: Boolean = hasFeature(bitmask, PAYMENT_SECRET_MANDATORY) || hasFeature(bitmask, PAYMENT_SECRET_OPTIONAL) lazy val requirePaymentSecret: Boolean = hasFeature(bitmask, PAYMENT_SECRET_MANDATORY) lazy val allowTrampoline: Boolean = hasFeature(bitmask, TRAMPOLINE_PAYMENT_MANDATORY) || hasFeature(bitmask, TRAMPOLINE_PAYMENT_OPTIONAL) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/send/PaymentInitiator.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/send/PaymentInitiator.scala index 9d85b09a3..aa9864e27 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/send/PaymentInitiator.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/send/PaymentInitiator.scala @@ -47,12 +47,14 @@ class PaymentInitiator(nodeParams: NodeParams, router: ActorRef, relayer: ActorR val finalExpiry = r.finalExpiry(nodeParams.currentBlockHeight) r.paymentRequest match { case Some(invoice) if !invoice.features.supported => - sender ! PaymentFailed(paymentId, r.paymentHash, LocalFailure(new IllegalArgumentException(s"can't send payment: unknown invoice features (${r.paymentRequest.get.features})")) :: Nil) - case Some(invoice) if invoice.features.allowMultiPart => - r.predefinedRoute match { - case Nil => spawnMultiPartPaymentFsm(paymentCfg) forward SendMultiPartPayment(r.paymentHash, invoice.paymentSecret.get, r.targetNodeId, r.amount, finalExpiry, r.maxAttempts, r.assistedRoutes, r.routeParams) - case hops => spawnPaymentFsm(paymentCfg) forward SendPaymentToRoute(r.paymentHash, hops, Onion.createMultiPartPayload(r.amount, invoice.amount.getOrElse(r.amount), finalExpiry, invoice.paymentSecret.get)) + sender ! PaymentFailed(paymentId, r.paymentHash, LocalFailure(new IllegalArgumentException(s"can't send payment: unknown invoice features (${invoice.features})")) :: Nil) + case Some(invoice) if invoice.features.allowMultiPart => invoice.paymentSecret match { + case Some(paymentSecret) => r.predefinedRoute match { + case Nil => spawnMultiPartPaymentFsm(paymentCfg) forward SendMultiPartPayment(r.paymentHash, paymentSecret, r.targetNodeId, r.amount, finalExpiry, r.maxAttempts, r.assistedRoutes, r.routeParams) + case hops => spawnPaymentFsm(paymentCfg) forward SendPaymentToRoute(r.paymentHash, hops, Onion.createMultiPartPayload(r.amount, invoice.amount.getOrElse(r.amount), finalExpiry, paymentSecret)) } + case None => sender ! PaymentFailed(paymentId, r.paymentHash, LocalFailure(new IllegalArgumentException("can't send payment: multi-part invoice is missing a payment secret")) :: Nil) + } case _ => val payFsm = spawnPaymentFsm(paymentCfg) // NB: we only generate legacy payment onions for now for maximum compatibility. @@ -64,26 +66,30 @@ class PaymentInitiator(nodeParams: NodeParams, router: ActorRef, relayer: ActorR case r: SendTrampolinePaymentRequest => val paymentId = UUID.randomUUID() - val paymentCfg = SendPaymentConfig(paymentId, paymentId, None, r.paymentRequest.paymentHash, r.trampolineNodeId, Some(r.paymentRequest), storeInDb = true, publishEvent = true) - val trampolineRoute = Seq( - NodeHop(nodeParams.nodeId, r.trampolineNodeId, nodeParams.expiryDeltaBlocks, 0 msat), - NodeHop(r.trampolineNodeId, r.paymentRequest.nodeId, r.trampolineExpiryDelta, r.trampolineFees) // for now we only use a single trampoline hop - ) - // We generate a random secret for this payment to avoid leaking the invoice secret to the first trampoline node. - val trampolineSecret = randomBytes32 - val finalPayload = if (r.paymentRequest.features.allowMultiPart) { - Onion.createMultiPartPayload(r.finalAmount, r.finalAmount, r.finalExpiry(nodeParams.currentBlockHeight), r.paymentRequest.paymentSecret.get) - } else { - Onion.createSinglePartPayload(r.finalAmount, r.finalExpiry(nodeParams.currentBlockHeight), r.paymentRequest.paymentSecret) - } - // We assume that the trampoline node supports multi-part payments (it should). - val (trampolineAmount, trampolineExpiry, trampolineOnion) = if (r.paymentRequest.features.allowTrampoline) { - OutgoingPacket.buildPacket(Sphinx.TrampolinePacket)(r.paymentRequest.paymentHash, trampolineRoute, finalPayload) - } else { - OutgoingPacket.buildTrampolineToLegacyPacket(r.paymentRequest, trampolineRoute, finalPayload) - } - spawnMultiPartPaymentFsm(paymentCfg) forward SendMultiPartPayment(r.paymentRequest.paymentHash, trampolineSecret, r.trampolineNodeId, trampolineAmount, trampolineExpiry, 1, r.paymentRequest.routingInfo, r.routeParams, Seq(OnionTlv.TrampolineOnion(trampolineOnion.packet))) sender ! paymentId + if (!r.paymentRequest.features.allowTrampoline && r.paymentRequest.amount.isEmpty) { + sender ! PaymentFailed(paymentId, r.paymentRequest.paymentHash, LocalFailure(new IllegalArgumentException("cannot pay a 0-value invoice via trampoline-to-legacy (trampoline may steal funds)")) :: Nil) + } else { + val paymentCfg = SendPaymentConfig(paymentId, paymentId, None, r.paymentRequest.paymentHash, r.trampolineNodeId, Some(r.paymentRequest), storeInDb = true, publishEvent = true) + val finalPayload = if (r.paymentRequest.features.allowMultiPart) { + Onion.createMultiPartPayload(r.finalAmount, r.finalAmount, r.finalExpiry(nodeParams.currentBlockHeight), r.paymentRequest.paymentSecret.get) + } else { + Onion.createSinglePartPayload(r.finalAmount, r.finalExpiry(nodeParams.currentBlockHeight), r.paymentRequest.paymentSecret) + } + val trampolineRoute = Seq( + NodeHop(nodeParams.nodeId, r.trampolineNodeId, nodeParams.expiryDeltaBlocks, 0 msat), + NodeHop(r.trampolineNodeId, r.paymentRequest.nodeId, r.trampolineExpiryDelta, r.trampolineFees) // for now we only use a single trampoline hop + ) + // We assume that the trampoline node supports multi-part payments (it should). + val (trampolineAmount, trampolineExpiry, trampolineOnion) = if (r.paymentRequest.features.allowTrampoline) { + OutgoingPacket.buildPacket(Sphinx.TrampolinePacket)(r.paymentRequest.paymentHash, trampolineRoute, finalPayload) + } else { + OutgoingPacket.buildTrampolineToLegacyPacket(r.paymentRequest, trampolineRoute, finalPayload) + } + // We generate a random secret for this payment to avoid leaking the invoice secret to the first trampoline node. + val trampolineSecret = randomBytes32 + spawnMultiPartPaymentFsm(paymentCfg) forward SendMultiPartPayment(r.paymentRequest.paymentHash, trampolineSecret, r.trampolineNodeId, trampolineAmount, trampolineExpiry, 1, r.paymentRequest.routingInfo, r.routeParams, Seq(OnionTlv.TrampolineOnion(trampolineOnion.packet))) + } } def spawnPaymentFsm(paymentCfg: SendPaymentConfig): ActorRef = context.actorOf(PaymentLifecycle.props(nodeParams, paymentCfg, router, register)) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/wire/Onion.scala b/eclair-core/src/main/scala/fr/acinq/eclair/wire/Onion.scala index 2cbbc7544..37b13cac6 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/wire/Onion.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/wire/Onion.scala @@ -271,7 +271,6 @@ object Onion { /** Create a trampoline inner payload instructing the trampoline node to relay via a non-trampoline payment. */ def createNodeRelayToNonTrampolinePayload(amount: MilliSatoshi, totalAmount: MilliSatoshi, expiry: CltvExpiry, targetNodeId: PublicKey, invoice: PaymentRequest): NodeRelayPayload = { - require(invoice.amount.nonEmpty, "cannot pay a 0-value invoice via trampoline-to-legacy (trampoline may steal funds)") val tlvs = Seq[OnionTlv](AmountToForward(amount), OutgoingCltv(expiry), OutgoingNodeId(targetNodeId), InvoiceFeatures(invoice.features.bitmask.bytes), InvoiceRoutingInfo(invoice.routingInfo.toList.map(_.toList))) val tlvs2 = invoice.paymentSecret.map(s => tlvs :+ PaymentData(s, totalAmount)).getOrElse(tlvs) NodeRelayPayload(TlvStream(tlvs2)) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentInitiatorSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentInitiatorSpec.scala index f6a947a0c..69fb9aa99 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentInitiatorSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentInitiatorSpec.scala @@ -36,6 +36,8 @@ import fr.acinq.eclair.wire.{OnionCodecs, OnionTlv} import fr.acinq.eclair.{CltvExpiryDelta, LongToBtcAmount, NodeParams, TestConstants, randomKey} import org.scalatest.{Outcome, fixture} +import scala.concurrent.duration._ + /** * Created by t-bast on 25/07/2019. */ @@ -100,7 +102,7 @@ class PaymentInitiatorSpec extends TestKit(ActorSystem("test")) with fixture.Fun test("forward multi-part payment") { f => import f._ - val pr = PaymentRequest(Block.LivenetGenesisBlock.hash, Some(finalAmount), paymentHash, randomKey, "Some invoice", features = Some(Features(BASIC_MULTI_PART_PAYMENT_OPTIONAL))) + val pr = PaymentRequest(Block.LivenetGenesisBlock.hash, Some(finalAmount), paymentHash, randomKey, "Some invoice", features = Some(Features(BASIC_MULTI_PART_PAYMENT_OPTIONAL, PAYMENT_SECRET_OPTIONAL))) val req = SendPaymentRequest(finalAmount + 100.msat, paymentHash, c, 1, CltvExpiryDelta(42), Some(pr)) sender.send(initiator, req) val id = sender.expectMsgType[UUID] @@ -110,7 +112,7 @@ class PaymentInitiatorSpec extends TestKit(ActorSystem("test")) with fixture.Fun test("forward multi-part payment with pre-defined route") { f => import f._ - val pr = PaymentRequest(Block.LivenetGenesisBlock.hash, Some(finalAmount), paymentHash, randomKey, "Some invoice", features = Some(Features(BASIC_MULTI_PART_PAYMENT_OPTIONAL))) + val pr = PaymentRequest(Block.LivenetGenesisBlock.hash, Some(finalAmount), paymentHash, randomKey, "Some invoice", features = Some(Features(BASIC_MULTI_PART_PAYMENT_OPTIONAL, PAYMENT_SECRET_OPTIONAL))) val req = SendPaymentRequest(finalAmount / 2, paymentHash, c, 1, paymentRequest = Some(pr), predefinedRoute = Seq(a, b, c)) sender.send(initiator, req) val id = sender.expectMsgType[UUID] @@ -195,4 +197,22 @@ class PaymentInitiatorSpec extends TestKit(ActorSystem("test")) with fixture.Fun assert(trampolinePayload.invoiceFeatures === Some(pr.features.bitmask.bytes)) } + test("reject trampoline to legacy payment for 0-value invoice") { f => + import f._ + // This is disabled because it would let the trampoline node steal the whole payment (if malicious). + val routingHints = List(List(PaymentRequest.ExtraHop(b, channelUpdate_bc.shortChannelId, 10 msat, 100, CltvExpiryDelta(144)))) + val features = Features(PAYMENT_SECRET_OPTIONAL, BASIC_MULTI_PART_PAYMENT_OPTIONAL) + val pr = PaymentRequest(Block.RegtestGenesisBlock.hash, None, paymentHash, priv_a.privateKey, "#abittooreckless", None, None, routingHints, features = Some(features)) + val trampolineFees = 21000 msat + val req = SendTrampolinePaymentRequest(finalAmount, trampolineFees, pr, b, CltvExpiryDelta(9), CltvExpiryDelta(12)) + sender.send(initiator, req) + val id = sender.expectMsgType[UUID] + val fail = sender.expectMsgType[PaymentFailed] + assert(fail.id === id) + assert(fail.failures.head.isInstanceOf[LocalFailure]) + + multiPartPayFsm.expectNoMsg(50 millis) + payFsm.expectNoMsg(50 millis) + } + } diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentPacketSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentPacketSpec.scala index 9419723d3..295f7f4b2 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentPacketSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentPacketSpec.scala @@ -259,16 +259,6 @@ class PaymentPacketSpec extends FunSuite with BeforeAndAfterAll { assert(inner_d.invoiceRoutingInfo === Some(routingHints)) } - test("fail to build a trampoline payment to non-trampoline recipient for 0-value invoice") { - // This is disabled because it would let the trampoline node steal the whole payment (if malicious). - val routingHints = List(List(PaymentRequest.ExtraHop(randomKey.publicKey, ShortChannelId(42), 10 msat, 100, CltvExpiryDelta(144)))) - val invoiceFeatures = Features(PAYMENT_SECRET_OPTIONAL, BASIC_MULTI_PART_PAYMENT_OPTIONAL) - val invoice = PaymentRequest(Block.RegtestGenesisBlock.hash, None, paymentHash, priv_a.privateKey, "#abittooreckless", None, None, routingHints, features = Some(invoiceFeatures)) - assertThrows[IllegalArgumentException]( - buildTrampolineToLegacyPacket(invoice, trampolineHops, FinalLegacyPayload(finalAmount, finalExpiry)) - ) - } - test("fail to build a trampoline payment when too much invoice data is provided") { val routingHintOverflow = List(List.fill(7)(PaymentRequest.ExtraHop(randomKey.publicKey, ShortChannelId(1), 10 msat, 100, CltvExpiryDelta(12)))) val invoice = PaymentRequest(Block.RegtestGenesisBlock.hash, Some(finalAmount), paymentHash, priv_a.privateKey, "#reckless", None, None, routingHintOverflow) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentRequestSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentRequestSpec.scala index 7afd6c78a..06889d28e 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentRequestSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentRequestSpec.scala @@ -312,8 +312,8 @@ class PaymentRequestSpec extends FunSuite { val featureBits = Map( Features(bin" 00000000000000000000") -> Result(allowMultiPart = false, requirePaymentSecret = false, areSupported = true), - Features(bin" 00010000000000000000") -> Result(allowMultiPart = true, requirePaymentSecret = false, areSupported = true), - Features(bin" 00100000000000000000") -> Result(allowMultiPart = true, requirePaymentSecret = false, areSupported = true), + Features(bin" 00011000000000000000") -> Result(allowMultiPart = true, requirePaymentSecret = false, areSupported = true), + Features(bin" 00101000000000000000") -> Result(allowMultiPart = true, requirePaymentSecret = false, areSupported = true), Features(bin" 00010100000000000000") -> Result(allowMultiPart = true, requirePaymentSecret = true, areSupported = true), Features(bin" 00011000000000000000") -> Result(allowMultiPart = true, requirePaymentSecret = false, areSupported = true), Features(bin" 00101000000000000000") -> Result(allowMultiPart = true, requirePaymentSecret = false, areSupported = true), @@ -349,6 +349,16 @@ class PaymentRequestSpec extends FunSuite { val pr2 = PaymentRequest.read("lnbc40n1pw9qjvwpp5qq3w2ln6krepcslqszkrsfzwy49y0407hvks30ec6pu9s07jur3sdpstfshq5n9v9jzucm0d5s8vmm5v5s8qmmnwssyj3p6yqenwdencqzysxqrrss7ju0s4dwx6w8a95a9p2xc5vudl09gjl0w2n02sjrvffde632nxwh2l4w35nqepj4j5njhh4z65wyfc724yj6dn9wajvajfn5j7em6wsq2elakl") assert(!pr2.features.requirePaymentSecret) assert(pr2.paymentSecret === None) + + // An invoice that sets the payment secret feature bit must provide a payment secret. + assertThrows[IllegalArgumentException]( + PaymentRequest.read("lntb15u1pwahg4kpp56hhnss8tshz2qz539a0u69yjlcq4vpm7776tuqqv53mqn8eeslpsdq4xysyymr0vd4kzcmrd9hx7cqp29qy9qqq6t3lep4wkqeuj4y58fwxj6lqykzqdaa7a7cak4elywptgft0mcfnl0k243870ek8dwnnqww67wrak5kxpfw428rgu58z66er76sh5zsqw0zvns") + ) + + // A multi-part invoice must use a payment secret. + assertThrows[IllegalArgumentException]( + PaymentRequest(Block.LivenetGenesisBlock.hash, Some(123 msat), ByteVector32.One, priv, "MPP without secrets", features = Some(Features(BASIC_MULTI_PART_PAYMENT_OPTIONAL))) + ) } test("trampoline") { @@ -359,7 +369,7 @@ class PaymentRequestSpec extends FunSuite { assert(!pr1.features.allowMultiPart) assert(pr1.features.allowTrampoline) - val pr2 = PaymentRequest(Block.LivenetGenesisBlock.hash, Some(123 msat), ByteVector32.One, priv, "Some invoice", features = Some(Features(TRAMPOLINE_PAYMENT_MANDATORY, BASIC_MULTI_PART_PAYMENT_OPTIONAL))) + val pr2 = PaymentRequest(Block.LivenetGenesisBlock.hash, Some(123 msat), ByteVector32.One, priv, "Some invoice", features = Some(Features(TRAMPOLINE_PAYMENT_MANDATORY, BASIC_MULTI_PART_PAYMENT_OPTIONAL, PAYMENT_SECRET_OPTIONAL))) assert(pr2.features.allowMultiPart) assert(pr2.features.allowTrampoline)