Skip to content

Commit

Permalink
fix: configure certificates for secure proxy (#8704)
Browse files Browse the repository at this point in the history
Fixes #7683

Signed-off-by: Jeff MAURY <[email protected]>
  • Loading branch information
jeffmaury authored Sep 10, 2024
1 parent 6043c96 commit 82b5f37
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 27 deletions.
4 changes: 4 additions & 0 deletions packages/main/src/plugin/extension-loader.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import type * as containerDesktopAPI from '@podman-desktop/api';
import { app } from 'electron';
import { beforeAll, beforeEach, describe, expect, test, vi } from 'vitest';

import type { Certificates } from '/@/plugin/certificates.js';
import type { ContributionManager } from '/@/plugin/contribution-manager.js';
import type { KubeGeneratorRegistry } from '/@/plugin/kubernetes/kube-generator-registry.js';
import { NavigationManager } from '/@/plugin/navigation/navigation-manager.js';
Expand Down Expand Up @@ -241,6 +242,8 @@ const dialogRegistry: DialogRegistry = {
saveDialog: saveDialogMock,
} as unknown as DialogRegistry;

const certificates: Certificates = {} as unknown as Certificates;

vi.mock('electron', () => {
return {
app: {
Expand Down Expand Up @@ -292,6 +295,7 @@ beforeAll(() => {
colorRegistry,
dialogRegistry,
safeStorageRegistry,
certificates,
);
});

Expand Down
4 changes: 3 additions & 1 deletion packages/main/src/plugin/extension-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import type { ApiSenderType } from './api.js';
import type { PodInfo } from './api/pod-info.js';
import type { AuthenticationImpl } from './authentication.js';
import { CancellationTokenSource } from './cancellation-token.js';
import type { Certificates } from './certificates.js';
import type { CliToolRegistry } from './cli-tool-registry.js';
import type { CommandRegistry } from './command-registry.js';
import type { ConfigurationRegistry, IConfigurationNode } from './configuration-registry.js';
Expand Down Expand Up @@ -192,6 +193,7 @@ export class ExtensionLoader {
private colorRegistry: ColorRegistry,
private dialogRegistry: DialogRegistry,
private safeStorageRegistry: SafeStorageRegistry,
private certificates: Certificates,
) {
this.pluginsDirectory = directories.getPluginsDirectory();
this.pluginsScanDirectory = directories.getPluginsScanDirectory();
Expand Down Expand Up @@ -289,7 +291,7 @@ export class ExtensionLoader {
fs.mkdirSync(this.pluginsScanDirectory, { recursive: true });
}

this.moduleLoader.addOverride(createHttpPatchedModules(this.proxy)); // add patched http and https
this.moduleLoader.addOverride(createHttpPatchedModules(this.proxy, this.certificates)); // add patched http and https
this.moduleLoader.addOverride({ '@podman-desktop/api': ext => ext.api }); // add podman desktop API

this.moduleLoader.overrideRequire();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,10 @@ export class ExtensionsCatalog {
maxFreeSockets: 256,
scheduling: 'lifo',
proxy: httpsProxyUrl,
ca: this.certificates.getAllCertificates(),
proxyRequestOptions: {
ca: this.certificates.getAllCertificates(),
},
});
} catch (error) {
throw new Error(`Failed to create https proxy agent from ${httpsProxyUrl}: ${error}`);
Expand Down
7 changes: 4 additions & 3 deletions packages/main/src/plugin/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,9 @@ export class PluginSystem {
const colorRegistry = new ColorRegistry(apiSender, configurationRegistry);
colorRegistry.init();

const proxy = new Proxy(configurationRegistry);
const certificates = new Certificates();
await certificates.init();
const proxy = new Proxy(configurationRegistry, certificates);
await proxy.init();

const telemetry = new Telemetry(configurationRegistry);
Expand All @@ -471,8 +473,6 @@ export class PluginSystem {
const notificationRegistry = new NotificationRegistry(apiSender, taskManager);
const menuRegistry = new MenuRegistry(commandRegistry);
const kubeGeneratorRegistry = new KubeGeneratorRegistry();
const certificates = new Certificates();
await certificates.init();
const imageRegistry = new ImageRegistry(apiSender, telemetry, certificates, proxy);
const viewRegistry = new ViewRegistry();
const context = new Context(apiSender);
Expand Down Expand Up @@ -670,6 +670,7 @@ export class PluginSystem {
colorRegistry,
dialogRegistry,
safeStorageRegistry,
certificates,
);
await this.extensionLoader.init();

Expand Down
21 changes: 13 additions & 8 deletions packages/main/src/plugin/proxy-resolver.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import type { HttpsProxyAgentOptions } from 'hpagent';
import { HttpsProxyAgent } from 'hpagent';
import { beforeEach, expect, test, vi } from 'vitest';

import type { Certificates } from './certificates.js';
import type { Proxy } from './proxy.js';
import * as ProxyResolver from './proxy-resolver.js';

Expand Down Expand Up @@ -83,48 +84,52 @@ const Http = 'http';
const HttpProxyUrl = `${Http}://proxy.url`;
const HttpsProxyUrl = 'https://proxy.url';

const certificates: Certificates = {
getAllCertificates: vi.fn(),
} as unknown as Certificates;

beforeEach(() => {
vi.clearAllMocks();
});

test('getOptions return options w/o agent if proxy not enabled', () => {
const proxy = createProxy(false);
const options = ProxyResolver.getOptions(proxy, false);
const options = ProxyResolver.getOptions(proxy, false, certificates);
expect(options.agent).toBeUndefined();
});

test('getOptions return options w/ agent for https proxy', () => {
const proxy = createProxy(true, HttpsProxyUrl, HttpProxyUrl);
const options = ProxyResolver.getOptions(proxy, true);
const options = ProxyResolver.getOptions(proxy, true, certificates);
expect(options.agent).not.toBeUndefined();
expect((options.agent as any).https).toBeTruthy();
});

test('getOptions return options w/ https.Agent for https proxy', () => {
const proxy = createProxy(true, undefined, HttpProxyUrl);
const options = ProxyResolver.getOptions(proxy, false);
const options = ProxyResolver.getOptions(proxy, false, certificates);
expect(options.agent).not.toBeUndefined();
expect((options.agent as any).https).toBeFalsy();
});

test('patched http get calls original with the original parameters when proxy is not enabled', () => {
const proxy = createProxy(false, HttpsProxyUrl, HttpProxyUrl);
const patched = ProxyResolver.createHttpPatchedModules(proxy);
const patched = ProxyResolver.createHttpPatchedModules(proxy, certificates);
patched.http.get(`${Http}://site.url`);
expect(get).toBeCalledWith(`${Http}://site.url`);
});

test('patched http get calls original method with the original parameters when proxy is enabled and socketPath is requested', () => {
const proxy = createProxy(true, HttpsProxyUrl, HttpProxyUrl);
const patched = ProxyResolver.createHttpPatchedModules(proxy);
const patched = ProxyResolver.createHttpPatchedModules(proxy, certificates);
const socketOptions = { socketPath: '/var/socket/path' };
patched.http.get(socketOptions);
expect(get).toBeCalledWith(socketOptions, undefined);
});

test('patched http get when called with url and callback calls original with options and callback', () => {
const proxy = createProxy(true, HttpsProxyUrl, HttpProxyUrl);
const patched = ProxyResolver.createHttpPatchedModules(proxy);
const patched = ProxyResolver.createHttpPatchedModules(proxy, certificates);
const colon = ':';
const url = `https://[fe80${colon}${colon}1802${colon}20ff${colon}fe8d${colon}d4ce]`;
const callback = vi.fn();
Expand All @@ -145,7 +150,7 @@ test('patched http get when called with url and callback calls original with opt

test('patched http get translates username@password in url to auth option', () => {
const proxy = createProxy(true, HttpsProxyUrl, HttpProxyUrl);
const patched = ProxyResolver.createHttpPatchedModules(proxy);
const patched = ProxyResolver.createHttpPatchedModules(proxy, certificates);
const url = 'https://usr:[email protected]';
const callback = vi.fn();
patched.http.get(url, callback);
Expand All @@ -166,7 +171,7 @@ test('patched http get translates username@password in url to auth option', () =

test('patched http get works when url passed as protocol and hostname in options', () => {
const proxy = createProxy(true, HttpsProxyUrl, HttpProxyUrl);
const patched = ProxyResolver.createHttpPatchedModules(proxy);
const patched = ProxyResolver.createHttpPatchedModules(proxy, certificates);
const callback = vi.fn();
const options = {
hostname: 'rest.url',
Expand Down
24 changes: 13 additions & 11 deletions packages/main/src/plugin/proxy-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import * as nodeurl from 'node:url';
import type { HttpProxyAgentOptions, HttpsProxyAgentOptions } from 'hpagent';
import { HttpProxyAgent, HttpsProxyAgent } from 'hpagent';

import type { Certificates } from './certificates.js';
import type { Proxy } from './proxy.js';

// Agents usage table
Expand All @@ -39,14 +40,15 @@ import type { Proxy } from './proxy.js';
// ------------------------------------
// Source - https://github.com/delvedor/hpagent/tree/main#usage

function createProxyAgent(secure: boolean, proxyUrl: string): http.Agent | https.Agent {
function createProxyAgent(secure: boolean, proxyUrl: string, certificates: Certificates): http.Agent | https.Agent {
const options = {
keepAlive: true,
keepAliveMsecs: 1000,
maxSockets: 256,
maxFreeSockets: 256,
scheduling: 'lifo',
proxy: proxyUrl,
ca: certificates.getAllCertificates(),
};
return secure
? new HttpsProxyAgent(options as HttpsProxyAgentOptions)
Expand All @@ -62,24 +64,24 @@ export function getProxyUrl(proxy: Proxy, secure: boolean): string | undefined {

type ProxyOptions = { agent?: http.Agent | https.Agent };

export function getOptions(proxy: Proxy, secure: boolean): ProxyOptions {
export function getOptions(proxy: Proxy, secure: boolean, certificates: Certificates): ProxyOptions {
const options: ProxyOptions = {};
const proxyUrl = getProxyUrl(proxy, secure);
if (proxyUrl) {
options.agent = createProxyAgent(secure, proxyUrl);
options.agent = createProxyAgent(secure, proxyUrl, certificates);
}
return options;
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function createHttpPatch(originals: typeof http | typeof https, proxy: Proxy): any {
export function createHttpPatch(originals: typeof http | typeof https, proxy: Proxy, certificates: Certificates): any {
return {
get: patch(originals.get),
request: patch(originals.request),
get: patch(originals.get, certificates),
request: patch(originals.request, certificates),
};

// eslint-disable-next-line @typescript-eslint/no-explicit-any
function patch(original: typeof http.get): any {
function patch(original: typeof http.get, certificates: Certificates): any {
function patched(
url?: string | nodeurl.URL | null,
options?: http.RequestOptions | null,
Expand Down Expand Up @@ -127,7 +129,7 @@ export function createHttpPatch(originals: typeof http | typeof https, proxy: Pr
const host = options.hostname ?? options.host;
const isLocalhost = !host || host === 'localhost' || host === '127.0.0.1';
if (!isLocalhost) {
options = { ...options, ...getOptions(proxy, options.protocol === 'https:') };
options = { ...options, ...getOptions(proxy, options.protocol === 'https:', certificates) };
}

return original(options, callback);
Expand All @@ -139,10 +141,10 @@ export function createHttpPatch(originals: typeof http | typeof https, proxy: Pr
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function createHttpPatchedModules(proxy: Proxy): any {
export function createHttpPatchedModules(proxy: Proxy, certificates: Certificates): any {
const res = {
http: { ...http, ...createHttpPatch(http, proxy) },
https: { ...https, ...createHttpPatch(https, proxy) },
http: { ...http, ...createHttpPatch(http, proxy, certificates) },
https: { ...https, ...createHttpPatch(https, proxy, certificates) },
};
return { ...res, 'node:https': res.https, 'node:http': res.http };
}
9 changes: 7 additions & 2 deletions packages/main/src/plugin/proxy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,16 @@ import type { AddressInfo } from 'node:net';
import { createProxy, type ProxyServer } from 'proxy';
import { describe, expect, test, vi } from 'vitest';

import type { Certificates } from '/@/plugin/certificates.js';
import type { ConfigurationRegistry } from '/@/plugin/configuration-registry.js';
import { ensureURL, Proxy } from '/@/plugin/proxy.js';

const URL = 'https://podman-desktop.io';

const certificates: Certificates = {
getAllCertificates: vi.fn(),
} as unknown as Certificates;

function getConfigurationRegistry(
enabled: boolean,
http: string | undefined,
Expand Down Expand Up @@ -64,7 +69,7 @@ async function buildProxy(): Promise<ProxyServer> {

test('fetch without proxy', async () => {
const configurationRegistry = getConfigurationRegistry(false, undefined, undefined, undefined);
const proxy = new Proxy(configurationRegistry);
const proxy = new Proxy(configurationRegistry, certificates);
await proxy.init();
await fetch(URL);
});
Expand All @@ -73,7 +78,7 @@ test('fetch with http proxy', async () => {
const proxyServer = await buildProxy();
const address = proxyServer.address() as AddressInfo;
const configurationRegistry = getConfigurationRegistry(true, `127.0.0.1:${address.port}`, undefined, undefined);
const proxy = new Proxy(configurationRegistry);
const proxy = new Proxy(configurationRegistry, certificates);
await proxy.init();
let connectDone = false;
proxyServer.on('connect', () => (connectDone = true));
Expand Down
12 changes: 10 additions & 2 deletions packages/main/src/plugin/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import type { Event, ProxySettings } from '@podman-desktop/api';
import { ProxyAgent } from 'undici';

import type { Certificates } from '/@/plugin/certificates.js';

import type { ConfigurationRegistry, IConfigurationNode } from './configuration-registry.js';
import { Emitter } from './events/emitter.js';
import { getProxyUrl } from './proxy-resolver.js';
Expand Down Expand Up @@ -61,7 +63,10 @@ export class Proxy {
private readonly _onDidStateChange = new Emitter<boolean>();
public readonly onDidStateChange: Event<boolean> = this._onDidStateChange.event;

constructor(private configurationRegistry: ConfigurationRegistry) {}
constructor(
private configurationRegistry: ConfigurationRegistry,
private certificates: Certificates,
) {}

async init(): Promise<void> {
const proxyConfigurationNode: IConfigurationNode = {
Expand Down Expand Up @@ -171,7 +176,10 @@ export class Proxy {
globalThis.fetch = function (url: any, opts?: any): Promise<Response> {
const proxyurl = getProxyUrl(_me, asURL(url).protocol === 'https');
if (proxyurl) {
opts = { ...opts, dispatcher: new ProxyAgent(proxyurl) };
opts = {
...opts,
dispatcher: new ProxyAgent({ uri: proxyurl, proxyTls: { ca: _me.certificates.getAllCertificates() } }),
};
}

return original(url, opts);
Expand Down

0 comments on commit 82b5f37

Please sign in to comment.