tj / node-cookie-signature Goto Github PK

https://github.com/visionmedia/node-cookie-signature/blob/master/test/index.js#L14

val.should.not.equal('hhello.DGDUkGlIkCzPz+C0B064FNgHdEjox7ch8tOBGslZ5QI');

hhello is an h to many.

Hi Team,
at the #L42 val and mac are already equal strings (in case signing was successful), should they be signed anyway before equallity check?
Cheers, Alex

When you stringify a string unsigning doesnt work:

works
const val = 'whatever'
const signedCookie = cookie.sign(val, 'secret')
cookie.unsign(signedCookie, 'secret')

doesn't work
const val = JSON.stringify('whatever')
const signedCookie = cookie.sign(val, 'secret')
cookie.unsign(signedCookie, 'secret')

This doesn't make sense because many cookie values need to be stringified especially when they have object values. Is there anway for this to work with stringified text?

Please add a separate License file as it makes the oss automation easy.

Hi, I'm trying to load something like this in a Cloudflare Worker environment. I got something working that selectively loads either the Node.js or the Browser crypto implementation (which is available in a worker), would you be interested in this functionality within this package? With esm it's like:

const nodeHash = async message => {
  const crypto = await import("crypto");
  return crypto
    .createHash("sha256")
    .update(message)
    .digest("hex");
};

const browserHash = async message => {
  const msgUint8 = new TextEncoder().encode(message);
  const hashBuffer = await crypto.subtle.digest("SHA-256", msgUint8);
  const hashArray = Array.from(new Uint8Array(hashBuffer));
  return hashArray.map(b => b.toString(16).padStart(2, "0")).join("");
};

export const hash = typeof crypto === "undefined" ? nodeHash : browserHash;

I would convert it to ES5/commonjs for a PR and convert it to e.g. the base64 and hmac() methods used in node-cookie-signature.

Or any other of the allowed types for the key parameter: <string> | <Buffer> | <TypedArray> | <DataView> | <KeyObject>

Cryptographically secure keys are usually binary encoded, converting them to strings is not always possible.

This is a request that the sign function be exported so it can be used by services that want to generate a cookie (like creating and driving a headless browser session for screenshot/PDF generation). Currently, we're doing this by having our own function that matches the behavior, but this is fragile and it would be nice to be able to use the actual function.

Follow on from this issue: expressjs/session#805

Hi,

Nice piece of code. Just by curiosity i can't understand why you remove the characters "+" and "=" in your code :
https://github.com/tj/node-cookie-signature/blob/master/index.js#L23

A) Why doesn't you replace the "+" character them by some "safe" characters like "-" and "_" for a base64 url safe mecanisum (the = is for the padding so mostly useless) ?
B) Why doesn't you remove the character "/" that can happen in a base64 string.
C) Does the fact to remove thoses characters improve security ?

Thanks and keep up the good work.

https://nodejs.org/dist/latest-v20.x/docs/api/crypto.html#cryptosubtle

CC/ @natevw

When I attempt the Example, TypeError came up on my screen at unsign function.

timingSafeEqual is already implemented in docs

Does somebody know how resolve this error?

my envs => cookie-signature: v1.2.0, Node: v12.19.0, react: 16.14.0, Next.js: v10

var cookie = require('cookie-signature');

var val = cookie.sign('hello', 'tobiiscool');
val.should.equal('hello.DGDUkGlIkCzPz+C0B064FNgHdEjox7ch8tOBGslZ5QI');

var val = cookie.sign('hello', 'tobiiscool');
cookie.unsign(val, 'tobiiscool').should.equal('hello'); <- TypeError at unsign
cookie.unsign(val, 'luna').should.be.false;

The signature is cryptographically random, so why not just compare the received signature to the expected signature? That way timing attacks are only being done against randomized values, and there is no need to perform two computationally expensive SHA1 hashes for each cookie.

According to Wikipedia, a birthday attack on SHA-256 would require 4.8 * 10^35 different in-use signatures in order for the attacker to have a one-millionth percent chance of selecting an in-use signature. Those are pretty slim odds for even a highly improbable scenario.

Moreover, the currently in-use SHA1 hash maps the 256 cryptographically random bits of the SHA-256 signature (plus the data bits) into only 160 random bits, increasing the odds of collision available to an attacker.

It seems that comparing signatures directly may be sufficient to make timing attacks infeasible, while also greatly improving performance and reducing attack vulnerability. What am I missing?

(See also issue #15 discussion on dealing with timing attacks via SHA1.)

Do we support key rotation? That is

var cookie = require('cookie-signature');
var key = [keyForSetting, key1ForGetting, key2ForGetting, ...];

Also how to check if the cookie is tampered? Thank you

Hello,

our systems (in Debian and Ubuntu) tracks software in github via tags...

Can you please use them when release new software?

It seems tag "v1.0.5" is missing here.

Any reason not to also replace + and / (basically make it "base64-urlsafe")?

if try set a signed cookie with a number, the express app will be broked by an odd error:

TypeError: cookie required
at exports.sign (/Users/xxxxxx/node_modules/express/node_modules/cookie-signature/index.js:18:37)

but the cookie related modules are already installed.

in the source, the two lines are the reasons:

   if ('string' != typeof val) throw new TypeError('cookie required');
   if ('string' != typeof secret) throw new TypeError('secret required');

pls check out

Happened to take a peek at your code in the process of looking at an app using express-session.

https://github.com/tj/node-cookie-signature/blob/master/index.js#L42

Hashing the resulting hmac's doesn't increase the security, in fact it could probably be argued it lowers it as you are taking a sha256 hmac and making it rely on the weaker sha1 hash. I don't think this leads to an exploitable bug, but you should really just compare the resulting mac's directly to each other without messing with them.

edit: Read the rationalizations on the other closed tickets for people who pointed out sha1

Appreciate it is somewhat new, but any reason not to use crypto.timingSafeEqual() and get rid of the sha1 to avoid any timing side-channels?

https://nodejs.org/api/crypto.html#crypto_crypto_timingsafeequal_a_b

This bit of code is a potential maintenance issue:

var str = val.slice(0, val.lastIndexOf('.'))
    , mac = exports.sign(str, secret);

We have three variables val, str, mac involved in some important logic and their names are confusing.

• val is the incoming untrusted (but probably signed) input, e.g. "iamroot.f5da773d717015ffd4ffa4d0f9a87ed57b30811a", or "iamroot.b629a45f1f6175ddcddfbaac08dd2562" or "will.i.am" or "moe" or "\\"><script>alert('gotcha')</script>" or "" [or in a buggy app 42 or undefined or [Object object]]
• str is the value we would like to return, iff it was signed by us, e.g. "iamroot"
• mac is what val should be, if str were signed by us, e.g. "iamroot.f5da773d717015ffd4ffa4d0f9a87ed57b30811a"

Maybe rename these something like:

• val → input
• str → tentativeResult
• mac → originalOutput [???]

Not urgent.

Hello.

After a few weeks ago, when I do npm outdated get me 1.0.7 as latest package, but I have 1.2.1

package.json

...
"cookie-signature": "^1.2.1",
...

I think it is no correct

fd9b451#commitcomment-3015126 stops Etherpad sessions from functioning

Discovered through a series of bisects documented at ether/etherpad-lite#1738

Hi dear team.
Thank you for the amazing project!

This is my reproduce example:

const cookie = require('cookie-signature');
const value = 'value';
const secret = 'secret';
const signed = cookie.sign(value, secret);
const result = cookie.unsign(signed + 'some', secret);
console.log(result)

I expected to see a false result. But got a 'value'.

Could you use the buffer-from ponyfill so the library becomes compatible with older Node versions (from node 0.10)?

I'd like to know why use "sha1(mac) == sha1(val)" instead of "mac == val" at line 42 in index.js.
Is this some kind of security issue? Is it necessary to use sha1 here?

A bit of recent activity here (posed as a feature request but after reading more carefully I think just needed some help using existing functionality, or consider the older #33 idea for example, but regardless…) has me thinking about the maintenance of this library.

Context: I seem to have ± inherited it from TJ years ago, at least I am able to push to this repository (though not change settings on it) as well as publish releases on npm, and I seem to be the only one proactively responding to discussions here. So afaik I am in practice the only current maintainer.

This isn't a huge problem, since my vision for this particular library is that it remain a simple, stable, secure piece of reusable code. The easiest way to do that has been to simply not mess with it unless/until another security issue ever comes to light. So I answer the occasional question or address a maintenance issue every now and then but that's it. So I'm happy to keep doing that, but it's not an ideal situation for the users of this library.

• As an open source project it should be more responsive to the hopes and dreams of its users. Even if this library is already in an "actively finished" state it doesn't seem right for me to make that declaration unilaterally.
• as critical infrastructure it should have more formal review/release process, vulnerability disclosure point-of-contact, etc. etc.

I think it'd be good for this library to find a more "sustainable" home this yearone of these years!

IIUC, the https://github.com/senchalabs/connect project (and therefore also the https://github.com/expressjs/express project?) still rely on this library as a critical piece of their session-handling infrastructure. Assuming TJ would be on board, I wonder if this repository could be moved to one of those accounts and be maintained under the umbrella of one of those communities?

Is it possible to revisit my PR #25 ? This is affecting Express and showing that there is a vulnerability, and a lot of folks have their deployment process fail on a vulnerability from Snyk: https://snyk.io/vuln/npm:cookie-signature:20180111

I though @natevw we discussed this and that prior versions were not vulnerable? Has something changed here? Also cc @jodevsa @drewjenkins @wesleytodd

Greetings,

I see this repository is owned by TJ Holowaychuk but the copyright holder appears to be "LearnBoost" when one takes a look at the license info posted on the main page.

Is TJ Holowaychuck the copyright holder? or is LearnBoost, Inc.?

Thanks.
Brian

Naming conventions create confusion

having an unnamed function to becall as an object clouds the learning.

Most of the workshop is spent on the inaccurate naming of this workshop