lightningd/hsm_control: use a simple fd for HSM.

Now we're always sync, just use an fd.  Put the hsm_sync_read() helper
here, too, and do HSM init sync which makes things much simpler.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

lightningd/hsm_control.c

@@ -6,16 +6,51 @@
 #include <ccan/io/io.h>
 #include <ccan/take/take.h>
 #include <daemon/log.h>
+#include <errno.h>
 #include <inttypes.h>
 #include <lightningd/hsm/gen_hsm_wire.h>
+#include <lightningd/status.h>
+#include <string.h>
 #include <wally_bip32.h>
+#include <wire/wire_sync.h>
 
-static bool hsm_init_done(struct subd *hsm, const u8 *msg, const int *fds,
-			  struct lightningd *ld)
+u8 *hsm_sync_read(const tal_t *ctx, struct lightningd *ld)
 {
-	u8 *serialized_extkey;
+	for (;;) {
+		u8 *msg = wire_sync_read(ctx, ld->hsm_fd);
+		if (!msg)
+			fatal("Could not write from HSM: %s", strerror(errno));
+		if (fromwire_peektype(msg) != STATUS_TRACE)
+			return msg;
 
-	if (!fromwire_hsmctl_init_reply(hsm, msg, NULL, &ld->dstate.id,
+		log_debug(ld->log, "HSM TRACE: %.*s",
+			  (int)(tal_len(msg) - sizeof(be16)),
+			  (char *)msg + sizeof(be16));
+		tal_free(msg);
+	}
+}
+
+void hsm_init(struct lightningd *ld, bool newdir)
+{
+	const tal_t *tmpctx = tal_tmpctx(ld);
+	u8 *msg, *serialized_extkey;
+	bool create;
+
+	ld->hsm_fd = subd_raw(ld, "lightningd_hsm");
+	if (ld->hsm_fd < 0)
+		err(1, "Could not subd hsm");
+
+	if (newdir)
+		create = true;
+	else
+		create = (access("hsm_secret", F_OK) != 0);
+
+	if (!wire_sync_write(ld->hsm_fd, towire_hsmctl_init(tmpctx, create)))
+		err(1, "Writing init msg to hsm");
+
+	msg = hsm_sync_read(tmpctx, ld);
+	if (!fromwire_hsmctl_init_reply(tmpctx, msg, NULL,
+					&ld->dstate.id,
 					&ld->peer_seed,
 					&serialized_extkey))
 		errx(1, "HSM did not give init reply");
@@ -25,88 +60,5 @@ static bool hsm_init_done(struct subd *hsm, const u8 *msg, const int *fds,
 	if (bip32_key_unserialize(serialized_extkey, tal_len(serialized_extkey),
 				  ld->bip32_base) != WALLY_OK)
 		errx(1, "HSM did not give unserializable BIP32 extkey");
-
 	ld->wallet->bip32_base = ld->bip32_base;
-	io_break(ld->hsm);
-	return true;
 }
-
-static void hsm_finished(struct subd *hsm, int status)
-{
-	if (WIFEXITED(status))
-		errx(1, "HSM failed (exit status %i), exiting.",
-		     WEXITSTATUS(status));
-	errx(1, "HSM failed (signal %u), exiting.", WTERMSIG(status));
-}
-
-static int hsm_msg(struct subd *hsm, const u8 *msg, const int *fds)
-{
-	enum hsm_wire_type t = fromwire_peektype(msg);
-	u8 *badmsg;
-	struct peer *peer;
-	u64 id;
-
-	switch (t) {
-	case WIRE_HSMSTATUS_CLIENT_BAD_REQUEST:
-		if (!fromwire_hsmstatus_client_bad_request(msg, msg, NULL,
-							   &id, &badmsg))
-			errx(1, "HSM bad status %s", tal_hex(msg, msg));
-		peer = peer_by_unique_id(hsm->ld, id);
-
-		/* "Shouldn't happen" */
-		errx(1, "HSM says bad cmd from %"PRIu64" (%s): %s",
-		     id,
-		     peer ? type_to_string(msg, struct pubkey, &peer->id)
-		     : "unknown peer",
-		     tal_hex(msg, badmsg));
-
-	/* subd already logs fatal errors. */
-	case WIRE_HSMSTATUS_INIT_FAILED:
-	case WIRE_HSMSTATUS_WRITEMSG_FAILED:
-	case WIRE_HSMSTATUS_BAD_REQUEST:
-	case WIRE_HSMSTATUS_FD_FAILED:
-	case WIRE_HSMSTATUS_KEY_FAILED:
-		break;
-
-	/* HSM doesn't send these */
-	case WIRE_HSMCTL_INIT:
-	case WIRE_HSMCTL_HSMFD_ECDH:
-	case WIRE_HSMCTL_HSMFD_CHANNELD:
-	case WIRE_HSMCTL_SIGN_FUNDING:
-	case WIRE_HSMCTL_SIGN_WITHDRAWAL:
-	case WIRE_HSMCTL_NODE_ANNOUNCEMENT_SIG_REQ:
-
-	/* Replies should be paired to individual requests. */
-	case WIRE_HSMCTL_INIT_REPLY:
-	case WIRE_HSMCTL_HSMFD_CHANNELD_REPLY:
-	case WIRE_HSMCTL_HSMFD_ECDH_FD_REPLY:
-	case WIRE_HSMCTL_SIGN_FUNDING_REPLY:
-	case WIRE_HSMCTL_SIGN_WITHDRAWAL_REPLY:
-	case WIRE_HSMCTL_NODE_ANNOUNCEMENT_SIG_REPLY:
-		errx(1, "HSM gave invalid message %s", hsm_wire_type_name(t));
-	}
-	return 0;
-}
-
-void hsm_init(struct lightningd *ld, bool newdir)
-{
-	bool create;
-
-	ld->hsm = new_subd(ld, ld, "lightningd_hsm", NULL,
-			   hsm_wire_type_name,
-			   hsm_msg, hsm_finished, NULL);
-	if (!ld->hsm)
-		err(1, "Could not subd hsm");
-
-	if (newdir)
-		create = true;
-	else
-		create = (access("hsm_secret", F_OK) != 0);
-
-	subd_req(ld->hsm, ld->hsm, take(towire_hsmctl_init(ld->hsm, create)),
-		 -1, 0, hsm_init_done, ld);
-
-	if (io_loop(NULL, NULL) != ld->hsm)
-		errx(1, "Unexpected io exit during HSM startup");
-}
-

lightningd/hsm_control.h

@@ -1,9 +1,12 @@
 #ifndef LIGHTNING_LIGHTNINGD_HSM_CONTROL_H
 #define LIGHTNING_LIGHTNINGD_HSM_CONTROL_H
 #include "config.h"
+#include <ccan/short_types/short_types.h>
+#include <ccan/tal/tal.h>
 #include <stdbool.h>
 
 struct lightningd;
 
+u8 *hsm_sync_read(const tal_t *ctx, struct lightningd *ld);
 void hsm_init(struct lightningd *ld, bool newdir);
 #endif /* LIGHTNING_LIGHTNINGD_HSM_CONTROL_H */

lightningd/lightningd.c

@@ -204,7 +204,7 @@ static void shutdown_subdaemons(struct lightningd *ld)
 	struct peer *p;
 
 	/* Let everyone shutdown cleanly. */
-	subd_shutdown(ld->hsm, 10);
+	close(ld->hsm_fd);
 	subd_shutdown(ld->gossip, 10);
 
 	/* Duplicates are OK: no need to check here. */

lightningd/lightningd.h

@@ -26,7 +26,7 @@ struct lightningd {
 	struct log *log;
 
 	/* Bearer of all my secrets. */
-	struct subd *hsm;
+	int hsm_fd;
 
 	/* Daemon looking after peers during init / before channel. */
 	struct subd *gossip;

lightningd/new_connection.c

@@ -7,6 +7,7 @@
 #include <lightningd/cryptomsg.h>
 #include <lightningd/handshake/gen_handshake_wire.h>
 #include <lightningd/hsm/gen_hsm_wire.h>
+#include <lightningd/hsm_control.h>
 #include <lightningd/lightningd.h>
 #include <lightningd/new_connection.h>
 #include <lightningd/peer_control.h>
@@ -52,23 +53,6 @@ static void set_blocking(int fd, bool block)
 	fcntl(fd, F_SETFL, flags);
 }
 
-
-static u8 *hsm_sync_read(const tal_t *ctx, struct lightningd *ld)
-{
-	for (;;) {
-		u8 *msg = wire_sync_read(ctx, io_conn_fd(ld->hsm->conn));
-		if (!msg)
-			fatal("Could not write from HSM: %s", strerror(errno));
-		if (fromwire_peektype(msg) != STATUS_TRACE)
-			return msg;
-
-		log_debug(ld->hsm->log, "TRACE: %.*s",
-			  (int)(tal_len(msg) - sizeof(be16)),
-			  (char *)msg + sizeof(be16));
-		tal_free(msg);
-	}
-}
-
 static void
 PRINTF_FMT(3,4) connection_failed(struct connection *c, struct log *log,
 				  const char *fmt, ...)
@@ -205,21 +189,17 @@ static struct io_plan *hsm_then_handshake(struct io_conn *conn,
 	u8 *msg;
 
 	/* Get HSM fd for this peer. */
-	/* FIXME: don't use hsm->conn */
-	set_blocking(io_conn_fd(ld->hsm->conn), true);
 	msg = towire_hsmctl_hsmfd_ecdh(tmpctx, c->unique_id);
-
-	if (!wire_sync_write(io_conn_fd(ld->hsm->conn), msg))
+	if (!wire_sync_write(ld->hsm_fd, msg))
 		fatal("Could not write to HSM: %s", strerror(errno));
 
 	msg = hsm_sync_read(tmpctx, ld);
 	if (!fromwire_hsmctl_hsmfd_ecdh_fd_reply(msg, NULL))
 		fatal("Malformed hsmfd response: %s", tal_hex(msg, msg));
 
-	hsmfd = fdpass_recv(io_conn_fd(ld->hsm->conn));
+	hsmfd = fdpass_recv(ld->hsm_fd);
 	if (hsmfd < 0)
 		fatal("Could not read fd from HSM: %s", strerror(errno));
-	set_blocking(io_conn_fd(ld->hsm->conn), false);
 
 	/* Make sure connection fd is blocking */
 	set_blocking(connfd, true);

lightningd/peer_control.c

@@ -24,6 +24,7 @@
 #include <lightningd/gen_peer_state_names.h>
 #include <lightningd/gossip/gen_gossip_wire.h>
 #include <lightningd/hsm/gen_hsm_wire.h>
+#include <lightningd/hsm_control.h>
 #include <lightningd/key_derive.h>
 #include <lightningd/new_connection.h>
 #include <lightningd/opening/gen_opening_wire.h>
@@ -39,18 +40,6 @@
 #include <wire/gen_peer_wire.h>
 #include <wire/wire_sync.h>
 
-static void set_blocking(int fd, bool block)
-{
-	int flags = fcntl(fd, F_GETFL);
-
-	if (block)
-		flags &= ~O_NONBLOCK;
-	else
-		flags |= O_NONBLOCK;
-
-	fcntl(fd, F_SETFL, flags);
-}
-
 static void destroy_peer(struct peer *peer)
 {
 	list_del_from(&peer->ld->peers, &peer->list);
@@ -58,22 +47,6 @@ static void destroy_peer(struct peer *peer)
 		close(peer->gossip_client_fd);
 }
 
-static u8 *hsm_sync_read(const tal_t *ctx, struct lightningd *ld)
-{
-	for (;;) {
-		u8 *msg = wire_sync_read(ctx, io_conn_fd(ld->hsm->conn));
-		if (!msg)
-			fatal("Could not write from HSM: %s", strerror(errno));
-		if (fromwire_peektype(msg) != STATUS_TRACE)
-			return msg;
-
-		log_debug(ld->hsm->log, "TRACE: %.*s",
-			  (int)(tal_len(msg) - sizeof(be16)),
-			  (char *)msg + sizeof(be16));
-		tal_free(msg);
-	}
-}
-
 /* Mutual recursion, sets timer. */
 static void peer_reconnect(struct peer *peer);
 
@@ -861,14 +834,10 @@ static int peer_channel_announced(struct peer *peer, const u8 *msg)
 	msg = towire_hsmctl_node_announcement_sig_req(
 		tmpctx, create_node_announcement(tmpctx, ld, NULL));
 
-	/* FIXME: don't use hsm->conn */
-	set_blocking(io_conn_fd(ld->hsm->conn), true);
-	if (!wire_sync_write(io_conn_fd(ld->hsm->conn), take(msg)))
+	if (!wire_sync_write(ld->hsm_fd, take(msg)))
 		fatal("Could not write to HSM: %s", strerror(errno));
 
 	msg = hsm_sync_read(tmpctx, ld);
-	set_blocking(io_conn_fd(ld->hsm->conn), false);
-
 	if (!fromwire_hsmctl_node_announcement_sig_reply(msg, NULL, &sig))
 		fatal("HSM returned an invalid node_announcement sig");
 
@@ -998,21 +967,17 @@ static bool peer_start_channeld(struct peer *peer,
 	else
 		*peer->balance = peer->push_msat;
 
-	/* FIXME: don't use hsm->conn */
-	set_blocking(io_conn_fd(peer->ld->hsm->conn), true);
 	msg = towire_hsmctl_hsmfd_channeld(tmpctx, peer->unique_id);
-
-	if (!wire_sync_write(io_conn_fd(peer->ld->hsm->conn), take(msg)))
+	if (!wire_sync_write(peer->ld->hsm_fd, take(msg)))
 		fatal("Could not write to HSM: %s", strerror(errno));
 
 	msg = hsm_sync_read(tmpctx, peer->ld);
 	if (!fromwire_hsmctl_hsmfd_channeld_reply(msg, NULL))
 		fatal("Bad reply from HSM: %s", tal_hex(tmpctx, msg));
 
-	hsmfd = fdpass_recv(io_conn_fd(peer->ld->hsm->conn));
+	hsmfd = fdpass_recv(peer->ld->hsm_fd);
 	if (hsmfd < 0)
 		fatal("Could not read fd from HSM: %s", strerror(errno));
-	set_blocking(io_conn_fd(peer->ld->hsm->conn), false);
 
 	peer->owner = new_subd(peer->ld, peer->ld,
 			       "lightningd_channel", peer,
@@ -1182,15 +1147,10 @@ static bool opening_funder_finished(struct subd *opening, const u8 *resp,
 
 	fc->peer->owner = NULL;
 
-	/* FIXME: don't use hsm->conn */
-	set_blocking(io_conn_fd(fc->peer->ld->hsm->conn), true);
-	if (!wire_sync_write(io_conn_fd(fc->peer->ld->hsm->conn),
-			     take(msg)))
+	if (!wire_sync_write(fc->peer->ld->hsm_fd, take(msg)))
 		fatal("Could not write to HSM: %s", strerror(errno));
 
 	msg = hsm_sync_read(fc, fc->peer->ld);
-	set_blocking(io_conn_fd(fc->peer->ld->hsm->conn), false);
-
 	opening_got_hsm_funding_sig(fc, fds[0], msg, &cs);
 
 	/* Tell opening daemon to exit. */

wallet/walletrpc.c

@@ -6,8 +6,8 @@
 #include <daemon/chaintopology.h>
 #include <daemon/jsonrpc.h>
 #include <errno.h>
-#include <fcntl.h>
 #include <lightningd/hsm/gen_hsm_wire.h>
+#include <lightningd/hsm_control.h>
 #include <lightningd/key_derive.h>
 #include <lightningd/lightningd.h>
 #include <lightningd/status.h>
@@ -15,7 +15,6 @@
 #include <lightningd/utxo.h>
 #include <lightningd/withdraw_tx.h>
 #include <permute_tx.h>
-#include <unistd.h>
 #include <wally_bip32.h>
 #include <wire/wire_sync.h>
 
@@ -28,34 +27,6 @@ struct withdrawal {
 	const char *hextx;
 };
 
-static void set_blocking(int fd, bool block)
-{
-	int flags = fcntl(fd, F_GETFL);
-
-	if (block)
-		flags &= ~O_NONBLOCK;
-	else
-		flags |= O_NONBLOCK;
-
-	fcntl(fd, F_SETFL, flags);
-}
-
-static u8 *hsm_sync_read(const tal_t *ctx, struct lightningd *ld)
-{
-	for (;;) {
-		u8 *msg = wire_sync_read(ctx, io_conn_fd(ld->hsm->conn));
-		if (!msg)
-			fatal("Could not write from HSM: %s", strerror(errno));
-		if (fromwire_peektype(msg) != STATUS_TRACE)
-			return msg;
-
-		log_debug(ld->hsm->log, "TRACE: %.*s",
-			  (int)(tal_len(msg) - sizeof(be16)),
-			  (char *)msg + sizeof(be16));
-		tal_free(msg);
-	}
-}
-
 /**
  * wallet_extract_owned_outputs - given a tx, extract all of our outputs
  */
@@ -204,14 +175,11 @@ static void json_withdraw(struct command *cmd,
 						utxos);
 	tal_free(utxos);
 
-	/* FIXME: don't use hsm->conn */
-	set_blocking(io_conn_fd(ld->hsm->conn), true);
-	if (!wire_sync_write(io_conn_fd(ld->hsm->conn), take(msg)))
+	if (!wire_sync_write(ld->hsm_fd, take(msg)))
 		fatal("Could not write sign_withdrawal to HSM: %s",
 		      strerror(errno));
 
 	msg = hsm_sync_read(cmd, ld);
-	set_blocking(io_conn_fd(ld->hsm->conn), false);
 
 	if (!fromwire_hsmctl_sign_withdrawal_reply(withdraw, msg, NULL, &sigs))
 		fatal("HSM gave bad sign_withdrawal_reply %s",