mirror of
https://github.com/elastic/kibana.git
synced 2025-04-24 09:48:58 -04:00
Use proper Content-Type request header instead of kbn-version and kbn-xsrf for server CSRF check. (#12019)
This commit is contained in:
Aleh Zasypkin
GitHub
parent
f054b53a72
commit
fb40168305
2 changed files with 267 additions and 37 deletions
|
|
@ -1,14 +1,16 @@
|
|||
import expect from 'expect.js';
|
||||
import sinon from 'sinon';
|
||||
import { fromNode as fn } from 'bluebird';
|
||||
import { resolve } from 'path';
|
||||
import * as kbnTestServer from '../../../test_utils/kbn_server';
|
||||
|
||||
const nonDestructiveMethods = ['GET', 'HEAD'];
|
||||
const destructiveMethods = ['POST', 'PUT', 'DELETE'];
|
||||
const src = resolve.bind(null, __dirname, '../../../../src');
|
||||
|
||||
const xsrfHeader = 'kbn-xsrf';
|
||||
const versionHeader = 'kbn-version';
|
||||
const contentTypeHeader = 'content-type';
|
||||
const testPath = '/xsrf/test/route';
|
||||
const actualVersion = require(src('../package.json')).version;
|
||||
|
||||
describe('xsrf request filter', function () {
|
||||
|
|
@ -29,10 +31,23 @@ describe('xsrf request filter', function () {
|
|||
|
||||
await kbnServer.ready();
|
||||
|
||||
const routeMethods = nonDestructiveMethods.filter(method => method !== 'HEAD').concat(destructiveMethods);
|
||||
kbnServer.server.route({
|
||||
path: '/xsrf/test/route',
|
||||
method: routeMethods,
|
||||
path: testPath,
|
||||
method: 'GET',
|
||||
handler: function (req, reply) {
|
||||
reply(null, 'ok');
|
||||
}
|
||||
});
|
||||
|
||||
kbnServer.server.route({
|
||||
path: testPath,
|
||||
method: destructiveMethods,
|
||||
config: {
|
||||
// Disable payload parsing to make HapiJS server accept any content-type header.
|
||||
payload: {
|
||||
parse: false
|
||||
}
|
||||
},
|
||||
handler: function (req, reply) {
|
||||
reply(null, 'ok');
|
||||
}
|
||||
|
|
@ -42,43 +57,102 @@ describe('xsrf request filter', function () {
|
|||
};
|
||||
|
||||
let kbnServer;
|
||||
beforeEach(async () => kbnServer = await makeServer());
|
||||
afterEach(async () => await kbnServer.close());
|
||||
beforeEach(async () => {
|
||||
kbnServer = await makeServer();
|
||||
sinon.spy(kbnServer.server, 'log');
|
||||
});
|
||||
|
||||
for (const method of nonDestructiveMethods) {
|
||||
describe(`nonDestructiveMethod: ${method}`, function () { // eslint-disable-line no-loop-func
|
||||
it('accepts requests without a token', async function () {
|
||||
const resp = await inject(kbnServer, {
|
||||
url: '/xsrf/test/route',
|
||||
method: method
|
||||
});
|
||||
afterEach(async () => {
|
||||
await kbnServer.close();
|
||||
});
|
||||
|
||||
expect(resp.statusCode).to.be(200);
|
||||
if (method === 'HEAD') expect(resp.payload).to.be.empty();
|
||||
else expect(resp.payload).to.be('ok');
|
||||
describe(`nonDestructiveMethod: GET`, function () {
|
||||
it('accepts requests without a token', async function () {
|
||||
const resp = await inject(kbnServer, {
|
||||
url: testPath,
|
||||
method: 'GET'
|
||||
});
|
||||
|
||||
it('accepts requests with the xsrf header', async function () {
|
||||
const resp = await inject(kbnServer, {
|
||||
url: '/xsrf/test/route',
|
||||
method: method,
|
||||
headers: {
|
||||
[xsrfHeader]: 'anything',
|
||||
},
|
||||
});
|
||||
|
||||
expect(resp.statusCode).to.be(200);
|
||||
if (method === 'HEAD') expect(resp.payload).to.be.empty();
|
||||
else expect(resp.payload).to.be('ok');
|
||||
});
|
||||
expect(resp.statusCode).to.be(200);
|
||||
expect(resp.payload).to.be('ok');
|
||||
sinon.assert.notCalled(kbnServer.server.log);
|
||||
});
|
||||
}
|
||||
|
||||
it('accepts requests with the xsrf header', async function () {
|
||||
const resp = await inject(kbnServer, {
|
||||
url: testPath,
|
||||
method: 'GET',
|
||||
headers: {
|
||||
[xsrfHeader]: 'anything',
|
||||
},
|
||||
});
|
||||
|
||||
expect(resp.statusCode).to.be(200);
|
||||
expect(resp.payload).to.be('ok');
|
||||
sinon.assert.notCalled(kbnServer.server.log);
|
||||
});
|
||||
|
||||
it('accepts requests with any content-type header', async function () {
|
||||
const resp = await inject(kbnServer, {
|
||||
url: testPath,
|
||||
method: 'GET',
|
||||
headers: {
|
||||
[contentTypeHeader]: 'anything',
|
||||
},
|
||||
});
|
||||
|
||||
expect(resp.statusCode).to.be(200);
|
||||
expect(resp.payload).to.be('ok');
|
||||
sinon.assert.notCalled(kbnServer.server.log);
|
||||
});
|
||||
});
|
||||
|
||||
describe(`nonDestructiveMethod: HEAD`, function () {
|
||||
it('accepts requests without a token', async function () {
|
||||
const resp = await inject(kbnServer, {
|
||||
url: testPath,
|
||||
method: 'HEAD'
|
||||
});
|
||||
|
||||
expect(resp.statusCode).to.be(200);
|
||||
expect(resp.payload).to.be.empty();
|
||||
sinon.assert.notCalled(kbnServer.server.log);
|
||||
});
|
||||
|
||||
it('accepts requests with the xsrf header', async function () {
|
||||
const resp = await inject(kbnServer, {
|
||||
url: testPath,
|
||||
method: 'HEAD',
|
||||
headers: {
|
||||
[xsrfHeader]: 'anything',
|
||||
},
|
||||
});
|
||||
|
||||
expect(resp.statusCode).to.be(200);
|
||||
expect(resp.payload).to.be.empty();
|
||||
sinon.assert.notCalled(kbnServer.server.log);
|
||||
});
|
||||
|
||||
it('accepts requests with any content-type header', async function () {
|
||||
const resp = await inject(kbnServer, {
|
||||
url: testPath,
|
||||
method: 'HEAD',
|
||||
headers: {
|
||||
[contentTypeHeader]: 'anything',
|
||||
},
|
||||
});
|
||||
|
||||
expect(resp.statusCode).to.be(200);
|
||||
expect(resp.payload).to.be.empty();
|
||||
sinon.assert.notCalled(kbnServer.server.log);
|
||||
});
|
||||
});
|
||||
|
||||
for (const method of destructiveMethods) {
|
||||
describe(`destructiveMethod: ${method}`, function () { // eslint-disable-line no-loop-func
|
||||
it('accepts requests with the xsrf header', async function () {
|
||||
const resp = await inject(kbnServer, {
|
||||
url: '/xsrf/test/route',
|
||||
url: testPath,
|
||||
method: method,
|
||||
headers: {
|
||||
[xsrfHeader]: 'anything',
|
||||
|
|
@ -87,13 +161,20 @@ describe('xsrf request filter', function () {
|
|||
|
||||
expect(resp.statusCode).to.be(200);
|
||||
expect(resp.payload).to.be('ok');
|
||||
sinon.assert.calledOnce(kbnServer.server.log);
|
||||
sinon.assert.calledWith(
|
||||
kbnServer.server.log,
|
||||
['warning', 'deprecation'],
|
||||
`The ${xsrfHeader} header is deprecated and will be removed in a future version of Kibana.` +
|
||||
` Specify a ${contentTypeHeader} header of either application/json or application/x-ndjson instead.`
|
||||
);
|
||||
});
|
||||
|
||||
// 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',
|
||||
url: testPath,
|
||||
method: method,
|
||||
headers: {
|
||||
[versionHeader]: actualVersion,
|
||||
|
|
@ -102,16 +183,131 @@ describe('xsrf request filter', function () {
|
|||
|
||||
expect(resp.statusCode).to.be(200);
|
||||
expect(resp.payload).to.be('ok');
|
||||
sinon.assert.calledOnce(kbnServer.server.log);
|
||||
sinon.assert.calledWith(
|
||||
kbnServer.server.log,
|
||||
['warning', 'deprecation'],
|
||||
`The ${versionHeader} header will no longer be accepted for CSRF protection in a future version of Kibana.` +
|
||||
` Specify a ${contentTypeHeader} header of either application/json or application/x-ndjson instead.`
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects requests without either an xsrf or version header', async function () {
|
||||
it('accepts requests with any allowed media type', async function () {
|
||||
const allowedContentTypes = [
|
||||
'application/json',
|
||||
'application/x-ndjson',
|
||||
'application/x-ndjson;charset=UTF-8',
|
||||
'application/json;charset=UTF-8'
|
||||
];
|
||||
|
||||
for (const contentType of allowedContentTypes) {
|
||||
const resp = await inject(kbnServer, {
|
||||
url: testPath,
|
||||
method: method,
|
||||
headers: {
|
||||
[contentTypeHeader]: contentType,
|
||||
}
|
||||
});
|
||||
|
||||
expect(resp.statusCode).to.be(200);
|
||||
expect(resp.payload).to.be('ok');
|
||||
sinon.assert.notCalled(kbnServer.server.log);
|
||||
}
|
||||
});
|
||||
|
||||
it('accepts requests with any allowed media type, but warns if xsrf header is presented', async function () {
|
||||
const allowedContentTypes = [
|
||||
'application/json',
|
||||
'application/x-ndjson',
|
||||
'application/x-ndjson;charset=UTF-8',
|
||||
'application/json;charset=UTF-8'
|
||||
];
|
||||
|
||||
for (const contentType of allowedContentTypes) {
|
||||
const resp = await inject(kbnServer, {
|
||||
url: testPath,
|
||||
method: method,
|
||||
headers: {
|
||||
[contentTypeHeader]: contentType,
|
||||
[xsrfHeader]: 'anything',
|
||||
}
|
||||
});
|
||||
|
||||
expect(resp.statusCode).to.be(200);
|
||||
expect(resp.payload).to.be('ok');
|
||||
|
||||
sinon.assert.calledOnce(kbnServer.server.log);
|
||||
sinon.assert.calledWith(
|
||||
kbnServer.server.log,
|
||||
['warning', 'deprecation'],
|
||||
`The ${xsrfHeader} header is deprecated and will be removed in a future version of Kibana.`
|
||||
);
|
||||
|
||||
kbnServer.server.log.reset();
|
||||
}
|
||||
});
|
||||
|
||||
it('does not warn about version header if warned about xsrf header already', async function () {
|
||||
const resp = await inject(kbnServer, {
|
||||
url: '/xsrf/test/route',
|
||||
url: testPath,
|
||||
method: method,
|
||||
headers: {
|
||||
[contentTypeHeader]: 'plain/text',
|
||||
[xsrfHeader]: 'anything',
|
||||
[versionHeader]: actualVersion,
|
||||
}
|
||||
});
|
||||
|
||||
expect(resp.statusCode).to.be(200);
|
||||
expect(resp.payload).to.be('ok');
|
||||
|
||||
sinon.assert.calledOnce(kbnServer.server.log);
|
||||
sinon.assert.calledWith(
|
||||
kbnServer.server.log,
|
||||
['warning', 'deprecation'],
|
||||
`The ${xsrfHeader} header is deprecated and will be removed in a future version of Kibana.` +
|
||||
` Specify a ${contentTypeHeader} header of either application/json or application/x-ndjson instead.`
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects requests without either an xsrf, version header or acceptable content-type', async function () {
|
||||
const resp = await inject(kbnServer, {
|
||||
url: testPath,
|
||||
method: method
|
||||
});
|
||||
|
||||
expect(resp.statusCode).to.be(400);
|
||||
expect(resp.payload).to.match(/"Request must contain an kbn-xsrf header/);
|
||||
expect(resp.result.message).to.be(
|
||||
'Request must contain a content-type header of either application/json or application/x-ndjson.' +
|
||||
` The content-type header for current request is undefined.`
|
||||
);
|
||||
sinon.assert.notCalled(kbnServer.server.log);
|
||||
});
|
||||
|
||||
it('rejects requests with content-type that is not allowed', async function () {
|
||||
const notAllowedContentTypes = [
|
||||
'application/json-like',
|
||||
'application/x-www-form-urlencoded',
|
||||
'multipart/form-data; boundary=0',
|
||||
'text/plain;charset=UTF-8'
|
||||
];
|
||||
|
||||
for (const contentType of notAllowedContentTypes) {
|
||||
const resp = await inject(kbnServer, {
|
||||
url: testPath,
|
||||
method: method,
|
||||
headers: {
|
||||
[contentTypeHeader]: contentType,
|
||||
}
|
||||
});
|
||||
|
||||
expect(resp.statusCode).to.be(400);
|
||||
expect(resp.result.message).to.be(
|
||||
'Request must contain a content-type header of either application/json or application/x-ndjson.' +
|
||||
` The content-type header for current request is ${contentType}.`
|
||||
);
|
||||
sinon.assert.notCalled(kbnServer.server.log);
|
||||
}
|
||||
});
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -4,6 +4,8 @@ export default function (kbnServer, server, config) {
|
|||
const disabled = config.get('server.xsrf.disableProtection');
|
||||
const versionHeader = 'kbn-version';
|
||||
const xsrfHeader = 'kbn-xsrf';
|
||||
const contentTypeHeader = 'content-type';
|
||||
const allowedRequestMediaTypes = ['application/json', 'application/x-ndjson'];
|
||||
|
||||
server.ext('onPostAuth', function (req, reply) {
|
||||
if (disabled) {
|
||||
|
|
@ -11,11 +13,43 @@ export default function (kbnServer, server, config) {
|
|||
}
|
||||
|
||||
const isSafeMethod = req.method === 'get' || req.method === 'head';
|
||||
if (isSafeMethod) {
|
||||
// There is no need to verify XSRF for GET or HEAD requests.
|
||||
return reply.continue();
|
||||
}
|
||||
|
||||
const hasVersionHeader = versionHeader in req.headers;
|
||||
const hasXsrfHeader = xsrfHeader in req.headers;
|
||||
const hasContentTypeHeader = contentTypeHeader in req.headers;
|
||||
|
||||
if (!isSafeMethod && !hasVersionHeader && !hasXsrfHeader) {
|
||||
return reply(badRequest(`Request must contain an ${xsrfHeader} header`));
|
||||
// Since we're only interested in media type let's extract it from the content type ("media type [;parameter]")
|
||||
// and leave off parameter portion (e.g. charset) of the header.
|
||||
const hasAllowedMediaType = hasContentTypeHeader && allowedRequestMediaTypes.includes(
|
||||
req.headers[contentTypeHeader].split(';')[0]
|
||||
);
|
||||
|
||||
if (hasXsrfHeader) {
|
||||
let xsrfHeaderDeprecationMessage =
|
||||
`The ${xsrfHeader} header is deprecated and will be removed in a future version of Kibana.`;
|
||||
if (!hasAllowedMediaType) {
|
||||
xsrfHeaderDeprecationMessage +=
|
||||
` Specify a ${contentTypeHeader} header of either application/json or application/x-ndjson instead.`;
|
||||
}
|
||||
|
||||
server.log(['warning', 'deprecation'], xsrfHeaderDeprecationMessage);
|
||||
} else if (!hasAllowedMediaType && hasVersionHeader) {
|
||||
server.log(
|
||||
['warning', 'deprecation'],
|
||||
`The ${versionHeader} header will no longer be accepted for CSRF protection in a future version of Kibana.` +
|
||||
` Specify a ${contentTypeHeader} header of either application/json or application/x-ndjson instead.`
|
||||
);
|
||||
}
|
||||
|
||||
if (!hasAllowedMediaType && !hasVersionHeader && !hasXsrfHeader) {
|
||||
return reply(badRequest(
|
||||
`Request must contain a ${contentTypeHeader} header of either application/json or application/x-ndjson.` +
|
||||
` The ${contentTypeHeader} header for current request is ${req.headers[contentTypeHeader]}.`
|
||||
));
|
||||
}
|
||||
|
||||
return reply.continue();
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue