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

Add app/feature to sign Nostr messages using m/44'/1237'/* keys. #4160

Open
rleed opened this issue Sep 6, 2024 · 18 comments · May be fixed by #4436
Open

Add app/feature to sign Nostr messages using m/44'/1237'/* keys. #4160

rleed opened this issue Sep 6, 2024 · 18 comments · May be fixed by #4436
Assignees

Comments

@rleed
Copy link

rleed commented Sep 6, 2024

MVP: When signing a text message, detect purpose code 1237 (per SLIP-44) in the derivation path. If present, calculate the signature in the manner described in NIP-01. See also NIP-06: Basic key derivation from mnemonic seed phrase.

Note: existing UI and protobuf are sufficient.

Nice to have: don't warn that derivation path m/44'/1237'/* is unrecognized

@AlexanderPavlenko
Copy link

#1943

@rleed
Copy link
Author

rleed commented Sep 11, 2024

#1943

To be clear: this has no dependency on #1943. Nostr signing is already in the wild and there's no decision to wait for.

@ibz ibz self-assigned this Sep 18, 2024
@ibz
Copy link
Contributor

ibz commented Oct 3, 2024

calculate the signature in the manner described in NIP-01.

Arguably, Trezor does not need to know about NIP-01.

NIP-01 says the sig (signature of the Nostr event) is lowercase hex of the signature of the sha256 hash of the serialized event data, which is the same as the "id".

The way I interpret that is that signing a Nostr event means:

  1. calculating its id
  2. signing this id with the key derived from SLIP44 with coin type 1237
  3. taking the signature of the id and adding it to the NIP-01 JSON struct

Trezor's role then would be point 2 above, basically signing a text message which is the Event ID.

Points 1 and 3 would be the role of the browser extension, or some higher level entity whose job is to sign Nostr events, while Trezor's role is to sign text messages without needing to know that the text messages represent the ID of a Nostr event (actually it would know, because of the coin type 1237, but it would not be able to know what the event is about).

The main trade off is that Trezor would not be able to ask you to confirm the message being signed. It would just ask you to confirm the ID of the event, which you would have to double-check with the extension.

But in either way, having Trezor ask you to confirm a JSON on its screen would not be very user friendly to begin with! And the alternative at the other end of the spectrum is probably something we really do not want, namely having Trezor have knowledge of NIPS and asking you to confirm the event based on its kind:

  • Do you want to post "Hello world" (NIP-01)?
  • Do you want to send a "thumbs up" reaction to this message (NIP-25)?
  • Do you want to list this item for sale on the Nostr marketplace (NIP-15)?

This would be extremely user friendly of course, but would essentially turn Trezor into a Nostr client, which it is not.

@ibz
Copy link
Contributor

ibz commented Oct 3, 2024

To be clear: this has no dependency on #1943.

Actually there is, at least partially. See this comment: #1943 (comment)

SignMessage does not work for Taproot and raises an error before it has a chance to return the result.

@matejcik
Copy link
Contributor

matejcik commented Oct 3, 2024

Nice to have: don't warn that derivation path m/44'/1237'/* is unrecognized

We can whitelist this path specifically, or we can add a virtual "coin" with SLIP44 id 1237, which you would specify as the SignMessage parameter.

I know little about Nostr, but I'm assuming you have a "public address" that would be confirmed alongside the message? How do those addresses look, are they the same thing as Taproot addresses?

@ibz
Copy link
Contributor

ibz commented Oct 3, 2024

We can whitelist this path specifically, or we can add a virtual "coin" with SLIP44 id 1237, which you would specify as the SignMessage parameter.

Yes, this is probably the way to go.

[...] you have a "public address" [...]

Not in the sense of having an address you send a transaction to. In Nostr you just sign events, which are JSON, and broadcast them to relays.

[...] that would be confirmed alongside the message [...]

I think we want to confirm the event as a whole - which can be anything (post an update, send a reaction, list an item for sale, update the list of people you follow, ...). But that means turning Trezor into a Nostr client. The alternative is to confirm the Event ID - which is a hash of the event and which is what Trezor will sign.

Confirming parts of the event (like the message being posted, the pubkey a DM is meant for...) requires Trezor to have knowledge of different kinds of events, which seems beyond Trezor's role to me.

@matejcik
Copy link
Contributor

matejcik commented Oct 3, 2024

no, what I mean is the source account. Like, who is it that posts the 👍, whose public key is producing the signature -- I'm guessing Nostr doesn't allow me to sign someone else's actions :)

@AlexanderPavlenko
Copy link

no, what I mean is the source account. Like, who is it that posts the 👍, whose public key is producing the signature -- I'm guessing Nostr doesn't allow me to sign someone else's actions :)

One can sign anything given private key. Source account = public key.

@ibz
Copy link
Contributor

ibz commented Oct 3, 2024

I'm guessing Nostr doesn't allow me to sign someone else's actions

Relays might look at that as a sanity check, but I don't think it is required by the protocol. In general relays are pretty dumb and just store and broadcast whatever comes in.

It is the responsibility of your client to verify that some message you got from some relay is properly signed, because what you are trusting is your client, not the relays - which are not entities you can control, really.

@ibz
Copy link
Contributor

ibz commented Oct 3, 2024

One can sign anything given private key. Source account = public key.

So we could display the public key and ask the user to confirm that, right?

I am thinking this may be useful, because we might end up deriving multiple keys from the same seed - you could have a key for your personal account, and a key for your business account... depending on the derivation path. But this would be more like a sanity check than something essential, if it is the web extension that ultimately builds the event. Of course it would be up to the extension to eventually add the correct key (the one corresponding to the private key Trezor used) in the event that it broadcasts.

@AlexanderPavlenko
Copy link

How do those addresses look, are they the same thing as Taproot addresses?

https://github.com/rot13maxi/key-convertr

@AlexanderPavlenko
Copy link

One can sign anything given private key. Source account = public key.

So we could display the public key and ask the user to confirm that, right?

I am thinking this may be useful, because we might end up deriving multiple keys from the same seed - you could have a key for your personal account, and a key for your business account... depending on the derivation path. But this would be more like a sanity check than something essential, if it is the web extension that ultimately builds the event. Of course it would be up to the extension to eventually add the correct key (the one corresponding to the private key Trezor used) in the event that it broadcasts.

Yes, seems useful to confirm pubkey. Maybe also provide its derivation without signing. This would allow to build an event given only the derivation path and trezorctl.

@andrewkozlik
Copy link
Contributor

andrewkozlik commented Oct 3, 2024

See this comment: #1943 (comment)

I think that this is unrelated to Taproot signing. The NIPs define how the message should be hashed and what keys to use. There is no key tweaking like in Taproot. In Bitcoin the message signatures contain script type information and public key recovery information in the first byte of the signature, so that you can recover the address from the signature. All these things are not defined for Taproot. Nostr on the other hand doesn't do any of this.

The good news is that we don't need to wait for #1934. The bad news is that I don't think this is as simple as adding another coin to the list of Bitcoin-like coins, because as I just explained, the message signing is not Bitcoin-like, not even Taproot-like due to the lack of key tweaking.

So we could display the public key and ask the user to confirm that, right?

During signing the user should see the account number. If they want to see the public key for the given account they should use some kind of GetPublicKey message, probably a dedicated NostrGetPublicKey.

Confirming parts of the event (like the message being posted, the pubkey a DM is meant for...) requires Trezor to have knowledge of different kinds of events, which seems beyond Trezor's role to me.

I think that is Trezor's role. When signing Bitcoin transactions we don't show the hash that is being signed. We display the transaction information in a human-readable form. Given that a special app will probably be needed to support Nostr signing, this is not all that much extra effort, although signing JSON might turn out to be a PITA. (Trezor shouldn't have to parse the JSON. It should get it in some parsed form as a protobuf message, then display the information to the user, internally serialize it into JSON while hashing it and sign the resulting hash.)

@ibz
Copy link
Contributor

ibz commented Oct 3, 2024

I think that is Trezor's role. When signing Bitcoin transactions we don't show the hash that is being signed.

OK, you got me on this. Makes sense.

Trezor shouldn't have to parse the JSON.

Not the full JSON of the event, indeed, that would be overkill. Maybe the JSON-array variant of it, which is then hashed to obtain the ID which is then signed? Seems simple enough compared to full JSON:

[0, pubkey, created_at, kind, tags <as an array of arrays of non-null strings>, content <as a string>] (see NIP-01)

If this is what we get via protobuf (as a string), all we need to do is:

  1. confirm this
  • in a first iteration we could just display it as it is without parsing - not great, but at least it is clear what you are signing - all information is there
  • in a later iteration, we could extract kind and content and show them first on a nicer screen, perhaps hard-coding some of the most-used kinds, but allowing essentially any unknown kind, and have timestamp, tags, etc. on another screen
  1. hash this string (no need to serialize it - just hash what we received as a string!)
  2. sign the hash
  3. now the caller (browser extension, or some user of trezorctl) can build the actual JSON and attach the signature to it

@andrewkozlik
Copy link
Contributor

More like this:

message NostrSign {
  repeated uint32 address_n = 1; // Used to derive pubkey.
  optional uint32 created_at = 2;
  optional uint32 kind = 3;
  repeated string tags = 4;
  optional string content = 5;
}

@ibz
Copy link
Contributor

ibz commented Oct 3, 2024

Even better. Then we don't need to parse anything. But then it is not really part of SignMessage, which has just a string. Which is even better, since we are not subject to the other limitations described. Basically it's a whole new app, which would be invoked using trezorctl nostr sign-event as opposed to trezorctl btc sign-message.

PS: still need to serialize as the JSON-array in order to hash & sign.

@AlexanderPavlenko
Copy link

the message signing is not Bitcoin-like, not even Taproot-like due to the lack of key tweaking

Indeed. BIP341 also says "taproot outputs should never reuse keys".

Improper signatures may leak the key?
https://b10c.me/blog/009-schnorr-nonce-reuse-challenge/

Key rotation may become a thing:
nostr-protocol/nips#103

@rleed
Copy link
Author

rleed commented Oct 9, 2024

Even better. Then we don't need to parse anything. But then it is not really part of SignMessage, which has just a string. Which is even better, since we are not subject to the other limitations described. Basically it's a whole new app, which would be invoked using trezorctl nostr sign-event as opposed to trezorctl btc sign-message.

PS: still need to serialize as the JSON-array in order to hash & sign.

This is a good plan! I'm excited!

@ibz ibz linked a pull request Dec 10, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

5 participants