Separate xsrf handling and version checking

Traditionally we've relied on the kbn-version header for csrf
protection, but that is also used for the client-side Kibana version
check that alerts users when their client is out of date and needs to be
refreshed, so it must match the version of Kibana exactly.

This poses a problem for any programmatic access that would only get set
up once but may run repeatedly throughout the future (e.g. watcher), so
there's now the additional option of specifying the kbn-xsrf header
instead. The value of the header does not matter, but its very presence
is all that is necessary to avoid xsrf attacks.

The xsrf protection just needs either one of these headers to exist.

src/server/http/__tests__/version_check.js

@@ -0,0 +1,76 @@
+import expect from 'expect.js';
+import { fromNode } from 'bluebird';
+import { resolve } from 'path';
+import * as kbnTestServer from '../../../../test/utils/kbn_server';
+
+const src = resolve.bind(null, __dirname, '../../../../src');
+
+const versionHeader = 'kbn-version';
+const version = require(src('../package.json')).version;
+
+describe('version_check request filter', function () {
+  function makeRequest(kbnServer, opts) {
+    return fromNode(cb => {
+      kbnTestServer.makeRequest(kbnServer, opts, (resp) => {
+        cb(null, resp);
+      });
+    });
+  }
+
+  async function makeServer() {
+    const kbnServer = kbnTestServer.createServer();
+
+    await kbnServer.ready();
+
+    kbnServer.server.route({
+      path: '/version_check/test/route',
+      method: 'GET',
+      handler: function (req, reply) {
+        reply(null, 'ok');
+      }
+    });
+
+    return kbnServer;
+  };
+
+  let kbnServer;
+  beforeEach(async () => kbnServer = await makeServer());
+  afterEach(async () => await kbnServer.close());
+
+  it('accepts requests with the correct version passed in the version header', async function () {
+    const resp = await makeRequest(kbnServer, {
+      url: '/version_check/test/route',
+      method: 'GET',
+      headers: {
+        [versionHeader]: version,
+      },
+    });
+
+    expect(resp.statusCode).to.be(200);
+    expect(resp.payload).to.be('ok');
+  });
+
+  it('rejects requests with an incorrect version passed in the version header', async function () {
+    const resp = await makeRequest(kbnServer, {
+      url: '/version_check/test/route',
+      method: 'GET',
+      headers: {
+        [versionHeader]: `invalid:${version}`,
+      },
+    });
+
+    expect(resp.statusCode).to.be(400);
+    expect(resp.headers).to.have.property(versionHeader, version);
+    expect(resp.payload).to.match(/"Browser client is out of date/);
+  });
+
+  it('accepts requests that do not include a version header', async function () {
+    const resp = await makeRequest(kbnServer, {
+      url: '/version_check/test/route',
+      method: 'GET'
+    });
+
+    expect(resp.statusCode).to.be(200);
+    expect(resp.payload).to.be('ok');
+  });
+});

src/server/http/__tests__/xsrf.js

@@ -7,8 +7,9 @@ const nonDestructiveMethods = ['GET', 'HEAD'];
 const destructiveMethods = ['POST', 'PUT', 'DELETE'];
 const src = resolve.bind(null, __dirname, '../../../../src');
 
-const xsrfHeader = 'kbn-version';
-const version = require(src('../package.json')).version;
+const xsrfHeader = 'kbn-xsrf';
+const versionHeader = 'kbn-version';
+const actualVersion = require(src('../package.json')).version;
 
 describe('xsrf request filter', function () {
   function inject(kbnServer, opts) {
@@ -57,59 +58,60 @@ describe('xsrf request filter', function () {
         else expect(resp.payload).to.be('ok');
       });
 
-      it('failes on invalid tokens', async function () {
+      it('accepts requests with the xsrf header', async function () {
         const resp = await inject(kbnServer, {
           url: '/xsrf/test/route',
           method: method,
           headers: {
-            [xsrfHeader]: `invalid:${version}`,
+            [xsrfHeader]: 'anything',
           },
         });
 
-        expect(resp.statusCode).to.be(400);
-        expect(resp.headers).to.have.property(xsrfHeader, version);
+        expect(resp.statusCode).to.be(200);
         if (method === 'HEAD') expect(resp.payload).to.be.empty();
-        else expect(resp.payload).to.match(/"Browser client is out of date/);
+        else expect(resp.payload).to.be('ok');
       });
     });
   }
 
   for (const method of destructiveMethods) {
     context(`destructiveMethod: ${method}`, function () { // eslint-disable-line no-loop-func
-      it('accepts requests with the correct token', async function () {
+      it('accepts requests with the xsrf header', async function () {
         const resp = await inject(kbnServer, {
           url: '/xsrf/test/route',
           method: method,
           headers: {
-            [xsrfHeader]: version,
+            [xsrfHeader]: 'anything',
           },
         });
 
         expect(resp.statusCode).to.be(200);
         expect(resp.payload).to.be('ok');
       });
 
-      it('rejects requests without a token', async function () {
+      // this is still valid for existing csrf protection support
+      // it does not actually do any validation on the version value itself
+      it('accepts requests with the version header', async function () {
         const resp = await inject(kbnServer, {
           url: '/xsrf/test/route',
-          method: method
+          method: method,
+          headers: {
+            [versionHeader]: actualVersion,
+          },
         });
 
-        expect(resp.statusCode).to.be(400);
-        expect(resp.payload).to.match(/"Missing kbn-version header/);
+        expect(resp.statusCode).to.be(200);
+        expect(resp.payload).to.be('ok');
       });
 
-      it('rejects requests with an invalid token', async function () {
+      it('rejects requests without either an xsrf or version header', async function () {
         const resp = await inject(kbnServer, {
           url: '/xsrf/test/route',
-          method: method,
-          headers: {
-            [xsrfHeader]: `invalid:${version}`,
-          },
+          method: method
         });
 
         expect(resp.statusCode).to.be(400);
-        expect(resp.payload).to.match(/"Browser client is out of date/);
+        expect(resp.payload).to.match(/"Request must contain an kbn-xsrf header/);
       });
     });
   }

src/server/http/index.js

@@ -5,6 +5,8 @@ import fs from 'fs';
 import Boom from 'boom';
 import Hapi from 'hapi';
 import getDefaultRoute from './get_default_route';
+import versionCheckMixin from './version_check';
+
 module.exports = async function (kbnServer, server, config) {
 
 
@@ -132,5 +134,7 @@ module.exports = async function (kbnServer, server, config) {
     }
   });
 
+  kbnServer.mixin(versionCheckMixin);
+
   return kbnServer.mixin(require('./xsrf'));
 };

src/server/http/version_check.js

@@ -0,0 +1,19 @@
+import { badRequest } from 'boom';
+
+export default function (kbnServer, server, config) {
+  const versionHeader = 'kbn-version';
+  const actualVersion = config.get('pkg.version');
+
+  server.ext('onPostAuth', function (req, reply) {
+    const versionRequested = req.headers[versionHeader];
+
+    if (versionRequested && versionRequested !== actualVersion) {
+      return reply(badRequest('Browser client is out of date, please refresh the page', {
+        expected: actualVersion,
+        got: versionRequested
+      }));
+    }
+
+    return reply.continue();
+  });
+}

src/server/http/xsrf.js

@@ -1,21 +1,21 @@
 import { badRequest } from 'boom';
 
 export default function (kbnServer, server, config) {
-  const version = config.get('pkg.version');
   const disabled = config.get('server.xsrf.disableProtection');
-  const header = 'kbn-version';
+  const versionHeader = 'kbn-version';
+  const xsrfHeader = 'kbn-xsrf';
 
   server.ext('onPostAuth', function (req, reply) {
-    const noHeaderGet = (req.method === 'get' || req.method === 'head') && !req.headers[header];
-    if (disabled || noHeaderGet) return reply.continue();
+    if (disabled) {
+      return reply.continue();
+    }
+
+    const isSafeMethod = req.method === 'get' || req.method === 'head';
+    const hasVersionHeader = versionHeader in req.headers;
+    const hasXsrfHeader = xsrfHeader in req.headers;
 
-    const submission = req.headers[header];
-    if (!submission) return reply(badRequest(`Missing ${header} header`));
-    if (submission !== version) {
-      return reply(badRequest('Browser client is out of date, please refresh the page', {
-        expected: version,
-        got: submission
-      }));
+    if (!isSafeMethod && !hasVersionHeader && !hasXsrfHeader) {
+      return reply(badRequest(`Request must contain an ${xsrfHeader} header`));
     }
 
     return reply.continue();