nips icon indicating copy to clipboard operation
nips copied to clipboard

NIP-49 Encrypted Private Key export/import

Open mikedilger opened this issue 2 years ago • 12 comments

This is a scheme for exporting/importing a private key and sharing it on nostr so that it can be moved between clients securely without exposing it (displaying it, cutting and pasting it, etc).

This is currently implemented in gossip for saving/loading to its internal database. But if other clients implement this I will make a gossip function to export/import from other clients.

mikedilger avatar Dec 30 '22 01:12 mikedilger

As for the 11 bytes, I pulled them out of my arse. The point was to verify that a decryption is correct. Due to AES needing padding, most failed decryptions will result in an AES padding error, but I still like having these 11 bytes for more certainty.

mikedilger avatar Dec 30 '22 02:12 mikedilger

Changes made based on feedback from SPA.

I increased the number of rounds and added a version number. SPA mentioned use of 10,000,000 rounds in borderwallets.com. OWASP recommended 310,000 rounds for password hashing in 2021.

I based the number of rounds (100,000) on what would require one core of my computer (AMD Ryzen 9 5900X) to compute for about a second. We can increase it more if people lean that way.

I also made the salt random, however I think it is unnecessary. The private key already salts the hell out of the password. But leaving it out makes some people feel less secure and it doesn't hurt to throw in a random salt. It also made a convenient place to put a versioning byte in case we need it.

mikedilger avatar Jan 02 '23 19:01 mikedilger

Who is SPA?

I like the idea of having private keys be encrypted according to a standard. But I would like it even more if they were encoded like the NIP-19 entities, bech32 with a different prefix, say, ncryptsec (I don't know), just so everything follows the same pattern. Or is this a bad idea?

I didn't read this very carefully, but I think it would be just taking whatever the bytes we have here at the end and bech32ing them.

Another thing I would like to standardize is a location in the computer to store encrypted private keys so multiple apps could access them, but I guess this is another topic, and one that is much more hairy.

fiatjaf avatar Jan 07 '23 23:01 fiatjaf

SPA: 4a38463c2a75e68c24416e7720a3b3befbb0ea6872d5a04692c39e18e8f2dcac , someone that contacted me on nostr about it.

I like the ncryptsec bech32 thing. We could drop the base64 encoding and switch to that instead. Keeps things more consistent. I'll make the change.

mikedilger avatar Jan 08 '23 00:01 mikedilger

I'm going to respond to comments from the twitterati.

https://twitter.com/ColbySerpa/status/1614233927312605184 This one is off topic. It's a suggestion to tightly tie nostr to bitcoin, the benefit of which seems rather nil to me.

https://twitter.com/4moonsettler/status/1614607087568392193
Apparently there is a BIP-38 that aims to do something similar https://github.com/bitcoin/bips/blob/master/bip-0038.mediawiki I will look at it later. The suggestion to use Diffie Hellman isn't applicable. There is no online back-and-forth protocol between old client and new client. In fact, when "old client" writes the encrypted key to a relay, the user may have no idea what his new client is going to be yet. Further, if they somehow established a secure channel, that doesn't authenticate the new client to the old client, it just creates a secure channel to an unknown party.

https://twitter.com/pandrewhk/status/1614245968597352449 Again on the Diffie Hellman. He is right this is for "non-interactive backups". It is what gossip uses to store the key in its local database. Avoiding private key transfer over the protocol would be great, but nostr doesn't support mutiple-keys-per-identity and I wasn't trying to crack that nut.

https://twitter.com/isaacfain/status/1614264974012039169 This looks like a better comment, ~~but at first glance I think he's wrong about most of what he said. He doesn't like that I appended a checksum in step 1, then he doesn't like that there is no checksum on the input before encryption. Maybe he didn't have his coffee yet.~~ And where is the IV reuse? I'm using IV in the standard way, am I not? Show me how I'm not using IV exactly how you are supposed to? If you don't append the IV to the ciphertext, it cannot be deciphered. Notice all the examples on this page https://docs.rs/cbc/latest/cbc/ require an IV to decrypt. How can you get the IV if you didn't append it? The 'bool' creates 7 bits of known plaintext, maybe it could change to a single bit. XChaCha-Poly1305 > AES is probably correct but I'm not going to get my panties in a twist about it. And bech32 is more uniform with the rest of nostr and has nothing to do with the crypto part.

https://twitter.com/samecwilliams/status/1614492981943480323 This guy has a good idea. That is to let the user choose how many cycles to put PBKDF2 through. They can ramp it up and spend half an hour encrypting their private key if they want, or do like I do and encrytped it in less than a second. So I will add this. Someone go like his post for me.

https://twitter.com/sj_mackenzie/status/1614536989239566337 This guy wants 1 million PBKDF2 cycles. Given the previous comment, he can have that. It slows down password cracking so if you want to use terribly short guessable passwords I guess you can mitigate that by ramping up the PBKDF2. I use long hard to crack passwords with many many bits. But to each his own. As for XChaCha-Poly1305 versus AES, everybody has their favorites. AES isn't my favorite, I was just reusing something used elsewhere in nostr.

Nobody mentioned that the salt was useless. I would expect a good cryptographer to notice that. So these people on Twitter are not good cryptographers, or simply didn't spend any real time on this. I bet if I didn't put it in there, though, someone would point out it was missing.

And now a point from me: When people are cutting-and-pasting private keys around on their computer and into web pages on their browser, nobody seems to see anything wrong with that. But as soon as you suggest a much more secure method of private key handling, because it's not quite perfect everybody gets their panties in a twist. Funny how people are.

Happy to take feedback on my feedback.

mikedilger avatar Jan 15 '23 14:01 mikedilger

Actually isaacfain did say one thing of use I didn't comment on. Forcing 11 known bytes into the plaintext helps an attacker because they can quickly determine in each iteration if they cracked your AES or not. It would be better not to have known plaintext. The sacrifice is that you couldn't detect if the decryption succeeded or not. Maybe a 'checksum' is better because it is not a fixed known sequence. Perhaps it is I that hadn't had my coffee yet.

mikedilger avatar Jan 15 '23 14:01 mikedilger

So I propose to change this scheme to work like this instead (I'll write it up later):

  • Use XChaCha20-Poly1305 -- people like this better, it's less associated with the US government, it's faster (in software) and the Poly1305 creates a checksum. The checksum is useful, but we have no 'associated data' to include.
  • Encrypt ONLY the 32-byte nostr private key with nothing else inside of the encryption, so there can be no concerns about partial known plaintext attacks.
  • Put the key security flag outside of the encryption
  • Specify a user-selectable number of rounds for PBKDF2 based on powers of 2, recommending from 16-24 or so, but letting the user decide.
  • Drop the salt. It's purpose (making multiple hashes of the same password all different) makes no difference here because we are using PBKDF2 for key derivation not for password hashing.

mikedilger avatar Jan 15 '23 15:01 mikedilger

I think the scheme shouldn't use too much arcane stuff or be too complex otherwise no one is going to implement it.

fiatjaf avatar Jan 17 '23 01:01 fiatjaf

I think the scheme shouldn't use too much arcane stuff or be too complex otherwise no one is going to implement it.

So... remove the bech32 encoding? 😝

My other reply could be: That is why I made it simple.

XChaCha20-Poly1305 and PBKDF2 are top-line very standard and common cryptographic algorithms that BTW nobody should try to implement, they should use a cryptographic library.

If someone wants to come up with a different scheme though, I'm all ears. The scheme of the original PR was born out of necessity, and modified to suit others.

mikedilger avatar Jan 17 '23 02:01 mikedilger

Yes, I was thinking about using a library.

I didn't mean to say this scheme was not simple, I didn't read it fully or thought about the implementation, I just got afraid, by reading all the names of these algorithms, that if I were to implement it I would have to import a bunch of new libraries which would come with a lot of dependencies. And in Scala Native I would have to write C bindings to random C libraries that implement these things that I would find on GitHub from unknown developers.

fiatjaf avatar Jan 17 '23 11:01 fiatjaf

I think of it like 2 steps (2 algorithms) but technically it's 5. PBKDF2 using HMAC-SHA-256 is essentially a single process, but technically using three algorithms in a pattern. And XChaCha20-Poly1305 is a second single process, but technically a combination of an encryption algorithm and a digital signature algorithm. Generally if a library has PBKDF2 it has the other two (because that is by far the most common way to run PBKDF2). And if a library has XChaCha20 it has the Poly1305 (because that is commonly used to use XChaCha20 as AEAD (authenticated encryption with associated data))

I understand not wanting to pull in a bunch of libraries you aren't already pulling in.

BIP-38 solves the same problem: passphrase encryption of private key material. PKCS#8 also solves the same thing.

BIP-38 uses scrypt (~~which is like PBKDF2 but less preferred by cryptographers~~ I may be wrong about this. Now I'm reading somewhere that scrypt is stronger, but also about argon2 and balloon hashing being even better. Remember that I am not a cryptographer! Sorry.), it uses salting (I skipped salting because it makes no sense to me in this context), it uses AES256 (again a less preferred algorithm, but really not bad at all IMHO), does some splitting and XORing again which I do not understand the purpose of, and some base58 encoding (weird bitcoin world stuff, but probably in your library).

It also has a mode where you cannot encrypt an existing private key, you can only encrypt-generate together, and uses some secp256k1 magic that I do not understand to do it. That offers some assurance that nobody knows the seed words because you just generated it.

~~scrypt and~~ AES256 aren't terrible. They were probably the right choice in 2012. ~~PBKDF2-HMAC-SHA256 and~~ argon2 and XChaCha20-Poly1305 are probably the right choice today if cryptographic strength was the only concern.

EDIT: From what I'm reading today, scrypt is newer and better than PBKDF2, although PBKDF2 is still in widespread usage. Argon2 is better than both of those.

Finally (and sorry this is so long), I'm having second thoughts about suggesting that people publish these encrypted private keys. Because there will be people who use bad passphrases that will have their identity stolen.

mikedilger avatar Jan 17 '23 18:01 mikedilger

The last changes I made to this specified scrypt for slow KDF and XChaCha20--Poly1305 for encryption. This has been in use in gossip for some time, albeit in gossip we hard coded the scrypt parameter n=18 (whereas this standard says the user should be able to choose).

mikedilger avatar Mar 02 '23 00:03 mikedilger

I try to implement, but as TLV values not as concat, why introduce a new share link format.

immetoo2 avatar Jun 08 '23 15:06 immetoo2

I try to implement, but as TLV values not as concat, why introduce a new share link format.

That makes sense. But this is deployed in the gossip client for a long time now so I would have to version bump it to change it to TLV, a pain. And we have a lot of fields all of which have known exact lengths so the flexibility of TLV isn't needed.

But if TLV got more buy-in, I would make that sacrifice. Nobody seems to have bought into this NIP and gossip private keys can't be transferred to other clients without opening them first (a sad state of affairs).

mikedilger avatar Jun 14 '23 04:06 mikedilger

I tried to implement it in JavaScript: https://github.com/AsaiToshiya/nip-49.

AsaiToshiya avatar Oct 27 '23 17:10 AsaiToshiya

I've implemented this at https://github.com/nbd-wtf/go-nostr/blob/39f541fc034bdf29e45d66751e8c9ae49d7ed3b4/nip49/nip49.go

Looks good to me.

fiatjaf avatar Jan 23 '24 01:01 fiatjaf

Love that this is a bech32 entity 👌

alexgleason avatar Jan 23 '24 01:01 alexgleason

Reading it again now after a long time I think it is too long. I should take out the opinionated advice and some of the excessive reasoning.

mikedilger avatar Jan 23 '24 06:01 mikedilger

I've implemented this at https://github.com/nbd-wtf/go-nostr/blob/39f541fc034bdf29e45d66751e8c9ae49d7ed3b4/nip49/nip49.go

That's awesome. And the code is actually quite short, the heavy lifting being done in the libs.

mikedilger avatar Jan 23 '24 06:01 mikedilger

https://github.com/fiatjaf/gitstr now supports reading, storing and decrypting ncrypticsec keys (I was afraid of storing nsec keys as plaintext).

fiatjaf avatar Jan 23 '24 14:01 fiatjaf

Looks great! But what's the new kind 10002 doing? I couldn't find it in the text.

vitorpamplona avatar Jan 23 '24 14:01 vitorpamplona

I think that's legacy stuff.

fiatjaf avatar Jan 23 '24 15:01 fiatjaf

10002

Yeah this NIP was started ages ago. Most of the comments before Jan 17, 2023 probably won't apply. Better to just look at the text.

mikedilger avatar Jan 24 '24 03:01 mikedilger

Maybe rename it to just "private key encryption" and not specify what to do with the encrypted material?

fiatjaf avatar Jan 24 '24 12:01 fiatjaf

Implemented this in https://github.com/fiatjaf/nak:

nak key encrypt <secret-key> <password>
nak key decrypt <ncryptsec> <password>

This is probably not very useful in practice, but for good for debugging and learning.

fiatjaf avatar Jan 25 '24 12:01 fiatjaf

Implemented this in https://github.com/nbd-wtf/nostr-tools.

fiatjaf avatar Jan 25 '24 15:01 fiatjaf

Have you implemented this in ngit @DanConwayDev?

I think we should merge this.

fiatjaf avatar Jan 25 '24 16:01 fiatjaf

I'm about to push a change to the title. Then I'm in favor of merging. We can add more test data vectors, but it does have one.

mikedilger avatar Jan 25 '24 23:01 mikedilger

Oh this is awesome; going to implement this scheme in nsecbunker

pablof7z avatar Jan 27 '24 00:01 pablof7z

Using good encryption standards is important for encrypting a secret with a potentially weak password but I'm not sure there should a nostr specific standard. The string shouldn't be shared so there is no need for cross compatibility. I suppose if the standard is not here then where should it be?

Have you implemented this in ngit @DanConwayDev?

Mostly - thanks for the good work on this @mikedilger.

To quote from my commit message: https://github.com/DanConwayDev/ngit-cli/commit/96660a90e4cd296a2922d7a547de4cd9d0b1928b

The approach to encryption draws heavily from that used by the gossip
nostr client.
...
There is UX trade-off between decryption speed and key-stretching
computation. This UX challenge is exacerbated in a cli tool as
decryption must take place more regularly. Thought was put into the
selected n_log and a heavily reduced value is provided for long
passwords where security benefits are smaller.

A more granular reducing in computation was also considered by
rejected to avoided to revealing just how weak a password is as most
weak passwords are reused.

I use 0x1 for the version number but most notably a log_n of 15 or 1 if the password is > 20 characters.

I'd recommend some flexibility around log_n depending on device and usage pattern.

The rust scrypt library recommended 17 at the time of my commit. It will need to go up as computing power increases.

DanConwayDev avatar Jan 27 '24 01:01 DanConwayDev