Skip to content

Commit

Permalink
cherry-pick(#32008): chore(client-certificates): rewrite error for un…
Browse files Browse the repository at this point in the history
…supported PFX errors
  • Loading branch information
mxschmitt committed Aug 5, 2024
1 parent ed9b4d9 commit cf31aa8
Show file tree
Hide file tree
Showing 6 changed files with 89 additions and 15 deletions.
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 @@ -444,7 +444,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')
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

0 comments on commit cf31aa8

Please sign in to comment.