lightningd: check payment secret on htlc receipt.

We don't set the secret to compulsory (yet!) but put code in for the
future.  Meanwhile, if there is a secret, check it is correct.

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

common/bolt11.c

@@ -302,7 +302,7 @@ static char *decode_n(struct bolt11 *b11,
         return NULL;
 }
 
-/* BOLT-e36f7b6517e1173dcbd49da3b516cfe1f48ae556 #11:
+/* BOLT-412c260b537be95b105ae2062463f1d992024ca7 #11:
  *
  * * `s` (16): `data_length` 52. This 256-bit secret prevents
  *    forwarding nodes from probing the payment recipient.
@@ -317,7 +317,7 @@ static char *decode_s(struct bolt11 *b11,
                 return unknown_field(b11, hu5, data, data_len, 's',
                                      data_length);
 
-        /* BOLT-e36f7b6517e1173dcbd49da3b516cfe1f48ae556 #11:
+        /* BOLT-412c260b537be95b105ae2062463f1d992024ca7 #11:
 	 *
 	 * A reader... MUST skip over unknown fields, OR an `f` field
 	 * with unknown `version`, OR `p`, `h`, `s` or `n` fields that do

lightningd/invoice.c

@@ -9,6 +9,7 @@
 #include <common/amount.h>
 #include <common/bech32.h>
 #include <common/bolt11.h>
+#include <common/features.h>
 #include <common/json_command.h>
 #include <common/json_helpers.h>
 #include <common/jsonrpc_errors.h>
@@ -235,7 +236,8 @@ REGISTER_PLUGIN_HOOK(invoice_payment,
 void invoice_try_pay(struct lightningd *ld,
 		     struct htlc_in *hin,
 		     const struct sha256 *payment_hash,
-		     const struct amount_msat msat)
+		     const struct amount_msat msat,
+		     const struct secret *payment_secret)
 {
 	struct invoice invoice;
 	const struct invoice_details *details;
@@ -247,6 +249,42 @@ void invoice_try_pay(struct lightningd *ld,
 	}
 	details = wallet_invoice_details(tmpctx, ld->wallet, invoice);
 
+	log_debug(ld->log, "payment_secret is %s",
+		  payment_secret ? "set": "NULL");
+
+	/* BOLT-e36f7b6517e1173dcbd49da3b516cfe1f48ae556 #4:
+	 *
+	 * - if the `payment_secret` doesn't match the expected value for that
+	 *   `payment_hash`, or the `payment_secret` is required and is not
+	 *   present:
+	 *   - MUST fail the HTLC.
+	 *   - MUST return an `incorrect_or_unknown_payment_details` error.
+	 */
+	/* BOLT-e36f7b6517e1173dcbd49da3b516cfe1f48ae556 #1:
+	 *
+	 * - if `payment_secret` is required in the onion:
+	 *    - MUST set the even feature `var_onion_optin`.
+	 */
+	if (feature_is_set(details->features, COMPULSORY_FEATURE(OPT_VAR_ONION))
+	    && !payment_secret) {
+		fail_htlc(hin, WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS);
+		return;
+	}
+
+	if (payment_secret) {
+		struct secret expected;
+
+		invoice_secret(&details->r, &expected);
+		log_debug(ld->log, "payment_secret %s vs %s",
+			  type_to_string(tmpctx, struct secret, payment_secret),
+			  type_to_string(tmpctx, struct secret, &expected));
+		if (!secret_eq_consttime(payment_secret, &expected)) {
+			fail_htlc(hin,
+				  WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS);
+			return;
+		}
+	}
+
 	/* BOLT #4:
 	 *
 	 * An _intermediate hop_ MUST NOT, but the _final node_:

lightningd/invoice.h

@@ -14,12 +14,14 @@ struct sha256;
  * @hin: the input HTLC which is offering to pay.
  * @payment_hash: hash of preimage they want.
  * @msat: amount they offer to pay.
+ * @payment_secret: they payment secret they sent, if any.
  *
  * Either calls fulfill_htlc() or fail_htlcs().
  */
 void invoice_try_pay(struct lightningd *ld,
 		     struct htlc_in *hin,
 		     const struct sha256 *payment_hash,
-		     const struct amount_msat msat);
+		     const struct amount_msat msat,
+		     const struct secret *payment_secret);
 
 #endif /* LIGHTNING_LIGHTNINGD_INVOICE_H */

lightningd/peer_htlcs.c

@@ -342,7 +342,7 @@ static void handle_localpay(struct htlc_in *hin,
 		goto fail;
 	}
 
-	invoice_try_pay(ld, hin, payment_hash, amt_to_forward);
+	invoice_try_pay(ld, hin, payment_hash, amt_to_forward, payment_secret);
 	return;
 
 fail:

lightningd/test/run-invoice-select-inchan.c

@@ -93,6 +93,9 @@ void fail_htlc(struct htlc_in *hin UNNEEDED, enum onion_type failcode UNNEEDED)
 /* Generated stub for fatal */
 void   fatal(const char *fmt UNNEEDED, ...)
 { fprintf(stderr, "fatal called!\n"); abort(); }
+/* Generated stub for feature_is_set */
+bool feature_is_set(const u8 *features UNNEEDED, size_t bit UNNEEDED)
+{ fprintf(stderr, "feature_is_set called!\n"); abort(); }
 /* Generated stub for fromwire_channel_dev_memleak_reply */
 bool fromwire_channel_dev_memleak_reply(const void *p UNNEEDED, bool *leak UNNEEDED)
 { fprintf(stderr, "fromwire_channel_dev_memleak_reply called!\n"); abort(); }

tests/test_pay.py

@@ -2411,6 +2411,25 @@ def test_tlv_or_legacy(node_factory, bitcoind):
         l3.daemon.wait_for_log("Got onion.*'type': 'legacy'")
 
 
+@unittest.skipIf(not EXPERIMENTAL_FEATURES, 'Needs invoice secret support')
+@unittest.skipIf(not DEVELOPER, 'Needs dev-routes')
+def test_pay_no_secret(node_factory, bitcoind):
+    l1, l2 = node_factory.line_graph(2, wait_for_announce=True)
+
+    l2.rpc.invoice(100000, "test_pay_no_secret", "test_pay_no_secret",
+                   preimage='00' * 32, expiry=2000000000)
+
+    # Produced from modified version (different secret!).
+    inv_badsecret = 'lnbcrt1u1pwuedm6pp5ve584t0cv27hwmy0cx9ca8uwyqyfw9y9dm3r8vus9fv36r2l9yjsdqaw3jhxazlwpshjhmwda0hxetrwfjhgxq8pmnt9qqcqp9sp52au0npwmw4xxv2rfrat04kh9p3jlmklgavhfxqukx0l05pw5tccs9qypqsqa286dmt2xh3jy8cd8ndeyr845q8a7nhgjkerdqjns76jraux6j25ddx9f5k5r2ey0kk942x3uhaff66794kyjxxcd48uevf7p6ja53gqjj5ur7'
+    with pytest.raises(RpcError, match=r"INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS.*'erring_index': 1"):
+        l1.rpc.pay(inv_badsecret)
+
+    # Produced from old version (no secret!)
+    inv_nosecret = 'lnbcrt1u1pwue4vapp5ve584t0cv27hwmy0cx9ca8uwyqyfw9y9dm3r8vus9fv36r2l9yjsdqaw3jhxazlwpshjhmwda0hxetrwfjhgxq8pmnt9qqcqp9570xsjyykvssa6ty8fjth6f2y8h09myngad9utesttwjwclv95fz3lgd402f9e5yzpnxmkypg55rkvpg522gcz4ymsjl2w3m4jhw4jsp55m7tl'
+    # This succeeds until we make secrets compulsory.
+    l1.rpc.pay(inv_nosecret)
+
+
 @flaky
 def test_shadow_routing(node_factory):
     """

wallet/test/run-wallet.c

@@ -207,7 +207,8 @@ void invoices_waitone(const tal_t *ctx UNNEEDED,
 void invoice_try_pay(struct lightningd *ld UNNEEDED,
 		     struct htlc_in *hin UNNEEDED,
 		     const struct sha256 *payment_hash UNNEEDED,
-		     const struct amount_msat msat UNNEEDED)
+		     const struct amount_msat msat UNNEEDED,
+		     const struct secret *payment_secret UNNEEDED)
 { fprintf(stderr, "invoice_try_pay called!\n"); abort(); }
 /* Generated stub for json_add_address */
 void json_add_address(struct json_stream *response UNNEEDED, const char *fieldname UNNEEDED,