diff --git a/src/nodes/utils.ts b/src/nodes/utils.ts index 9752ed3d5..68f192002 100644 --- a/src/nodes/utils.ts +++ b/src/nodes/utils.ts @@ -647,10 +647,11 @@ async function verifyClientCertificateChain( certChain.push(cert); } const now = new Date(); + let leafCert = true; for (let certIndex = 0; certIndex < certChain.length; certIndex++) { const cert = certChain[certIndex]; const certNext = certChain[certIndex + 1]; - if (now < cert.notBefore || now > cert.notAfter) { + if (leafCert && (now < cert.notBefore || now > cert.notAfter)) { return CryptoError.CertificateExpired; } const certNodeId = keysUtils.certNodeId(cert); @@ -675,6 +676,7 @@ async function verifyClientCertificateChain( return CryptoError.BadCertificate; } } + leafCert = false; } // Undefined means success return undefined; diff --git a/tests/client/utils.test.ts b/tests/client/utils.test.ts index a4e93484f..283c4dbf6 100644 --- a/tests/client/utils.test.ts +++ b/tests/client/utils.test.ts @@ -21,9 +21,9 @@ describe('client/utils', () => { beforeAll(async () => { cert = await testTlsUtils.createTLSConfigWithChain([ - keyPairRoot, - keyPairIntermediate, - keyPairLeaf, + [keyPairRoot, undefined], + [keyPairIntermediate, undefined], + [keyPairLeaf, undefined], ]); }); diff --git a/tests/nodes/utils.test.ts b/tests/nodes/utils.test.ts index 2fe25ddf2..ac6be9a91 100644 --- a/tests/nodes/utils.test.ts +++ b/tests/nodes/utils.test.ts @@ -9,6 +9,7 @@ import { IdInternal } from '@matrixai/id'; import { DB } from '@matrixai/db'; import { errors as rpcErrors } from '@matrixai/rpc'; import { utils as wsUtils } from '@matrixai/ws'; +import { CryptoError } from '@matrixai/quic/dist/native'; import * as nodesUtils from '@/nodes/utils'; import * as keysUtils from '@/keys/utils'; import * as validationErrors from '@/validation/errors'; @@ -334,9 +335,9 @@ describe('nodes/utils', () => { beforeAll(async () => { cert = await testTlsUtils.createTLSConfigWithChain([ - keyPairRoot, - keyPairIntermediate, - keyPairLeaf, + [keyPairRoot, undefined], + [keyPairIntermediate, undefined], + [keyPairLeaf, undefined], ]); }); @@ -384,6 +385,51 @@ describe('nodes/utils', () => { if (result2.result === 'fail') fail(); expect(Buffer.compare(result2.nodeId, nodeIdLeaf)).toBe(0); }); + test('verify on leaf cert with expired intermediate certs', async () => { + cert = await testTlsUtils.createTLSConfigWithChain([ + [keyPairRoot, 0], + [keyPairIntermediate, 0], + [keyPairIntermediate, 0], + [keyPairLeaf, undefined], + ]); + const result = await nodesUtils.verifyServerCertificateChain( + [nodeIdLeaf], + cert.certChainPem.map((v) => wsUtils.pemToDER(v)), + ); + expect(result.result).toBe('success'); + if (result.result === 'fail') fail(); + expect(Buffer.compare(result.nodeId, nodeIdLeaf)).toBe(0); + }); + test('verify on intermediate cert with expired intermediate certs', async () => { + cert = await testTlsUtils.createTLSConfigWithChain([ + [keyPairRoot, 0], + [keyPairIntermediate, 0], + [keyPairIntermediate, undefined], + [keyPairLeaf, undefined], + ]); + const result = await nodesUtils.verifyServerCertificateChain( + [nodeIdIntermediate], + cert.certChainPem.map((v) => wsUtils.pemToDER(v)), + ); + expect(result.result).toBe('success'); + if (result.result === 'fail') fail(); + expect(Buffer.compare(result.nodeId, nodeIdIntermediate)).toBe(0); + }); + test('fails with expired intermediate before valid target', async () => { + cert = await testTlsUtils.createTLSConfigWithChain([ + [keyPairRoot, 0], + [keyPairIntermediate, undefined], + [keyPairLeaf, 0], + [keyPairLeaf, undefined], + ]); + const result = await nodesUtils.verifyServerCertificateChain( + [nodeIdIntermediate], + cert.certChainPem.map((v) => wsUtils.pemToDER(v)), + ); + expect(result.result).toBe('fail'); + if (result.result !== 'fail') utils.never(); + expect(result.value).toBe(CryptoError.CertificateExpired); + }); }); describe('server verifyClientCertificateChain', () => { test('verify with multiple certs', async () => { @@ -392,6 +438,28 @@ describe('nodes/utils', () => { ); expect(result).toBeUndefined(); }); + test('verify with expired intermediate certs', async () => { + cert = await testTlsUtils.createTLSConfigWithChain([ + [keyPairRoot, 0], + [keyPairIntermediate, 0], + [keyPairLeaf, undefined], + ]); + const result = await nodesUtils.verifyClientCertificateChain( + cert.certChainPem.map((v) => wsUtils.pemToDER(v)), + ); + expect(result).toBeUndefined(); + }); + test('fails with expired leaf cert', async () => { + cert = await testTlsUtils.createTLSConfigWithChain([ + [keyPairRoot, undefined], + [keyPairIntermediate, undefined], + [keyPairLeaf, 0], + ]); + const result = await nodesUtils.verifyClientCertificateChain( + cert.certChainPem.map((v) => wsUtils.pemToDER(v)), + ); + expect(result).toBe(CryptoError.CertificateExpired); + }); }); }); }); diff --git a/tests/utils/tls.ts b/tests/utils/tls.ts index e43dd2456..58470655b 100644 --- a/tests/utils/tls.ts +++ b/tests/utils/tls.ts @@ -32,7 +32,7 @@ async function createTLSConfig( } async function createTLSConfigWithChain( - keyPairs: Array, + keyPairs: Array<[KeyPair, number | undefined]>, generateCertId?: () => CertId, ): Promise<{ keyPrivatePem: PrivateKeyPEM; @@ -43,10 +43,10 @@ async function createTLSConfigWithChain( let previousCert: Certificate | null = null; let previousKeyPair: KeyPair | null = null; const certChain: Array = []; - for (const keyPair of keyPairs) { + for (const [keyPair, duration] of keyPairs) { const newCert = await keysUtils.generateCertificate({ certId: generateCertId(), - duration: 31536000, + duration: duration ?? 31536000, issuerPrivateKey: previousKeyPair?.privateKey ?? keyPair.privateKey, subjectKeyPair: keyPair, issuerAttrsExtra: previousCert?.subjectName.toJSON(),