From f1c29aa3bde65da04d821b95ae0e14f4c2f7c06e Mon Sep 17 00:00:00 2001
From: Christian Decker <decker.christian@gmail.com>
Date: Wed, 22 Feb 2023 14:03:28 +0100
Subject: [PATCH] keysend: Do not strip even TLV types that were allowlisted

Changelog-Fixed: keysend: Keysend would strip even allowed extra TLV types before resolving, this is no longer the case.
---
 plugins/keysend.c | 12 ++++++++++++
 tests/test_pay.py | 16 +++++++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/plugins/keysend.c b/plugins/keysend.c
index 87f69bfa3..359f434bc 100644
--- a/plugins/keysend.c
+++ b/plugins/keysend.c
@@ -331,6 +331,15 @@ static int tlvfield_cmp(const struct tlv_field *a,
 	return 0;
 }
 
+/* Check to see if a given TLV type is in the allowlist. */
+static bool keysend_accept_extra_tlv_type(u64 type)
+{
+	for (size_t i=0; i<tal_count(accepted_extra_tlvs); i++)
+		if (type == accepted_extra_tlvs[i])
+			return true;
+	return false;
+}
+
 static struct command_result *
 htlc_accepted_invoice_created(struct command *cmd, const char *buf,
 			      const jsmntok_t *result,
@@ -348,6 +357,9 @@ htlc_accepted_invoice_created(struct command *cmd, const char *buf,
 		/* Same with known fields */
 		if (ki->payload->fields[i].meta)
 			continue;
+		/* If the type was explicitly allowed pass it through. */
+		if (keysend_accept_extra_tlv_type(ki->payload->fields[i].numtype))
+			continue;
 		/* Complain about it, at least. */
 		if (ki->preimage_field != &ki->payload->fields[i]) {
 			plugin_log(cmd->plugin, LOG_INFORM,
diff --git a/tests/test_pay.py b/tests/test_pay.py
index 6744e7394..f999a196f 100644
--- a/tests/test_pay.py
+++ b/tests/test_pay.py
@@ -3586,7 +3586,7 @@ def test_keysend(node_factory):
 def test_keysend_strip_tlvs(node_factory):
     """Use the extratlvs option to deliver a message with sphinx' TLV type, which keysend strips.
     """
-    amt = 10000
+    amt = 10**7
     l1, l2 = node_factory.line_graph(
         2,
         wait_for_announce=True,
@@ -3594,6 +3594,7 @@ def test_keysend_strip_tlvs(node_factory):
             {
                 # Not needed, just for listconfigs test.
                 'accept-htlc-tlv-types': '133773310,99990',
+                "plugin": os.path.join(os.path.dirname(__file__), "plugins/sphinx-receiver.py"),
             },
             {
                 "plugin": os.path.join(os.path.dirname(__file__), "plugins/sphinx-receiver.py"),
@@ -3604,6 +3605,7 @@ def test_keysend_strip_tlvs(node_factory):
     # Make sure listconfigs works here
     assert l1.rpc.listconfigs()['accept-htlc-tlv-types'] == '133773310,99990'
 
+    # l1 is configured to accept, so l2 should still filter them out
     l1.rpc.keysend(l2.info['id'], amt, extratlvs={133773310: 'FEEDC0DE'})
     inv = only_one(l2.rpc.listinvoices()['invoices'])
     assert not l2.daemon.is_in_log(r'plugin-sphinx-receiver.py.*extratlvs.*133773310.*feedc0de')
@@ -3636,6 +3638,18 @@ More info
     assert inv['description'] == 'keysend: ' + ksinfo
     l2.daemon.wait_for_log('Keysend payment uses illegal even field 133773310: stripping')
 
+    # Now reverse the direction. l1 accepts 133773310, but filters out
+    # other even unknown types (like 133773312).
+    l2.rpc.keysend(l1.info['id'], amt, extratlvs={
+        "133773310": b"helloworld".hex(),  # This one is allowlisted
+        "133773312": b"filterme".hex(),  # This one will get stripped
+    })
+
+    # The invoice_payment hook must contain the allowlisted TLV type,
+    # but not the stripped one.
+    assert l1.daemon.wait_for_log(r'plugin-sphinx-receiver.py: invoice_payment.*extratlvs.*133773310')
+    assert not l1.daemon.is_in_log(r'plugin-sphinx-receiver.py: invoice_payment.*extratlvs.*133773312')
+
 
 def test_keysend_routehint(node_factory):
     """Test whether we can deliver a keysend by adding a routehint on the cli