Skip to content

Commit d4d5c49

Browse files
committed
Improve x509 certificate parsing.
1 parent 7eef434 commit d4d5c49

File tree

2 files changed

+37
-18
lines changed

2 files changed

+37
-18
lines changed

lib/authorizationRequest.js

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,9 @@ export async function get({
5656
fetched = true;
5757
({
5858
payload: authorizationRequest, response, jwt
59-
} = await _fetch({requestUrl, getVerificationKey, agent}));
59+
} = await _fetch({
60+
requestUrl, getTrustedCertificates, getVerificationKey, agent
61+
}));
6062
}
6163

6264
// ensure authorization request is valid
@@ -205,13 +207,26 @@ async function _checkClientIdSchemeRequirements({
205207
if(!certificatePublicKey) {
206208
throw createNamedError({
207209
message:
208-
'Client ID schemes starting with "x509_" must use the "x5c" header ' +
210+
'No "x5c" header with an acceptable public key found; client ID ' +
211+
'schemes starting with "x509_" must use the "x5c" header ' +
209212
'to provide an X.509 certificate with the public key for verifying ' +
210213
'the request.',
211214
name: 'DataError'
212215
});
213216
}
214217

218+
// ensure trusted certs can be retrieved
219+
if(typeof getTrustedCertificates !== 'function') {
220+
throw createNamedError({
221+
message:
222+
'No "getTrustedCertificates" function provided; client ID schemes ' +
223+
'starting with "x509_" require such a function to be provided ' +
224+
'that will return the certificates that are to be trusted ' +
225+
'when verifying X.509 certificate chains.',
226+
name: 'DataError'
227+
});
228+
}
229+
215230
// get trusted certificates for `x5c` and verify chain
216231
const {x5c} = protectedHeader;
217232
const chain = parseCertificateChain({x5c});
@@ -230,7 +245,7 @@ async function _checkClientIdSchemeRequirements({
230245
});
231246
}
232247

233-
let {clientId} = authorizationRequest;
248+
let {client_id: clientId} = authorizationRequest;
234249
clientId = clientId.startsWith(`${clientIdScheme}:`) ?
235250
clientId.slice(clientIdScheme.length + 2) : clientId;
236251

@@ -271,7 +286,7 @@ async function _checkClientIdSchemeRequirements({
271286
// DID expressed in client ID; this is checked by default when a proper
272287
// DID resolver is used in `getVerificationKey` but this check provides
273288
// a partial additional sanity check
274-
let {clientId} = authorizationRequest;
289+
let {client_id: clientId} = authorizationRequest;
275290
clientId = clientId.startsWith('decentralized_identifier:did:') ?
276291
clientId.slice('decentralized_identifier:'.length + 1) : clientId;
277292
if(!protectedHeader?.kid?.startsWith(clientId + '#')) {
@@ -322,6 +337,14 @@ function _get(sp, name) {
322337
return value === null ? undefined : value;
323338
}
324339

340+
function _importPublicKeyFromX5c({x5c}) {
341+
if(x5c?.[0]) {
342+
const pem =
343+
`-----BEGIN CERTIFICATE-----\n${x5c[0]}\n-----END CERTIFICATE-----`;
344+
return importX509(pem, 'ES256');
345+
}
346+
}
347+
325348
async function _parseJwt({
326349
jwt, getTrustedCertificates, getVerificationKey, signatureRequired
327350
}) {
@@ -347,7 +370,7 @@ async function _parseJwt({
347370
// parse any `x5c` to get certificate public key and include that in
348371
// `getVerificationKey` params
349372
const {x5c} = protectedHeader;
350-
certificatePublicKey = x5c ? await importX509(x5c, 'ES256') : undefined;
373+
certificatePublicKey = await _importPublicKeyFromX5c({x5c});
351374
if(getVerificationKey) {
352375
return getVerificationKey({
353376
protectedHeader, certificatePublicKey,

lib/x509.js

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,12 @@ export function fromPemOrBase64(str) {
1212
const tag = 'CERTIFICATE';
1313
const pattern = new RegExp(
1414
`-{5}BEGIN ${tag}-{5}([a-zA-Z0-9=+\\/\\n\\r]+)-{5}END ${tag}-{5}`, 'g');
15-
16-
const certificates = [];
17-
// FIXME: use regex split instead
18-
// FIXME: ensure if there are no matches to just run base64Decode()
19-
let matches = pattern.exec(str);
20-
while(matches) {
21-
const base64 = matches[1].replace(/\r/g, '').replace(/\n/g, '');
22-
certificates.push(Certificate.fromBER(base64Decode(base64)));
23-
matches = pattern.exec(str);
15+
const matches = pattern.exec(str);
16+
if(!matches) {
17+
throw new Error('No PEM or Base64-formatted certificate found.');
2418
}
25-
26-
return certificates;
19+
const b64 = matches[1].replace(/\r/g, '').replace(/\n/g, '');
20+
return _fromBase64(b64);
2721
}
2822

2923
export function hasDomainSubjectAltName({certificate, name} = {}) {
@@ -38,8 +32,6 @@ export function hasDomainSubjectAltName({certificate, name} = {}) {
3832
}
3933
}
4034
}
41-
// FIXME: remove logging
42-
console.log('subjectAltNames', subjectAltNames);
4335
return subjectAltNames.has(name);
4436
}
4537

@@ -63,3 +55,7 @@ export async function verifyCertificateChain({
6355
const verifyResult = await chainEngine.verify();
6456
return verifyResult;
6557
}
58+
59+
function _fromBase64(str) {
60+
return Certificate.fromBER(base64Decode(str));
61+
}

0 commit comments

Comments
 (0)