Cookie signatures lose attributes upon re-signing, causing persistent cookie to become invalid

[jspilman]

When the .sig cookie is set the first time, you have all the 'opts' from the base cookie, and they propagate into the .sig cookie. But when re-signing due to expired key, the 'opts' simply aren't available, because all you have is the name=value that the client browser sends back.

Specifically… starting at Line 54 – we push the base cookie, then update the same object to become a signature, and push it. This should make both cookies share all the same options:

   headers = pushCookie(headers, cookie)
   if (opts && signed) {
     cookie.value = this.keys.sign(cookie.toString())
     cookie.name += ".sig"
     headers = pushCookie(headers, cookie)
   }

But up in the ‘get’ method, if we detect the signing key index was not zero on Line 34:

    index && this.set(sigName, this.keys.sign(data), { signed: false })

here all we have is name and value. The cookie we’re looking at could have been configured months ago, all the domain/path/expires/etc. settings are long gone from the server.

The largest impact I think is for persistent signed cookies. If the persistent cookie is re-signed due to a new key, the signature will always expire the next time the user closes their browser. since no ‘expire’ is set. Then next time we do a ‘get’ on the base cookie, the signature will be gone completely, and it will come back as having a invalid signature.

[jed]

hmm. this is a good point. i'm not sure there's much we can do other than giving a heads-up in the README. what do you think?

[jspilman]

The only way to deal with it transparently is to encode the options in the .sig, and then add another signature of the options as well.

 if (signed) {
  cookie.value = cookie.toSig();
  cookie.name += ".sig"
  headers = pushCookie(headers, cookie)
}

   toSig: function() {
      var options = JSON.stringify({ path: this.path, expires: this.expires, domain: this.domain, secure: this.secure })
      return JSON.stringify({
          sig: sign(cookie.toString()),
          opts: options,
          optsSig: sign(options)
      })
  }

Then you would have to pull out the ‘opts’ from the .sig cookie and use them when re-creating the .sig with a new signing key to keep the settings consistent.

Maybe not so bad when it’s all said and done.

[jed]

good point, @jspilman. i'm a little queasy about bloating the scope of the .sig cookie with this much state, to be honest. perhaps we'd be better of with opt-in defaults for a specific cookie implementations?

[HugoDellinger]

This bug has been causing us quite a lot of troubles. I think this should be fixed as soon as possible.

[rchampeimont]

This problem has caused trouble in our production website. We use expressjs/cookie-session (which uses this module as a dependency) and set the cookie to expire in 1 year (we have a server-side system to expire sessions). We rotated the key but then the session.sig gets to expire at browser close instead of 1 year when uses comes back and session is re-signed. Then uses closes his browser and comes back and is logged out (which we don't want). We have taken several hours to find the cause of this issue and finally found this.