Disable access to dangerous commands

[ZmnSCPxj]

Some commands are dangerous: we know for a fact that users have lost money by incorrect use of these commands:

• fundchannel_start, fundchannel_complete - People run fundchannel_complete before they can ensure that fundchannel_start has completed for all the other peers they are multifunding to, and getting their funds locked in unclaimable UTXOs.
• listsendpays - People think that "failed" in listsendpays means the payment as a whole failed, but in fact a pay command could retry the payment in case of failure. Detecting the payment as failed could make automated systems fail to deduct custodial funds from a client, who can keep re-attempting payouts to their own node until they drain the automated system.

Because of these, we should restrict access to these commands. In particular, these commands should only be accessible to plugins that provide safer interfaces (fundchannel, listpays) and the lower-level commands should be restricted.

My proposal is this:

• Add an option --allow-plugin-restricted-command=/path/to/plugin:command. This authorizes a particular plugin to access a particular command.
• Plugins in their getmanifest can include a restricted field to their commands, which if true means the command cannot be normally accessed.
• At startup, lightningd selects a random 256-bit number. Names of restricted commands are appended to that number and then hashed; restricted commands can only be accessed by using a method field (instead of the actual command name) containing the lowercase hex string representation of the resulting hash (called the "mangled method name").
• Plugins which have been authorized to access a restricted command will be given an additional field in their init command, unrestrictions. This contains an object mapping restricted command names to the mangled method names used by this execution of lighniingd, and will only contain those restricted commands that have been authorized via --allow-plugin-restricted-command.
• Built-in plugin fundchannel is allowed access to fundchannel_start and fundchannel_continue and fundchannel_cancel. Built-in plugin pay is allowed access to listsendpays. Those commands are now restricted and cannot be used by users.

[ZmnSCPxj]

Plugin libraries could add some additional field to their init wrappers / entrypoints to indicate that the plugin requires a specific restricted command, and will fail plugin startup if the command does not exist in unrestrictions. Further, they could internally wrap any command interface they have to look up unrestrictions for the mangled method names.

[rustyrussell]

I sympathize with this, but compatibility is also important. Making things harder to use is rarely a good idea.

I do wonder if there's a way to clearly demark all "you probably don't want to use this!" commands; we've used the dev- marker for clearly developer cases, but some of those have been marginal too (eg. dev-sendcustommsg especially).

Documentation is not sufficient for this, clearly.

Should we migrate all such commands use a raw- prefix? Or require a spurious dangerous=true argument? Or even danger- as a prefix?

[ZmnSCPxj]

If we are willing to settle for a prefix, then I would suggest adding : rather than - as separator, eg not-safe-to-use:listsendpays, if only because of its unusualness. But that still does not follow the principle that we should only provide an interface to users that would not cut them if they misuse it accidentally (this is the unsafePerformIO debate...)

We might still want to have multiple layers of interfaces to be used only at specific places, with a separate application layer that only accepts such simple (in interface) commands as pay, invoice, waitanyinvoice, listinvoices, delinvoice, listpays, paystatus, getinfo, listfunds, listpeers, newaddr, withdraw, fundchannel, autocleaninvoice, decodeinvoice (why is it decodepay and not decodeinvoice??).

[cdecker]

Just as another option we could adopt bitcoins modus operandi and just exclude commands we don't want users to use directly from the help output and add warnings in the manpages, so that the uninitiated user doesn't get tempted but they are still there.