nostr-protocol / nips

Nostr Implementation Possibilities
2.19k stars 519 forks source link

NIP-04 considered harmful #107

Open adiabat opened 1 year ago

adiabat commented 1 year ago

Hi - just nostr seems promising, and end to end encrypted communication is a very important part of it, but the NIP-04 spec as written should not be implemented.

There is another issue here: https://github.com/nostr-protocol/nips/issues/72 pointing out the non-uniform AES key. That issue has been closed but probably shouldn't be -- the spec still has the unhashed / truncated result of the DH as the AES key.

Another, I think more severe problem is that protocol as described uses aes-256-cbc with no message authentication. This means that messages can be undetectably altered in transit. Anyone relaying the message can change the message, and the receiver can't tell that it was changed. I would remove NIP-04 before people start trying to use it.

There are other encryption protocols that are used with the secp256k1 keys used in nostr that could be adapted for direct messages, such as BIP324 (https://github.com/bitcoin/bips/pull/1378) or lightning's bolt 8 (https://github.com/lightning/bolts/blob/master/08-transport.md). There are also ratcheting protocols which have forward secrecy like the one used by signal, but those have other trade-offs.

The BIP324 and bolt 8 protocols aren't for discrete messages; they are for encrypted & authenticated communication channels. That means it doesn't directly replace NIP-04; the bolt 4 (https://github.com/lightning/bolts/blob/master/04-onion-routing.md) onion messages would be a closer fit. But encrypted transport between nodes is also important.

The code from BIP324 and LN is available in different languages and open licenses so I think that's the best bet for getting some code that's been used and tested.

paulmillr commented 1 year ago

@fyookball I think hkdf is good. For this particular case it may not really matter, but I would go through with a solution that's used by other ECIES libraries; in order to not reinvent the wheel. I mean, if everyone else is using hkdf, makes sense to use it. If everyone else is using sha256, makes sense to use sha. etc

Other than that, your stuff looking good.

earonesty commented 1 year ago

apple uses ANSI X9.63 with SHA-256 as the hash

function X963KDF(sharedSecret, sharedInfo, keySize=32){
  var maxCount = Math.ceil(keySize/digestLen);
  var result = Buffer.allocUnsafe(0);
  for (var count = 1; count < maxCount + 1; count++){
      var counter = Buffer.allocUnsafe(4);
      counter.writeUInt32BE(count, 0);
      var current = Buffer.concat([sharedSecret, counter, sharedInfo]);
      var hash = crypto.createHash(digest).update(current).digest();
      result = Buffer.concat([result, hash]);
  }

since keysize==digestlen, there's only one loop, and there is no sharedInfo. So ... it's just a hash with a fixed salt. Using the iv as a salt would be better. the reason for [edited: complex/big/slow] kdfs is to turn a password->key. in our case we have a 16 byte random iv and 32 byte arbitrary coordinate. simply hashing those two should be fine, pretending that the "salt" is any more than a prefix is odd and imo pretends at security that doesn't exist. im cool with using the iv as a prefix/salt - we have it anyway. speed is also important here.

paulmillr commented 1 year ago

the reason for kdfs is to turn a password->key

@earonesty That's false. You are talking about a subset of all KDFs: specifically PBKDF2, Scrypt and Argon.

earonesty commented 1 year ago

sure, those are stretchers, but my point is, we're starting with a great source of randomness and nearly random point. and you can see from the "kdf" above, that it's really "just a hash" in our case. (hash of all the things we have in common). kdfs (of the kind you're talking about) are great when the hash output size != key input size. but i dont really care. KDF(IV, Shared Secret) is a great. it's just not any stronger or more secure than HASH(IV, SharedSecret) ..

paulmillr commented 1 year ago

apple uses ANSI X9.63 with SHA-256 as the hash

Apple does not implement secp256k1 anywhere, which means it's not relevant for nostr.

Relevant implementations:

we're starting with a great source of randomness and nearly random point

Cool, which means it's good enough for HKDF. Shitty source of randomness would make stuff like pbkdf2 more relevant.

HKDF is important, it provides kdf security, which is not the same as PRF security.

See the blog post on proper usage of hkdf: https://soatok.blog/2021/11/17/understanding-hkdf/

earonesty commented 1 year ago

well, im totally cool with hkdf(sha(256 or 3), shared-secret, '', iv) but in that blog post you can see that uniform-random-decent-hash(iv,ss) is equivalent for input-key-len==output-key-len, and when there's no need for auth (no way to change the message here, so all the attacks it talks about are impossible)

im ok with wasting a little time for making people happy. sha3 is definitely the better choice here, imo. but i get that people like sha256

fyookball commented 1 year ago

Whatever you guys want; I don't mind re-doing the implementation. Would love to get something going in the short term as opposed to discussing it for a very long time. Doesn't have to be perfect. Is there any consensus at all?

earonesty commented 1 year ago

ok, after all the comments, my proposal:

mode 1: (Back compat, no hkdf, deprecated) fields: data?iv

mode 2: (hkdf(sha256, shared secret, iv)) fields: data?version?iv

mode 3: (hkdf(sha256, ephemeral secret, iv)) fields: data?version?iv?epub

that should be enough to satisfy everyone, and gives us 2 great ways to encrypt: directional and mutual

if people really think directional is useless, i'm ok with just adding the version field. at the very least that gives us a nice UX for future compatibility.

in the future we can consider allowing nip4 events with encrypted signatures, so an observer can't easily know who sent it

paulmillr commented 1 year ago

@fyookball so, regarding forward secrecy, how do you feel about ephemereal messages, which cannot be decrypted by sender?

earonesty commented 1 year ago

this is not tls, so is there a notion of "forward secrecy"? a user's private key should be able to unlock every message intended for him, for as long as he holds the key. isn't the whole point of nostr that a user can connect from any client and expect the same/similar experience? or, at least, that's why it's popular right now. forcing people to download messages and destroy the key to access them, and lock them in to clients or lose history isn't very nostr-like

we can use private key delegates to send messages that are no longer readable if the receiver discards an ephemeral key. but they would use a nip04 message to establish the delegate with an expiration, and then send the comms to the delegate key with an expiration. this is fine, for some use cases, but certainly isn't the "default". this relies on clients backing up historic messages, or discarding them

using ephemeral keys gets us asymmetric/directional encryption. it improves things. but it's not a solution, it's a tool - and it may only be useful for certain kinds of UX (why would i not want to see events i sent to someone? maybe no good reason?)

nip4 is, very specifically, a permanent encrypted message. if you want to build other protocols on top of it, use different event kinds and wrap things like ephemeral keys and group invitations inside a nip4-encrypted tag. nip4 is fine for what it is. and "forward secrecy" for private messages is neither useful nor desirable for many scenarios (that's not what ecies provides)

with the hkdf and the epub key, it provides for some good tools to build future nips on top of

fyookball commented 1 year ago

@fyookball so, regarding forward secrecy, how do you feel about ephemereal messages, which cannot be decrypted by sender?

I think we should focus on fixing the basic encryption first and then use that as a new message type to build on with fancier schemes, such as those involving ephemeral messages and/or keys.

I'm not really familiar with the scheme you have in mind... What I had suggested at one point is a fairly simple ephemeral key system where, if Alice wants to message Bob, she can send a nip04 (or similar) from an ephemeral key (lets call it "A2"), and in the message she provides another key (A3) which Bob can then message with an ephemeral key of his own, so you end up with A3 talking to B2 and the public only sees that an unknown key messaged Bob one time.

paulmillr commented 1 year ago

@fyookball i've actually meant the scheme you've suggested, with ephemereal keys, not full forward secrecy.

I'm just asking of having two different type of messages (ephemereal / permanent) is useful.

@earonesty

using ephemeral keys gets us asymmetric/directional encryption. it improves things. but it's not a solution, it's a tool

Right. So, maybe it's not worth implementing, at all?

I'm leaning towards having just one type of msg, which is ECDH+KDF+AES-GCM. It can then be extended with additional features.

Or maybe you're thinking of some cases where such messages are useful?

paulmillr commented 1 year ago

With regards to naming we already have precedent of NIP-08 which was deprecated in favor of NIP-27, so i'm thinking the new functionality should also have new NIP.

The question remains whether we want the NIP versioned, with encryption version specified in messages, which I would support.

fyookball commented 1 year ago

@paulmillr probably different message types are useful. One for just basic encryption, and one that is paired with some kind of ephemeral scheme (which could actually have more than 1 type).

If anyone wants to form an ad-hoc workgroup to chat about this, i'm open to it , either on telegram or nostr etc

paulmillr commented 1 year ago

@fyookball could you send me a dm? npub10jcnehsxwrjepupvh602pl83up0dh3wv3fqfwv062smygqvpeuwsk03kag

fyookball commented 1 year ago

@paulmillr sure, I pinged you on damus relay.

npub15m304ydnxud22gmvw5uujc2z8v2qh7stxrldvl0lf76etgf05acqsexhr3

earonesty commented 1 year ago

npub18mmjwlwqsuxgcp7lpmnxs2vjsvq7h9tc2u26zncr9t99xjrzhtsqwx4vcz

paulmillr commented 1 year ago

can't find how to DM someone in damus app and not seeing any dms...probably gotta built my own client using nip-04. Do you folks have any recommendations / cli tools for that?

fyookball commented 1 year ago

@paulmillr My client OstrichGram does NIP-04. It is using some dart code from nostr tools. See here: https://github.com/OstrichGram/OstrichGram/blob/main/ostrichgram/lib/nip04.dart

earonesty commented 1 year ago

nostr-tools does nip04 nicely. amethyst supports it. you can use nostr-tools to read amethyst dm's. https://github.com/vitorpamplona/amethyst

earonesty commented 1 year ago

i think there is a fundamental misunderstanding of nostr floating around

nostr is not a "messaging" system

it's a persistent store of messages. you can connect at any time and see a recent history of messages. that's why it took off. that's why you can move between one client and another and see all your dms. thats the beginning and end of why it works.

that's why we have nip33 (replaceable) and kind 0 (replaceable) . we know, instinctively, what it provides. uncensorable data.

thats why it works. if we keep thinking its a messaging system we're thinking about it wrong, and we can actually kill it with phrases like "considered harmful"

aside from a kdf on the secret, nip04 is a reasonable, secure and functioning way to create a persistent store of messages between two users. pitch me a better one!

if you want to send someone a message that they can never retrieve later when logging in on another client or a new phone (forward secrecy) ... that's another protocol, and i would argue it's a niche use case.

fyookball commented 1 year ago

@earonesty I think we're basically on the same page. I'm all for just doing some simple fixes (kdf, etc) to nip 04 as a starting point.

The idea of ephemeral keys is just to provide some basic privacy so others can't see who is messaging who. The point isn't to ensure "they can never retrieve it on another client" , although that would be a consequence of using new keys.

paulmillr commented 1 year ago

@fyookball

The idea of ephemeral keys is just to provide some basic privacy so others can't see who is messaging who. The point isn't to ensure "they can never retrieve it on another client" , although that would be a consequence of using new keys.

How is that going to happen?

A creates msg with ephemeral key to B and encrypts it to B public key. The whole network can see it was A who created the msg and B, msg recipient.

fyookball commented 1 year ago

@paulmillr I thought we discussed this earlier in the thread.

In the scheme I have in mind, if Alice wants to privately message Bob, she doesn't use her main key. She would use "A2", so all the public sees is "A2 messaged Bob", but they don't know who A2 is. Inside that message, she can secretly tell him "hey its me Alice" and even provide a signature from A1 (her main key) but that would be all encapsulated inside the encrypted message so no one would see that. And she would provide another key (A3) where she can replied to.

paulmillr commented 1 year ago

@fyookball understood now. So it's about encapsulation. Thanks.

jb55 commented 1 year ago

On Fri, May 26, 2023 at 04:57:23AM -0700, Paul Miller wrote:

can't find how to DM someone in damus app and not seeing any dms

you just go to someones profile and click the little message icon near the follow button

paulmillr commented 1 year ago

@jb55 when I enter public keys specified in the thread into search tab, no profile is found.

jb55 commented 1 year ago

I guess we should have some way to DM someone without a profile ...

earonesty commented 1 year ago

@paulmillr I thought we discussed this earlier in the thread.

In the scheme I have in mind, if Alice wants to privately message Bob, she doesn't use her main key. She would use "A2", so all the public sees is "A2 messaged Bob", but they don't know who A2 is. Inside that message, she can secretly tell him "hey its me Alice" and even provide a signature from A1 (her main key) but that would be all encapsulated inside the encrypted message so no one would see that. And she would provide another key (A3) where she can replied to.

that's cool, you use standard ecies to encrypt the message (epub), and then sign, in the envelope, with the pubkey it came from.

however, the problem with this is that

fyookball commented 1 year ago
  • alice now needs to hang on to a2 forever if she wants to see her outbox

Not necessarily. It could be deterministic based on her main key.

paulmillr commented 1 year ago

The concern with aes_256_gcm is that IVs are just 12 bytes and they MUST be unique for every message. Re-using IV will lead to msg leak. I see those options:

B seems to be better than others.

How many bits of timestamp do we use? One option is to use 64 bits, which is standard, and will allow to use 292M years. I suggest 48, which will limit max_year in nostr msgs to 10889, but increases our collision resistance of the remaining nonce part, taken from CSPRNG, from 2^-32 to 2^-48.

NIST 800-38d, chapter 8.2.1 says:

If desired, the fixed field itself may be constructed from two or more smaller fields. Moreover, one of those smaller fields could consist of bits that are arbitrary (i.e., not necessarily deterministic nor unique to the device), as long as the remaining bits ensure that the fixed field is not repeated in its entirety for some other device with the same key. Similarly, the entire fixed field may consist of arbitrary bits when there is only one context to identify, such as when a fresh key is limited to a single session of a communications protocol. In this case, if different participants in the session share a common fixed field, then the protocol shall ensure that the invocation fields are distinct for distinct data inputs. The invocation field typically is either 1) an integer counter or 2) a linear feedback shift register that is driven by a primitive polynomial to ensure a maximal cycle length. In either case, the invocation field increments upon each invocation of the authenticated encryption function. The lengths and positions of the fixed field and the invocation field shall be fixed for each supported IV length for the life of the key. In order to promote interoperability for the default IV length of 96 bits, this Recommendation suggests, but does not require, that the leading (i.e., leftmost) 32 bits of the IV hold the fixed field; and that the trailing (i.e., rightmost) 64 bits hold the invocation field.

// Idea B
const Mode = {
  aes_256_gcm: 0 // 2^32 (4B) messages per conversation, MUST use a unique IV for every msg
};
// Returns current time in Bytes format.
// If current time is `1685133644543` milliseconds,
// it will return `new Uint8Array([0x00, 0x18, 0x85, 0x9c, 0xad, 0x2f])`.
function getCurrentTimeInBytes() {
  const msec = Date.now();
  const hex = msec.toString(16).padStart(6 * 2, '0'); // 18859cad2ff => 0018859cad2ff
  const bytes = hexToBytes(hex);
  return bytes;
}
function generateNonce() {
  const nonce = new Uint8Array(12);
  nonce.set(0, crypto.getRandomValues(6)); // bits 0-48 come from CSPRNG
  nonce.set(6, getCurrentTimeInBytes()); // bits 48-96 come from timestamp
  return nonce;
}
function getConversationKey(privA, pubB) {
  const x = secp256k1.getSharedSecret(privA, pubB);
  return sha256(x); // must only contain 32b of X coordinate without additional parity byte
}
function encrypt_nip99(conversationKey, plaintext, mode = Mode.aes_256_gcm) {
  if (mode === Mode.aes_256_gcm) {
    const iv = generateNonce();
    const { ciphertext, mac } = aes_256_gcm.encrypt(plaintext, conversationKey, iv);
    return { ciphertext, mac, iv };
  }
  throw new Error('Unsupported nip99 mode: ' + mode);
}
function decrypt_nip99(conversationKey, ciphertextObj, mode = Mode.aes_256_gcm) {
  if (mode === Mode.aes_256_gcm) {
    const key = getSharedKey(receiverPriv, senderPub);
    const { ciphertext, mac, iv } = ciphertextObj;
    aes_256_gcm.verifyAuthTag(ciphertext, mac);
    const plaintext = aes_256_gcm.decrypt(ciphertext, conversationKey, iv);
    return plaintext;
  }
  throw new Error('Unsupported nip99 mode: ' + mode);
}

Any feelings about this one?

earonesty commented 1 year ago

we can use 16 bytes. no need to limit ourselves. 16 bytes is a uuid, we're good on random collisions then. not sure where those limits come from. is it a browser thing? openssl lets you go to 16 bytes easily. any scheme other than "secure random uuid" is a recipe for failure/attack.

anyway, we're not going to reuse the key, remember, you're rolling a new ephemeral pubkey on the sends. so technically, the iv doesn't matter. but also it costs nothing to roll 16 bytes, so we should do it anyway.

new protocol is, and i like it and i've implemented it and it works fine, ( i thought we all agreed to hkdf instead of just sha? when did that change?)

alice:

we definitely need to filter on version. so it has to be a 1-character tag if we use a tag. im ok with "v" for the version tag. or just "new kind per cryptosystem". i feel like if this is adopted as a standard we could have a vast proliferation, so i'm a fan of "v" being a simple integer and then having a standards list for each "v"

earonesty commented 1 year ago

the more i think about it, the more it's a security feature that this be a layered protocol. tags and kind need to be in-envelope (as above), and then we can move on. only version and epub outside.

i would put in the spec that all other encrypted systems should use the same "external kind". that way it's not possible to know if you're sending to a group chat (internal kind 122) or to an individual (internal kind 100), or posting an encrypted marketplace (internal kind 124) as a new item for sale. or just sending some plausibly deniable noise

if there are zero objections to this, lets write it up. the horse is dead, imo.

i would like to formalize that ["v", 1] is the 16-byte iv, hkdf(sha256), thing we all talked about (and vaguely agreed on) above.

i would love @vishalxl to comment on this, but i think he would agree

if there's zero objections i will write this up and we need to ask @fiatjaf to give us a NIP number and a KIND number. or is there some standard way to do that that isn't asking for permission. (i would love if the two numbers are the same, maybe 128 = 4x32)

earonesty commented 1 year ago
async nipXXEncrypt(pubkey: string, event: Event, version: number): Promise<NostrEvent> {
    if (version != 1)
      throw new Error('Method not implemented.');
    const content = JSON.stringify(event)
    const epriv = generatePrivateKey()
    const epub = getPublicKey(epriv)
    const key = secp256k1.getSharedSecret(epriv, '02' + pubkey)
    const normalizedKey = key.slice(1,33)
    const iv = randomBytes(16)
    const derivedKey = hkdf(sha256, normalizedKey, iv, undefined, 32);
    let plaintext = utf8Encoder.encode(content)
    let cryptoKey = await crypto.subtle.importKey(
      'raw',
      derivedKey,
      {name: 'AES-CBC'},
      false,
      ['encrypt']
    )
    let ciphertext = await crypto.subtle.encrypt(
      {name: 'AES-CBC', iv},
      cryptoKey,
      plaintext
    )
    let ctb64 = base64.encode(new Uint8Array(ciphertext))
    let ivb64 = base64.encode(new Uint8Array(iv.buffer))

    const unsigned = {
      kind: 99,
      content: ctb64 + "?" + ivb64,
      pubkey: epub,
      tags: [["p", pubkey], ["v", JSON.stringify(version)]]
    }
    return await this.signEventWith(unsigned, epriv, epub)
  }
paulmillr commented 1 year ago

@earonesty

we can use 16 bytes. no need to limit ourselves. 16 bytes is a uuid, we're good on random collisions then. not sure where those limits come from. is it a browser thing? openssl lets you go to 16 bytes easily. any scheme other than "secure random uuid" is a recipe for failure/attack.

No, we can't. GCM IV is 96 bits. https://crypto.stackexchange.com/questions/41601/aes-gcm-recommended-iv-size-why-12-bytes

earonesty commented 1 year ago

nope, we use 16 in nostr-tools, and it supports 16 in openssl. check it out! it works fine. look at the code. 12 bytes is recommended. not required. look at the "bootnote" on that response you posted. i use 16 regularly. so do most impls. openssl added it for a reason

longer nonce is not less secure in case nonces are generated randomly (which is by far the most common approach)

which is why we use 16 (and is why it's almost always terrible idea to use 12, unless you have a perfect counter) . honestly, i would just say "always use 16" and be done with the minor hit on perfect counter impls. the hit is all performance / efficiency. to hell with that, we need primitives that are safe. let performance people optimize at their own risk

earonesty commented 1 year ago

ok, i checked this. crypto subtle has a serious bug. so yes, iv's of 16 don't work. this doesn't matter much if we use the 16 bytes iv in the salt. we're back to uuid-level

openssl is smart about long iv's - adding the bytes after 12 to the box (see EVP_MAX_IV_LENGTH, and set params, you can specify a length of 16 and it does the right thing)

crypto subtle is not - it TRUNCATES the iv

we can fix this by always just using the iv as an input to the salt of the hkdf, and we should probably force-truncate to 12 bytes to fix nostr-tools interop with openssl

good thing we're switching to an hkdf! honestly, this is so broken rn. we should switch to the new algo asap, and not sit on thumbs.

earonesty commented 1 year ago

ok, new code for replacement for nip04 (bidirectional persisted encrypted content)


async nip04XEncrypt(privkey: string, pubkey: string, content: string, version: number) : Promise<string> {
    const key = secp256k1.getSharedSecret(privkey, '02' + pubkey)
    const normalizedKey = key.slice(1,33)
    const iv = randomBytes(16)
    const derivedKey = hkdf(sha256, normalizedKey, iv, undefined, 32);

    let plaintext = utf8Encoder.encode(content)
    let cryptoKey = await crypto.subtle.importKey(
      'raw',
      derivedKey,
      {name: 'AES-GCM'},
      false,
      ['encrypt']
    )

    let ciphertext = await crypto.subtle.encrypt(
      {name: 'AES-GCM', iv},
      cryptoKey,
      plaintext
    )

    let ctb64 = base64.encode(new Uint8Array(ciphertext))
    let ivb64 = base64.encode(new Uint8Array(iv.buffer))

    return ctb64 + "??" + ivb64 + "??" + version.toString()
  }

async nip04XDecrypt(privkey: string, pubkey: string, data: string): Promise<string> {
    let [ctb64, ivb64, version] = data.split('??')
    if (version != "1")
      throw Error("unknown version")

    let iv = base64.decode(ivb64)
    let ciphertext = base64.decode(ctb64)
    const key = secp256k1.getSharedSecret(privkey, '02' + pubkey)
    const normalizedKey = key.slice(1,33)

    const derivedKey = hkdf(sha256, normalizedKey, iv, undefined, 32);

    let cryptoKey = await crypto.subtle.importKey(
      'raw',
      derivedKey,
      {name: 'AES-CBC'},
      false,
      ['decrypt']
    )

    let plaintext = await crypto.subtle.decrypt(
      {name: 'AES-CBC', iv},
      cryptoKey,
      ciphertext
    )

    let text = utf8Decoder.decode(plaintext)

    return text
  }  ```

  this is the base .  it fixes all the problems and in the way we discussed.  the "1" is the version.  the code above is "directed" and it can use this as a primitive (remove version from tag, re-use this code)

  base layer/primitive arch is important for security too
earonesty commented 1 year ago

now look how nice the directed-blinded message is:

 async nipXXEncrypt(pubkey: string, inner: UnsignedEvent, version: number): Promise<NostrEvent> {
    const event = await this.signEvent(inner)
    const content = JSON.stringify(event)
    const epriv = generatePrivateKey()
    const epub = getPublicKey(epriv)
    const encrypted = await this.nip04XEncrypt(epriv, pubkey, content, version)
    const unsigned = {
      kind: 99,
      content: encrypted,
      pubkey: this.pubKey,
      tags: [["p", pubkey], ["v", JSON.stringify(version)]]
    }
    const signed = await this.signEventWith(unsigned, epriv, epub)
    return signed
  }
paulmillr commented 1 year ago

it fixes all the problems and in the way we discussed

No. It doesn't bring AES-GCM.

earonesty commented 1 year ago

it never re-uses the key, so we have no problem. derived key is new each time. and then the 12byte iv is fine

paulmillr commented 1 year ago

The whole point was to ditch CBC mode because it's shitty.

earonesty commented 1 year ago

was that the "whole point?" i thought the point was to make a safe low-level primitive that was easy to implement and worked in settings like browser and mobile. (we're using gcm not cbc, technically). do you have a recommendation that is better? we know cbc is shitty, we're not using it. gcm with a salted hkdf is fine. check the code above. i think we're clean now

paulmillr commented 1 year ago

That was the point. CBC is a can of worms.

The recommendation is, as i've mentioned several times, AES-256-GCM, or ChaCha20 / XChaCha20 Poly1305, but for now no need to complicate and we can just keep GCM.

Your code above is NOT using gcm - it's using CBC. And it's not verifying GCM mac.

earonesty commented 1 year ago

agreed! it was a typo. i knew we were ditching cbc. cool!. thanks for that (edited the post), crypto subtle will fail if the mac fails. so i think we're good there. plus we're already checking the sig. which means auth is not really important. how is the code now? i replaced ?iv= with "??" so we can have 3 (or more) fields in payload. 3rd field is version. prob we should flip it (version first)?

i think this should be 1 NIP with 2 kinds, "shared encrypted content", and "directed encrypted content", with 1 "internal kind" supported ("direct mesaage") that can be used

ie: kind 99 - > kind->501 -> dm ie: kind 98 - > old-style shared dm

the advantage of 98 is that content is persisted for both parties the advantage of 99 is that the sender doesn't reveal their public key

there are good reasons you may want both, i see no reason to discard one over the other. we should recommend clients using nip04 now should switch, immediately to the new kind 98. this requires little change to the underlying code and structure, and supports the same features and is strictly an improvement.

we can use 99 as the base-layer for encrypted group-chat and "derived-epub-key direct messages". the hard part with derived epub is that there's no "author tag" to scan for. so you're left looking at all messages sent to someone and checking which is yours. which can be prohibitive.

paulmillr commented 1 year ago

Is there any other way to pass iv? Maybe as a tag or something? Isn't that how they are used? I mean, encoding ciphertext to base64 and then appending ?iv=base64(iv) seems shaky.

paulmillr commented 1 year ago

from docs it seems like content can be an object.

So we should make it an object:

{
  "iv": base64(iv),
  "ciphertext": base64(ciphertext),
  "mac": base64(mac)
}
earonesty commented 1 year ago

In crypto subtle, the ciphertext is a combination of <actualCipherText><MAC>.
The MAC is appended as the last 16 bytes of the ciphertext and is always validated on decryption (missing mac==fail)

Having a key-value split is probably safer. we can't pass objects around as strings. so we'll have to encode them.

We can JSON.string({ { "i": base64(iv), "c": base64(ciphertext_with_mac), "v": version } })

instead of iv??cipher_mac??ver

instead, if you think that's nicer. again, mac is already in there. no need to deviate from that web/standard way of gcm'ing. i guess we could split it out and put it back again. but that feels like a recipe for making mistakes.

paulmillr commented 1 year ago

<actualCipherText><MAC> is not too bad. It's not the standard, but it's a standard. That's how webcrypto works -- others don't work this way.

OK, here's another crazy idea.

How about we ensure this new corrected scheme is group-chat-friendly? I don't think more features are needed, we can just ensure this works properly in group chat contexts. Signal group chats, AFAIK, are just a collection of 1-to-1 messages between every chat member.

earonesty commented 1 year ago

group-chat can use the nipXXEncrypt directed code (above) to send a privkey out as an invite. so the new standard is already friendly as a base-layer for group chat where all the invitations are blinded and the group itself is blinded.

indeed, that's what i'm using it for. fixing dm's is just part 1, ok, i think we're all on the same page finally. lets write up the base-layer (shared private message) with the versioned encryption scheme and make an NIP out of it. it should be a drop-in replacement for nip04

then we can make an NIP for the "generic encrypted message" layer, and then mention, in there, that group chat and other encrypted systems should, ideally, use this instead.