search

getAlby / lightning-browser-extension

The Bitcoin Lightning Browser Extension that brings deep Lightning & Nostr integration to the web. Wallet interface to multiple lightning nodes and key signer for Nostr, Liquid and onchain use.
https://getalby.com/#extension
MIT License
540 stars 194 forks source link

fix: add remote sign to reasonable event kinds #3226

Closed alexgleason closed 2 months ago

alexgleason commented 3 months ago

Describe the changes you have made in this PR

When I select "Let's be reasonable", Alby should not prompt me to sign kind 24133 "remote sign" events (NIP-46).

Screenshot from 2024-08-17 12-11-49

Why?

Kind 24133 is merely a transport mechanism. I still have to sign the actual event separately. Let me give an example. Say the server wants me to remote sign a kind 1 event. I first have to call window.nostr.signEvent(kind1) with the kind 1. This will trigger Alby to check whether I am allowed to sign a kind 1. Then I will need to encode the signed event into a kind 24133, and make a separate call to Alby like window.nostr.signEvent(kind24133). But by that point I have already signed the kind 1 and handed it off to the client, so wrapping it in a transport event is inconsequential because it could be transported any other way.

Another example: the server wants me to remote sign a kind 5 (deletion request). Kind 5 is not considered a "reasonable event kind" by Alby. Because I have to sign this event before creating a kind 24133, I will have to call window.nostr.signEvent(kind5) first, which will create the Alby popup, prompting me to approve the deletion. Then, if I approve it, it doesn't matter if I subsequently wrap that event in a kind 24133, because any damage that could be done by the kind 5 is done more easily by just submitting it to relays.

The important security step is whether I was prompted to sign the inner event (kind 1 / kind 5), not the transport event (kind 24133).

With the current UX, I am seeing complaints like this: https://ditto.pub/@justinbieber@stemstr.app/posts/a9f83c843c6901d645735c28af9956e458f2c4d19e86735cb396f3921262b252

If you use a nip-07 extension to log in, there will be a lot of pop ups until you select always allow for a bunch of the event types. After that everything is smooth.

So I think this event kind should be considered "reasonable" since it has no security implications on its own. It's just a transport mechanism.

Type of change

(Remove other not matching type)

Checklist

pavanjoshi914 commented 2 months ago

thanks for the pr. tAck