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

chore(client-certificates): rewrite error for unsupported PFX errors #32008

Merged
merged 3 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions packages/playwright-core/src/server/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import { Tracing } from './trace/recorder/tracing';
import type * as types from './types';
import type { HeadersArray, ProxySettings } from './types';
import { kMaxCookieExpiresDateInSeconds } from './network';
import { clientCertificatesToTLSOptions } from './socksClientCertificatesInterceptor';
import { clientCertificatesToTLSOptions, rewriteOpenSSLErrorIfNeeded } from './socksClientCertificatesInterceptor';

type FetchRequestOptions = {
userAgent: string;
Expand Down Expand Up @@ -452,7 +452,7 @@ export abstract class APIRequestContext extends SdkObject {
body.on('data', chunk => chunks.push(chunk));
body.on('end', notifyBodyFinished);
});
request.on('error', reject);
request.on('error', error => reject(rewriteOpenSSLErrorIfNeeded(error)));

const disposeListener = () => {
reject(new Error('Request context disposed.'));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import fs from 'fs';
import tls from 'tls';
import stream from 'stream';
import { createSocket, createTLSSocket } from '../utils/happy-eyeballs';
import { ManualPromise } from '../utils';
import { escapeHTML, ManualPromise, rewriteErrorMessage } from '../utils';
import type { SocksSocketClosedPayload, SocksSocketDataPayload, SocksSocketRequestedPayload } from '../common/socksProxy';
import { SocksProxy } from '../common/socksProxy';
import type * as channels from '@protocol/channels';
Expand Down Expand Up @@ -150,8 +150,10 @@ class SocksProxyConnection {
};

const handleError = (error: Error) => {
debugLogger.log('client-certificates', `error when connecting to target: ${error.message}`);
const responseBody = 'Playwright client-certificate error: ' + error.message;
error = rewriteOpenSSLErrorIfNeeded(error);
debugLogger.log('client-certificates', `error when connecting to target: ${error.message.replaceAll('\n', ' ')}`);
const responseBody = escapeHTML('Playwright client-certificate error: ' + error.message)
.replaceAll('\n', ' <br>');
if (internalTLS?.alpnProtocol === 'h2') {
// This method is available only in Node.js 20+
if ('performServerHandshake' in http2) {
Expand Down Expand Up @@ -297,3 +299,14 @@ export function clientCertificatesToTLSOptions(
function rewriteToLocalhostIfNeeded(host: string): string {
return host === 'local.playwright' ? 'localhost' : host;
}

export function rewriteOpenSSLErrorIfNeeded(error: Error): Error {
if (error.message !== 'unsupported')
dgozman marked this conversation as resolved.
Show resolved Hide resolved
return error;
return rewriteErrorMessage(error, [
'Unsupported TLS certificate.',
'Most likely, the security algorithm of the given certificate was deprecated by OpenSSL.',
'For more details, see https://github.com/openssl/openssl/blob/master/README-PROVIDERS.md#the-legacy-provider',
'You could probably modernize the certificate by following the steps at https://github.com/nodejs/node/issues/40672#issuecomment-1243648223',
].join('\n'));
}
8 changes: 8 additions & 0 deletions packages/playwright-core/src/utils/isomorphic/stringUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,3 +132,11 @@ export function escapeRegExp(s: string) {
// From https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions#escaping
return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); // $& means the whole matched string
}

const escaped = { '&': '&amp;', '<': '&lt;', '>': '&gt;', '"': '&quot;', '\'': '&#39;' };
export function escapeHTMLAttribute(s: string): string {
return s.replace(/[&<>"']/ug, char => (escaped as any)[char]);
}
export function escapeHTML(s: string): string {
return s.replace(/[&<]/ug, char => (escaped as any)[char]);
}
13 changes: 3 additions & 10 deletions packages/trace-viewer/src/snapshotRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

import { escapeHTMLAttribute, escapeHTML } from '@isomorphic/stringUtils';
import type { FrameSnapshot, NodeNameAttributesChildNodesSnapshot, NodeSnapshot, RenderedFrameSnapshot, ResourceSnapshot, SubtreeReferenceSnapshot } from '@trace/snapshot';

function isNodeNameAttributesChildNodesSnapshot(n: NodeSnapshot): n is NodeNameAttributesChildNodesSnapshot {
Expand Down Expand Up @@ -57,7 +58,7 @@ export class SnapshotRenderer {
// Old snapshotter was sending lower-case.
if (parentTag === 'STYLE' || parentTag === 'style')
return rewriteURLsInStyleSheetForCustomProtocol(n);
return escapeText(n);
return escapeHTML(n);
}

if (!(n as any)._string) {
Expand Down Expand Up @@ -106,7 +107,7 @@ export class SnapshotRenderer {
attrValue = 'link://' + value;
else if (attr.toLowerCase() === 'href' || attr.toLowerCase() === 'src' || attr === kCurrentSrcAttribute)
attrValue = rewriteURLForCustomProtocol(value);
builder.push(' ', attrName, '="', escapeAttribute(attrValue), '"');
builder.push(' ', attrName, '="', escapeHTMLAttribute(attrValue), '"');
}
builder.push('>');
for (const child of children)
Expand Down Expand Up @@ -193,14 +194,6 @@ export class SnapshotRenderer {
}

const autoClosing = new Set(['AREA', 'BASE', 'BR', 'COL', 'COMMAND', 'EMBED', 'HR', 'IMG', 'INPUT', 'KEYGEN', 'LINK', 'MENUITEM', 'META', 'PARAM', 'SOURCE', 'TRACK', 'WBR']);
const escaped = { '&': '&amp;', '<': '&lt;', '>': '&gt;', '"': '&quot;', '\'': '&#39;' };

function escapeAttribute(s: string): string {
return s.replace(/[&<>"']/ug, char => (escaped as any)[char]);
}
function escapeText(s: string): string {
return s.replace(/[&<]/ug, char => (escaped as any)[char]);
}

function snapshotNodes(snapshot: FrameSnapshot): NodeSnapshot[] {
if (!(snapshot as any)._nodes) {
Expand Down
Binary file not shown.
60 changes: 60 additions & 0 deletions tests/library/client-certificates.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,51 @@ test.describe('fetch', () => {
await request.dispose();
});

test('pass with trusted client certificates in pfx format', async ({ playwright, startCCServer, asset }) => {
const serverURL = await startCCServer();
const request = await playwright.request.newContext({
ignoreHTTPSErrors: true,
clientCertificates: [{
origin: new URL(serverURL).origin,
pfxPath: asset('client-certificates/client/trusted/cert.pfx'),
passphrase: 'secure'
}],
});
const response = await request.get(serverURL);
expect(response.url()).toBe(serverURL);
expect(response.status()).toBe(200);
expect(await response.text()).toContain('Hello Alice, your certificate was issued by localhost!');
await request.dispose();
});

test('should throw a http error if the pfx passphrase is incorect', async ({ playwright, startCCServer, asset, browserName }) => {
const serverURL = await startCCServer({ useFakeLocalhost: browserName === 'webkit' && process.platform === 'darwin' });
const request = await playwright.request.newContext({
ignoreHTTPSErrors: true,
clientCertificates: [{
origin: new URL(serverURL).origin,
pfxPath: asset('client-certificates/client/trusted/cert.pfx'),
passphrase: 'this-password-is-incorrect'
}],
});
await expect(request.get(serverURL)).rejects.toThrow('mac verify failure');
await request.dispose();
});

test('should fail with matching certificates in legacy pfx format', async ({ playwright, startCCServer, asset, browserName }) => {
const serverURL = await startCCServer({ useFakeLocalhost: browserName === 'webkit' && process.platform === 'darwin' });
const request = await playwright.request.newContext({
ignoreHTTPSErrors: true,
clientCertificates: [{
origin: new URL(serverURL).origin,
pfxPath: asset('client-certificates/client/trusted/cert-legacy.pfx'),
passphrase: 'secure'
}],
});
await expect(request.get(serverURL)).rejects.toThrow('Unsupported TLS certificate');
await request.dispose();
});

test('should work in the browser with request interception', async ({ browser, playwright, startCCServer, asset }) => {
const serverURL = await startCCServer();
const request = await playwright.request.newContext({
Expand Down Expand Up @@ -272,6 +317,21 @@ test.describe('browser', () => {
await page.close();
});

test('should fail with matching certificates in legacy pfx format', async ({ browser, startCCServer, asset, browserName }) => {
const serverURL = await startCCServer({ useFakeLocalhost: browserName === 'webkit' && process.platform === 'darwin' });
const page = await browser.newPage({
ignoreHTTPSErrors: true,
clientCertificates: [{
origin: new URL(serverURL).origin,
pfxPath: asset('client-certificates/client/trusted/cert-legacy.pfx'),
passphrase: 'secure'
}],
});
await page.goto(serverURL);
await expect(page.getByText('Unsupported TLS certificate.')).toBeVisible();
await page.close();
});

test('should throw a http error if the pfx passphrase is incorect', async ({ browser, startCCServer, asset, browserName }) => {
const serverURL = await startCCServer({ useFakeLocalhost: browserName === 'webkit' && process.platform === 'darwin' });
const page = await browser.newPage({
Expand Down
Loading