From 6724db65de9ca977407c4721742729b3cb6290eb Mon Sep 17 00:00:00 2001 From: Erick Cestari Date: Thu, 3 Jul 2025 13:42:32 +0930 Subject: [PATCH] BOLT11: Make payment secret field ('s') mandatory Make the payment secret field ('s') mandatory for BOLT11 payment requests, implementing the requirement specified in BOLT11 spec PR #1242 (https://github.com/lightning/bolts/pull/1242). This security enhancement prevents payment probing attacks by requiring all invoices to include payment secrets. Changes include: 1. Adding validation in bolt11_decode_nosig() to reject invoices without the 's' field 2. Adding payment secrets to all test vectors 3. Updating expected encoded values in test cases to include payment secrets 4. Adding a specific test case that verifies proper rejection of invoices missing the payment secret field Changelog-Changed: Made payment secret ('s' field) mandatory in BOLT11 payment requests for improved security. --- common/bolt11.c | 3 +++ common/test/run-bolt11.c | 19 +++++++++++++------ tests/test_invoices.py | 7 ++++--- tests/test_pay.py | 2 +- 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/common/bolt11.c b/common/bolt11.c index f90bc978c..bcc00cf38 100644 --- a/common/bolt11.c +++ b/common/bolt11.c @@ -929,6 +929,9 @@ struct bolt11 *bolt11_decode_nosig(const tal_t *ctx, const char *str, if (!have_field[bech32_charset_rev['p']]) return decode_fail(b11, fail, "No valid 'p' field found"); + if (!have_field[bech32_charset_rev['s']]) + return decode_fail(b11, fail, "Missing required payment secret (s field)"); + if (have_field[bech32_charset_rev['h']] && description) { struct sha256 sha; diff --git a/common/test/run-bolt11.c b/common/test/run-bolt11.c index 630536eec..e4fbbd797 100644 --- a/common/test/run-bolt11.c +++ b/common/test/run-bolt11.c @@ -300,6 +300,8 @@ int main(int argc, char *argv[]) b11 = new_bolt11(tmpctx, &msatoshi); b11->chain = chainparams_for_network("bitcoin"); b11->timestamp = 1496314658; + b11->payment_secret = tal(b11, struct secret); + memset(b11->payment_secret, 0x11, sizeof(*b11->payment_secret)); if (!hex_decode("0001020304050607080900010203040506070809000102030405060708090102", strlen("0001020304050607080900010203040506070809000102030405060708090102"), &b11->payment_hash, sizeof(b11->payment_hash))) @@ -308,9 +310,7 @@ int main(int argc, char *argv[]) b11->description = "1 cup coffee"; b11->expiry = 60; - dev_bolt11_omit_c_value = true; - test_b11("LNBC2500U1PVJLUEZPP5QQQSYQCYQ5RQWZQFQQQSYQCYQ5RQWZQFQQQSYQCYQ5RQWZQFQYPQDQ5XYSXXATSYP3K7ENXV4JSXQZPUAZTRNWNGZN3KDZW5HYDLZF03QDGM2HDQ27CQV3AGM2AWHZ5SE903VRUATFHQ77W3LS4EVS3CH9ZW97J25EMUDUPQ63NYW24CG27H2RSPFJ9SRP", b11, NULL); - dev_bolt11_omit_c_value = false; + test_b11("LNBC2500U1PVJLUEZSP5ZYG3ZYG3ZYG3ZYG3ZYG3ZYG3ZYG3ZYG3ZYG3ZYG3ZYG3ZYG3ZYGSPP5QQQSYQCYQ5RQWZQFQQQSYQCYQ5RQWZQFQQQSYQCYQ5RQWZQFQYPQDQ5XYSXXATSYP3K7ENXV4JSXQZPUCQPJEJ3HDW922T23KQLTFQDTUF2MGFQESVYFKKAQVE0HW8VD0CHQSTEJ60K02J4W6NP4CUJV87TUS82EDUYX7YTWFW4S58CJTG0U0A6TD6CQ3ADH8K", b11, NULL); /* Unknown field handling */ if (!node_id_from_hexstr("02330d13587b67a85c0a36ea001c4dba14bcd48dda8988f7303275b040bffb6abd", strlen("02330d13587b67a85c0a36ea001c4dba14bcd48dda8988f7303275b040bffb6abd"), &node)) @@ -319,6 +319,8 @@ int main(int argc, char *argv[]) b11 = new_bolt11(tmpctx, &msatoshi); b11->chain = chainparams_for_network("testnet"); b11->timestamp = 1554294928; + b11->payment_secret = tal(b11, struct secret); + memset(b11->payment_secret, 0x11, sizeof(*b11->payment_secret)); if (!hex_decode("850aeaf5f69670e8889936fc2e0cff3ceb0c3b5eab8f04ae57767118db673a91", strlen("850aeaf5f69670e8889936fc2e0cff3ceb0c3b5eab8f04ae57767118db673a91"), &b11->payment_hash, sizeof(b11->payment_hash))) @@ -334,9 +336,7 @@ int main(int argc, char *argv[]) extra->data[i] = bech32_charset_rev[(u8)"dp68gup69uhnzwfj9cejuvf3xshrwde68qcrswf0d46kcarfwpshyaplw3skw0tdw4k8g6tsv9e8g"[i]]; list_add(&b11->extra_fields, &extra->list); - dev_bolt11_omit_c_value = true; - test_b11("lntb30m1pw2f2yspp5s59w4a0kjecw3zyexm7zur8l8n4scw674w8sftjhwec33km882gsdpa2pshjmt9de6zqun9w96k2um5ypmkjargypkh2mr5d9cxzun5ypeh2ursdae8gxqruyqvzddp68gup69uhnzwfj9cejuvf3xshrwde68qcrswf0d46kcarfwpshyaplw3skw0tdw4k8g6tsv9e8g4a3hx0v945csrmpm7yxyaamgt2xu7mu4xyt3vp7045n4k4czxf9kj0vw0m8dr5t3pjxuek04rtgyy8uzss5eet5gcyekd6m7u0mzv5sp7mdsag", b11, NULL); - dev_bolt11_omit_c_value = false; + test_b11("lntb30m1pw2f2yssp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygspp5s59w4a0kjecw3zyexm7zur8l8n4scw674w8sftjhwec33km882gsdpa2pshjmt9de6zqun9w96k2um5ypmkjargypkh2mr5d9cxzun5ypeh2ursdae8gxqruyqcqpjvzddp68gup69uhnzwfj9cejuvf3xshrwde68qcrswf0d46kcarfwpshyaplw3skw0tdw4k8g6tsv9e8gcz45tperrgam4d3t6hc9kaguyzx08mrqsfhfxkx5fmpufvxfvypypyjcpxzvnnzq9jwsm3htpkwkxqsp8jt95ekkzq8ck5vze4lpehqq5cdj22", b11, NULL); /* BOLT #11: * @@ -690,6 +690,13 @@ int main(int argc, char *argv[]) assert(!bolt11_decode(tmpctx, "lnbc2500000001p1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5xysxxatsyp3k7enxv4jsxqzpusp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygs9qrsgq0lzc236j96a95uv0m3umg28gclm5lqxtqqwk32uuk4k6673k6n5kfvx3d2h8s295fad45fdhmusm8sjudfhlf6dcsxmfvkeywmjdkxcp99202x", NULL, NULL, NULL, &fail)); assert(streq(fail, "Invalid sub-millisatoshi amount '2500000001p'")); + /* BOLT #11: + * > ### Missing required `s` field. + * > lnbc20m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqhp58yjmdan79s6qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahrqs9qrsgq7ea976txfraylvgzuxs8kgcw23ezlrszfnh8r6qtfpr6cxga50aj6txm9rxrydzd06dfeawfk6swupvz4erwnyutnjq7x39ymw6j38gp49qdkj + */ + assert(!bolt11_decode(tmpctx, "lnbc20m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqhp58yjmdan79s6qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahrqs9qrsgq7ea976txfraylvgzuxs8kgcw23ezlrszfnh8r6qtfpr6cxga50aj6txm9rxrydzd06dfeawfk6swupvz4erwnyutnjq7x39ymw6j38gp49qdkj", NULL, NULL, NULL, &fail)); + assert(streq(fail, "Missing required payment secret (s field)")); + /* Invalid UTF-8 tests. */ /* Description: Bad UTF-8: 0xC0" */ assert(!bolt11_decode(tmpctx, "lnbc1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5gfskggz423rz6wp6yrqqcqpjkkrsmq07c4ht7qgjdmf2a8savsafcy8lqn4av4gs80gz88ff2y780tdcve7sxp80kd4vk7hajt5mskcsegz2qfll4jywfwhap2q2n6cqyz5tv4", NULL, NULL, NULL, &fail)); diff --git a/tests/test_invoices.py b/tests/test_invoices.py index 1c810965b..1e0260415 100644 --- a/tests/test_invoices.py +++ b/tests/test_invoices.py @@ -571,19 +571,20 @@ def test_waitanyinvoice_reversed(node_factory, executor): def test_decode_unknown(node_factory): l1 = node_factory.get_node() - b11 = l1.rpc.decode('lntb30m1pw2f2yspp5s59w4a0kjecw3zyexm7zur8l8n4scw674w8sftjhwec33km882gsdpa2pshjmt9de6zqun9w96k2um5ypmkjargypkh2mr5d9cxzun5ypeh2ursdae8gxqruyqvzddp68gup69uhnzwfj9cejuvf3xshrwde68qcrswf0d46kcarfwpshyaplw3skw0tdw4k8g6tsv9e8gu2etcvsym36pdjpz04wm9nn96f9ntc3t3h5r08pe9d62p3js5wt5rkurqnrl7zkj2fjpvl3rmn7wwazt80letwxlm22hngu8n88g7hsp542qpl') + # Made with "devtools/bolt11-cli encode 41bfd2660762506c9933ade59f1debf7e6495b10c14a92dbcd2d623da2507d3d s=0000000000000000000000000000000000000000000000000000000000000000 timestamp=1554294928 p=850aeaf5f69670e8889936fc2e0cff3ceb0c3b5eab8f04ae57767118db673a91 d='Payment request with multipart support' x=28800 118=0d011a07081c011a051c1713020e0912051819121c0c0911061017030e0d191a07001803100e090f0d151a16181d03090e011017041d011f0e1110160e0f0b0d0e151607081a0b100c05190708" amount=3000000000 currency=tb + b11 = l1.rpc.decode('lntb30m1pw2f2yssp5qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqpp5s59w4a0kjecw3zyexm7zur8l8n4scw674w8sftjhwec33km882gsdpa2pshjmt9de6zqun9w96k2um5ypmkjargypkh2mr5d9cxzun5ypeh2ursdae8gxqruyqcqpjvzddp68gup69uhnzwfj9cejuvf3xshrwde68qcrswf0d46kcarfwpshyaplw3skw0tdw4k8g6tsv9e8g02pjf6p8tqx0fy6vmxnrvsjmnx7us54ml9uk0927wy4mq5wne8pr6g6mmew03y60lt5mvzyksf7yjwq4qxlun5cca9amgkxggr2xkvcqukuyuf') assert b11['type'] == 'bolt11 invoice' assert b11['currency'] == 'tb' assert b11['created_at'] == 1554294928 assert b11['payment_hash'] == '850aeaf5f69670e8889936fc2e0cff3ceb0c3b5eab8f04ae57767118db673a91' assert b11['description'] == 'Payment request with multipart support' assert b11['expiry'] == 28800 - assert b11['payee'] == '02330d13587b67a85c0a36ea001c4dba14bcd48dda8988f7303275b040bffb6abd' + assert b11['payee'] == '0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518' assert b11['min_final_cltv_expiry'] == 18 extra = only_one(b11['extra']) assert extra['tag'] == 'v' assert extra['data'] == 'dp68gup69uhnzwfj9cejuvf3xshrwde68qcrswf0d46kcarfwpshyaplw3skw0tdw4k8g6tsv9e8g' - assert b11['signature'] == '3045022100e2b2bc3204dc7416c8227d5db2ce65d24b35e22b8de8379c392b74a0c650a397022041db8304c7ff0ad25264167e23dcfce7744b3bff95b8dfda9579a38799ce8f5e' + assert b11['signature'] == '304402207a8324e827580cf4934cd9a636425b99bdc852bbf97967955e712bb051d3c9c202203d235bde5cf8934ffae9b60896827c49381501bfc9d318e97bb458c840d46b33' assert 'fallbacks' not in b11 assert 'routes' not in b11 diff --git a/tests/test_pay.py b/tests/test_pay.py index d39c72271..9a2f58b99 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -2961,7 +2961,7 @@ def test_pay_no_secret(node_factory, bitcoind): # Produced from old version (no secret!) inv_nosecret = 'lnbcrt1u1pwue4vapp5ve584t0cv27hwmy0cx9ca8uwyqyfw9y9dm3r8vus9fv36r2l9yjsdqaw3jhxazlwpshjhmwda0hxetrwfjhgxq8pmnt9qqcqp9570xsjyykvssa6ty8fjth6f2y8h09myngad9utesttwjwclv95fz3lgd402f9e5yzpnxmkypg55rkvpg522gcz4ymsjl2w3m4jhw4jsp55m7tl' - with pytest.raises(RpcError, match=r"INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS.*'erring_index': 1"): + with pytest.raises(RpcError, match=r"Invalid bolt11: Missing required payment secret \(s field\)"): l1.rpc.pay(inv_nosecret)