commando: add rate for maximum successful rune use per minute.

I'm assuming that nobody wants a rate slower than 1 per minute; we can
introduce 'drate' if we want a per-day kind of limit.

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

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
 ------------
 

plugins/commando.c

@@ -1,5 +1,7 @@
 #include "config.h"
 #include <ccan/array_size/array_size.h>
+#include <ccan/crypto/siphash24/siphash24.h>
+#include <ccan/htable/htable_type.h>
 #include <ccan/json_escape/json_escape.h>
 #include <ccan/json_out/json_out.h>
 #include <ccan/rune/rune.h>
@@ -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;
 }
 

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})