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

signrpc: sign and verify messages with custom key #3812

Merged
merged 3 commits into from Dec 12, 2019

Conversation

@guggero
Copy link
Collaborator

guggero commented Dec 10, 2019

The existing SignMessage and VerifyMessage in the main rpc.proto file are very restrictive in that they only allow creating signatures over the node's identity private key and that a signature's recovered public key must correspond to a nodes' identity public key known to the network.
The new methods in the signer.proto don't have those restrictions and allow signing and verifying over custom wallet keys (limited to the 1017' special purpose derivation branch).

lnrpc/signrpc/signer_server.go Outdated Show resolved Hide resolved
lnrpc/signrpc/signer_server.go Outdated Show resolved Hide resolved
@guggero guggero force-pushed the guggero:sign-custom branch from 44eecee to 869a79c Dec 11, 2019
@guggero

This comment has been minimized.

Copy link
Collaborator Author

guggero commented Dec 11, 2019

I switched to default ECDSA encoding but left the message prefixing in to avoid signing anything sensitive. This means that you can only verify signatures generated with the lnd signer which should be fine for our use case.

I've also given the VerifyMessage the signer/read permission instead of signer/generate.

@guggero guggero requested a review from Roasbeef Dec 11, 2019
@guggero guggero force-pushed the guggero:sign-custom branch 3 times, most recently from 39ab7b4 to ffbfc40 Dec 11, 2019
guggero added 2 commits Dec 10, 2019
To allow signing of messages with any key in the key chain
we add two new methods to the signer RPC. These behave differently
to the methods with the same name in the main RPC as described
in the documentation comment.
@guggero guggero force-pushed the guggero:sign-custom branch from ffbfc40 to 9522677 Dec 11, 2019
Copy link
Member

Roasbeef left a comment

LGTM 🛫

@Roasbeef Roasbeef merged commit 3381755 into lightningnetwork:master Dec 12, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.01%) to 62.247%
Details
@guggero guggero deleted the guggero:sign-custom branch Dec 12, 2019
@joostjager

This comment has been minimized.

Copy link
Collaborator

joostjager commented Dec 15, 2019

I think admin.macaroon needs to be regenerated? In that case it may be good to add that to the release notes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.