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

NIP-46 still uses NIP-04 encryption #1095

Open
alexgleason opened this issue Mar 1, 2024 · 13 comments
Open

NIP-46 still uses NIP-04 encryption #1095

alexgleason opened this issue Mar 1, 2024 · 13 comments

Comments

@alexgleason
Copy link
Member

NIP-04 is considered harmful. It could leak your secret key. We did so much work to ensure we made the right choices this time in NIP-44 and even did a proper security audit.

NIP-46 still depends on the insecure encryption. This goes against the entire fundamental purpose of NIP-46, which is to protect your private key.

There was pushback against changing NIP-46 because it would result in breaking changes. But a breaking change to NIP-44 is necessary. Otherwise why did we do all of that? And what's the point of protecting your key using encryption that could reveal your key?

So now we are stuck: not wanting to change NIP-46 so it doesn't have breaking changes, but using an outdated and deprecated form of encryption that goes against the goals of NIP-46.

What should we do?

@fiatjaf
Copy link
Member

fiatjaf commented Mar 2, 2024

I'll believe it is insecure when I see a stolen key.

@alexgleason
Copy link
Member Author

I'll believe it is insecure when I see a stolen key.

Then why did we do all of that? I thought it was the whole point.

cc @paulmillr

@paulmillr
Copy link
Contributor

@fiatjaf

I'll believe it is insecure when I see a stolen key.

Then it would be too late.

You know there are people who can steal your keys using radio, right? https://eprint.iacr.org/2015/170 Have you “seen” it?

Why did Signal and iMessage switch to post quantum cryptography? There are no pq computers today. Have you seen them?

@alexgleason i’m a security guy who saw a problem and wanted to get it done. If the community is not interested in this, there’s not much I can do 🤷‍♂️ It was complicated enough to even release nip44 out. I don’t want to spend more time agitating for “using” it.

what can be done by someone: modifying 46 to include proper encryption.

@mikedilger
Copy link
Contributor

I wouldn't worry about the breaking change. NIP-46 was broken recently. What does it matter that we break it again? Just please document it in BREAKING.md (which isn't being kept up to date so far)

@tyiu
Copy link
Contributor

tyiu commented Mar 21, 2024

For the kinds in NIP-46 that depend on NIP-04 encryption, we could just deprecate them in favor of new kinds. Not sure why there's pushback. NIP-04 is already officially unrecommended. So moving away from its usage is in line with that decision. The protocol is still relatively early in infancy so I don't think making breaking changes in how we do encryption to strengthen security (even if people believe it is theoretical in nature) is all that outlandish.

@vitorpamplona
Copy link
Collaborator

The only barrier is the lack of support for nip44 on extensions, but once they are there, nip46 should change as well.

And yes, let's do the change before it becomes too late.

I think the push back was more on the how to do it than on the migration itself.

@VnUgE
Copy link

VnUgE commented Apr 14, 2024

Bump on this? I have been building noscrypt without nip04 encryption in the API in favor of nip44 all together. Id like to be phasing out "consensus based" insecure forms of cryptography as fast as possible in my libraries.

@alexgleason
Copy link
Member Author

It's possible to detect whether the encrypted blob is nip44 or not just by looking at the first few characters.

If you're building a remote signer, I think you should detect it and support both. If you're building a client, I think you should accept an option like encryption?: 'nip04' | 'nip44' and default to nip04.

@mikedilger
Copy link
Contributor

It's possible to detect whether the encrypted blob is nip44 or not just by looking at the first few characters.

I think you have to look for the "?iv=" string to detect NIP-04, and base64 decode and then look at the first few characters to detect the version of NIP-44.

@mikedilger
Copy link
Contributor

mikedilger commented Apr 19, 2024

The change needs to happen in 2 phases:

  1. Everybody's code needs to be able to detect and decrypt either NIP-04 or NIP-44. Once we feel that has rolled out sufficiently,
  2. Everybody's code starts producing NIP-44 encrypted data instead of NIP-04.

EDIT: Gossip has done (1) and is waiting for some community organiser (@fiatjaf ?) to nudge all the clients towards completing it so we can move on to step (2).

@VnUgE
Copy link

VnUgE commented Apr 19, 2024

I think you have to look for the "?iv=" string to detect NIP-04, and base64 decode and then look at the first few characters to detect the version of NIP-44.

That is correct. There is no discriminating leading byte markers between 04 & 44 as utf8-encoded base64 text. '?' is not base64 and would cause a decoding error, so it needs to be parsed out first. If it does not exist, then it can be tested for nip-44, by decoding and looking for a leading version byte.

@vitorpamplona
Copy link
Collaborator

iv is always 24 chars, so don't "search" for it, just check the reference directly:

Given v as the NIP-04 string:

v[v.length-28] == '?' && v[v.length-27] == 'i' && v[v.length-26] == 'v' && v[v.length-25] == '='

@alexgleason
Copy link
Member Author

I added NIP-44 support to my remote signer class in Nostrify: https://nostrify.dev/sign/connect#encryption

I did not end up automatically detecting the decrypt method. It just uses an option to switch the encryption used: https://gitlab.com/soapbox-pub/nostrify/-/blob/main/src/NConnectSigner.ts?ref_type=heads#L159

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

No branches or pull requests

7 participants