keygrip icon indicating copy to clipboard operation
keygrip copied to clipboard

encryption/decryption

Open jonathanong opened this issue 10 years ago • 19 comments

i have an initial implementation here: https://github.com/expressjs/keygrip/commit/427f18803c38bf13a260fc0e467f363ad2f280bf

Some notes/questions/etc.

  • Should we using initialization vectors? not sure how much security that adds. maybe make it optional.
  • want to change .cipher = to something else so we can do .cipher(message) and .decipher(message). .hash_algorithm= and .cipher_algorithm=?

without any feedback i'm going to just chug along

/cc @expressjs/owners

jonathanong avatar May 17 '14 23:05 jonathanong

Should we using initialization vectors?

node.js will derive the IV from the password, but it's not as secure. You should have both sides handshake to exchange an IV to use, but I don't think this is feasible with cookies, so it probably just has to be left out.

Basically if you leave it out, the same cleartext always encrypts to the same ciphertext. But of course if you let the user set it and it just stays the same, that doesn't do anything, because the cleartext will still be the same for the same ciphertext.

dougwilson avatar May 17 '14 23:05 dougwilson

Nonces in the cookie can help mix up the cyphertext. On May 17, 2014 7:13 PM, "Douglas Christopher Wilson" < [email protected]> wrote:

Should we using initialization vectors?

node.js will derive the IV from the password, but it's not as secure. You should have both sides handshake to exchange an IV to use, but I don't think this is feasible with cookies, so it probably just has to be left out.

Basically if you leave it out, the same cleartext always encrypts to the same ciphertext. But of course if you let the user set it and it just stays the same, that doesn't do anything, because the cleartext will still be the same for the same ciphertext.

— Reply to this email directly or view it on GitHubhttps://github.com/expressjs/keygrip/issues/14#issuecomment-43426755 .

defunctzombie avatar May 17 '14 23:05 defunctzombie

Nonces in the cookie can help mix up the cyphertext.

Make sure the nonce is at the start of the cleartext if you do this, since changes cascade from start to end :)

dougwilson avatar May 17 '14 23:05 dougwilson

okay updated it with optional iv support. let me know what you guys think before i release 2.0.0

@dougwilson your code coverage magic would be appreciated :)

jonathanong avatar Jun 04 '14 06:06 jonathanong

What's the reasoning behind removing the encoding option? Just wondering, not sure if it actually mattered now.

Fishrock123 avatar Jun 04 '14 13:06 Fishrock123

@dougwilson your code coverage magic would be appreciated :)

Sure, can do :)

dougwilson avatar Jun 04 '14 13:06 dougwilson

What's the reasoning behind removing the encoding option?

The output from this should be opaque to the user; allowing them to say if they want it in hex or base64 is weird; they should just treat it as an opaque string i.m.o.

dougwilson avatar Jun 04 '14 13:06 dougwilson

Fair enough.

Fishrock123 avatar Jun 04 '14 13:06 Fishrock123

Actually, @Fishrock123 re-looking at the new docs, it looks like we're just returning Buffer objects directly, which would be the real reason the encoding was removed, haha, since it's no longer stringifying the Buffers.

dougwilson avatar Jun 04 '14 14:06 dougwilson

yeah trying to make this a more generic crypto library. also, encoding is a hassle when you've got a bunch of other stuff to deal with.

jonathanong avatar Jun 04 '14 15:06 jonathanong

@jonathanong I have some questions about this library's interface when you have some time; specifically regarding encodings.

dougwilson avatar Jul 06 '14 20:07 dougwilson

What is holding this back?

emilbayes avatar Aug 04 '14 10:08 emilbayes

What is holding this back?

There are a couple encoding issues I still need to fix (non-Latin1 chars are not stored correctly and can thus become corrupted, or unreadable by other languages), some cleanups, and documentation work.

Also @tixz since you seem interested, if you want, you can start trying it out with npm install expressjs/keygrip and give us feedback as basically a pre-release, if you want :)

dougwilson avatar Aug 04 '14 14:08 dougwilson

Done! I was doing essentially what Keygrip does in a private repo, but without hiding keys in a closure and without hardening agains timing attacks. Therefore I already knew the cause of the bug :sunny:

emilbayes avatar Aug 04 '14 15:08 emilbayes

Wow, @tixz you really stepped up there, I was not expecting that, that was awesome! :palm_tree:

dougwilson avatar Aug 04 '14 15:08 dougwilson

Ok, PR has been merged and I'm in the process of clean up and prep for pushing a 2.0.0.

dougwilson avatar May 10 '15 05:05 dougwilson

@dougwilson anything i can help with?

jonathanong avatar Jul 10 '15 06:07 jonathanong

Let's revive this ;-) After implementing this feature in https://github.com/expressjs/cookie-session/pull/141, what is missing for this feature to land in master?

mircohacker avatar Aug 07 '20 09:08 mircohacker

@dougwilson what is holding this back?

hcldan avatar Sep 19 '22 14:09 hcldan