Skip to content

Allow to import unarmored S/MIME certificate #3568

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tomholub merged 4 commits into from
Apr 18, 2021
Merged

Allow to import unarmored S/MIME certificate #3568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a504872
Allow to import unarmored S/MIME certificate
rrrooommmaaa Apr 16, 2021
67bd1dd
fix
rrrooommmaaa Apr 16, 2021
16f8be1
Extract emails from SubjectAltName extension of S/MIME certificate
rrrooommmaaa Apr 17, 2021
f7865f5
detect unique emails in S/MIME certificate
rrrooommmaaa Apr 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 20 additions & 11 deletions extension/js/common/core/crypto/key.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
Expand Down Expand Up @@ -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<Key[]> => {
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);
Expand All @@ -157,15 +158,23 @@ export class KeyUtil {
} catch (e) {
allErr.push(e as Error);
}
try {
allKeys.push(await SmimeKey.parseDecryptBinary(key, passPhrase));
} 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);
}
}
if (allKeys.length > 0) {
return allKeys;
return { keys: allKeys, err: allErr };
}

public static parseBinary = async (key: Uint8Array, passPhrase?: string | undefined): Promise<Key[]> => {
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 => {
Expand Down
64 changes: 44 additions & 20 deletions extension/js/common/core/crypto/smime/smime-key.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,18 @@ export class SmimeKey {

public static parseDecryptBinary = async (buffer: Uint8Array, password: string): Promise<Key> => {
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.');
Expand All @@ -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 emails = SmimeKey.getNormalizedEmailsFromCertificate(certificate);
const key = {
type: 'x509',
id: certificate.serialNumber.toUpperCase(),
Expand All @@ -49,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),
Expand Down Expand Up @@ -84,14 +91,26 @@ 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);
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[]);
}
if (emails.length) {
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}`);
}

private static getKeyFromCertificate = (certificate: forge.pki.Certificate, pem: string): Key => {
SmimeKey.removeWeakKeys(certificate);
const emails = SmimeKey.getNormalizedEmailsFromCertificate(certificate);
const key = {
type: 'x509',
id: certificate.serialNumber.toUpperCase(),
Copy link
Contributor Author

@rrrooommmaaa rrrooommmaaa Apr 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid we can't use serialNumber as fingerprint.
Should we open a new task for this as well as include migration to re-calculate ids?

Copy link
Collaborator

@tomholub tomholub Apr 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created an issue to discuss

Expand All @@ -100,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),
Expand All @@ -110,10 +129,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) {
Expand Down
97 changes: 97 additions & 0 deletions test/source/tests/unit-node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -530,6 +531,102 @@ 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();
});

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 => {
Expand Down