Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API description of VerifyMessage is wrong and dangerous #3596

Closed
rustyrussell opened this issue Oct 12, 2019 · 6 comments · Fixed by #7689
Closed

API description of VerifyMessage is wrong and dangerous #3596

rustyrussell opened this issue Oct 12, 2019 · 6 comments · Fixed by #7689
Labels
documentation Documentation changes that do not affect code behaviour rpc Related to the RPC interface

Comments

@rustyrussell
Copy link

Background

The API doc says:

"VerifyMessage verifies a signature over a msg. The signature must be zbase32 encoded and signed by an active node in the resident node's channel database. In addition to returning the validity of the signature, VerifyMessage also returns the recovered pubkey from the signature."

Let's break that down:

VerifyMessage verifies a signature over a msg.

No, it doesn't. It performs key recovery. 50% of the time, it will return a key, given any random message.

In addition to returning the validity of the signature, VerifyMessage also returns the recovered pubkey from the signature.

Again, it doesn't check the validity of the signature. And returning the recovered pubkey is not just a side-effect: checking that it is the expected key is a compulsory part of using the API.

This function should either require the pubkey and do the actual verification as the name promises, or be renamed to "SignedMessageExtractKey". At least you need to document how to use it, though I doubt that will help :(

I wonder how many users are relying on this?

@Roasbeef
Copy link
Member

Roasbeef commented Oct 12, 2019

Again, it doesn't check the validity of the signature.

If the signature was invalid, it wouldn't be able to be recovered (yes it may be the case that they actually don't know the private key to that pubkey). The pubkey is only returned if the pubkey is found in the graph. It was originally meant to be used as a sort of sybil resistant authentication mechanism (can require them to have N BTC in channels, etc). By returning the pubkey the caller is then also able to perform additional checks on it (this is really a user of my service, etc). Existence of the pubkey in the graph also confirms that the signer knows the private key for that public key as their node announcement has an authenticating signature (channel announcements too etc).

I wonder how many users are relying on this?

I'm not sure, from my PoV it isn't very widely used. I think some applications want to use it, but haven't completely thought through their use case as they want to use it for nodes that have 100% unadvertised channels. At that point there's no actual cost since the user can just generate an arbitrary amount of pubkeys to create an account or w/e. As the channels are unadvertised the service would need to require channel proofs, which defeats the purpose of not advertising. In essence, there's nothing linking the pubkey to the actual leaf Lightning node.

@rustyrussell
Copy link
Author

rustyrussell commented Oct 12, 2019

Checking against the graph makes this secure. Phew! But it's also very fragile! No signature validation from private nodes, or maybe if you're not up-to-date.

With Zap promoting this as an identification mechanism, these undocumented requirements make me wonder what insecurities we'll see as people experiment...

@rustyrussell
Copy link
Author

... still, I stole your method for my checkmessage API in c-lightning: if they don't provide a pubkey arg, we lookup a node with the recovered id. Ideally we could look through any invoices we know too, as another source of valid node ids (since that also uses key recovery, but on a different message).

@sr-gi
Copy link

sr-gi commented Feb 19, 2020

Is there any plan for including an optional parameter for the public key so messages can be verified without having to check if the node exists (in the same fashion c-lightning does)?

We were considering using SignMessage and VerifyMessage as the authentication mechanism for BOLT13 but realised it may conflict with the current way of verifying the messages for lnd.

The idea was to either sign the requests using the node's private key, or an ephemeral key that could be a tweak of private key (h/t @cdecker).

The issue with the former is that it will leak what node is sending data to the tower, which we may want to avoid.

The issue with the later is that it won't work with the current implementation since the tweak (or ephemeral key) cannot be provided, nor can a public key for verification.

@wpaulino
Copy link
Contributor

Does the version of SignMessage and VerifyMessage in signrpc work for your use-case? The RPC could also be extended to accept a tweak.

@sr-gi
Copy link

sr-gi commented Feb 19, 2020

Thanks! That's exactly what I was looking for. Yes, as long as it can be tweaked and verified using a public key it should do.

@wpaulino wpaulino added documentation Documentation changes that do not affect code behaviour rpc Related to the RPC interface verification labels Feb 20, 2020
guggero added a commit to guggero/lnd that referenced this issue Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation changes that do not affect code behaviour rpc Related to the RPC interface
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants