[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)
elastic · Oct 24, 2024 · 3da02c5 · 3da02c5
1 parent b61a627
commit 3da02c5
Show file tree

Hide file tree

Showing 6 changed files with 47 additions and 57 deletions.
diff --git a/x-pack/plugins/security_solution/public/management/cypress/e2e/serverless/metering.cy.ts b/x-pack/plugins/security_solution/public/management/cypress/e2e/serverless/metering.cy.ts
@@ -25,7 +25,7 @@ describe(
       ftrConfig: {
         kbnServerArgs: [
           `--xpack.securitySolutionServerless.usageReportingTaskInterval=1m`,
-          `--xpack.securitySolutionServerless.usageApi.url=https://localhost:3623`,
+          `--xpack.securitySolutionServerless.usageApi.url=http://localhost:3623`,
         ],
       },
     },

diff --git a/...ugins/security_solution_serverless/server/common/services/usage_reporting_service.test.ts b/...ugins/security_solution_serverless/server/common/services/usage_reporting_service.test.ts
@@ -14,8 +14,8 @@ import { KBN_CERT_PATH, KBN_KEY_PATH, CA_CERT_PATH } from '@kbn/dev-utils';
 import type { UsageApiConfigSchema } from '../../config';
 import type { UsageRecord } from '../../types';
 
+import { USAGE_REPORTING_ENDPOINT } from '../../constants';
 import { UsageReportingService } from './usage_reporting_service';
-import { USAGE_REPORTING_ENDPOINT, USAGE_SERVICE_USAGE_URL } from '../../constants';
 
 jest.mock('node-fetch');
 const { Response } = jest.requireActual('node-fetch');
@@ -24,6 +24,8 @@ describe('UsageReportingService', () => {
   let usageApiConfig: UsageApiConfigSchema;
   let service: UsageReportingService;
 
+  const kibanaVersion = '8.16.0';
+
   function generateUsageApiConfig(overrides?: Partial<UsageApiConfigSchema>): UsageApiConfigSchema {
     const DEFAULT_USAGE_API_CONFIG = { enabled: false };
     usageApiConfig = merge(DEFAULT_USAGE_API_CONFIG, overrides);
@@ -34,7 +36,7 @@ describe('UsageReportingService', () => {
   function setupService(
     usageApi: UsageApiConfigSchema = generateUsageApiConfig()
   ): UsageReportingService {
-    service = new UsageReportingService(usageApi);
+    service = new UsageReportingService(usageApi, kibanaVersion);
     return service;
   }
 
@@ -59,61 +61,42 @@ describe('UsageReportingService', () => {
       setupService();
     });
 
-    it('should still work if usageApi.url is not provided', async () => {
+    it('should not set agent if the URL is not https', async () => {
+      const url = 'http://usage-api.example';
+      setupService(generateUsageApiConfig({ url }));
       const usageRecord = generateUsageRecord();
       const records: UsageRecord[] = [usageRecord];
       const mockResponse = new Response(null, { status: 200 });
-      (fetch as jest.MockedFunction<typeof fetch>).mockResolvedValueOnce(mockResponse);
+      (fetch as jest.MockedFunction<typeof fetch>).mockResolvedValue(mockResponse);
 
       const response = await service.reportUsage(records);
 
       expect(fetch).toHaveBeenCalledTimes(1);
-      expect(fetch).toHaveBeenCalledWith(USAGE_SERVICE_USAGE_URL, {
+      expect(fetch).toHaveBeenCalledWith(`${url}${USAGE_REPORTING_ENDPOINT}`, {
         method: 'post',
         body: JSON.stringify(records),
-        headers: { 'Content-Type': 'application/json' },
-        agent: expect.any(https.Agent),
+        headers: {
+          'Content-Type': 'application/json',
+          'User-Agent': `Kibana/${kibanaVersion} node-fetch`,
+        },
       });
       expect(response).toBe(mockResponse);
     });
 
-    it('should use an agent with rejectUnauthorized false if config.enabled is false', async () => {
+    it('should throw if url not provided', async () => {
       const usageRecord = generateUsageRecord();
       const records: UsageRecord[] = [usageRecord];
-      const mockResponse = new Response(null, { status: 200 });
-      (fetch as jest.MockedFunction<typeof fetch>).mockResolvedValueOnce(mockResponse);
-
-      const response = await service.reportUsage(records);
-
-      expect(fetch).toHaveBeenCalledTimes(1);
-      expect(fetch).toHaveBeenCalledWith(USAGE_SERVICE_USAGE_URL, {
-        method: 'post',
-        body: JSON.stringify(records),
-        headers: { 'Content-Type': 'application/json' },
-        agent: expect.objectContaining({
-          options: expect.objectContaining({ rejectUnauthorized: false }),
-        }),
-      });
-      expect(response).toBe(mockResponse);
+      await expect(service.reportUsage(records)).rejects.toThrowError('usage-api url not provided');
     });
 
-    it('should not set agent if the URL is not https', async () => {
-      const url = 'http://usage-api.example';
+    it('should throw if TLS configs not provided', async () => {
+      const url = 'https://some-url';
       setupService(generateUsageApiConfig({ url }));
       const usageRecord = generateUsageRecord();
       const records: UsageRecord[] = [usageRecord];
-      const mockResponse = new Response(null, { status: 200 });
-      (fetch as jest.MockedFunction<typeof fetch>).mockResolvedValue(mockResponse);
-
-      const response = await service.reportUsage(records);
-
-      expect(fetch).toHaveBeenCalledTimes(1);
-      expect(fetch).toHaveBeenCalledWith(`${url}${USAGE_REPORTING_ENDPOINT}`, {
-        method: 'post',
-        body: JSON.stringify(records),
-        headers: { 'Content-Type': 'application/json' },
-      });
-      expect(response).toBe(mockResponse);
+      await expect(service.reportUsage(records)).rejects.toThrowError(
+        'usage-api TLS configs not provided'
+      );
     });
   });
 
@@ -132,7 +115,7 @@ describe('UsageReportingService', () => {
       setupService(generateUsageApiConfig(DEFAULT_CONFIG));
     });
 
-    it('should use usageApi.url if provided', async () => {
+    it('should correctly use usageApi.url', async () => {
       const usageRecord = generateUsageRecord();
       const records: UsageRecord[] = [usageRecord];
       const mockResponse = new Response(null, { status: 200 });
@@ -145,7 +128,10 @@ describe('UsageReportingService', () => {
       expect(fetch).toHaveBeenCalledWith(url, {
         method: 'post',
         body: JSON.stringify(records),
-        headers: { 'Content-Type': 'application/json' },
+        headers: {
+          'Content-Type': 'application/json',
+          'User-Agent': `Kibana/${kibanaVersion} node-fetch`,
+        },
         agent: expect.any(https.Agent),
       });
       expect(response).toBe(mockResponse);
@@ -164,7 +150,10 @@ describe('UsageReportingService', () => {
       expect(fetch).toHaveBeenCalledWith(url, {
         method: 'post',
         body: JSON.stringify(records),
-        headers: { 'Content-Type': 'application/json' },
+        headers: {
+          'Content-Type': 'application/json',
+          'User-Agent': `Kibana/${kibanaVersion} node-fetch`,
+        },
         agent: expect.objectContaining({
           options: expect.objectContaining({
             cert: expect.any(String),

diff --git a/...ck/plugins/security_solution_serverless/server/common/services/usage_reporting_service.ts b/...ck/plugins/security_solution_serverless/server/common/services/usage_reporting_service.ts
@@ -15,18 +15,24 @@ import { SslConfig, sslSchema } from '@kbn/server-http-tools';
 import type { UsageRecord } from '../../types';
 import type { UsageApiConfigSchema, TlsConfigSchema } from '../../config';
 
-import { USAGE_REPORTING_ENDPOINT, USAGE_SERVICE_USAGE_URL } from '../../constants';
+import { USAGE_REPORTING_ENDPOINT } from '../../constants';
 
 export class UsageReportingService {
   private agent: https.Agent | undefined;
 
-  constructor(private readonly config: UsageApiConfigSchema) {}
+  constructor(
+    private readonly config: UsageApiConfigSchema,
+    private readonly kibanaVersion: string
+  ) {}
 
   public async reportUsage(records: UsageRecord[]): Promise<Response> {
     const reqArgs: RequestInit = {
       method: 'post',
       body: JSON.stringify(records),
-      headers: { 'Content-Type': 'application/json' },
+      headers: {
+        'Content-Type': 'application/json',
+        'User-Agent': `Kibana/${this.kibanaVersion} node-fetch`,
+      },
     };
     if (this.usageApiUrl.includes('https')) {
       reqArgs.agent = this.httpAgent;
@@ -36,15 +42,15 @@ export class UsageReportingService {
 
   private get tlsConfigs(): NonNullable<TlsConfigSchema> {
     if (!this.config.tls) {
-      throw new Error('UsageReportingService: usageApi.tls configs not provided');
+      throw new Error('usage-api TLS configs not provided');
     }
 
     return this.config.tls;
   }
 
   private get usageApiUrl(): string {
     if (!this.config.url) {
-      return USAGE_SERVICE_USAGE_URL;
+      throw new Error('usage-api url not provided');
     }
 
     return `${this.config.url}${USAGE_REPORTING_ENDPOINT}`;
@@ -55,11 +61,6 @@ export class UsageReportingService {
       return this.agent;
     }
 
-    if (!this.config.enabled) {
-      this.agent = new https.Agent({ rejectUnauthorized: false });
-      return this.agent;
-    }
-
     const tlsConfig = new SslConfig(
       sslSchema.validate({
         enabled: true,

diff --git a/x-pack/plugins/security_solution_serverless/server/constants.ts b/x-pack/plugins/security_solution_serverless/server/constants.ts
@@ -5,9 +5,5 @@
  * 2.0.
  */
 
-const namespace = 'elastic-system';
-const USAGE_SERVICE_BASE_API_URL = `https://usage-api.${namespace}/api`;
-const USAGE_SERVICE_BASE_API_URL_V1 = `${USAGE_SERVICE_BASE_API_URL}/v1`;
-export const USAGE_SERVICE_USAGE_URL = `${USAGE_SERVICE_BASE_API_URL_V1}/usage`;
 export const USAGE_REPORTING_ENDPOINT = '/api/v1/usage';
 export const METERING_SERVICE_BATCH_SIZE = 1000;
diff --git a/x-pack/plugins/security_solution_serverless/server/plugin.ts b/x-pack/plugins/security_solution_serverless/server/plugin.ts
@@ -45,6 +45,7 @@ export class SecuritySolutionServerlessPlugin
       SecuritySolutionServerlessPluginStartDeps
     >
 {
+  private kibanaVersion: string;
   private config: ServerlessSecurityConfig;
   private cloudSecurityUsageReportingTask: SecurityUsageReportingTask | undefined;
   private endpointUsageReportingTask: SecurityUsageReportingTask | undefined;
@@ -53,10 +54,14 @@ export class SecuritySolutionServerlessPlugin
   private readonly usageReportingService: UsageReportingService;
 
   constructor(private readonly initializerContext: PluginInitializerContext) {
+    this.kibanaVersion = initializerContext.env.packageInfo.version;
     this.config = this.initializerContext.config.get<ServerlessSecurityConfig>();
     this.logger = this.initializerContext.logger.get();
 
-    this.usageReportingService = new UsageReportingService(this.config.usageApi);
+    this.usageReportingService = new UsageReportingService(
+      this.config.usageApi,
+      this.kibanaVersion
+    );
 
     const productTypesStr = JSON.stringify(this.config.productTypes, null, 2);
     this.logger.info(`Security Solution running with product types:\n${productTypesStr}`);

diff --git a/x-pack/plugins/security_solution_serverless/server/task_manager/usage_reporting_task.ts b/x-pack/plugins/security_solution_serverless/server/task_manager/usage_reporting_task.ts
@@ -188,7 +188,6 @@ export class SecurityUsageReportingTask {
             usageRecords.length
           }) usage records starting from ${lastSuccessfulReport.toISOString()}: ${err} `
         );
-        shouldRunAgain = true;
       }
     }
-Original file line number
+Diff line change
@@ Expand Up / @@ -188,7 +188,6 @@ export class SecurityUsageReportingTask { @@
                 usageRecords.length
               }) usage records starting from ${lastSuccessfulReport.toISOString()}: ${err} `
             );
-            shouldRunAgain = true;
           }
         }
@@ Expand Down @@