Skip to content

Commit 5bce509

Browse files
authored
Make node_id, bip32 xpub, and bolt12 pubkey persistent (#23)
* Persist node_id, bip32 xpub, and bolt12 pubkey * cleaned up the remotesigner skip predicates * addressed review comments: rename, let remote signer generate seed
1 parent b1ec803 commit 5bce509

File tree

10 files changed

+161
-66
lines changed

10 files changed

+161
-66
lines changed

contrib/remote_hsmd/Makefile

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ PROTOS_PATH = contrib/remote_hsmd
3434
RMTHSMD_COMMON_OBJS := \
3535
hsmd/hsmd_wiregen.o \
3636
common/amount.o \
37+
common/base32.o \
3738
common/bigsize.o \
3839
common/bip32.o \
3940
common/channel_id.o \
@@ -42,6 +43,9 @@ RMTHSMD_COMMON_OBJS := \
4243
common/derive_basepoints.o \
4344
common/status_wiregen.o \
4445
common/hash_u5.o \
46+
common/json.o \
47+
common/json_helpers.o \
48+
common/json_stream.o \
4549
common/key_derive.o \
4650
common/memleak.o \
4751
common/msg_queue.o \
@@ -55,7 +59,8 @@ RMTHSMD_COMMON_OBJS := \
5559
common/type_to_string.o \
5660
common/utils.o \
5761
common/utxo.o \
58-
common/version.o
62+
common/version.o \
63+
common/wireaddr.o
5964

6065
# For checking
6166
LIGHTNINGD_RMTHSM_ALLSRC_NOGEN := $(filter-out contrib/remote_hsmd/remotesigner.%, $(LIGHTNINGD_RMTHSM_SRC) $(LIGHTNINGD_RMTHSM_SRC))

contrib/remote_hsmd/hsmd.c

Lines changed: 113 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,19 @@
2020
#include <ccan/intmap/intmap.h>
2121
#include <ccan/io/fdpass/fdpass.h>
2222
#include <ccan/io/io.h>
23+
#include <ccan/json_out/json_out.h>
2324
#include <ccan/noerr/noerr.h>
2425
#include <ccan/ptrint/ptrint.h>
2526
#include <ccan/read_write_all/read_write_all.h>
27+
#include <ccan/str/hex/hex.h>
2628
#include <ccan/take/take.h>
29+
#include <ccan/tal/grab_file/grab_file.h>
2730
#include <ccan/tal/str/str.h>
2831
#include <common/daemon_conn.h>
2932
#include <common/derive_basepoints.h>
3033
#include <common/hash_u5.h>
34+
#include <common/json.h>
35+
#include <common/json_helpers.h>
3136
#include <common/key_derive.h>
3237
#include <common/memleak.h>
3338
#include <common/node_id.h>
@@ -391,6 +396,72 @@ static void workaround_init_bolt12(const struct secret *hsm_secret, struct pubke
391396
"Could derive bolt12 public key.");
392397
}
393398

399+
static void persist_node_id(const struct node_id *node_id,
400+
const struct ext_key *bip32,
401+
const struct pubkey32 *bolt12)
402+
{
403+
struct json_out *jout = json_out_new(tmpctx);
404+
json_out_start(jout, NULL, '{');
405+
406+
json_out_addstr(jout, "nodeid", type_to_string(jout, struct node_id, node_id));
407+
408+
char *xpub;
409+
tal_wally_start();
410+
int rv2 = bip32_key_to_base58(bip32, BIP32_FLAG_KEY_PUBLIC, &xpub);
411+
tal_wally_end(NULL);
412+
assert(rv2 == WALLY_OK);
413+
json_out_addstr(jout, "xpub", xpub);
414+
wally_free_string(xpub);
415+
416+
json_out_addstr(jout, "bolt12", type_to_string(jout, struct pubkey32, bolt12));
417+
418+
json_out_end(jout, '}');
419+
size_t len;
420+
const char *p = json_out_contents(jout, &len);
421+
422+
int fd = open("NODE_ID", O_WRONLY|O_TRUNC|O_CREAT, 0666);
423+
assert(fd != -1);
424+
write_all(fd, p, len);
425+
json_out_consume(jout, len);
426+
close(fd);
427+
}
428+
429+
static bool restore_node_id(struct node_id *node_id,
430+
struct ext_key *bip32,
431+
struct pubkey32 *bolt12)
432+
{
433+
if (access("NODE_ID", F_OK) == -1) {
434+
// This is a cold start, we don't have this yet.
435+
return false;
436+
}
437+
438+
// This is a warmstart, initialize our node_id.
439+
char *buffer = grab_file(tmpctx, "NODE_ID");
440+
const jsmntok_t *toks = json_parse_simple(buffer, buffer, strlen(buffer));
441+
const jsmntok_t *nodeidtok = json_get_member(buffer, toks, "nodeid");
442+
const jsmntok_t *xpubtok = json_get_member(buffer, toks, "xpub");
443+
const jsmntok_t *bolt12tok = json_get_member(buffer, toks, "bolt12");
444+
assert(nodeidtok != NULL);
445+
assert(xpubtok != NULL);
446+
assert(bolt12tok != NULL);
447+
448+
if (!json_to_node_id(buffer, nodeidtok, node_id))
449+
abort();
450+
451+
buffer[xpubtok->end] = '\0'; // need to null-terminate xpub string
452+
if (bip32_key_from_base58(buffer + xpubtok->start, bip32) != WALLY_OK)
453+
abort();
454+
455+
u8 raw[32];
456+
if (!hex_decode(buffer + bolt12tok->start, bolt12tok->end - bolt12tok->start,
457+
raw, sizeof(raw)))
458+
abort();
459+
if (secp256k1_xonly_pubkey_parse(secp256k1_ctx, &bolt12->pubkey, raw) != 1)
460+
abort();
461+
462+
return true;
463+
}
464+
394465
/*~ This is the response to lightningd's HSM_INIT request, which is the first
395466
* thing it sends. */
396467
static struct io_plan *init_hsm(struct io_conn *conn,
@@ -405,6 +476,7 @@ static struct io_plan *init_hsm(struct io_conn *conn,
405476
struct sha256 *force_channel_secrets_shaseed;
406477
struct secret *hsm_encryption_key;
407478
struct secret hsm_secret;
479+
struct secret *use_hsm_secret;
408480
bool coldstart;
409481

410482
/* This must be lightningd. */
@@ -444,41 +516,51 @@ static struct io_plan *init_hsm(struct io_conn *conn,
444516
* will use */
445517
c->chainparams = chainparams;
446518

447-
/* To support integration tests we honor any seed provided
448-
* in the hsm_secret file (testnet only). Otherwise we
449-
* generate a random seed.
450-
*/
451-
if (!read_test_seed(&hsm_secret)) {
452-
randombytes_buf(&hsm_secret, sizeof(hsm_secret));
453-
}
454-
455519
/* Is this a warm start (restart) or a cold start (first time)? */
456-
coldstart = access("WARM", F_OK) == -1;
457-
458-
proxy_stat rv = proxy_init_hsm(&bip32_key_version, chainparams,
459-
coldstart, &hsm_secret,
460-
&node_id, &pubstuff.bip32);
461-
if (PROXY_PERMANENT(rv)) {
462-
status_failed(STATUS_FAIL_INTERNAL_ERROR,
463-
"proxy_%s failed: %s", __FUNCTION__,
464-
proxy_last_message());
465-
}
466-
else if (!PROXY_SUCCESS(rv)) {
467-
status_unusual("proxy_%s failed: %s", __FUNCTION__,
468-
proxy_last_message());
469-
return bad_req_fmt(conn, c, msg_in,
470-
"proxy_%s error: %s", __FUNCTION__,
471-
proxy_last_message());
472-
}
520+
if (restore_node_id(&node_id, &pubstuff.bip32, &bolt12)) {
521+
// This is a warm start.
522+
proxy_set_node_id(&node_id);
523+
} else {
524+
status_unusual("cold start, initializing the remote signer");
525+
526+
// This is a cold start, initialize the remote signer.
527+
528+
/* To support integration tests we honor any seed provided
529+
* in the hsm_secret file (testnet only). Otherwise we
530+
* generate a random seed.
531+
*/
532+
if (read_test_seed(&hsm_secret)) {
533+
// We are running integration tests and the secret has been forced.
534+
use_hsm_secret = &hsm_secret;
535+
} else {
536+
// We are not running integration tests, remote signer generates.
537+
use_hsm_secret = NULL;
538+
}
539+
540+
coldstart = true; // this can go away in the API.
541+
proxy_stat rv = proxy_init_hsm(&bip32_key_version, chainparams,
542+
coldstart, use_hsm_secret,
543+
&node_id, &pubstuff.bip32);
544+
if (PROXY_PERMANENT(rv)) {
545+
status_failed(STATUS_FAIL_INTERNAL_ERROR,
546+
"proxy_%s failed: %s", __FUNCTION__,
547+
proxy_last_message());
548+
}
549+
else if (!PROXY_SUCCESS(rv)) {
550+
status_unusual("proxy_%s failed: %s", __FUNCTION__,
551+
proxy_last_message());
552+
return bad_req_fmt(conn, c, msg_in,
553+
"proxy_%s error: %s", __FUNCTION__,
554+
proxy_last_message());
555+
}
473556

474-
/* Mark this node as already inited. */
475-
int fd = open("WARM", O_WRONLY|O_TRUNC|O_CREAT, 0666);
476-
assert(fd != -1);
477-
close(fd);
478557

479-
// TODO - add support for bolt12
480-
workaround_init_bolt12(&hsm_secret, &bolt12);
558+
// TODO - add support for bolt12
559+
workaround_init_bolt12(&hsm_secret, &bolt12);
481560

561+
/* Mark this node as already inited. */
562+
persist_node_id(&node_id, &pubstuff.bip32, &bolt12);
563+
}
482564
/* Now we can consider ourselves initialized, and we won't get
483565
* upset if we get a non-init message. */
484566
initialized = true;

contrib/remote_hsmd/proxy.cc

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,11 @@ void proxy_setup()
324324
last_message = "";
325325
}
326326

327+
void proxy_set_node_id(const struct node_id *node_id)
328+
{
329+
self_id = *node_id;
330+
}
331+
327332
proxy_stat proxy_init_hsm(struct bip32_key_version *bip32_key_version,
328333
struct chainparams const *chainparams,
329334
bool coldstart,
@@ -334,7 +339,7 @@ proxy_stat proxy_init_hsm(struct bip32_key_version *bip32_key_version,
334339
STATUS_DEBUG(
335340
"%s:%d %s { \"hsm_secret\":%s, \"coldstart\":%s }",
336341
__FILE__, __LINE__, __FUNCTION__,
337-
dump_secret(hsm_secret).c_str(),
342+
hsm_secret != NULL ? dump_secret(hsm_secret).c_str() : "\"\"",
338343
coldstart ? "true" : "false"
339344
);
340345

@@ -351,9 +356,9 @@ proxy_stat proxy_init_hsm(struct bip32_key_version *bip32_key_version,
351356

352357
req.set_coldstart(coldstart);
353358

354-
/* FIXME - Sending the secret instead of generating on
355-
* the remote. */
356-
marshal_bip32seed(hsm_secret, req.mutable_hsm_secret());
359+
// If we are running integration tests the secret will be forced.
360+
if (hsm_secret != NULL)
361+
marshal_bip32seed(hsm_secret, req.mutable_hsm_secret());
357362

358363
ClientContext context;
359364
InitReply rsp;

contrib/remote_hsmd/proxy.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ char const *proxy_last_message(void);
3737

3838
void proxy_setup(void);
3939

40+
void proxy_set_node_id(const struct node_id *node_id);
41+
4042
proxy_stat proxy_init_hsm(
4143
struct bip32_key_version *bip32_key_version,
4244
struct chainparams const *chainparams,

tests/test_connection.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@ def test_disconnect_fundee(node_factory):
340340

341341
@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need')
342342
@unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1")
343-
@unittest.skipIf(os.getenv('SUBDAEMON', 'xxx') == 'hsmd:remote_hsmd', "remote_hsmd doesn't support dual-funding yet")
343+
@unittest.skipIf(os.getenv('SUBDAEMON') == 'hsmd:remote_hsmd', "remote_hsmd doesn't support dual-funding yet")
344344
def test_disconnect_fundee_v2(node_factory):
345345
# Now error on fundee side during channel open, with them funding
346346
disconnects = ['-WIRE_ACCEPT_CHANNEL2',
@@ -536,6 +536,7 @@ def test_reconnect_no_update(node_factory, executor, bitcoind):
536536
l1.daemon.wait_for_log(r"CLOSINGD_COMPLETE")
537537

538538

539+
@unittest.skipIf(os.getenv('SUBDAEMON') == 'hsmd:remote_hsmd', "remote_hsmd doesn't like hsm_secret shenanigans")
539540
def test_connect_stresstest(node_factory, executor):
540541
# This test is unreliable, but it's better than nothing.
541542
l1, l2, l3 = node_factory.get_nodes(3, opts={'may_reconnect': True})
@@ -952,7 +953,7 @@ def test_funding_toolarge(node_factory, bitcoind):
952953

953954

954955
@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need')
955-
@unittest.skipIf(os.getenv('SUBDAEMON', 'xxx') == 'hsmd:remote_hsmd', "remote_hsmd doesn't support dual-funding yet")
956+
@unittest.skipIf(os.getenv('SUBDAEMON') == 'hsmd:remote_hsmd', "remote_hsmd doesn't support dual-funding yet")
956957
def test_v2_open(node_factory, bitcoind, chainparams):
957958
l1, l2 = node_factory.get_nodes(2,
958959
opts=[{'experimental-dual-fund': None},
@@ -1400,7 +1401,7 @@ def test_funding_external_wallet(node_factory, bitcoind):
14001401

14011402

14021403
@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need')
1403-
@unittest.skipIf(os.getenv('SUBDAEMON', 'xxx') == 'hsmd:remote_hsmd', "remote_hsmd doesn't support dual-funding yet")
1404+
@unittest.skipIf(os.getenv('SUBDAEMON') == 'hsmd:remote_hsmd', "remote_hsmd doesn't support dual-funding yet")
14041405
def test_multifunding_v2_v1_mixed(node_factory, bitcoind):
14051406
'''
14061407
Simple test for multifundchannel, using v1 + v2
@@ -1439,7 +1440,7 @@ def test_multifunding_v2_v1_mixed(node_factory, bitcoind):
14391440

14401441

14411442
@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need')
1442-
@unittest.skipIf(os.getenv('SUBDAEMON', 'xxx') == 'hsmd:remote_hsmd', "remote_hsmd doesn't support dual-funding yet")
1443+
@unittest.skipIf(os.getenv('SUBDAEMON') == 'hsmd:remote_hsmd', "remote_hsmd doesn't support dual-funding yet")
14431444
def test_multifunding_v2_exclusive(node_factory, bitcoind):
14441445
'''
14451446
Simple test for multifundchannel, using v2
@@ -2812,7 +2813,7 @@ def test_fail_unconfirmed(node_factory, bitcoind, executor):
28122813

28132814
@unittest.skipIf(not DEVELOPER, "need dev-disconnect")
28142815
@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need')
2815-
@unittest.skipIf(os.getenv('SUBDAEMON', 'xxx') == 'hsmd:remote_hsmd', "remote_hsmd doesn't support dual-funding yet")
2816+
@unittest.skipIf(os.getenv('SUBDAEMON') == 'hsmd:remote_hsmd', "remote_hsmd doesn't support dual-funding yet")
28162817
def test_fail_unconfirmed_openchannel2(node_factory, bitcoind, executor):
28172818
"""Test that if we crash with an unconfirmed connection to a known
28182819
peer, we don't have a dangling peer in db"""
@@ -3095,7 +3096,7 @@ def test_channel_features(node_factory, bitcoind):
30953096

30963097

30973098
@unittest.skipIf(not DEVELOPER, "need dev-force-features")
3098-
@unittest.skipIf(os.getenv('SUBDAEMON', 'xxx') == 'hsmd:remote_hsmd', "remote_hsmd doesn't support non-option_static_remotekey")
3099+
@unittest.skipIf(os.getenv('SUBDAEMON') == 'hsmd:remote_hsmd', "remote_hsmd doesn't support non-option_static_remotekey")
30993100
def test_nonstatic_channel(node_factory, bitcoind):
31003101
"""Smoke test for a channel without option_static_remotekey"""
31013102
l1, l2 = node_factory.line_graph(2,

tests/test_db.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ def test_max_channel_id(node_factory, bitcoind):
133133
@unittest.skipIf(not COMPAT, "needs COMPAT to convert obsolete db")
134134
@unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "This test is based on a sqlite3 snapshot")
135135
@unittest.skipIf(TEST_NETWORK != 'regtest', "The network must match the DB snapshot")
136-
@unittest.skipIf(os.getenv('SUBDAEMON', 'xxx') == 'hsmd:remote_hsmd', "remote_hsmd doesn't like channel_nonce changing")
136+
@unittest.skipIf(os.getenv('SUBDAEMON') == 'hsmd:remote_hsmd', "remote_hsmd doesn't like channel_nonce changing")
137137
def test_scid_upgrade(node_factory, bitcoind):
138138
bitcoind.generate_block(1)
139139

@@ -147,7 +147,7 @@ def test_scid_upgrade(node_factory, bitcoind):
147147
@unittest.skipIf(not COMPAT, "needs COMPAT to convert obsolete db")
148148
@unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "This test is based on a sqlite3 snapshot")
149149
@unittest.skipIf(TEST_NETWORK != 'regtest', "The network must match the DB snapshot")
150-
@unittest.skipIf(os.getenv('SUBDAEMON', 'xxx') == 'hsmd:remote_hsmd', "remote_hsmd doesn't like channel_nonce changing")
150+
@unittest.skipIf(os.getenv('SUBDAEMON') == 'hsmd:remote_hsmd', "remote_hsmd doesn't like channel_nonce changing")
151151
def test_last_tx_psbt_upgrade(node_factory, bitcoind):
152152
bitcoind.generate_block(12)
153153

@@ -184,7 +184,7 @@ def test_last_tx_psbt_upgrade(node_factory, bitcoind):
184184

185185
@unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "This test is based on a sqlite3 snapshot")
186186
@unittest.skipIf(TEST_NETWORK != 'regtest', "The network must match the DB snapshot")
187-
@unittest.skipIf(os.getenv('SUBDAEMON', 'xxx') == 'hsmd:remote_hsmd', "remote_hsmd doesn't support backfill yet")
187+
@unittest.skipIf(os.getenv('SUBDAEMON') == 'hsmd:remote_hsmd', "remote_hsmd doesn't support backfill yet")
188188
def test_backfill_scriptpubkeys(node_factory, bitcoind):
189189
bitcoind.generate_block(214)
190190

@@ -311,7 +311,7 @@ def get_dsn(self):
311311
os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3',
312312
"This test is based on a sqlite3 snapshot"
313313
)
314-
@unittest.skipIf(os.getenv('SUBDAEMON', 'xxx') == 'hsmd:remote_hsmd', "remote_hsmd doesn't support DB migration")
314+
@unittest.skipIf(os.getenv('SUBDAEMON') == 'hsmd:remote_hsmd', "remote_hsmd doesn't support DB migration")
315315
def test_local_basepoints_cache(bitcoind, node_factory):
316316
"""XXX started caching the local basepoints as well as the remote ones.
317317

tests/test_misc.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1106,7 +1106,7 @@ def test_daemon_option(node_factory):
11061106

11071107
@flaky
11081108
@unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1")
1109-
@unittest.skipIf(os.getenv('SUBDAEMON', 'xxx') == 'hsmd:remote_hsmd', "remote_hsmd doesn't have repeatable random seeding")
1109+
@unittest.skipIf(os.getenv('SUBDAEMON') == 'hsmd:remote_hsmd', "remote_hsmd doesn't have repeatable random seeding")
11101110
def test_blockchaintrack(node_factory, bitcoind):
11111111
"""Check that we track the blockchain correctly across reorgs
11121112
"""
@@ -1809,7 +1809,7 @@ def mock_fail(*args):
18091809

18101810
@unittest.skipIf(not DEVELOPER, "needs --dev-force-bip32-seed")
18111811
@unittest.skipIf(TEST_NETWORK != 'regtest', "Addresses are network specific")
1812-
@unittest.skipIf(os.getenv('SUBDAEMON', 'xxx') == 'hsmd:remote_hsmd', "remote_hsmd doesn't support forced secrets")
1812+
@unittest.skipIf(os.getenv('SUBDAEMON') == 'hsmd:remote_hsmd', "remote_hsmd doesn't support forced secrets")
18131813
def test_dev_force_bip32_seed(node_factory):
18141814
l1 = node_factory.get_node(options={'dev-force-bip32-seed': '0000000000000000000000000000000000000000000000000000000000000001'})
18151815
# First is m/0/0/1 ..
@@ -2136,7 +2136,7 @@ def test_regtest_upgrade(node_factory):
21362136

21372137
@unittest.skipIf(VALGRIND, "valgrind files can't be written since we rmdir")
21382138
@unittest.skipIf(TEST_NETWORK != "regtest", "needs bitcoin mainnet")
2139-
@unittest.skipIf(os.getenv('SUBDAEMON', 'xxx') == 'hsmd:remote_hsmd', "remote_hsmd doesn't create hsm_secret file")
2139+
@unittest.skipIf(os.getenv('SUBDAEMON') == 'hsmd:remote_hsmd', "remote_hsmd doesn't create hsm_secret file")
21402140
def test_new_node_is_mainnet(node_factory):
21412141
"""Test that an empty directory causes us to be on mainnet"""
21422142
l1 = node_factory.get_node(start=False, may_fail=True)
@@ -2374,7 +2374,7 @@ def test_sendonionmessage_reply(node_factory):
23742374

23752375

23762376
@unittest.skipIf(not DEVELOPER, "needs --dev-force-privkey")
2377-
@unittest.skipIf(os.getenv('SUBDAEMON', 'xxx') == 'hsmd:remote_hsmd', "remote_hsmd doesn't support dev-force-privkey")
2377+
@unittest.skipIf(os.getenv('SUBDAEMON') == 'hsmd:remote_hsmd', "remote_hsmd doesn't support dev-force-privkey")
23782378
def test_getsharedsecret(node_factory):
23792379
"""
23802380
Test getsharedsecret command.

0 commit comments

Comments
 (0)