Skip to content

Commit 3da02c5

Browse files
joeypoonjoeypoon
authored
[Security Solution] always validate usage-api cert (#196741)
## Summary * enables usage-api cert validation for all requests within Kibana serverless security * removes hardcoded usage-api url, must be passed in configs now * adds user-agent to usage-api requests * fixes a potential issue with usage-api requests retrying too quickly Fixes: https://github.com/elastic/kibana/security/code-scanning/460 ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)
1 parent b61a627 commit 3da02c5

6 files changed

Lines changed: 47 additions & 57 deletions

File tree

‎x-pack/plugins/security_solution/public/management/cypress/e2e/serverless/metering.cy.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ describe(
2525
ftrConfig: {
2626
kbnServerArgs: [
2727
`--xpack.securitySolutionServerless.usageReportingTaskInterval=1m`,
28-
`--xpack.securitySolutionServerless.usageApi.url=https://localhost:3623`,
28+
`--xpack.securitySolutionServerless.usageApi.url=http://localhost:3623`,
2929
],
3030
},
3131
},

‎x-pack/plugins/security_solution_serverless/server/common/services/usage_reporting_service.test.ts‎

Lines changed: 29 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ import { KBN_CERT_PATH, KBN_KEY_PATH, CA_CERT_PATH } from '@kbn/dev-utils';
1414
import type { UsageApiConfigSchema } from '../../config';
1515
import type { UsageRecord } from '../../types';
1616

17+
import { USAGE_REPORTING_ENDPOINT } from '../../constants';
1718
import { UsageReportingService } from './usage_reporting_service';
18-
import { USAGE_REPORTING_ENDPOINT, USAGE_SERVICE_USAGE_URL } from '../../constants';
1919

2020
jest.mock('node-fetch');
2121
const { Response } = jest.requireActual('node-fetch');
@@ -24,6 +24,8 @@ describe('UsageReportingService', () => {
2424
let usageApiConfig: UsageApiConfigSchema;
2525
let service: UsageReportingService;
2626

27+
const kibanaVersion = '8.16.0';
28+
2729
function generateUsageApiConfig(overrides?: Partial<UsageApiConfigSchema>): UsageApiConfigSchema {
2830
const DEFAULT_USAGE_API_CONFIG = { enabled: false };
2931
usageApiConfig = merge(DEFAULT_USAGE_API_CONFIG, overrides);
@@ -34,7 +36,7 @@ describe('UsageReportingService', () => {
3436
function setupService(
3537
usageApi: UsageApiConfigSchema = generateUsageApiConfig()
3638
): UsageReportingService {
37-
service = new UsageReportingService(usageApi);
39+
service = new UsageReportingService(usageApi, kibanaVersion);
3840
return service;
3941
}
4042

@@ -59,61 +61,42 @@ describe('UsageReportingService', () => {
5961
setupService();
6062
});
6163

62-
it('should still work if usageApi.url is not provided', async () => {
64+
it('should not set agent if the URL is not https', async () => {
65+
const url = 'http://usage-api.example';
66+
setupService(generateUsageApiConfig({ url }));
6367
const usageRecord = generateUsageRecord();
6468
const records: UsageRecord[] = [usageRecord];
6569
const mockResponse = new Response(null, { status: 200 });
66-
(fetch as jest.MockedFunction<typeof fetch>).mockResolvedValueOnce(mockResponse);
70+
(fetch as jest.MockedFunction<typeof fetch>).mockResolvedValue(mockResponse);
6771

6872
const response = await service.reportUsage(records);
6973

7074
expect(fetch).toHaveBeenCalledTimes(1);
71-
expect(fetch).toHaveBeenCalledWith(USAGE_SERVICE_USAGE_URL, {
75+
expect(fetch).toHaveBeenCalledWith(`${url}${USAGE_REPORTING_ENDPOINT}`, {
7276
method: 'post',
7377
body: JSON.stringify(records),
74-
headers: { 'Content-Type': 'application/json' },
75-
agent: expect.any(https.Agent),
78+
headers: {
79+
'Content-Type': 'application/json',
80+
'User-Agent': `Kibana/${kibanaVersion} node-fetch`,
81+
},
7682
});
7783
expect(response).toBe(mockResponse);
7884
});
7985

80-
it('should use an agent with rejectUnauthorized false if config.enabled is false', async () => {
86+
it('should throw if url not provided', async () => {
8187
const usageRecord = generateUsageRecord();
8288
const records: UsageRecord[] = [usageRecord];
83-
const mockResponse = new Response(null, { status: 200 });
84-
(fetch as jest.MockedFunction<typeof fetch>).mockResolvedValueOnce(mockResponse);
85-
86-
const response = await service.reportUsage(records);
87-
88-
expect(fetch).toHaveBeenCalledTimes(1);
89-
expect(fetch).toHaveBeenCalledWith(USAGE_SERVICE_USAGE_URL, {
90-
method: 'post',
91-
body: JSON.stringify(records),
92-
headers: { 'Content-Type': 'application/json' },
93-
agent: expect.objectContaining({
94-
options: expect.objectContaining({ rejectUnauthorized: false }),
95-
}),
96-
});
97-
expect(response).toBe(mockResponse);
89+
await expect(service.reportUsage(records)).rejects.toThrowError('usage-api url not provided');
9890
});
9991

100-
it('should not set agent if the URL is not https', async () => {
101-
const url = 'http://usage-api.example';
92+
it('should throw if TLS configs not provided', async () => {
93+
const url = 'https://some-url';
10294
setupService(generateUsageApiConfig({ url }));
10395
const usageRecord = generateUsageRecord();
10496
const records: UsageRecord[] = [usageRecord];
105-
const mockResponse = new Response(null, { status: 200 });
106-
(fetch as jest.MockedFunction<typeof fetch>).mockResolvedValue(mockResponse);
107-
108-
const response = await service.reportUsage(records);
109-
110-
expect(fetch).toHaveBeenCalledTimes(1);
111-
expect(fetch).toHaveBeenCalledWith(`${url}${USAGE_REPORTING_ENDPOINT}`, {
112-
method: 'post',
113-
body: JSON.stringify(records),
114-
headers: { 'Content-Type': 'application/json' },
115-
});
116-
expect(response).toBe(mockResponse);
97+
await expect(service.reportUsage(records)).rejects.toThrowError(
98+
'usage-api TLS configs not provided'
99+
);
117100
});
118101
});
119102

@@ -132,7 +115,7 @@ describe('UsageReportingService', () => {
132115
setupService(generateUsageApiConfig(DEFAULT_CONFIG));
133116
});
134117

135-
it('should use usageApi.url if provided', async () => {
118+
it('should correctly use usageApi.url', async () => {
136119
const usageRecord = generateUsageRecord();
137120
const records: UsageRecord[] = [usageRecord];
138121
const mockResponse = new Response(null, { status: 200 });
@@ -145,7 +128,10 @@ describe('UsageReportingService', () => {
145128
expect(fetch).toHaveBeenCalledWith(url, {
146129
method: 'post',
147130
body: JSON.stringify(records),
148-
headers: { 'Content-Type': 'application/json' },
131+
headers: {
132+
'Content-Type': 'application/json',
133+
'User-Agent': `Kibana/${kibanaVersion} node-fetch`,
134+
},
149135
agent: expect.any(https.Agent),
150136
});
151137
expect(response).toBe(mockResponse);
@@ -164,7 +150,10 @@ describe('UsageReportingService', () => {
164150
expect(fetch).toHaveBeenCalledWith(url, {
165151
method: 'post',
166152
body: JSON.stringify(records),
167-
headers: { 'Content-Type': 'application/json' },
153+
headers: {
154+
'Content-Type': 'application/json',
155+
'User-Agent': `Kibana/${kibanaVersion} node-fetch`,
156+
},
168157
agent: expect.objectContaining({
169158
options: expect.objectContaining({
170159
cert: expect.any(String),

‎x-pack/plugins/security_solution_serverless/server/common/services/usage_reporting_service.ts‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,24 @@ import { SslConfig, sslSchema } from '@kbn/server-http-tools';
1515
import type { UsageRecord } from '../../types';
1616
import type { UsageApiConfigSchema, TlsConfigSchema } from '../../config';
1717

18-
import { USAGE_REPORTING_ENDPOINT, USAGE_SERVICE_USAGE_URL } from '../../constants';
18+
import { USAGE_REPORTING_ENDPOINT } from '../../constants';
1919

2020
export class UsageReportingService {
2121
private agent: https.Agent | undefined;
2222

23-
constructor(private readonly config: UsageApiConfigSchema) {}
23+
constructor(
24+
private readonly config: UsageApiConfigSchema,
25+
private readonly kibanaVersion: string
26+
) {}
2427

2528
public async reportUsage(records: UsageRecord[]): Promise<Response> {
2629
const reqArgs: RequestInit = {
2730
method: 'post',
2831
body: JSON.stringify(records),
29-
headers: { 'Content-Type': 'application/json' },
32+
headers: {
33+
'Content-Type': 'application/json',
34+
'User-Agent': `Kibana/${this.kibanaVersion} node-fetch`,
35+
},
3036
};
3137
if (this.usageApiUrl.includes('https')) {
3238
reqArgs.agent = this.httpAgent;
@@ -36,15 +42,15 @@ export class UsageReportingService {
3642

3743
private get tlsConfigs(): NonNullable<TlsConfigSchema> {
3844
if (!this.config.tls) {
39-
throw new Error('UsageReportingService: usageApi.tls configs not provided');
45+
throw new Error('usage-api TLS configs not provided');
4046
}
4147

4248
return this.config.tls;
4349
}
4450

4551
private get usageApiUrl(): string {
4652
if (!this.config.url) {
47-
return USAGE_SERVICE_USAGE_URL;
53+
throw new Error('usage-api url not provided');
4854
}
4955

5056
return `${this.config.url}${USAGE_REPORTING_ENDPOINT}`;
@@ -55,11 +61,6 @@ export class UsageReportingService {
5561
return this.agent;
5662
}
5763

58-
if (!this.config.enabled) {
59-
this.agent = new https.Agent({ rejectUnauthorized: false });
60-
return this.agent;
61-
}
62-
6364
const tlsConfig = new SslConfig(
6465
sslSchema.validate({
6566
enabled: true,

‎x-pack/plugins/security_solution_serverless/server/constants.ts‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,5 @@
55
* 2.0.
66
*/
77

8-
const namespace = 'elastic-system';
9-
const USAGE_SERVICE_BASE_API_URL = `https://usage-api.${namespace}/api`;
10-
const USAGE_SERVICE_BASE_API_URL_V1 = `${USAGE_SERVICE_BASE_API_URL}/v1`;
11-
export const USAGE_SERVICE_USAGE_URL = `${USAGE_SERVICE_BASE_API_URL_V1}/usage`;
128
export const USAGE_REPORTING_ENDPOINT = '/api/v1/usage';
139
export const METERING_SERVICE_BATCH_SIZE = 1000;

‎x-pack/plugins/security_solution_serverless/server/plugin.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ export class SecuritySolutionServerlessPlugin
4545
SecuritySolutionServerlessPluginStartDeps
4646
>
4747
{
48+
private kibanaVersion: string;
4849
private config: ServerlessSecurityConfig;
4950
private cloudSecurityUsageReportingTask: SecurityUsageReportingTask | undefined;
5051
private endpointUsageReportingTask: SecurityUsageReportingTask | undefined;
@@ -53,10 +54,14 @@ export class SecuritySolutionServerlessPlugin
5354
private readonly usageReportingService: UsageReportingService;
5455

5556
constructor(private readonly initializerContext: PluginInitializerContext) {
57+
this.kibanaVersion = initializerContext.env.packageInfo.version;
5658
this.config = this.initializerContext.config.get<ServerlessSecurityConfig>();
5759
this.logger = this.initializerContext.logger.get();
5860

59-
this.usageReportingService = new UsageReportingService(this.config.usageApi);
61+
this.usageReportingService = new UsageReportingService(
62+
this.config.usageApi,
63+
this.kibanaVersion
64+
);
6065

6166
const productTypesStr = JSON.stringify(this.config.productTypes, null, 2);
6267
this.logger.info(`Security Solution running with product types:\n${productTypesStr}`);

‎x-pack/plugins/security_solution_serverless/server/task_manager/usage_reporting_task.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,6 @@ export class SecurityUsageReportingTask {
188188
usageRecords.length
189189
}) usage records starting from ${lastSuccessfulReport.toISOString()}: ${err} `
190190
);
191-
shouldRunAgain = true;
192191
}
193192
}
194193

0 commit comments

Comments
 (0)