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

Feature request: please integrate NIP04 (encrypted message) #15

Closed
galets opened this issue Feb 3, 2023 · 17 comments
Closed

Feature request: please integrate NIP04 (encrypted message) #15

galets opened this issue Feb 3, 2023 · 17 comments

Comments

@galets
Copy link

galets commented Feb 3, 2023

This is a request to add NIP04 decoder to the library

Following implementation works with this library: https://github.com/galets/nostr_message_fetch/blob/master/lib/nip04.dart

I'm consenting for you to re-license the code to LGPL-3.0

@ethicnology
Copy link
Owner

Ok I will take a look at this

@ethicnology
Copy link
Owner

Initial work: #25

WIP: 385f211

@no-prob I reworked your contribution to make it more compatible with the actual usage of the library, could you take a look at the changes ?
Could generates random nostr keys to generate unit tests for Encrypted Direct Message with the hardcoded keys ?

@ryzizub Yo! Could you take a look to the contribution and the rework behind ? If you also have some NIP4 Event that you could provide as test vectors with the correct pubkey/privkey

To finish the job we really need to cover these unit tests cases:

  • Nip4.cipher with a valid plaintext and the expected ciphertext output
  • Nip4.decipher with a valid ciphertext and the expected plaintext output
  • Instanciate a Nip4 event from real data, it should pass verify = true
  • Create a EncryptedDirectMessage.quick message that is nostr valid
  • EncryptedDirectMessage.getPlaintext
  • EncryptedDirectMessage.getCiphertext

@ntheden
Copy link
Contributor

ntheden commented Apr 17, 2023

Could generates random nostr keys to generate unit tests for Encrypted Direct Message with the hardcoded keys ?

For unit tests it makes sense to hard-code keys and iv so that the expected cipher text can be compared with known values.

EncryptedDirectMessage.getPlaintext

I suppose the usage model for receive side is if kind=4 then cast the Event to EncryptedDirectMessage and call getPlaintext?

@ethicnology
Copy link
Owner

@no-prob

You can take a look at my refactoring of your #26 –> 6427f60

@ethicnology
Copy link
Owner

more changes for #26 –> 0ca4cc8

@ethicnology
Copy link
Owner

ethicnology commented Apr 20, 2023

rename/reorganize -> 75a9731

I tried to make it as clean and readable:

@no-prob remaining work for us before PR into main:

  • improve code coverage
  • example/nip_004.dart
  • README.md

@ethicnology
Copy link
Owner

@no-prob
#26 (comment)

I will write the rest of the mentioned test cases now, the ones mentioned in #15 (comment)

We only need test case for these line shown in red: https://app.codecov.io/gh/ethicnology/dart-nostr/blob/develop/lib/src/nips/nip_004.dart

@ethicnology
Copy link
Owner

@no-prob Do you still want to do these tasks?

@ntheden
Copy link
Contributor

ntheden commented Apr 28, 2023

@no-prob Do you still want to do these tasks?

Hi, I won't be making new PR's until I get the app that depends on this library stable. Thanks

@ethicnology
Copy link
Owner

@no-prob I feel you, I'm working/maintaining this project but i have no apps that depends on it.

I will wait for @ryzizub review to finish the PR.

@ryzizub
Copy link
Contributor

ryzizub commented Apr 30, 2023

@ethicnology I can look into it next week

@hazeycode
Copy link
Contributor

hazeycode commented May 21, 2023

FWIW there are problems with NIP-04 and as such it is not recommended. There are ongoing efforts to supersede with a more secure specification for private messaging. See nostr-protocol/nips#107

@ntheden
Copy link
Contributor

ntheden commented May 23, 2023

FWIW there are problems with NIP-04 and as such it is not recommended. There are ongoing efforts to supersede with a more secure specification for private messaging. See nostr-protocol/nips#107

Yeah, we will want to implement the better messaging NIPs. NIP 4 is just to get started.

@ethicnology
Copy link
Owner

ethicnology commented May 25, 2023

answer from @jonasschnelli

I guess this is a fair observation. While the NIP04 encryption itself is missing a MAC, the envelope signature done by the same key might provide a similar result (but again, this is cooking your own crypto scheme and should be reviewed carefully).

Marked as deprecated in 44fd3f2 to warns users about controversial discussions regarding this NIP.

@ntheden
Copy link
Contributor

ntheden commented May 25, 2023

I would have chosen to mark it deprecated when there is a working NIP to replace it but not before that.

@ethicnology
Copy link
Owner

The annotation deprecated is a shorthand for deprecating until an unspecified "next release" without migration instructions.

I decided to mark it deprecated so that users of the library are aware of the discussions for the future replacement of this feature, but they can still use it until the "unspecified next release".

@ethicnology
Copy link
Owner

merged in #30

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

5 participants