From a50487275c23ac2fe7680bb9bb03d9206cab08dc Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Fri, 16 Apr 2021 05:54:04 -0400 Subject: [PATCH 1/4] Allow to import unarmored S/MIME certificate --- extension/js/common/core/crypto/key.ts | 23 +++++++---- .../js/common/core/crypto/smime/smime-key.ts | 40 +++++++++++++------ test/source/tests/unit-node.ts | 11 +++++ 3 files changed, 54 insertions(+), 20 deletions(-) diff --git a/extension/js/common/core/crypto/key.ts b/extension/js/common/core/crypto/key.ts index 2e87d6a802b..0e6f3507364 100644 --- a/extension/js/common/core/crypto/key.ts +++ b/extension/js/common/core/crypto/key.ts @@ -101,9 +101,10 @@ export class KeyUtil { if (isArmored) { allKeys.push(...await KeyUtil.parseMany(content.toString())); } else { - const { err, keys } = await opgp.key.read(typeof content === 'string' ? Buf.fromUtfStr(content) : content); - allErrs.push(...(err || [])); - allKeys.push(...await Promise.all(keys.map(key => OpenPGPKey.convertExternalLibraryObjToKey(key)))); + const buf = typeof content === 'string' ? Buf.fromUtfStr(content) : content; + const { keys, err } = await KeyUtil.readBinary(buf); + allKeys.push(...keys); + allErrs.push(...err); } } catch (e) { allErrs.push(e instanceof Error ? e : new Error(String(e))); @@ -134,7 +135,7 @@ export class KeyUtil { throw new UnexpectedKeyTypeError(`Key type is ${keyType}, expecting OpenPGP or x509 S/MIME`); } - public static parseBinary = async (key: Uint8Array, passPhrase: string): Promise => { + public static readBinary = async (key: Uint8Array, passPhrase?: string | undefined): Promise<{ keys: Key[], err: Error[] }> => { const allKeys: Key[] = [], allErr: Error[] = []; try { const { keys, err } = await opgp.key.read(key); @@ -158,14 +159,20 @@ export class KeyUtil { allErr.push(e as Error); } try { - allKeys.push(await SmimeKey.parseDecryptBinary(key, passPhrase)); + allKeys.push(await SmimeKey.parseDecryptBinary(key, passPhrase ?? '')); + return { keys: allKeys, err: [] }; } catch (e) { allErr.push(e as Error); } - if (allKeys.length > 0) { - return allKeys; + return { keys: allKeys, err: allErr }; + } + + public static parseBinary = async (key: Uint8Array, passPhrase?: string | undefined): Promise => { + const { keys, err } = await KeyUtil.readBinary(key, passPhrase); + if (keys.length > 0) { + return keys; } - throw new Error(allErr ? allErr.map((err, i) => (i + 1) + '. ' + err.message).join('\n') : 'Should not happen: no keys and no errors.'); + throw new Error(err.length ? err.map((e, i) => (i + 1) + '. ' + e.message).join('\n') : 'Should not happen: no keys and no errors.'); } public static armor = (pubkey: Key): string => { diff --git a/extension/js/common/core/crypto/smime/smime-key.ts b/extension/js/common/core/crypto/smime/smime-key.ts index 330639fb3a2..075a9cc7954 100644 --- a/extension/js/common/core/crypto/smime/smime-key.ts +++ b/extension/js/common/core/crypto/smime/smime-key.ts @@ -22,8 +22,18 @@ export class SmimeKey { public static parseDecryptBinary = async (buffer: Uint8Array, password: string): Promise => { const bytes = String.fromCharCode.apply(undefined, new Uint8Array(buffer) as unknown as number[]) as string; - const p12Asn1 = forge.asn1.fromDer(bytes); - const p12 = forge.pkcs12.pkcs12FromAsn1(p12Asn1, password); + const asn1 = forge.asn1.fromDer(bytes); + let certificate: forge.pki.Certificate | undefined; + try { + // try to recognize a certificate + certificate = forge.pki.certificateFromAsn1(asn1); + } catch (e) { + // fall back to p12 + } + if (certificate) { + return SmimeKey.getKeyFromCertificate(certificate, forge.pki.certificateToPem(certificate)); + } + const p12 = forge.pkcs12.pkcs12FromAsn1(asn1, password); const bags = p12.getBags({ bagType: forge.pki.oids.certBag }); if (!bags) { throw new Error('No user certificate found.'); @@ -32,15 +42,12 @@ export class SmimeKey { if (!bag) { throw new Error('No user certificate found.'); } - const certificate = bag[0]?.cert; + certificate = bag[0]?.cert; if (!certificate) { throw new Error('No user certificate found.'); } - const email = (certificate.subject.getField('CN') as { value: string }).value; - const normalizedEmail = Str.parseEmail(email).email; - if (!normalizedEmail) { - throw new UnreportableError(`This S/MIME x.509 certificate has an invalid recipient email: ${email}`); - } + SmimeKey.removeWeakKeys(certificate); + const normalizedEmail = SmimeKey.getNormalizedEmailFromCertificate(certificate); const key = { type: 'x509', id: certificate.serialNumber.toUpperCase(), @@ -84,14 +91,18 @@ export class SmimeKey { return { data: new Uint8Array(arr), type: 'smime' }; } - private static parsePemCertificate = (text: string): Key => { - const certificate = forge.pki.certificateFromPem(text); - SmimeKey.removeWeakKeys(certificate); + private static getNormalizedEmailFromCertificate = (certificate: forge.pki.Certificate): string => { const email = (certificate.subject.getField('CN') as { value: string }).value; const normalizedEmail = Str.parseEmail(email).email; if (!normalizedEmail) { throw new UnreportableError(`This S/MIME x.509 certificate has an invalid recipient email: ${email}`); } + return normalizedEmail; + } + + private static getKeyFromCertificate = (certificate: forge.pki.Certificate, pem: string): Key => { + SmimeKey.removeWeakKeys(certificate); + const normalizedEmail = SmimeKey.getNormalizedEmailFromCertificate(certificate); const key = { type: 'x509', id: certificate.serialNumber.toUpperCase(), @@ -110,10 +121,15 @@ export class SmimeKey { isPublic: certificate.publicKey && !certificate.privateKey, isPrivate: !!certificate.privateKey, } as Key; - (key as unknown as { rawArmored: string }).rawArmored = text; + (key as unknown as { rawArmored: string }).rawArmored = pem; return key; } + private static parsePemCertificate = (text: string): Key => { + const certificate = forge.pki.certificateFromPem(text); + return SmimeKey.getKeyFromCertificate(certificate, text); + } + private static removeWeakKeys = (certificate: forge.pki.Certificate) => { const publicKeyN = (certificate.publicKey as forge.pki.rsa.PublicKey)?.n; if (publicKeyN && publicKeyN.bitLength() < 2048) { diff --git a/test/source/tests/unit-node.ts b/test/source/tests/unit-node.ts index 7bb22a97f47..9ec4366d6ab 100644 --- a/test/source/tests/unit-node.ts +++ b/test/source/tests/unit-node.ts @@ -20,6 +20,7 @@ import { GoogleData, GmailParser, GmailMsg } from '../mock/google/google-data'; import { testConstants } from './tooling/consts'; import { PgpArmor } from '../core/crypto/pgp/pgp-armor'; import { equals } from '../buf.js'; +import * as forge from 'node-forge'; chai.use(chaiAsPromised); const expect = chai.expect; @@ -530,6 +531,16 @@ vpQiyk4ceuTNkUZ/qmgiMpQLxXZnDDo= t.pass(); }); + ava.default('[unit][KeyUtil.readMany] Parsing unarmored S/MIME certificate', async t => { + const pem = forge.pem.decode(smimeCert)[0]; + const { keys, errs } = await KeyUtil.readMany(Buf.fromRawBytesStr(pem.body)); + expect(keys.length).to.equal(1); + expect(errs.length).to.equal(0); + expect(keys[0].id).to.equal('63F7025E700F3945301FB2FBA5674F84'); + expect(keys[0].type).to.equal('x509'); + t.pass(); + }); + const smimeAndPgp = smimeCert + '\r\n' + expiredPgp; ava.default('[unit][KeyUtil.readMany] Parsing one S/MIME and one OpenPGP armored keys', async t => { From 67bd1dd9e941d2e93b0bddba56d6d74b961f9c0d Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Fri, 16 Apr 2021 09:45:40 -0400 Subject: [PATCH 2/4] fix --- extension/js/common/core/crypto/key.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/extension/js/common/core/crypto/key.ts b/extension/js/common/core/crypto/key.ts index 0e6f3507364..318a6a26a37 100644 --- a/extension/js/common/core/crypto/key.ts +++ b/extension/js/common/core/crypto/key.ts @@ -158,11 +158,13 @@ export class KeyUtil { } catch (e) { allErr.push(e as Error); } - try { - allKeys.push(await SmimeKey.parseDecryptBinary(key, passPhrase ?? '')); - return { keys: allKeys, err: [] }; - } catch (e) { - allErr.push(e as Error); + if (!allKeys.length) { + try { + allKeys.push(await SmimeKey.parseDecryptBinary(key, passPhrase ?? '')); + return { keys: allKeys, err: [] }; + } catch (e) { + allErr.push(e as Error); + } } return { keys: allKeys, err: allErr }; } From 16f8be18e355586858db33b664e4877301986d25 Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Sat, 17 Apr 2021 09:08:21 -0400 Subject: [PATCH 3/4] Extract emails from SubjectAltName extension of S/MIME certificate --- .../js/common/core/crypto/smime/smime-key.ts | 32 ++++--- test/source/tests/unit-node.ts | 86 +++++++++++++++++++ 2 files changed, 106 insertions(+), 12 deletions(-) diff --git a/extension/js/common/core/crypto/smime/smime-key.ts b/extension/js/common/core/crypto/smime/smime-key.ts index 075a9cc7954..2f7593e649a 100644 --- a/extension/js/common/core/crypto/smime/smime-key.ts +++ b/extension/js/common/core/crypto/smime/smime-key.ts @@ -47,7 +47,7 @@ export class SmimeKey { throw new Error('No user certificate found.'); } SmimeKey.removeWeakKeys(certificate); - const normalizedEmail = SmimeKey.getNormalizedEmailFromCertificate(certificate); + const emails = SmimeKey.getNormalizedEmailsFromCertificate(certificate); const key = { type: 'x509', id: certificate.serialNumber.toUpperCase(), @@ -56,8 +56,8 @@ export class SmimeKey { usableForSigning: SmimeKey.isEmailCertificate(certificate), usableForEncryptionButExpired: false, usableForSigningButExpired: false, - emails: [normalizedEmail], - identities: [normalizedEmail], + emails, + identities: emails, created: SmimeKey.dateToNumber(certificate.validity.notBefore), lastModified: SmimeKey.dateToNumber(certificate.validity.notBefore), expiration: SmimeKey.dateToNumber(certificate.validity.notAfter), @@ -91,18 +91,26 @@ export class SmimeKey { return { data: new Uint8Array(arr), type: 'smime' }; } - private static getNormalizedEmailFromCertificate = (certificate: forge.pki.Certificate): string => { - const email = (certificate.subject.getField('CN') as { value: string }).value; - const normalizedEmail = Str.parseEmail(email).email; - if (!normalizedEmail) { - throw new UnreportableError(`This S/MIME x.509 certificate has an invalid recipient email: ${email}`); + private static getNormalizedEmailsFromCertificate = (certificate: forge.pki.Certificate): string[] => { + const emailFromSubject = (certificate.subject.getField('CN') as { value: string }).value; + const normalizedEmail = Str.parseEmail(emailFromSubject).email; + const emails = normalizedEmail ? [normalizedEmail] : []; + // search for e-mails in subjectAltName extension + const subjectAltName = certificate.getExtension('subjectAltName') as { altNames: { type: number, value: string }[] }; + if (subjectAltName && subjectAltName.altNames) { + const emailsFromAltNames = subjectAltName.altNames.filter(entry => entry.type === 1). + map(entry => Str.parseEmail(entry.value).email).filter(Boolean); + emails.push(...emailsFromAltNames as string[]); } - return normalizedEmail; + if (emails.length) { + return emails; + } + throw new UnreportableError(`This S/MIME x.509 certificate has an invalid recipient email: ${emailFromSubject}`); } private static getKeyFromCertificate = (certificate: forge.pki.Certificate, pem: string): Key => { SmimeKey.removeWeakKeys(certificate); - const normalizedEmail = SmimeKey.getNormalizedEmailFromCertificate(certificate); + const emails = SmimeKey.getNormalizedEmailsFromCertificate(certificate); const key = { type: 'x509', id: certificate.serialNumber.toUpperCase(), @@ -111,8 +119,8 @@ export class SmimeKey { usableForSigning: certificate.publicKey && SmimeKey.isEmailCertificate(certificate), usableForEncryptionButExpired: false, usableForSigningButExpired: false, - emails: [normalizedEmail], - identities: [normalizedEmail], + emails, + identities: emails, created: SmimeKey.dateToNumber(certificate.validity.notBefore), lastModified: SmimeKey.dateToNumber(certificate.validity.notBefore), expiration: SmimeKey.dateToNumber(certificate.validity.notAfter), diff --git a/test/source/tests/unit-node.ts b/test/source/tests/unit-node.ts index 9ec4366d6ab..d3f2e867d16 100644 --- a/test/source/tests/unit-node.ts +++ b/test/source/tests/unit-node.ts @@ -541,6 +541,92 @@ vpQiyk4ceuTNkUZ/qmgiMpQLxXZnDDo= t.pass(); }); + ava.default('[unit][KeyUtil.parse] Correctly extracting email from SubjectAltName of S/MIME certificate', async t => { + /* + // generate a key pair + const keys = forge.pki.rsa.generateKeyPair(2048); + // create a certification request (CSR) + const csr = forge.pki.createCertificationRequest(); + csr.publicKey = keys.publicKey; + csr.setSubject([{ + name: 'commonName', + value: 'Jack Doe' + }]); + // set (optional) attributes + const subjectAltName = { + name: 'subjectAltName', + altNames: [{ + // 1 is RFC822Name type + type: 1, + value: 'email@embedded.in.subj.alt.name' + }] + } + const extensions = [subjectAltName]; + (csr as any).setAttributes([{ + name: 'extensionRequest', + extensions + }]); + csr.sign(keys.privateKey); + // issue a certificate based on the csr + const cert = forge.pki.createCertificate(); + cert.serialNumber = '1'; + cert.validity.notBefore = new Date(); + cert.validity.notAfter = new Date(); + cert.validity.notAfter.setFullYear(cert.validity.notBefore.getFullYear() + 30); + cert.setSubject(csr.subject.attributes); + const caCertPem = fs.readFileSync("./ca.crt", 'utf8'); + const caKeyPem = fs.readFileSync("./ca.key", 'utf8'); + const caCert = forge.pki.certificateFromPem(caCertPem); + const caKey = forge.pki.decryptRsaPrivateKey(caKeyPem, '1234'); + cert.setIssuer(caCert.subject.attributes); + cert.setExtensions([{ + name: 'basicConstraints', + cA: true + }, { + name: 'keyUsage', + keyCertSign: true, + digitalSignature: true, + nonRepudiation: true, + keyEncipherment: true, + dataEncipherment: true + }, subjectAltName + ]); + cert.publicKey = csr.publicKey; + cert.sign(caKey); + const pem = forge.pki.certificateToPem(cert); + */ + const pem = `-----BEGIN CERTIFICATE----- +MIIETTCCAjWgAwIBAgIBATANBgkqhkiG9w0BAQUFADB0MRMwEQYDVQQIDApTb21l +LVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQxFzAVBgNV +BAMMDlNvbWUgQXV0aG9yaXR5MSEwHwYJKoZIhvcNAQkBFhJhdXRob3JpdHlAdGVz +dC5jb20wIBcNMjEwNDE3MTIyMTMxWhgPMjA1MTA0MTcxMjIxMzFaMBMxETAPBgNV +BAMTCEphY2sgRG9lMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAyKOw +VX51bduPdwSLR4u1O4HuOrELZjlOx8SWlOdU2yDZmp9iTZ/jP318xUs7XL1gIMDF +mXuDZB+KU9rwvECOecazWp8vpfLV/Tn/lp5lDLz+QqwlSWruzz0Z49F6zCWfBMQQ +Y475a03pd0oo6Soxt89A5PXuQhIBgdniyxUeQe0Okd7MC5/w0R+95aqZB47ui7ur +R7HcyGzkvfADXvdeZQsKSjja0lVFUJAJ6Uj2o0R9Z1YHtZKH9/D75IiYY3gqYJtt +BZoZPOMpl7Jam5Hz7PVWV3aeeMsAAHALWK7qvfaNx3IOCVh5KYQZ544P7cGGgpuw +UamKkF+wR7H4d7OYPwIDAQABo0kwRzAMBgNVHRMEBTADAQH/MAsGA1UdDwQEAwIC +9DAqBgNVHREEIzAhgR9lbWFpbEBlbWJlZGRlZC5pbi5zdWJqLmFsdC5uYW1lMA0G +CSqGSIb3DQEBBQUAA4ICAQCeGSsJNYsyQXnRal3L0HDF8PTj5zBa2jCSVAuwMe9Z +LWSJEXetF6uwH3yJzCxe/ZGNheEUAMGnMC1lYwsZ8x/hO8WcnzGxC1kqS71jV0us +rYZGsSb6dOoSigUfrzEcImx33n5yKYS8cHN/tUMvPiULX9RlSWnKlAfQClQeIxEA +6Y1Jeu0AVP3ugMajxqHoA10JOOrqjKuvkkM3gha9iS+q0w0mqhJ8GzZfOTdFJj/G +/erHQ/HWL7mqJoGh+i6I9N5qBNmdNEZazXJ/ACfR46Zav7nOXBF9CZ4k4g3mr/Po +1L3FXotxDQaTITY4xrse/GNCd92Q2Pc3ASS1SWRozpefyY414qfDP4x7IYwFOnK/ +swVjxFEyniiliYOiUV7tEm5FYRkAaQIAMiAXsZQB5LwatJN7WCQMh3xfPiuW91wL +Qmq47Rku8zPVsmQ5oBF9Ip4RraLOapoL09abmhyS9CFiT+bqZYSa9erT81eZnEfY +p07CH3yZBVSw7nRTIS8ScDHRvTt+FzrcchVcPfXMfYeydosmgQdDFFy/fm2alb8B +JKEHXc4KK04f6Fa90Uo+1hVInMziuLRWN6vubkHUDSXY4jhGm84OksTyW3AFKigC +jLwe8W9IMt765T5x5oux9MmPDXF05xHfm4qfH/BMO3a802x5u2gJjJjuknrFdgXY +9Q== +-----END CERTIFICATE-----`; + const key = await KeyUtil.parse(pem); + expect(key.emails.length).to.equal(1); + expect(key.emails[0]).to.equal('email@embedded.in.subj.alt.name'); + t.pass(); + }); + const smimeAndPgp = smimeCert + '\r\n' + expiredPgp; ava.default('[unit][KeyUtil.readMany] Parsing one S/MIME and one OpenPGP armored keys', async t => { From f7865f51775c809d0f9d1ddce8ecee4b6f851d0b Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Sun, 18 Apr 2021 03:53:19 -0400 Subject: [PATCH 4/4] detect unique emails in S/MIME certificate --- extension/js/common/core/crypto/smime/smime-key.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extension/js/common/core/crypto/smime/smime-key.ts b/extension/js/common/core/crypto/smime/smime-key.ts index 2f7593e649a..b4557050f65 100644 --- a/extension/js/common/core/crypto/smime/smime-key.ts +++ b/extension/js/common/core/crypto/smime/smime-key.ts @@ -103,7 +103,7 @@ export class SmimeKey { emails.push(...emailsFromAltNames as string[]); } if (emails.length) { - return emails; + return emails.filter((value, index, self) => self.indexOf(value) === index); } throw new UnreportableError(`This S/MIME x.509 certificate has an invalid recipient email: ${emailFromSubject}`); }