diff --git a/doc/lightning-commando-rune.7.md b/doc/lightning-commando-rune.7.md index ca31e2e96..97bc4ec9a 100644 --- a/doc/lightning-commando-rune.7.md +++ b/doc/lightning-commando-rune.7.md @@ -33,6 +33,7 @@ being run: * time: the current UNIX time, e.g. "time<1656759180". * id: the node_id of the peer, e.g. "id=024b9a1fa8e006f1e3937f65f66c408e6da8e1ca728ea43222a7381df1cc449605". * method: the command being run, e.g. "method=withdraw". +* rate: the rate limit, per minute, e.g. "rate=60". * pnum: the number of parameters. e.g. "pnum<2". * pnameX: the parameter named X. e.g. "pnamedestination=1RustyRX2oai4EYYDpQGWvEL62BBGqN9T". * parrN: the N'th parameter. e.g. "parr0=1RustyRX2oai4EYYDpQGWvEL62BBGqN9T". @@ -108,13 +109,14 @@ This allows `listpeers` with 1 argument (`pnum=1`), which is either by name (`pn "unique_id": "3" } -Before we give this to our peer, let's add another restriction: that -it only be usable for 24 hours from now. `date +%s` can give us the -current time in seconds: +Before we give this to our peer, let's add two more restrictions: that +it only be usable for 24 hours from now (`time<`), and that it can only +be used twice a minute (`rate=2`). `date +%s` can give us the current +time in seconds: - $ lightning-cli commando-rune rune=fTQnfL05coEbiBO8SS0cvQwCcPLxE9c02pZCC6HRVEY9MyZpZD0wMjRiOWExZmE4ZTAwNmYxZTM5MzdmNjVmNjZjNDA4ZTZkYThlMWNhNzI4ZWE0MzIyMmE3MzgxZGYxY2M0NDk2MDUmbWV0aG9kPWxpc3RwZWVycyZwbnVtPTEmcG5hbWVpZF4wMjRiOWExZmE4ZTAwNmYxZTM5M3xwYXJyMF4wMjRiOWExZmE4ZTAwNmYxZTM5Mw== restrictions="t<$(($(date +%s) + 24*60*60))" + $ lightning-cli commando-rune rune=fTQnfL05coEbiBO8SS0cvQwCcPLxE9c02pZCC6HRVEY9MyZpZD0wMjRiOWExZmE4ZTAwNmYxZTM5MzdmNjVmNjZjNDA4ZTZkYThlMWNhNzI4ZWE0MzIyMmE3MzgxZGYxY2M0NDk2MDUmbWV0aG9kPWxpc3RwZWVycyZwbnVtPTEmcG5hbWVpZF4wMjRiOWExZmE4ZTAwNmYxZTM5M3xwYXJyMF4wMjRiOWExZmE4ZTAwNmYxZTM5Mw== restrictions='["time<'$(($(date +%s) + 24*60*60))'","rate=2"]' { - "rune": "Sh-jGdfO9UGByLvah2AHgc_VwgoNujckPNkxTx54ugg9MyZpZD0wMjRiOWExZmE4ZTAwNmYxZTM5MzdmNjVmNjZjNDA4ZTZkYThlMWNhNzI4ZWE0MzIyMmE3MzgxZGYxY2M0NDk2MDUmbWV0aG9kPWxpc3RwZWVycyZwbnVtPTEmcG5hbWVpZF4wMjRiOWExZmE4ZTAwNmYxZTM5M3xwYXJyMF4wMjRiOWExZmE4ZTAwNmYxZTM5MyZ0PDE2NTY4OTc5MjU=", + "rune": "tU-RLjMiDpY2U0o3W1oFowar36RFGpWloPbW9-RuZdo9MyZpZD0wMjRiOWExZmE4ZTAwNmYxZTM5MzdmNjVmNjZjNDA4ZTZkYThlMWNhNzI4ZWE0MzIyMmE3MzgxZGYxY2M0NDk2MDUmbWV0aG9kPWxpc3RwZWVycyZwbnVtPTEmcG5hbWVpZF4wMjRiOWExZmE4ZTAwNmYxZTM5M3xwYXJyMF4wMjRiOWExZmE4ZTAwNmYxZTM5MyZ0aW1lPDE2NTY5MjA1MzgmcmF0ZT0y", "unique_id": "3" } @@ -126,6 +128,13 @@ normal rune into a read-only rune, or restrict access for 30 minutes from the time you give it to someone. Adding restrictions before sharing runes is best practice. +If a rune has a ratelimit, any derived rune will have the same id, and +thus will compete for that ratelimit. You might want to consider +adding a tighter ratelimit to a rune before sharing it, so you will +keep the remainder. For example, if you rune has a limit of 60 times +per minute, adding a limit of 5 times per minute and handing that rune +out means you can still use your original rune 55 times per minute. + RETURN VALUE ------------ diff --git a/plugins/commando.c b/plugins/commando.c index b7e44f82f..2f39079a2 100644 --- a/plugins/commando.c +++ b/plugins/commando.c @@ -1,5 +1,7 @@ #include "config.h" #include +#include +#include #include #include #include @@ -42,6 +44,45 @@ static struct commando **incoming_commands; static u64 *rune_counter; static struct rune *master_rune; +struct usage { + /* If you really issue more than 2^32 runes, they'll share ratelimit buckets */ + u32 id; + u32 counter; +}; + +static u64 usage_id(const struct usage *u) +{ + return u->id; +} + +static size_t id_hash(u64 id) +{ + return siphash24(siphash_seed(), &id, sizeof(id)); +} + +static bool usage_eq_id(const struct usage *u, u64 id) +{ + return u->id == id; +} +HTABLE_DEFINE_TYPE(struct usage, usage_id, id_hash, usage_eq_id, usage_table); +static struct usage_table usage_table; + +/* Every minute we forget entries. */ +static void flush_usage_table(void *unused) +{ + struct usage *u; + struct usage_table_iter it; + + for (u = usage_table_first(&usage_table, &it); + u; + u = usage_table_next(&usage_table, &it)) { + usage_table_delval(&usage_table, &it); + tal_free(u); + } + + notleak(plugin_timer(plugin, time_from_sec(60), flush_usage_table, NULL)); +} + /* NULL peer: don't care about peer. NULL id: don't care about id */ static struct commando *find_commando(struct commando **arr, const struct node_id *peer, @@ -183,8 +224,40 @@ struct cond_info { const jsmntok_t *method; const jsmntok_t *params; STRMAP(const jsmntok_t *) cached_params; + struct usage *usage; }; +static const char *rate_limit_check(const tal_t *ctx, + const struct rune *rune, + const struct rune_altern *alt, + struct cond_info *cinfo) +{ + unsigned long r; + char *endp; + if (alt->condition != '=') + return "rate operator must be ="; + + r = strtoul(alt->value, &endp, 10); + if (endp == alt->value || *endp || r == 0 || r >= UINT32_MAX) + return "malformed rate"; + + /* We cache this: we only add usage counter if whole rune succeeds! */ + if (!cinfo->usage) { + cinfo->usage = usage_table_get(&usage_table, atol(rune->unique_id)); + if (!cinfo->usage) { + cinfo->usage = tal(plugin, struct usage); + cinfo->usage->id = atol(rune->unique_id); + cinfo->usage->counter = 0; + usage_table_add(&usage_table, cinfo->usage); + } + } + + /* >= becuase if we allow this, counter will increment */ + if (cinfo->usage->counter >= r) + return tal_fmt(ctx, "Rate of %lu per minute exceeded", r); + return NULL; +} + static const char *check_condition(const tal_t *ctx, const struct rune *rune, const struct rune_altern *alt, @@ -203,6 +276,8 @@ static const char *check_condition(const tal_t *ctx, cinfo->method->end - cinfo->method->start); } else if (streq(alt->fieldname, "pnum")) { return rune_alt_single_int(ctx, alt, cinfo->params->size); + } else if (streq(alt->fieldname, "rate")) { + return rate_limit_check(ctx, rune, alt, cinfo); } /* Rest are params looksup: generate this once! */ @@ -267,12 +342,17 @@ static const char *check_rune(const tal_t *ctx, cinfo.buf = buf; cinfo.method = method; cinfo.params = params; + cinfo.usage = NULL; strmap_init(&cinfo.cached_params); err = rune_test(ctx, master_rune, rune, check_condition, &cinfo); /* Just in case they manage to make us speak non-JSON, escape! */ if (err) err = json_escape(ctx, take(err))->s; strmap_clear(&cinfo.cached_params); + + /* If it succeeded, *now* we increment any associated usage counter. */ + if (!err && cinfo.usage) + cinfo.usage->counter++; return err; } @@ -770,6 +850,7 @@ static void memleak_mark_globals(struct plugin *p, struct htable *memtable) memleak_remove_region(memtable, outgoing_commands, tal_bytelen(outgoing_commands)); memleak_remove_region(memtable, incoming_commands, tal_bytelen(incoming_commands)); memleak_remove_region(memtable, master_rune, sizeof(*master_rune)); + memleak_remove_htable(memtable, &usage_table.raw); if (rune_counter) memleak_remove_region(memtable, rune_counter, sizeof(*rune_counter)); } @@ -782,6 +863,7 @@ static const char *init(struct plugin *p, outgoing_commands = tal_arr(p, struct commando *, 0); incoming_commands = tal_arr(p, struct commando *, 0); + usage_table_init(&usage_table); plugin = p; #if DEVELOPER plugin_set_memleak_handler(p, memleak_mark_globals); @@ -814,6 +896,8 @@ static const char *init(struct plugin *p, master_rune = rune_new(plugin, rune_secret.data, ARRAY_SIZE(rune_secret.data), NULL); + /* Start flush timer. */ + flush_usage_table(NULL); return NULL; } diff --git a/tests/test_plugin.py b/tests/test_plugin.py index b7ddd0350..9061ac715 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -2666,10 +2666,19 @@ def test_commando_rune(node_factory): rune7 = l1.rpc.commando_rune(restrictions="pnum=0") assert rune7['rune'] == 'QJonN6ySDFw-P5VnilZxlOGRs_tST1ejtd-bAYuZfjk9NCZwbnVtPTA=' assert rune7['unique_id'] == '4' + rune8 = l1.rpc.commando_rune(rune7['rune'], "rate=3") + assert rune8['rune'] == 'kSYFx6ON9hr_ExcQLwVkm1ABnvc1TcMFBwLrAVee0EA9NCZwbnVtPTAmcmF0ZT0z' + assert rune8['unique_id'] == '4' + rune9 = l1.rpc.commando_rune(rune8['rune'], "rate=1") + assert rune9['rune'] == 'O8Zr-ULTBKO3_pKYz0QKE9xYl1vQ4Xx9PtlHuist9Rk9NCZwbnVtPTAmcmF0ZT0zJnJhdGU9MQ==' + assert rune9['unique_id'] == '4' # Replace rune3 with a more useful timestamp! expiry = int(time.time()) + 15 rune3 = l1.rpc.commando_rune(restrictions="time<{}".format(expiry)) + ratelimit_successes = ((rune9, "getinfo", {}), + (rune8, "getinfo", {}), + (rune8, "getinfo", {})) successes = ((rune1, "listpeers", {}), (rune2, "listpeers", {}), (rune2, "getinfo", {}), @@ -2681,7 +2690,7 @@ def test_commando_rune(node_factory): (rune6, "listpeers", [l2.info['id'], 'broken']), (rune6, "listpeers", [l2.info['id']]), (rune7, "listpeers", []), - (rune7, "getinfo", {})) + (rune7, "getinfo", {})) + ratelimit_successes failures = ((rune2, "withdraw", {}), (rune2, "plugin", {'subcommand': 'list'}), (rune3, "getinfo", {}), @@ -2689,7 +2698,9 @@ def test_commando_rune(node_factory): (rune5, "listpeers", {'id': l2.info['id'], 'level': 'io'}), (rune6, "listpeers", [l2.info['id'], 'io']), (rune7, "listpeers", [l2.info['id']]), - (rune7, "listpeers", {'id': l2.info['id']})) + (rune7, "listpeers", {'id': l2.info['id']}), + (rune9, "getinfo", {}), + (rune8, "getinfo", {})) for rune, cmd, params in successes: l2.rpc.call(method='commando', @@ -2721,3 +2732,13 @@ def test_commando_rune(node_factory): 'method': "listpeers", 'params': {}}) assert exc_info.value.error['code'] == 0x4c51 + + # Now wait for ratelimit expiry, ratelimits should reset. + time.sleep(61) + + for rune, cmd, params in ratelimit_successes: + l2.rpc.call(method='commando', + payload={'peer_id': l1.info['id'], + 'rune': rune['rune'], + 'method': cmd, + 'params': params})