WIP: Remote Signer Proxy #4
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Since the probing binaries compiled by the configurator needs to run on the host machine we provide a variable CONFIGURATOR_WRAPPER that can be set to anything that you want to wrap the calls with. One example is `qemu-aarch64-static`.
The usual json_stream starters and command_result enders.
This adds helpers to start and send a jsonrpc request using json_stream in order to benefit from the helpers. This then simplifies existing plugins RPC requests by using json_stream helpers.
We mark the test as xfail() as it exposes that libplugin's PLUGIN_RESTARTABLE was not taken into account !
As a separated commit because it was pre-existent (changelog + xfail test). This also fix a logical problem in lightningd/plugin_control: we were assuming a plugin started with 'plugin start' but which did not comport a 'dynamic' entry in its manifest to be dynamic, though it should have been treated as static. Changelog-fixed: plugins: Dynamic C plugins can now be managed when lightningd is up
Co-authored-by: Rusty Russell <rusty@rustcorp.com.au>
This also remove the now duplicate plugin_hook_unregister_all(), added in the tal destructor of the struct plugin.
Changelog-Added: pyln-testing adds wait_for_htlcs helper to settle HTLCs
Changelog-Fixed: Fixed factual errors in `lightning-listchannels.7` documentation. `delay` is ***not*** the unilateral close delay for that channel-direction!
restrict fundchannel_cancel usage to only the opener side Changelog-Changed: Only the opener of a fundchannel can cancel the channel open with fundchannel_cancel
Break out a method for canceling a channel that will either loop through contacting the peer to tell them of the error or just directly cleans up if the peer is currently disconnected.
Use the new forget_channel method to cancel, which checks that peer is still connected before attempting to send message.
if the channel hasn't been locked in yet, allow for a 'hard' error to kill the channel
we have 4 venues in which we can add features, 3 of which are unilaterally controlled (`init`, `node_announcement`, and `invoices`) the `channel_announcement` is co-signed by both parties, so we can't add featurebits without additional coordination overhead. Each location is encoded as a key-value pair in a dict called `featurebits` in the manifest (omitted if no custom featurebits are set).
We'll collect the featurebits on-demand from all currently active plugins when needed.
We will be doing this when collecting featurebits from the plugins, so make this a reusable function.
The `init_featurebits` are computed at startup, and then cached indefinitely. They are then used whenever a new `init` handshake is performed. We could add a new message to push updates to `connectd` whenever a plugin is added or removed, but that's up for discussion.
This is the last venue we need to add custom featurebits to, so we also unmark the test as xfail. Changelog-Added: plugin: Plugins can now signal support for experimental protocol extensions by registering featurebits for `node_announcement`s, the connection handshake, and for invoices. For now this is limited to non-dynamic plugins only
|
closing to recycle, overview and commits get messed up by rebasing |
ksedgwic
pushed a commit
that referenced
this pull request
Sep 8, 2021
The variable `block` (instace of `struct block`) is
allocated on the stack without being initialized, i.e. its
member `prev` points to nowhere. This causes a segmentation
fault on my machine on the binding of "prev_hash" on running
`wallet_block_add`, as the following core-dump analysis
shows:
$ egdb ./wallet/test/run-wallet ./run-wallet.core
[...]
Core was generated by `run-wallet'.
Program terminated with signal SIGSEGV, Segmentation fault.
---Type <return> to continue, or q <return> to quit---
#0 0x000008f67a04b660 in memcpy (dst0=<optimized out>, src0=0x100007f8c, length=32) at /usr/src/lib/libc/string/memcpy.c:97
97 TLOOP1(*dst++ = *src++);
(gdb) bt
#0 0x000008f67a04b660 in memcpy (dst0=<optimized out>, src0=0x100007f8c, length=32) at /usr/src/lib/libc/string/memcpy.c:97
#1 0x000008f73e838f60 in sqlite3VdbeMemSetStr () from /usr/local/lib/libsqlite3.so.37.12
#2 0x000008f73e83cb11 in bindText () from /usr/local/lib/libsqlite3.so.37.12
#3 0x000008f44bc91345 in db_sqlite3_query (stmt=0x8f6845bf028) at wallet/db_sqlite3.c:77
#4 0x000008f44bc91122 in db_sqlite3_exec (stmt=0x8f6845bf028) at wallet/db_sqlite3.c:110
#5 0x000008f44bcbb3b2 in db_exec_prepared_v2 (stmt=0x8f6845bf028) at ./wallet/db.c:2055
#6 0x000008f44bcc6890 in wallet_block_add (w=0x8f688b5bba8, b=0x7f7ffffca788) at ./wallet/wallet.c:3556
#7 0x000008f44bce2607 in test_wallet_outputs (ld=0x8f6a35a7828, ctx=0x8f6a35c0268) at wallet/test/run-wallet.c:1104
#8 0x000008f44bcddec0 in main (argc=1, argv=0x7f7ffffcaaf8) at wallet/test/run-wallet.c:1930
Fix by explicitely setting the whole structure to zero.
[ Rebuilt generated files, too --RR ]
ksedgwic
pushed a commit
that referenced
this pull request
Mar 3, 2023
This will fix a crash that I caused on armv7 and by looking inside the coredump with gdb (by adding an assert on n that must be different from null) I get the following stacktrace ``` (gdb) bt \#0 0x00000000 in ?? () \#1 0x0043a038 in send_backtrace (why=0xbe9e3600 "FATAL SIGNAL 11") at common/daemon.c:36 \#2 0x0043a0ec in crashdump (sig=11) at common/daemon.c:46 \#3 <signal handler called> \#4 0x00406d04 in node_announcement (map=0x938ecc, nann_off=495146) at common/gossmap.c:586 \#5 0x00406fec in map_catchup (map=0x938ecc, num_rejected=0xbe9e3a40) at common/gossmap.c:643 \#6 0x004073a4 in load_gossip_store (map=0x938ecc, num_rejected=0xbe9e3a40) at common/gossmap.c:697 \#7 0x00408244 in gossmap_load (ctx=0x0, filename=0x4e16b8 "gossip_store", num_channel_updates_rejected=0xbe9e3a40) at common/gossmap.c:976 \#8 0x0041a548 in init (p=0x93831c, buf=0x9399d4 "\n\n{\"jsonrpc\":\"2.0\",\"id\":\"cln:init#25\",\"method\":\"init\",\"params\":{\"options\":{},\"configuration\":{\"lightning-dir\":\"/home/vincent/.lightning/testnet\",\"rpc-file\":\"lightning-rpc\",\"startup\":true,\"network\":\"te"..., config=0x939cdc) at plugins/topology.c:622 \#9 0x0041e5d0 in handle_init (cmd=0x938934, buf=0x9399d4 "\n\n{\"jsonrpc\":\"2.0\",\"id\":\"cln:init#25\",\"method\":\"init\",\"params\":{\"options\":{},\"configuration\":{\"lightning-dir\":\"/home/vincent/.lightning/testnet\",\"rpc-file\":\"lightning-rpc\",\"startup\":true,\"network\":\"te"..., params=0x939c8c) at plugins/libplugin.c:1208 \#10 0x0041fc04 in ld_command_handle (plugin=0x93831c, toks=0x939bec) at plugins/libplugin.c:1572 \#11 0x00420050 in ld_read_json_one (plugin=0x93831c) at plugins/libplugin.c:1667 \#12 0x004201bc in ld_read_json (conn=0x9391c4, plugin=0x93831c) at plugins/libplugin.c:1687 \#13 0x004cb82c in next_plan (conn=0x9391c4, plan=0x9391d8) at ccan/ccan/io/io.c:59 \#14 0x004cc67c in do_plan (conn=0x9391c4, plan=0x9391d8, idle_on_epipe=false) at ccan/ccan/io/io.c:407 \#15 0x004cc6dc in io_ready (conn=0x9391c4, pollflags=1) at ccan/ccan/io/io.c:417 \#16 0x004cf8cc in io_loop (timers=0x9383c4, expired=0xbe9e3ce4) at ccan/ccan/io/poll.c:453 \#17 0x00420af4 in plugin_main (argv=0xbe9e3eb4, init=0x41a46c <init>, restartability=PLUGIN_STATIC, init_rpc=true, features=0x0, commands=0x6167e8 <commands>, num_commands=4, notif_subs=0x0, num_notif_subs=0, hook_subs=0x0, num_hook_subs=0, notif_topics=0x0, num_notif_topics=0) at plugins/libplugin.c:1891 \#18 0x0041a6f8 in main (argc=1, argv=0xbe9e3eb4) at plugins/topology.c:679 ``` I do not know if this is a solution because I do not know when I can parse a node announcement for a node that it is not longer in the gossip map. So, I hope this is just usefult for @rustyrussell Changelog-Fixed: fixes `FATAL SIGNAL 11` on gossmap node announcement parsing. Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
ksedgwic
pushed a commit
that referenced
this pull request
Apr 21, 2023
The issue is that common_setup() wasn't called by the fuzz target,
leaving secp256k1_ctx as NULL.
UBSan error:
$ UBSAN_OPTIONS="print_stacktrace=1:halt_on_error=1" \
./fuzz-channel_id crash-1575b41ef09e62e4c09c165e6dc037a110b113f2
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 1153355603
INFO: Loaded 1 modules (25915 inline 8-bit counters): 25915 [0x563bae7ac3a8, 0x563bae7b28e3),
INFO: Loaded 1 PC tables (25915 PCs): 25915 [0x563bae7b28e8,0x563bae817c98),
./fuzz-channel_id: Running 1 inputs 1 time(s) each.
Running: crash-1575b41ef09e62e4c09c165e6dc037a110b113f2
bitcoin/pubkey.c:22:33: runtime error: null pointer passed as argument 1, which is declared to never be null
external/libwally-core/src/secp256k1/include/secp256k1.h:373:3: note: nonnull attribute specified here
#0 0x563bae41e3db in pubkey_from_der bitcoin/pubkey.c:19:7
#1 0x563bae4205e0 in fromwire_pubkey bitcoin/pubkey.c:111:7
#2 0x563bae46437c in run tests/fuzz/fuzz-channel_id.c:42:3
#3 0x563bae2f6016 in LLVMFuzzerTestOneInput tests/fuzz/libfuzz.c:23:2
#4 0x563bae20a450 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long)
#5 0x563bae1f4c3f in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long)
#6 0x563bae1fa6e6 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long))
#7 0x563bae223052 in main (tests/fuzz/fuzz-channel_id+0x181052) (BuildId: f7f56e14ffc06df54ab732d79ea922e773de1f25)
#8 0x7fa7fa113082 in __libc_start_main
#9 0x563bae1efbdd in _start
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior bitcoin/pubkey.c:22:33 in
ksedgwic
pushed a commit
that referenced
this pull request
Aug 3, 2023
It is possible for db_column_bytes() to return 0 and for db_column_blob() to return NULL even when db_column_is_null() returns false. We need to short circuit in this case. Detected by UBSan: db/bindings.c:479:12: runtime error: null pointer passed as argument 2, which is declared to never be null /usr/include/string.h:44:28: note: nonnull attribute specified here #0 0x95f117 in db_col_arr_ db/bindings.c:479:2 #1 0x95ef85 in db_col_channel_type db/bindings.c:459:32 #2 0x852c03 in wallet_stmt2channel wallet/wallet.c:1483:9 #3 0x81f396 in wallet_channels_load_active wallet/wallet.c:1749:23 #4 0x81f03d in wallet_init_channels wallet/wallet.c:1765:9 #5 0x72f1f9 in load_channels_from_wallet lightningd/peer_control.c:2257:7 #6 0x672856 in main lightningd/lightningd.c:1121:25
ksedgwic
pushed a commit
that referenced
this pull request
Aug 3, 2023
Fixes nullability errors detected by UBSan:
wire/fromwire.c:173:46: runtime error: null pointer passed as argument 1, which is declared to never be null
external/libwally-core/src/secp256k1/include/secp256k1.h:432:3: note: nonnull attribute specified here
#0 0x65214a in fromwire_secp256k1_ecdsa_signature wire/fromwire.c:173:6
#1 0x659500 in printwire_secp256k1_ecdsa_signature devtools/print_wire.c:331:1
#2 0x646ba2 in printwire_channel_update wire/peer_printgen.c:1900:7
#3 0x637182 in printpeer_wire_message wire/peer_printgen.c:128:11
#4 0x65a097 in main devtools/decodemsg.c:85:10
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds an alternate implementation of c-lightning's hsmd daemon. The private keys and secrets are sequestered in an external program, the Remote Lightning Signer. This remote-hsmd daemon proxies all signing requests to the remote daemon.
Please see the Lightning Signer Overview for more information.