Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core.logging] Add RewriteAppender for filtering LogMeta. #91492

Merged
merged 29 commits into from
Feb 24, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
752bdf1
Add RewriteAppender with a 'meta' rewrite policy.
lukeelmers Feb 16, 2021
71e46ce
Add unit tests.
lukeelmers Feb 16, 2021
e1c6402
Filter sensitive headers using rewrite appender.
lukeelmers Feb 17, 2021
ae5facc
Update generated docs.
lukeelmers Feb 17, 2021
03a0397
Rename policy.transform to policy.rewrite for consistency with log4j.
lukeelmers Feb 17, 2021
61d4450
Address initial feedback.
lukeelmers Feb 18, 2021
0dc0afc
Switch from appenders.update to appenders.addAppender to align with l…
lukeelmers Feb 18, 2021
44d1440
Fix missed transform > rewrite rename.
lukeelmers Feb 18, 2021
524cc31
Shallow clone http request/response headers.
lukeelmers Feb 18, 2021
c72c75b
Test for circular refs between appenders.
lukeelmers Feb 19, 2021
7cbe347
Update README with documentation.
lukeelmers Feb 19, 2021
e9b9bcc
Merge branch 'master' into feat/rewrite-appender
kibanamachine Feb 19, 2021
8fb040e
Take Appender inside addAppender
lukeelmers Feb 19, 2021
88397a5
Clean up policy comments
lukeelmers Feb 19, 2021
a6632ee
Update snapshots
lukeelmers Feb 19, 2021
eabb888
Merge branch 'master' into feat/rewrite-appender
kibanamachine Feb 21, 2021
0516db3
Re-add hardcoded headers filtering to http logs.
lukeelmers Feb 21, 2021
c777305
Use fixed timestamp in meta policy tests.
lukeelmers Feb 21, 2021
e0f4c58
Clean up documentation.
lukeelmers Feb 21, 2021
11b5512
Prevent mutating array headers.
lukeelmers Feb 22, 2021
b46e404
Extract inline types from MetaRewritePolicy.
lukeelmers Feb 22, 2021
f6ae2ea
Clean up unknown appender error message.
lukeelmers Feb 22, 2021
353d8e5
Resolve merge conflicts.
lukeelmers Feb 22, 2021
dcee4b9
Merge branch 'master' into feat/rewrite-appender
lukeelmers Feb 22, 2021
f9f28dd
Remove MetaRewritePolicy 'add' mode.
lukeelmers Feb 23, 2021
3d2a945
Remove default rewrite-appender.
lukeelmers Feb 23, 2021
102d490
Merge branch 'master' into feat/rewrite-appender
kibanamachine Feb 23, 2021
aa5d21e
Merge branch 'master' into feat/rewrite-appender
kibanamachine Feb 24, 2021
fb23cb1
Fix logging system jest tests.
lukeelmers Feb 24, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@
<b>Signature:</b>

```typescript
export declare type AppenderConfigType = ConsoleAppenderConfig | FileAppenderConfig | LegacyAppenderConfig | RollingFileAppenderConfig;
export declare type AppenderConfigType = ConsoleAppenderConfig | FileAppenderConfig | LegacyAppenderConfig | RewriteAppenderConfig | RollingFileAppenderConfig;
```
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const page1 = await savedObjectsClient.find({
type: 'visualization',
sortField: 'updated_at',
sortOrder: 'asc',
pit,
pit: { id },
lukeelmers marked this conversation as resolved.
Show resolved Hide resolved
});
const lastHit = page1.saved_objects[page1.saved_objects.length - 1];
const page2 = await savedObjectsClient.find({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,16 @@ openPointInTimeForType(type: string | string[], { keepAlive, preference }?: Save


```ts
const repository = coreStart.savedObjects.createInternalRepository();

const { id } = await repository.openPointInTimeForType(
type: 'index-pattern',
{ keepAlive: '2m' },
const { id } = await savedObjectsClient.openPointInTimeForType(
type: 'visualization',
{ keepAlive: '5m' },
);
const page1 = await savedObjectsClient.find({
type: 'visualization',
sortField: 'updated_at',
sortOrder: 'asc',
pit,
pit: { id, keepAlive: '2m' },
});

const lastHit = page1.saved_objects[page1.saved_objects.length - 1];
const page2 = await savedObjectsClient.find({
type: 'visualization',
Expand All @@ -50,7 +47,6 @@ const page2 = await savedObjectsClient.find({
pit: { id: page1.pit_id },
searchAfter: lastHit.sort,
});

await savedObjectsClient.closePointInTime(page2.pit_id);

```
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-logging/src/appenders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { LogRecord } from './log_record';
*/
export interface Appender {
append(record: LogRecord): void;
update(config: { appenders?: Map<string, Appender> }): void;
mshustov marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
72 changes: 70 additions & 2 deletions src/core/server/http/integration_tests/logging.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,42 @@ describe('request logging', () => {
expect(JSON.parse(meta).http.response.headers.bar).toBe('world');
});

it('filters sensitive request headers', async () => {
it('filters sensitive request headers when RewriteAppender is configured', async () => {
root = kbnTestServer.createRoot({
logging: {
silent: true,
appenders: {
'test-console': {
type: 'console',
layout: {
type: 'pattern',
pattern: '%level|%logger|%message|%meta',
},
},
rewrite: {
type: 'rewrite',
appenders: ['test-console'],
policy: {
type: 'meta',
mode: 'update',
properties: [
{ path: 'http.request.headers.authorization', value: '[REDACTED]' },
],
},
},
},
loggers: [
{
name: 'http.server.response',
appenders: ['rewrite'],
level: 'debug',
},
],
},
plugins: {
initialize: false,
},
});
const { http } = await root.setup();

http.createRouter('/').post(
Expand Down Expand Up @@ -283,7 +318,40 @@ describe('request logging', () => {
expect(JSON.parse(meta).http.request.headers.authorization).toBe('[REDACTED]');
});

it('filters sensitive response headers', async () => {
it('filters sensitive response headers when RewriteAppender is configured', async () => {
root = kbnTestServer.createRoot({
logging: {
silent: true,
appenders: {
'test-console': {
type: 'console',
layout: {
type: 'pattern',
pattern: '%level|%logger|%message|%meta',
},
},
rewrite: {
type: 'rewrite',
appenders: ['test-console'],
policy: {
type: 'meta',
mode: 'update',
properties: [{ path: 'http.response.headers.set-cookie', value: '[REDACTED]' }],
},
},
},
loggers: [
{
name: 'http.server.response',
appenders: ['rewrite'],
level: 'debug',
},
],
},
plugins: {
initialize: false,
},
});
const { http } = await root.setup();

http.createRouter('/').post(
Expand Down
48 changes: 0 additions & 48 deletions src/core/server/http/logging/get_response_log.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,54 +148,6 @@ describe('getEcsResponseLog', () => {
expect(result.http.response.status_code).toBe(400);
});

describe('filters sensitive headers', () => {
test('redacts Authorization and Cookie headers by default', () => {
const req = createMockHapiRequest({
headers: { authorization: 'a', cookie: 'b', 'user-agent': 'hi' },
response: { headers: { 'content-length': 123, 'set-cookie': 'c' } },
});
const result = getEcsResponseLog(req, logger);
expect(result.http.request.headers).toMatchInlineSnapshot(`
Object {
"authorization": "[REDACTED]",
"cookie": "[REDACTED]",
"user-agent": "hi",
}
`);
expect(result.http.response.headers).toMatchInlineSnapshot(`
Object {
"content-length": 123,
"set-cookie": "[REDACTED]",
}
`);
});

test('does not mutate original headers', () => {
const reqHeaders = { authorization: 'a', cookie: 'b', 'user-agent': 'hi' };
const resHeaders = { headers: { 'content-length': 123, 'set-cookie': 'c' } };
const req = createMockHapiRequest({
headers: reqHeaders,
response: { headers: resHeaders },
});
getEcsResponseLog(req, logger);
expect(reqHeaders).toMatchInlineSnapshot(`
Object {
"authorization": "a",
"cookie": "b",
"user-agent": "hi",
}
`);
expect(resHeaders).toMatchInlineSnapshot(`
Object {
"headers": Object {
"content-length": 123,
"set-cookie": "c",
},
}
`);
});
});

describe('ecs', () => {
test('specifies correct ECS version', () => {
const req = createMockHapiRequest();
Expand Down
19 changes: 2 additions & 17 deletions src/core/server/http/logging/get_response_log.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,6 @@ import { EcsEvent, Logger } from '../../logging';
import { getResponsePayloadBytes } from './get_payload_size';

const ECS_VERSION = '1.7.0';
const FORBIDDEN_HEADERS = ['authorization', 'cookie', 'set-cookie'];
const REDACTED_HEADER_TEXT = '[REDACTED]';

// We are excluding sensitive headers by default, until we have a log filtering mechanism.
function redactSensitiveHeaders(
headers?: Record<string, string | string[]>
): Record<string, string | string[]> {
const result = {} as Record<string, string | string[]>;
if (headers) {
for (const key of Object.keys(headers)) {
result[key] = FORBIDDEN_HEADERS.includes(key) ? REDACTED_HEADER_TEXT : headers[key];
}
}
return result;
}

/**
* Converts a hapi `Request` into ECS-compliant `LogMeta` for logging.
Expand Down Expand Up @@ -66,15 +51,15 @@ export function getEcsResponseLog(request: Request, log: Logger): LogMeta {
mime_type: request.mime,
referrer: request.info.referrer,
// @ts-expect-error Headers are not yet part of ECS: https://github.com/elastic/ecs/issues/232.
headers: redactSensitiveHeaders(request.headers),
headers: request.headers,
},
response: {
body: {
bytes,
},
status_code,
// @ts-expect-error Headers are not yet part of ECS: https://github.com/elastic/ecs/issues/232.
headers: redactSensitiveHeaders(responseHeaders),
headers: responseHeaders,
// responseTime is a custom non-ECS field
responseTime: !isNaN(responseTime) ? responseTime : undefined,
},
Expand Down
7 changes: 7 additions & 0 deletions src/core/server/legacy/logging/appenders/legacy_appender.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@ export class LegacyAppender implements DisposableAppender {
this.loggingServer.log(record);
}

/**
* Updates `LegacyAppender` configuration.
*/
public update() {
// noop
}

public dispose() {
this.loggingServer.stop();
}
Expand Down
5 changes: 5 additions & 0 deletions src/core/server/logging/appenders/appenders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
import { Layouts } from '../layouts/layouts';
import { ConsoleAppender, ConsoleAppenderConfig } from './console/console_appender';
import { FileAppender, FileAppenderConfig } from './file/file_appender';
import { RewriteAppender, RewriteAppenderConfig } from './rewrite/rewrite_appender';
import {
RollingFileAppender,
RollingFileAppenderConfig,
Expand All @@ -32,6 +33,7 @@ export const appendersSchema = schema.oneOf([
ConsoleAppender.configSchema,
FileAppender.configSchema,
LegacyAppender.configSchema,
RewriteAppender.configSchema,
RollingFileAppender.configSchema,
]);

Expand All @@ -40,6 +42,7 @@ export type AppenderConfigType =
| ConsoleAppenderConfig
| FileAppenderConfig
| LegacyAppenderConfig
| RewriteAppenderConfig
| RollingFileAppenderConfig;

/** @internal */
Expand All @@ -57,6 +60,8 @@ export class Appenders {
return new ConsoleAppender(Layouts.create(config.layout));
case 'file':
return new FileAppender(Layouts.create(config.layout), config.fileName);
case 'rewrite':
return new RewriteAppender(config);
case 'rolling-file':
return new RollingFileAppender(config);
case 'legacy-appender':
Expand Down
7 changes: 7 additions & 0 deletions src/core/server/logging/appenders/buffer/buffer_appender.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@ export class BufferAppender implements DisposableAppender {
this.buffer.push(record);
}

/**
* Updates `BufferAppender` configuration.
*/
public update() {
// noop
}

/**
* Clears buffer and returns all records that it had.
*/
Expand Down
7 changes: 7 additions & 0 deletions src/core/server/logging/appenders/console/console_appender.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ export class ConsoleAppender implements DisposableAppender {
console.log(this.layout.format(record));
}

/**
* Updates `ConsoleAppender` configuration.
*/
public update() {
// noop
}

/**
* Disposes `ConsoleAppender`.
*/
Expand Down
7 changes: 7 additions & 0 deletions src/core/server/logging/appenders/file/file_appender.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,13 @@ export class FileAppender implements DisposableAppender {
this.outputStream.write(`${this.layout.format(record)}\n`);
}

/**
* Updates `FileAppender` configuration.
*/
public update() {
// noop
}

/**
* Disposes `FileAppender`. Waits for the underlying file stream to be completely flushed and closed.
*/
Expand Down
20 changes: 20 additions & 0 deletions src/core/server/logging/appenders/rewrite/mocks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import type { RewritePolicy } from './policies/policy';

const createPolicyMock = () => {
const mock: jest.Mocked<RewritePolicy> = {
transform: jest.fn((x) => x),
};
return mock;
};

export const rewriteAppenderMocks = {
createPolicy: createPolicyMock,
};
31 changes: 31 additions & 0 deletions src/core/server/logging/appenders/rewrite/policies/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { schema } from '@kbn/config-schema';
import { assertNever } from '@kbn/std';
import { RewritePolicy } from './policy';
import { MetaRewritePolicy, MetaRewritePolicyConfig, metaRewritePolicyConfigSchema } from './meta';

export { RewritePolicy };

/**
* Available rewrite policies which specify what part of a {@link LogRecord}
* can be modified.
*/
export type RewritePolicyConfig = MetaRewritePolicyConfig;

export const rewritePolicyConfigSchema = schema.oneOf([metaRewritePolicyConfigSchema]);
lukeelmers marked this conversation as resolved.
Show resolved Hide resolved

export const createRewritePolicy = (config: RewritePolicyConfig): RewritePolicy => {
switch (config.type) {
case 'meta':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find MetaRewritePolicy in log4j http://logging.apache.org/log4j/2.x/manual/appenders.html#RewritePolicy
Do we call so because we apply a policy to Meta object only?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think MetaRewritePolicy is a combined modification of log4j's MapRewritePolicy and PropertiesRewritePolicy where we're using the type field and the properties field rather than the keyValuePair in MetaRewritePolicy and we're allowing fields to be added too ( what PropertiesRewritePolicy does). And, yes, the policy is applied to the meta property. It's hard to tell though because our implementation isn't a 1-to-1 with log4j.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we call so because we apply a policy to Meta object only?

Yeah exactly, they have a map and properties rewrite policy, so I called this meta as it is only scoped to the LogMeta. With this pattern, in the future we could have something like a message or level policy that will rewrite those items specifically.

return new MetaRewritePolicy(config);
default:
return assertNever(config.type);
}
};
13 changes: 13 additions & 0 deletions src/core/server/logging/appenders/rewrite/policies/meta/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

export {
MetaRewritePolicy,
MetaRewritePolicyConfig,
metaRewritePolicyConfigSchema,
} from './meta_policy';
Loading