From 9459d22d6623af223bf745b98003da7ea3c240ae Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 4 Oct 2022 09:31:30 +1030 Subject: [PATCH] keysend: just strip even unknown fields. Signed-off-by: Rusty Russell Changelog-Fixed: Plugins: `keysend` now removes unknown even (technically illegal!) fields, to try to accept more payments. --- plugins/keysend.c | 39 +++++++++++++++++++++++---------------- tests/test_pay.py | 11 ++++++----- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/plugins/keysend.c b/plugins/keysend.c index d9d298709825..ea806c3545ca 100644 --- a/plugins/keysend.c +++ b/plugins/keysend.c @@ -248,7 +248,7 @@ struct keysend_in { struct preimage payment_preimage; char *label; struct tlv_tlv_payload *payload; - struct tlv_field *preimage_field; + struct tlv_field *preimage_field, *desc_field; }; static int tlvfield_cmp(const struct tlv_field *a, @@ -267,11 +267,27 @@ htlc_accepted_invoice_created(struct command *cmd, const char *buf, struct keysend_in *ki) { struct tlv_field field; - int preimage_field_idx = ki->preimage_field - ki->payload->fields; - /* Remove the preimage field so `lightningd` knows how to handle - * this. */ - tal_arr_remove(&ki->payload->fields, preimage_field_idx); + /* Remove all unknown fields: complain about unused even ones! + * (Keysend is not a design, it's a kludge by people who + * didn't read the spec, and Just Made Something Work). */ + for (size_t i = 0; i < tal_count(ki->payload->fields); i++) { + /* Odd fields are fine, leave them. */ + if (ki->payload->fields[i].numtype % 2) + continue; + /* Same with known fields */ + if (ki->payload->fields[i].meta) + continue; + /* Complain about it, at least. */ + if (ki->preimage_field != &ki->payload->fields[i]) { + plugin_log(cmd->plugin, LOG_INFORM, + "Keysend payment uses illegal even field %" + PRIu64 ": stripping", + ki->payload->fields[i].numtype); + } + tal_arr_remove(&ki->payload->fields, i); + i--; + } /* Now we can fill in the payment secret, from invoice. */ ki->payload->payment_data = tal(ki->payload, @@ -331,7 +347,7 @@ static struct command_result *htlc_accepted_call(struct command *cmd, struct sha256 payment_hash; size_t max; struct tlv_tlv_payload *payload; - struct tlv_field *preimage_field = NULL, *unknown_field = NULL, *desc_field = NULL; + struct tlv_field *preimage_field = NULL, *desc_field = NULL; bigsize_t s; struct keysend_in *ki; struct out_req *req; @@ -375,9 +391,6 @@ static struct command_result *htlc_accepted_call(struct command *cmd, if (field->numtype == PREIMAGE_TLV_TYPE) { preimage_field = field; continue; - } else if (field->numtype % 2 == 0 && field->meta == NULL) { - /* This can only happen with FROMWIRE_TLV_ANY_TYPE! */ - unknown_field = field; } /* Longest (unknown) text field wins. */ @@ -392,13 +405,6 @@ static struct command_result *htlc_accepted_call(struct command *cmd, if (preimage_field == NULL) return htlc_accepted_continue(cmd, NULL); - if (unknown_field != NULL) { - plugin_log(cmd->plugin, LOG_INFORM, - "Experimental: Accepting the keysend payment " - "despite having unknown even TLV type %" PRIu64 ".", - unknown_field->numtype); - } - /* If malformed (amt is compulsory), let lightningd handle it. */ if (!payload->amt_to_forward) { plugin_log(cmd->plugin, LOG_UNUSUAL, @@ -421,6 +427,7 @@ static struct command_result *htlc_accepted_call(struct command *cmd, ki->label = tal_fmt(ki, "keysend-%lu.%09lu", (unsigned long)now.ts.tv_sec, now.ts.tv_nsec); ki->payload = tal_steal(ki, payload); ki->preimage_field = preimage_field; + ki->desc_field = desc_field; /* If the preimage doesn't hash to the payment_hash we must continue, * maybe someone else knows how to handle these. */ diff --git a/tests/test_pay.py b/tests/test_pay.py index 232a7df3fa8d..49c778507208 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -3580,8 +3580,8 @@ def test_keysend(node_factory): l3.rpc.keysend(l4.info['id'], amt) -def test_keysend_extra_tlvs(node_factory): - """Use the extratlvs option to deliver a message with sphinx' TLV type. +def test_keysend_strip_tlvs(node_factory): + """Use the extratlvs option to deliver a message with sphinx' TLV type, which keysend strips. """ amt = 10000 l1, l2 = node_factory.line_graph( @@ -3593,7 +3593,6 @@ def test_keysend_extra_tlvs(node_factory): 'accept-htlc-tlv-types': '133773310,99990', }, { - 'accept-htlc-tlv-types': '133773310', "plugin": os.path.join(os.path.dirname(__file__), "plugins/sphinx-receiver.py"), }, ] @@ -3601,11 +3600,10 @@ def test_keysend_extra_tlvs(node_factory): # Make sure listconfigs works here assert l1.rpc.listconfigs()['accept-htlc-tlv-types'] == '133773310,99990' - assert l2.rpc.listconfigs()['accept-htlc-tlv-types'] == '133773310' l1.rpc.keysend(l2.info['id'], amt, extratlvs={133773310: 'FEEDC0DE'}) inv = only_one(l2.rpc.listinvoices()['invoices']) - assert(l2.daemon.is_in_log(r'plugin-sphinx-receiver.py.*extratlvs.*133773310.*feedc0de')) + assert not l2.daemon.is_in_log(r'plugin-sphinx-receiver.py.*extratlvs.*133773310.*feedc0de') assert(inv['amount_received_msat'] >= Millisatoshi(amt)) assert inv['description'] == 'keysend' @@ -3615,6 +3613,7 @@ def test_keysend_extra_tlvs(node_factory): l1.rpc.keysend(l2.info['id'], amt, extratlvs={133773310: b'hello there'.hex()}) inv = only_one(l2.rpc.listinvoices()['invoices']) assert inv['description'] == 'keysend: hello there' + l2.daemon.wait_for_log('Keysend payment uses illegal even field 133773310: stripping') l2.rpc.delinvoice(inv['label'], 'paid') # We can (just!) fit a giant description in. @@ -3622,6 +3621,7 @@ def test_keysend_extra_tlvs(node_factory): inv = only_one(l2.rpc.listinvoices()['invoices']) assert inv['description'] == 'keysend: ' + 'a' * 1100 l2.rpc.delinvoice(inv['label'], 'paid') + l2.daemon.wait_for_log('Keysend payment uses illegal even field 133773310: stripping') # Now try with some special characters ksinfo = """💕 ₿"' @@ -3630,6 +3630,7 @@ def test_keysend_extra_tlvs(node_factory): l1.rpc.keysend(l2.info['id'], amt, extratlvs={133773310: bytes(ksinfo, encoding='utf8').hex()}) inv = only_one(l2.rpc.listinvoices()['invoices']) assert inv['description'] == 'keysend: ' + ksinfo + l2.daemon.wait_for_log('Keysend payment uses illegal even field 133773310: stripping') def test_keysend_routehint(node_factory):