Library should not throw an error when attempting to set a "Secure" cookie on insecure connection

[kevinburke]

I'm running a Koa app behind a load balancer that terminates SSL. I set app.proxy to true, and have these lines to enable secure session cookies:

const session = require('koa-session');
app.proxy = true;
app.use(session({secure: true}))

When the load balancer terminates SSL and sends through X-Forwarded-Proto: https requests, everything works fine and a secure cookie gets set.

However, any request without a X-Forwarded-Proto: https header generates the following stack trace:

Error: Cannot send secure cookie over unencrypted connection
    at Cookies.set (/Users/kevin/src/github.com/bigco/website/node_modules/koa/node_modules/cookies/lib/cookies.js:86:11)
    at Session.save (/Users/kevin/src/github.com/bigco/website/node_modules/koa-session/index.js:296:15)
    at commit (/Users/kevin/src/github.com/bigco/website/node_modules/koa-session/index.js:163:10)
    at Object.session (/Users/kevin/src/github.com/bigco/website/node_modules/koa-session/index.js:120:7)
    at session.next (<anonymous>)
    at Object.bodyParser (/Users/kevin/src/github.com/bigco/website/node_modules/koa-bodyparser/index.js:68:12)
    at bodyParser.next (<anonymous>)
    at onFulfilled (/Users/kevin/src/github.com/bigco/website/node_modules/koa/node_modules/co/index.js:65:19)
    at process._tickDomainCallback (internal/process/next_tick.js:135:7)

The offending code is here, in the cookies library:

    , secure = this.secure !== undefined ? !!this.secure : req.protocol === 'https' || req.connection.encrypted
    , cookie = new Cookie(name, value, opts)
    , signed = opts && opts.signed !== undefined ? opts.signed : !!this.keys

  if (typeof headers == "string") headers = [headers]

  if (!secure && opts && opts.secure) {
    throw new Error('Cannot send secure cookie over unencrypted connection')
  }

A response is that I should be performing http redirects at the load balancer layer and my Node app shouldn't handle http requests. Of course.

But there are a variety of reasons I might send requests without that header:

• my Node app needs to do custom logic to decide how to redirect HTTP requests to HTTPS
• the load balancer sends health checks and I can't configure the headers it sends, also it is not forwarding them for a health check
• automated tooling makes requests directly to an app host (say, a stats or expvar server)

Basically it doesn't seem like setting a secure cookie in an insecure environment should be an error, or that a library whose primary purpose is cookie setting should be trying to sniff out details about where and how I'm terminating TLS.

It's confusing that every request has to set X-Forwarded-Proto: https even when it's not forwarding the request from anywhere else. Other features that might be useless when set in some conditions usually don't cause the server to error. An HSTS header has no effect when sent on a plain HTTP response, but frameworks don't usually throw a server error if you try to set it.

[kevinburke]

Previously, previously, previously, previously, previously, previously, previously.

[dougwilson]

Set "secure: true" in the options and this module wont check if the request is secure.

[dougwilson]

From docs:

A Boolean can optionally be passed as options.secure to explicitally specify if the connection is secure, rather than this module examining request.

[kevinburke]

Okay, I guess I need to kick this back up to Koa then.

Here is how they set secure:

  context.cookies = new Cookies(req, res, {
    keys: this.keys,
    secure: request.secure
  });

here's how request.secure gets checked:

  get protocol() {
    var proxy = this.app.proxy;
    if (this.socket.encrypted) return 'https';
    if (!proxy) return 'http';
    var proto = this.get('X-Forwarded-Proto') || 'http';
    return proto.split(/\s*,\s*/)[0];
  },

  /**
   * Short-hand for:
   *
   *    this.protocol == 'https'
   *
   * @return {Boolean}
   * @api public
   */

  get secure() {
    return 'https' == this.protocol;
  },

So it doesn't seem like I can really configure that.

[kevinburke]

I'm currently reading through the source code for webpack-dev-server because I am specifying the --https flag for that library, and loading content over HTTPS, but this.socket.encrypted is still being set to false.

[dougwilson]

Gotcha. The reason the original author added the sniffing is because it was really easy for people to accidentally send out secure cookies over HTTP, which defeats the purpose because an attacker in the same coffee shop would be able to see the cookie's value now. After fixing up the detection a few times, I added a constructor option so you can simply override the detection to skip it if you desired.

For your configuration, where you have an HTTP app that would always be behind a SSL terminator, I would recommend just setting "secure: true" to always send the cookies instead of doing auto detection. That would make it easy to still get health checks and the like that a reverse proxy will attempt on the server, and makes it easy to just to quick cURL calls internally against your server.

[kevinburke]

Okay. The problem is that there's no easy way, as far as I can tell, to set secure: true in a Koa app.

[kevinburke]

See the issue linked there ^^ another place where I wanted to set a Secure cookie in an HTTP environment.

[dougwilson]

Yep, sounds just exactly the use case where you should be setting "secure: true" when you create the Cookies object :) then everything works very well.

[dougwilson]

So I'm not sure there is anything to change here, looking at the Koa code you linked. Koa is completely opting out of the auto detection this module does and is explicitly setting "secure: true" or "secure: false" based on it's own assessment. Perhaps Koa could add an option to override that behavior ?

[kevinburke]

Ah, yikes, I meant to leave my most recent comment on the Koa issue and ask them to reopen their ticket.