Skip to content

Commit bfffacd

Browse files
gribnoysupgribnoysup
committed
chore(devtools-proxy-support): push expired certs to the bottom of the system CA list
1 parent ca8aea2 commit bfffacd

File tree

2 files changed

+173
-63
lines changed

2 files changed

+173
-63
lines changed

‎packages/devtools-proxy-support/src/system-ca.spec.ts

Lines changed: 68 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
11
import { expect } from 'chai';
2-
import { removeCertificatesWithoutIssuer } from './system-ca';
2+
import type { ParsedX509Cert } from './system-ca';
3+
import {
4+
parseCACerts,
5+
removeCertificatesWithoutIssuer,
6+
sortByExpirationDate,
7+
} from './system-ca';
38

4-
describe('removeCertificatesWithoutIssuer', function () {
5-
it('removes certificates that do not have an issuer', function () {
6-
const certs = [
7-
`-----BEGIN CERTIFICATE-----
9+
describe('system-ca helpers', function () {
10+
describe('removeCertificatesWithoutIssuer', function () {
11+
it('removes certificates that do not have an issuer', function () {
12+
const certs = [
13+
`-----BEGIN CERTIFICATE-----
814
MIIFazCCA1OgAwIBAgIRAIIQz7DSQONZRGPgu2OCiwAwDQYJKoZIhvcNAQELBQAwTzELMAkG
915
A1UEBhMCVVMxKTAnBgNVBAoTIEludGVybmV0IFNlY3VyaXR5IFJlc2VhcmNoIEdyb3VwMRUw
1016
EwYDVQQDEwxJU1JHIFJvb3QgWDEwHhcNMTUwNjA0MTEwNDM4WhcNMzUwNjA0MTEwNDM4WjBP
@@ -32,7 +38,7 @@ qKOJ2qxq4RgqsahDYVvTH9w7jXbyLeiNdd8XM2w9U/t7y0Ff/9yi0GE44Za4rF2LN9d11TPA
3238
mRGunUHBcnWEvgJBQl9nJEiU0Zsnvgc/ubhPgXRR4Xq37Z0j4r7g1SgEEzwxA57demyPxgcY
3339
xn/eR44/KJ4EBs+lVDR3veyJm+kXQ99b21/+jh5Xos1AnX5iItreGCc=
3440
-----END CERTIFICATE-----`,
35-
`-----BEGIN CERTIFICATE-----
41+
`-----BEGIN CERTIFICATE-----
3642
MIIFYDCCBEigAwIBAgIQQAF3ITfU6UK47naqPGQKtzANBgkqhkiG9w0BAQsFADA/
3743
MSQwIgYDVQQKExtEaWdpdGFsIFNpZ25hdHVyZSBUcnVzdCBDby4xFzAVBgNVBAMT
3844
DkRTVCBSb290IENBIFgzMB4XDTIxMDEyMDE5MTQwM1oXDTI0MDkzMDE4MTQwM1ow
@@ -63,12 +69,63 @@ WCLKTVXkcGdtwlfFRjlBz4pYg1htmf5X6DYO8A4jqv2Il9DjXA6USbW1FzXSLr9O
6369
he8Y4IWS6wY7bCkjCWDcRQJMEhg76fsO3txE+FiYruq9RUWhiF1myv4Q6W+CyBFC
6470
Dfvp7OOGAN6dEOM4+qR9sdjoSYKEBpsr6GtPAQw4dy753ec5
6571
-----END CERTIFICATE-----`,
66-
];
67-
expect(removeCertificatesWithoutIssuer(certs)).to.deep.equal({
68-
ca: [certs[0]],
69-
messages: [
72+
];
73+
const messages = [];
74+
const filtered = removeCertificatesWithoutIssuer(
75+
parseCACerts(certs, messages),
76+
messages
77+
);
78+
expect(
79+
filtered.map((cert) => {
80+
return cert.pem;
81+
})
82+
).to.deep.equal([certs[0]]);
83+
expect(messages).to.deep.eq([
7084
`Removing certificate for 'C=US\nO=Internet Security Research Group\nCN=ISRG Root X1' because issuer 'O=Digital Signature Trust Co.\nCN=DST Root CA X3' could not be found (serial no '4001772137D4E942B8EE76AA3C640AB7')`,
71-
],
85+
]);
86+
});
87+
});
88+
89+
describe('sortByExpirationDate', function () {
90+
it('pushes all expired certs to the bottom', function () {
91+
const mockCerts = [
92+
{
93+
serialNumber: '01',
94+
validTo: new Date(Date.now() + 10_000).toUTCString(),
95+
},
96+
{
97+
serialNumber: '02',
98+
validTo: new Date(Date.now() - 60_000).toUTCString(),
99+
},
100+
{
101+
serialNumber: '03',
102+
validTo: new Date(Date.now() - 50_000).toUTCString(),
103+
},
104+
{
105+
serialNumber: '04',
106+
validTo: new Date(Date.now() + 30_000).toUTCString(),
107+
},
108+
{
109+
serialNumber: '05',
110+
validTo: new Date(Date.now() + 20_000).toUTCString(),
111+
},
112+
{
113+
serialNumber: '06',
114+
validTo: new Date(Date.now() + 20_000).toUTCString(),
115+
},
116+
];
117+
118+
const sorted = sortByExpirationDate(
119+
mockCerts.map((parsed) => {
120+
return { pem: '', parsed } as ParsedX509Cert;
121+
})
122+
);
123+
124+
expect(
125+
sorted.map((cert) => {
126+
return cert.parsed?.serialNumber;
127+
})
128+
).to.deep.eq(['04', '05', '06', '01', '03', '02']);
72129
});
73130
});
74131
});

‎packages/devtools-proxy-support/src/system-ca.ts

Lines changed: 105 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -31,50 +31,39 @@ function systemCertsCached(systemCAOpts: SystemCAOptions = {}): Promise<{
3131
return systemCertsCachePromise;
3232
}
3333

34+
function certToString(cert: string | Uint8Array) {
35+
return typeof cert === 'string'
36+
? cert
37+
: Buffer.from(cert.buffer, cert.byteOffset, cert.byteLength).toString(
38+
'utf8'
39+
);
40+
}
41+
3442
export function mergeCA(...args: (NodeJSCAOption | undefined)[]): string {
3543
const ca = new Set<string>();
3644
for (const item of args) {
3745
if (!item) continue;
38-
const caList: readonly (string | Uint8Array)[] = Array.isArray(item)
39-
? item
40-
: [item];
46+
const caList = Array.isArray(item) ? item : [item];
4147
for (const cert of caList) {
42-
const asString =
43-
typeof cert === 'string'
44-
? cert
45-
: Buffer.from(cert.buffer, cert.byteOffset, cert.byteLength).toString(
46-
'utf8'
47-
);
48-
ca.add(asString);
48+
ca.add(certToString(cert));
4949
}
5050
}
5151
return [...ca].join('\n');
5252
}
5353

54-
const pemWithParsedCache = new WeakMap<
55-
string[],
56-
{
57-
ca: string[];
58-
messages: string[];
59-
}
60-
>();
61-
// TODO(COMPASS-8253): Remove this in favor of OpenSSL's X509_V_FLAG_PARTIAL_CHAIN
62-
// See linked tickets for details on why we need this (tl;dr: the system certificate
63-
// store may contain intermediate certficiates without the corresponding trusted root,
64-
// and OpenSSL does not seem to accept that)
65-
export function removeCertificatesWithoutIssuer(ca: string[]): {
66-
ca: string[];
67-
messages: string[];
68-
} {
69-
let result:
70-
| {
71-
ca: string[];
72-
messages: string[];
73-
}
74-
| undefined = pemWithParsedCache.get(ca);
54+
export type ParsedX509Cert = { pem: string; parsed: X509Certificate | null };
7555

76-
const messages: string[] = [];
77-
let caWithParsedCerts = ca.map((pem) => {
56+
/**
57+
* Safely parse provided certs, push any encountered errors to the provided
58+
* messages array
59+
*/
60+
export function parseCACerts(
61+
ca: NodeJSCAOption,
62+
messages: string[]
63+
): ParsedX509Cert[] {
64+
ca = Array.isArray(ca) ? ca : [ca];
65+
return ca.map((cert) => {
66+
const pem = certToString(cert);
7867
let parsed: X509Certificate | null = null;
7968
try {
8069
parsed = new X509Certificate(pem);
@@ -89,23 +78,84 @@ export function removeCertificatesWithoutIssuer(ca: string[]): {
8978
}
9079
return { pem, parsed };
9180
});
92-
caWithParsedCerts = caWithParsedCerts.filter(({ parsed }) => {
93-
const keep =
94-
!parsed ||
95-
parsed.checkIssued(parsed) ||
96-
caWithParsedCerts.find(
97-
({ parsed: issuer }) => issuer && parsed.checkIssued(issuer)
98-
);
99-
if (!keep) {
100-
messages.push(
81+
}
82+
83+
function doesCertificateHasMatchingIssuer(
84+
{ parsed }: ParsedX509Cert,
85+
certs: ParsedX509Cert[]
86+
) {
87+
return (
88+
!parsed ||
89+
parsed.checkIssued(parsed) ||
90+
certs.some(({ parsed: issuer }) => {
91+
return issuer && parsed.checkIssued(issuer);
92+
})
93+
);
94+
}
95+
96+
const withRemovedMissingIssuerCache = new WeakMap<
97+
ParsedX509Cert[],
98+
{
99+
ca: ParsedX509Cert[];
100+
messages: string[];
101+
}
102+
>();
103+
104+
// TODO(COMPASS-8253): Remove this in favor of OpenSSL's X509_V_FLAG_PARTIAL_CHAIN
105+
// See linked tickets for details on why we need this (tl;dr: the system certificate
106+
// store may contain intermediate certficiates without the corresponding trusted root,
107+
// and OpenSSL does not seem to accept that)
108+
export function removeCertificatesWithoutIssuer(
109+
ca: ParsedX509Cert[],
110+
messages: string[]
111+
): ParsedX509Cert[] {
112+
const result:
113+
| {
114+
ca: ParsedX509Cert[];
115+
messages: string[];
116+
}
117+
| undefined = withRemovedMissingIssuerCache.get(ca);
118+
119+
if (result) {
120+
messages.push(...result.messages);
121+
return result.ca;
122+
}
123+
124+
const _messages: string[] = [];
125+
const filteredCAlist = ca.filter((cert) => {
126+
const keep = doesCertificateHasMatchingIssuer(cert, ca);
127+
if (!keep && cert.parsed) {
128+
const { parsed } = cert;
129+
_messages.push(
101130
`Removing certificate for '${parsed.subject}' because issuer '${parsed.issuer}' could not be found (serial no '${parsed.serialNumber}')`
102131
);
103132
}
104133
return keep;
105134
});
106-
result = { ca: caWithParsedCerts.map(({ pem }) => pem), messages };
107-
pemWithParsedCache.set(ca, result);
108-
return result;
135+
withRemovedMissingIssuerCache.set(ca, {
136+
ca: filteredCAlist,
137+
messages: _messages,
138+
});
139+
messages.push(..._messages);
140+
return filteredCAlist;
141+
}
142+
143+
/**
144+
* Sorts cerificates by the Not After value. Items that are higher in the list
145+
* get picked up first by the CA issuer finding logic
146+
*
147+
* @see {@link https://jira.mongodb.org/browse/COMPASS-8322}
148+
*/
149+
export function sortByExpirationDate(ca: ParsedX509Cert[]) {
150+
return ca.slice().sort((a, b) => {
151+
if (!a.parsed || !b.parsed) {
152+
return 0;
153+
}
154+
return (
155+
new Date(b.parsed.validTo).getTime() -
156+
new Date(a.parsed.validTo).getTime()
157+
);
158+
});
109159
}
110160

111161
// Thin wrapper around system-ca, which merges:
@@ -135,11 +185,14 @@ export async function systemCA(
135185

136186
let systemCertsError: Error | undefined;
137187
let asyncFallbackError: Error | undefined;
138-
let systemCerts: string[] = [];
139-
let messages: string[] = [];
188+
let systemCerts: ParsedX509Cert[] = [];
189+
190+
const messages: string[] = [];
140191

141192
try {
142-
({ certs: systemCerts, asyncFallbackError } = await systemCertsCached());
193+
const systemCertsResult = await systemCertsCached();
194+
asyncFallbackError = systemCertsResult.asyncFallbackError;
195+
systemCerts = parseCACerts(systemCertsResult.certs, messages);
143196
} catch (err: any) {
144197
systemCertsError = err;
145198
}
@@ -150,14 +203,14 @@ export async function systemCA(
150203
!!process.env.DEVTOOLS_ALLOW_CERTIFICATES_WITHOUT_ISSUER
151204
)
152205
) {
153-
const reducedList = removeCertificatesWithoutIssuer(systemCerts);
154-
systemCerts = reducedList.ca;
155-
messages = messages.concat(reducedList.messages);
206+
systemCerts = removeCertificatesWithoutIssuer(systemCerts, messages);
156207
}
157208

158209
return {
159210
ca: mergeCA(
160-
systemCerts,
211+
sortByExpirationDate(systemCerts).map((cert) => {
212+
return cert.pem;
213+
}),
161214
rootCertificates,
162215
existingOptions.ca,
163216
await readTLSCAFilePromise

0 commit comments

Comments
 (0)