-
Notifications
You must be signed in to change notification settings - Fork 15
feat(server): support a custom CA bundle for the backend connection #537
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| import { readFileSync } from 'node:fs'; | ||
| import * as https from 'node:https'; | ||
| import { join } from 'node:path'; | ||
| import request from 'supertest'; | ||
|
|
||
| import { ADCServer } from '../../src/server'; | ||
|
|
||
| const readCert = (fileName: string) => | ||
| readFileSync(join(__dirname, '../assets/tls/', fileName), 'utf-8'); | ||
|
|
||
| // a backend whose certificate is signed by a CA the system does not trust | ||
| const backendPort = 48570; | ||
| const backendURL = `https://localhost:${backendPort}`; | ||
|
|
||
| describe('Server - Backend TLS', () => { | ||
| let server: ADCServer; | ||
| let backend: https.Server; | ||
|
|
||
| const syncTo = (opts: Record<string, unknown>) => | ||
| request(server.TEST_ONLY_getExpress()) | ||
| .put('/sync') | ||
| .send({ | ||
| task: { | ||
| opts: { | ||
| backend: 'apisix', | ||
| server: backendURL, | ||
| token: 'mock', | ||
| cacheKey: 'default', | ||
| ...opts, | ||
| }, | ||
| config: {}, | ||
| }, | ||
| }); | ||
|
|
||
| beforeAll(async () => { | ||
| server = new ADCServer({ | ||
| listen: new URL('http://127.0.0.1:3000'), | ||
| listenStatus: 3001, | ||
| }); | ||
| backend = https.createServer( | ||
| { cert: readCert('server.cer'), key: readCert('server.key') }, | ||
| (_, res) => (res.writeHead(404), res.end('{}')), | ||
| ); | ||
| await new Promise<void>((resolve) => | ||
| backend.listen(backendPort, '127.0.0.1', resolve), | ||
| ); | ||
| }); | ||
|
|
||
| afterAll(async () => { | ||
| await new Promise<void>((resolve) => backend.close(() => resolve())); | ||
| }); | ||
|
Comment on lines
+57
to
+59
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C3 'afterAll|backend\.close' apps/cli/e2e/server/tls.e2e-spec.ts
fd -H -t f '^package\.json$' . -x rg -n -C2 '"node"|`@types/node`|engines' {} || trueRepository: api7/adc Length of output: 1694 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n== tls.e2e-spec.ts ==\n'
cat -n apps/cli/e2e/server/tls.e2e-spec.ts | sed -n '1,140p'
printf '\n== backend.close references ==\n'
rg -n -C3 'close\(' apps/cli/e2e/server libs apps | sed -n '1,220p'
printf '\n== backend type / creation ==\n'
rg -n -C4 'backend\s*=|create.*backend|createServer|listen\(' apps/cli/e2e/server libs apps | sed -n '1,260p'Repository: api7/adc Length of output: 26277 🌐 Web query:
💡 Result: In Node.js, the Citations:
Propagate 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
|
|
||
| it('rejects an untrusted certificate', async () => { | ||
| const { status, body } = await syncTo({}); | ||
|
|
||
| expect(status).toEqual(500); | ||
| expect(body.message).toMatch(/unable to verify the first certificate/); | ||
| }); | ||
|
|
||
| it('accepts the certificate when its CA is provided', async () => { | ||
| const { status, body } = await syncTo({ caCert: readCert('ca.cer') }); | ||
|
|
||
| // the TLS handshake succeeds, so the backend's HTTP error surfaces instead | ||
| expect(status).toEqual(500); | ||
| expect(body.message).toMatch(/status code 404/); | ||
| }); | ||
|
|
||
| it('ignores the CA bundle when verification is off', async () => { | ||
| const { status, body } = await syncTo({ | ||
| tlsSkipVerify: true, | ||
| caCert: readCert('ca.cer'), | ||
| }); | ||
|
|
||
| expect(status).toEqual(500); | ||
| expect(body.message).toMatch(/status code 404/); | ||
| }); | ||
|
|
||
| it('rejects a CA bundle that is not PEM encoded', async () => { | ||
| const { status, body } = await syncTo({ caCert: 'not-a-certificate' }); | ||
|
|
||
| expect(status).toEqual(400); | ||
| expect(body.message).toMatch(/caCert must be a PEM-encoded certificate/); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| import { resolveHttpsAgent } from './agents'; | ||
|
|
||
| const CA_A = `-----BEGIN CERTIFICATE-----\nAAAA\n-----END CERTIFICATE-----`; | ||
| const CA_B = `-----BEGIN CERTIFICATE-----\nBBBB\n-----END CERTIFICATE-----`; | ||
|
|
||
| describe('Server - HTTPS agents', () => { | ||
| it('verifies against the system trust store by default', () => { | ||
| const agent = resolveHttpsAgent({}); | ||
| expect(agent.options.rejectUnauthorized).toEqual(true); | ||
| expect(agent.options.ca).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('skips verification when tlsSkipVerify is set', () => { | ||
| const agent = resolveHttpsAgent({ tlsSkipVerify: true }); | ||
| expect(agent.options.rejectUnauthorized).toEqual(false); | ||
| }); | ||
|
|
||
| it('verifies against the given CA bundle', () => { | ||
| const agent = resolveHttpsAgent({ caCert: CA_A }); | ||
| expect(agent.options.rejectUnauthorized).toEqual(true); | ||
| expect(agent.options.ca).toEqual(CA_A); | ||
| }); | ||
|
|
||
| it('reuses one agent per CA bundle, so sockets stay pooled', () => { | ||
| expect(resolveHttpsAgent({ caCert: CA_A })).toBe( | ||
| resolveHttpsAgent({ caCert: CA_A }), | ||
| ); | ||
| expect(resolveHttpsAgent({ caCert: CA_A })).not.toBe( | ||
| resolveHttpsAgent({ caCert: CA_B }), | ||
| ); | ||
| expect(resolveHttpsAgent({ caCert: CA_A })).not.toBe(resolveHttpsAgent({})); | ||
| }); | ||
|
|
||
| it('tlsSkipVerify takes precedence over the CA bundle', () => { | ||
| const agent = resolveHttpsAgent({ tlsSkipVerify: true, caCert: CA_A }); | ||
| expect(agent.options.rejectUnauthorized).toEqual(false); | ||
| expect(agent.options.ca).toBeUndefined(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import { HttpAgent, HttpOptions, HttpsAgent } from 'agentkeepalive'; | ||
|
|
||
| // create connection pool | ||
| const keepAlive: HttpOptions = { | ||
| keepAlive: true, | ||
| maxSockets: 256, // per host | ||
| maxFreeSockets: 16, // per host free | ||
| freeSocketTimeout: | ||
| parseInt(process.env.ADC_INGRESS_FREE_SOCKET_TIMEOUT ?? '') || 50000, // free socket keepalive for 50 seconds, and if the ADC_INGRESS_FREE_SOCKET_TIMEOUT environment variable is provided, it takes precedence. | ||
| }; | ||
|
|
||
| export const httpAgent = new HttpAgent(keepAlive); | ||
|
|
||
| const httpsAgent = new HttpsAgent({ | ||
| rejectUnauthorized: true, | ||
| ...keepAlive, | ||
| }); | ||
| const httpsInsecureAgent = new HttpsAgent({ | ||
| rejectUnauthorized: false, | ||
| ...keepAlive, | ||
| }); | ||
|
|
||
| // one agent per CA bundle, so sockets stay pooled across requests. | ||
| // keyed by the bundle itself; the key space is bounded by the number of | ||
| // distinct backends the server talks to. | ||
| const httpsCACertAgents = new Map<string, HttpsAgent>(); | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| export interface TLSOptions { | ||
| tlsSkipVerify?: boolean; | ||
|
|
||
| // PEM-encoded CA certificate (or bundle) to verify the backend against, | ||
| // instead of the system trust store. Ignored when tlsSkipVerify is set. | ||
| caCert?: string; | ||
| } | ||
|
|
||
| //TODO: support mTLS | ||
| export const resolveHttpsAgent = ({ | ||
| tlsSkipVerify, | ||
| caCert, | ||
| }: TLSOptions): HttpsAgent => { | ||
| if (tlsSkipVerify) return httpsInsecureAgent; | ||
| if (!caCert) return httpsAgent; | ||
|
|
||
| const cached = httpsCACertAgents.get(caCert); | ||
| if (cached) return cached; | ||
|
|
||
| const agent = new HttpsAgent({ | ||
| rejectUnauthorized: true, | ||
| ca: caCert, | ||
| ...keepAlive, | ||
| }); | ||
| httpsCACertAgents.set(caCert, agent); | ||
| return agent; | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 146ed37 — the key is now |
||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,18 @@ | ||
| import * as ADCSDK from '@api7/adc-sdk'; | ||
| import { z } from 'zod'; | ||
|
|
||
| const tlsSkipVerify = z.boolean().optional(); | ||
|
|
||
| // PEM-encoded CA certificate (or bundle) used to verify the backend, | ||
| // instead of the system trust store. | ||
| const caCert = z | ||
| .string() | ||
| .min(1) | ||
| .refine((cert) => cert.includes('-----BEGIN CERTIFICATE-----'), { | ||
| error: 'caCert must be a PEM-encoded certificate', | ||
| }) | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This one is inaccurate — .refine((val) => !val.path_prefix || val.path_prefix.startsWith("/"), {
error: "Path prefix must start with \"/\"",
})Verified rather than assumed — the e2e asserts on the response body and passes: |
||
| .optional(); | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| const SyncTask = z.strictObject({ | ||
| opts: z.looseObject({ | ||
| backend: z.string().min(1), | ||
|
|
@@ -12,6 +24,8 @@ const SyncTask = z.strictObject({ | |
| labelSelector: z.record(z.string(), z.string()).optional(), | ||
| cacheKey: z.string(), | ||
| bypassCache: z.boolean().optional().default(false), | ||
| tlsSkipVerify, | ||
| caCert, | ||
| }), | ||
| config: z.looseObject({}), | ||
| }); | ||
|
|
@@ -31,6 +45,8 @@ const ValidateTask = z.strictObject({ | |
| excludeResourceType: z.array(z.enum(ADCSDK.ResourceType)).optional(), | ||
| labelSelector: z.record(z.string(), z.string()).optional(), | ||
| cacheKey: z.string(), | ||
| tlsSkipVerify, | ||
| caCert, | ||
| }), | ||
| config: z.looseObject({}), | ||
| }); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.