koajs / session

Simple session middleware for koa
MIT License
901 stars 113 forks source link

Are cookies actually being signed? #181

Open rnd256 opened 5 years ago

rnd256 commented 5 years ago

Using the example code in your readme, I can't find any trace of the app.keys 'some secret hurr' actually being used to sign the cookie.

I assumed it would be used to generate the koa:sess.sig cookie, but it looks like that's generated by simply stringifying the session's contents and running it through crc32 - A process that a malicious user could easily replicate after modifying the contents of the koa:sess cookie.

What am I missing?

jmitchell38488 commented 5 years ago

tl;dr the content of the cookie is not encoded, only the .sig when signed is used. Also, by default it uses the sha1 algorithm to generate the .sig cookie, which can be cracked in minutes.

keys is referenced in the koa module in lib/context.js:

  get cookies() {
    if (!this[COOKIES]) {
      this[COOKIES] = new Cookies(this.req, this.res, {
        keys: this.app.keys,
        secure: this.request.secure
      });
    }
    return this[COOKIES];
  },

It originally used the KeyGrip library to manage keys for signing, which is used by the module cookies to create the *.sig cookie. This behaviour was changed in koa in version 0.4.0:

* remove app.keys getter/setter, update cookies, and remove keygrip deps

The code above in context.js is the only reference in the koa module to keys, which is used whenever cookies are manipulated.

In the cookies module, this is used to determine if a cookie should be signed:

var signed = opts && opts.signed !== undefined ? opts.signed : !!this.keys

From the cookies api doc:

A Keygrip object or an array of keys can optionally be passed as options.keys to enable cryptographic signing based on SHA1 HMAC, using rotated credentials.

While koa doesn't use keygrip, cookies does.

function Cookies(request, response, options) {
  if (!(this instanceof Cookies)) return new Cookies(request, response, options)
  ...
  if (options) {
    if (Array.isArray(options)) {
      this.keys = new Keygrip(options)
    } else if (options.constructor && options.constructor.name === 'Keygrip') {
      this.keys = options
    } else {
      this.keys = Array.isArray(options.keys) ? new Keygrip(options.keys) : options.keys
      ...
    }
  }
}

The signing method in keygrip:

  function sign(data, key) {
    return crypto
      .createHmac(algorithm, key)
      .update(data).digest(encoding)
      .replace(/\/|\+|=/g, function(x) {
        return ({ "/": "_", "+": "-", "=": "" })[x]
      })
  }

The cookie data itself that could store session data is not encrypted, just the .sig cookie is. Therefore, it's trivial to decode the signature method when:

The only signed aspect of the cookie is the .sig cookie, which by default is the sha1 hash, which can be cracked in minutes.

rnd256 commented 5 years ago

Thanks for the thorough details! I'm not particularly concerned about how trivial it is to parse the session cookie, but the .sig cookie using a sha1 hash by default seems like a serious issue. Most people will use the default and assume it's secure.

jmitchell38488 commented 5 years ago

I'm not particularly concerned about how trivial it is to parse the session cookie

Given the encrypted hash, and the raw data (which is just obfuscated in base64), it's trivial to determine the key. Signed cookies are inherently weak and offer no security benefit. Session ids are also not hashed or encrypted, so you can literally copy+paste cookies if you were to gain access to them.

There's also no origin checking on the cookies to prevent man in the middle, intercept, or even xss attacks if one were to gain access to the cookies.

That's not to say that a lot of the session management options out there are particularly strong either, they're all susceptible in one way or another.

zbo14 commented 4 years ago

@jmitchell38488

The only signed aspect of the cookie is the .sig cookie, which by default is the sha1 hash, which can be cracked in minutes.

Unless I'm seriously mistaken it's HMAC-SHA1, not just a SHA1 hash. Despite the collision vulnerability of SHA1, HMAC is still relatively secure assuming the secret keys are strong and an attacker has no knowledge of them.

This thread has a nice explanation though it's several years old. The second comment here is more recent, reiterates the point, and links to a nice infographic. If there's an attack on HMAC-SHA1 I'm not aware of, I'd be interested to hear.

All that being said, in an ideal world something like SHA256 would be the default algo in Keygrip.

rejetto commented 1 year ago

so, can we consider the tampering of the content of a cookie unfeasable? (once a decent key is provided)

I verified that Koa is actually checking the signature, making the session appear empty when a simple change is attempted to the content of the session (without updating the signature).

btw, I'm using a 30-chars random string conveying ~153 bits of actual randomness, generated at startup

rnd256 commented 1 year ago

so, can we consider the tampering of the content of a cookie unfeasable? (once a decent key is provided)

No, it seems alarmingly easy to tamper with these "signed" cookies. I wonder if the creator intended for them to be used alongside server-side session tracking in a DB? I bet a lot of people have been using these cookies as JWTs instead, which looks like it would be a critical security vulnerability for them (an attacker could brute force the signing key that's only protected by a SHA1 hash, change the userId in the cookie, re-sign it themselves, and get access to any account).

btw, I'm using a 30-chars random string conveying ~153 bits of actual randomness, generated at startup

If you generate it randomly at startup, then all your cookies will be invalidated every time the server restarts, and cookies won't work across multiple servers if you have more than one.

rejetto commented 1 year ago

an attacker could brute force the signing key that's only protected by a SHA1 hash

what do you think of the https://github.com/koajs/session/issues/181#issuecomment-584404285 saying that HMAC-SHA1 is actually used and that it is enough?

If you generate it randomly at startup, then all your cookies will be invalidated every time the server restarts, and cookies won't work across multiple servers if you have more than one.

correct. I'm accepting these limitations ATM.