From 262e4c840fbb55b96f7dc499fe01dd72d8911925 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 23 Jan 2020 16:24:10 +1030 Subject: [PATCH] sphinx: use struct secret for shared secret. Generally I prefer structures over u8, since the size is enforced at runtime; and in several places we were doing conversions as the code using Sphinx does treat struct secret as type of the secret. Note that passing an array is the same as passing the address, so changing from 'u8 secret[32]' to 'struct secret secret' means various 'secret' parameters change to '&secret'. Technically, '&secret' also would have worked before, since '&' is a noop on array, but that's always seemed a bit weird. Signed-off-by: Rusty Russell --- channeld/channeld.c | 2 +- common/sphinx.c | 43 ++++++++++++++++++++-------------------- common/sphinx.h | 6 +++--- devtools/onion.c | 6 +++--- lightningd/peer_htlcs.c | 2 +- wallet/test/run-wallet.c | 2 +- 6 files changed, 31 insertions(+), 30 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index 4e6bf6f44..30ae01f84 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -602,7 +602,7 @@ static struct secret *get_shared_secret(const tal_t *ctx, /* We make sure we can parse onion packet, so we know if shared secret * is actually valid (this checks hmac). */ - rs = process_onionpacket(tmpctx, &op, secret->data, + rs = process_onionpacket(tmpctx, &op, secret, htlc->rhash.u.u8, sizeof(htlc->rhash)); if (!rs) { diff --git a/common/sphinx.c b/common/sphinx.c index 1aef8551a..aea8b6ae8 100644 --- a/common/sphinx.c +++ b/common/sphinx.c @@ -18,7 +18,6 @@ #include #define BLINDING_FACTOR_SIZE 32 -#define SHARED_SECRET_SIZE 32 #define KEY_LEN 32 #define NUM_STREAM_BYTES (2*ROUTING_INFO_SIZE) @@ -27,7 +26,7 @@ #define RHO_KEYTYPE "rho" struct hop_params { - u8 secret[SHARED_SECRET_SIZE]; + struct secret secret; u8 blind[BLINDING_FACTOR_SIZE]; struct pubkey ephemeralkey; }; @@ -211,9 +210,10 @@ static void compute_packet_hmac(const struct onionpacket *packet, memcpy(hmac, mac, HMAC_SIZE); } -static bool generate_key(void *k, const char *t, u8 tlen, const u8 *s) +static bool generate_key(void *k, const char *t, u8 tlen, + const struct secret *s) { - return compute_hmac(k, s, KEY_LEN, t, tlen); + return compute_hmac(k, s->data, KEY_LEN, t, tlen); } static bool generate_header_padding(void *dst, size_t dstlen, @@ -227,7 +227,7 @@ static bool generate_header_padding(void *dst, size_t dstlen, memset(dst, 0, dstlen); for (int i = 0; i < tal_count(path->hops) - 1; i++) { if (!generate_key(&key, RHO_KEYTYPE, strlen(RHO_KEYTYPE), - params[i].secret)) + ¶ms[i].secret)) return false; generate_cipher_stream(stream, key, sizeof(stream)); @@ -253,7 +253,7 @@ static bool generate_header_padding(void *dst, size_t dstlen, } static void compute_blinding_factor(const struct pubkey *key, - const u8 sharedsecret[SHARED_SECRET_SIZE], + const struct secret *sharedsecret, u8 res[BLINDING_FACTOR_SIZE]) { struct sha256_ctx ctx; @@ -263,7 +263,7 @@ static void compute_blinding_factor(const struct pubkey *key, pubkey_to_der(der, key); sha256_init(&ctx); sha256_update(&ctx, der, sizeof(der)); - sha256_update(&ctx, sharedsecret, SHARED_SECRET_SIZE); + sha256_update(&ctx, sharedsecret->data, sizeof(sharedsecret->data)); sha256_done(&ctx, &temp); memcpy(res, &temp, 32); } @@ -281,17 +281,18 @@ static bool blind_group_element(struct pubkey *blindedelement, return true; } -static bool create_shared_secret(u8 *secret, const struct pubkey *pubkey, +static bool create_shared_secret(struct secret *secret, + const struct pubkey *pubkey, const struct secret *session_key) { - if (secp256k1_ecdh(secp256k1_ctx, secret, &pubkey->pubkey, + if (secp256k1_ecdh(secp256k1_ctx, secret->data, &pubkey->pubkey, session_key->data, NULL, NULL) != 1) return false; return true; } bool onion_shared_secret( - u8 *secret, + struct secret *secret, const struct onionpacket *packet, const struct privkey *privkey) { @@ -299,7 +300,7 @@ bool onion_shared_secret( &privkey->secret); } -static void generate_key_set(const u8 secret[SHARED_SECRET_SIZE], +static void generate_key_set(const struct secret *secret, struct keyset *keys) { generate_key(keys->rho, "rho", 3, secret); @@ -324,12 +325,12 @@ static struct hop_params *generate_hop_params( path->session_key->data) != 1) return NULL; - if (!create_shared_secret(params[0].secret, &path->hops[0].pubkey, + if (!create_shared_secret(¶ms[0].secret, &path->hops[0].pubkey, path->session_key)) return NULL; compute_blinding_factor( - ¶ms[0].ephemeralkey, params[0].secret, + ¶ms[0].ephemeralkey, ¶ms[0].secret, params[0].blind); /* Recursively compute all following ephemeral public keys, @@ -367,7 +368,7 @@ static struct hop_params *generate_hop_params( compute_blinding_factor( ¶ms[i].ephemeralkey, - params[i].secret, params[i].blind); + ¶ms[i].secret, params[i].blind); } return params; } @@ -423,14 +424,14 @@ struct onionpacket *create_onionpacket( */ /* Note that this is just hop_payloads: the rest of the packet is * overwritten below or above anyway. */ - generate_key(padkey, "pad", 3, sp->session_key->data); + generate_key(padkey, "pad", 3, sp->session_key); generate_cipher_stream(stream, padkey, ROUTING_INFO_SIZE); generate_header_padding(filler, sizeof(filler), sp, params); for (i = num_hops - 1; i >= 0; i--) { memcpy(sp->hops[i].hmac, nexthmac, HMAC_SIZE); - generate_key_set(params[i].secret, &keys); + generate_key_set(¶ms[i].secret, &keys); generate_cipher_stream(stream, keys.rho, ROUTING_INFO_SIZE); /* Rightshift mix-header by FRAME_SIZE */ @@ -451,7 +452,7 @@ struct onionpacket *create_onionpacket( memcpy(&packet->ephemeralkey, ¶ms[0].ephemeralkey, sizeof(secp256k1_pubkey)); for (i=0; idata); + generate_key(key, "um", 2, shared_secret); compute_hmac(hmac, payload, tal_count(payload), key, KEY_LEN); reply->contents = tal_arr(reply, u8, 0), @@ -603,7 +604,7 @@ struct onionreply *wrap_onionreply(const tal_t *ctx, * * The obfuscation step is repeated by every hop along the return path. */ - generate_key(key, "ammag", 5, shared_secret->data); + generate_key(key, "ammag", 5, shared_secret); generate_cipher_stream(stream, key, streamlen); result->contents = tal_arr(result, u8, streamlen); xorbytes(result->contents, stream, reply->contents, streamlen); @@ -637,7 +638,7 @@ u8 *unwrap_onionreply(const tal_t *ctx, /* Check if the HMAC matches, this means that this is * the origin */ - generate_key(key, "um", 2, shared_secrets[i].data); + generate_key(key, "um", 2, &shared_secrets[i]); compute_hmac(hmac, r->contents + sizeof(hmac), tal_count(r->contents) - sizeof(hmac), key, KEY_LEN); diff --git a/common/sphinx.h b/common/sphinx.h index 94bbdd72a..1f687bd7b 100644 --- a/common/sphinx.h +++ b/common/sphinx.h @@ -107,12 +107,12 @@ struct onionpacket *create_onionpacket( /** * onion_shared_secret - calculate ECDH shared secret between nodes. * - * @secret: the shared secret (32 bytes long) + * @secret: the shared secret * @pubkey: the public key of the other node * @privkey: the private key of this node (32 bytes long) */ bool onion_shared_secret( - u8 *secret, + struct secret *secret, const struct onionpacket *packet, const struct privkey *privkey); @@ -130,7 +130,7 @@ bool onion_shared_secret( struct route_step *process_onionpacket( const tal_t * ctx, const struct onionpacket *packet, - const u8 *shared_secret, + const struct secret *shared_secret, const u8 *assocdata, const size_t assocdatalen ); diff --git a/devtools/onion.c b/devtools/onion.c index a4ef5d934..5ebca0578 100644 --- a/devtools/onion.c +++ b/devtools/onion.c @@ -105,7 +105,7 @@ static struct route_step *decode_with_privkey(const tal_t *ctx, const u8 *onion, struct route_step *step; struct onionpacket packet; enum onion_type why_bad; - u8 shared_secret[32]; + struct secret shared_secret; if (!hex_decode(hexprivkey, strlen(hexprivkey), &seckey, sizeof(seckey))) errx(1, "Invalid private key hex '%s'", hexprivkey); @@ -114,10 +114,10 @@ static struct route_step *decode_with_privkey(const tal_t *ctx, const u8 *onion, if (why_bad != 0) errx(1, "Error parsing message: %s", onion_type_name(why_bad)); - if (!onion_shared_secret(shared_secret, &packet, &seckey)) + if (!onion_shared_secret(&shared_secret, &packet, &seckey)) errx(1, "Error creating shared secret."); - step = process_onionpacket(ctx, &packet, shared_secret, assocdata, + step = process_onionpacket(ctx, &packet, &shared_secret, assocdata, tal_bytelen(assocdata)); return step; diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index ae545660c..12f47214f 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -946,7 +946,7 @@ static bool peer_accepted_htlc(struct channel *channel, u64 id, return false; } - rs = process_onionpacket(tmpctx, &op, hin->shared_secret->data, + rs = process_onionpacket(tmpctx, &op, hin->shared_secret, hin->payment_hash.u.u8, sizeof(hin->payment_hash)); if (!rs) { diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 5571bd912..3e1d48356 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -523,7 +523,7 @@ void plugin_hook_call_(struct lightningd *ld UNNEEDED, const struct plugin_hook struct route_step *process_onionpacket( const tal_t * ctx UNNEEDED, const struct onionpacket *packet UNNEEDED, - const u8 *shared_secret UNNEEDED, + const struct secret *shared_secret UNNEEDED, const u8 *assocdata UNNEEDED, const size_t assocdatalen )