From 8bde02c6999b46469f18a1944a5d1ef2a2af118a Mon Sep 17 00:00:00 2001 From: hc-github-team-secure-vault-core <82990506+hc-github-team-secure-vault-core@users.noreply.github.com> Date: Wed, 19 Jul 2023 12:01:28 -0400 Subject: [PATCH] backport of commit 96bb63442204a1c8981947f3182640d04f29ff85 (#21931) Co-authored-by: claire bontempo <68122737+hellobontempo@users.noreply.github.com> --- changelog/21926.txt | 3 ++ ui/app/utils/parse-pki-cert-oids.js | 2 +- ui/app/utils/parse-pki-cert.js | 29 ++++++++++++--- .../addon/components/page/pki-issuer-list.hbs | 2 +- .../acceptance/pki/pki-configuration-test.js | 27 +++++++++++++- ui/tests/helpers/pki/values.js | 3 ++ ui/tests/helpers/pki/workflow.js | 3 ++ .../integration/utils/parse-pki-cert-test.js | 36 ++++++++++++++++++- 8 files changed, 96 insertions(+), 9 deletions(-) create mode 100644 changelog/21926.txt diff --git a/changelog/21926.txt b/changelog/21926.txt new file mode 100644 index 000000000..a6020204b --- /dev/null +++ b/changelog/21926.txt @@ -0,0 +1,3 @@ +```release-note:bug +ui: Fixes problem displaying certificates issued with unsupported signature algorithms (i.e. ed25519) +``` \ No newline at end of file diff --git a/ui/app/utils/parse-pki-cert-oids.js b/ui/app/utils/parse-pki-cert-oids.js index 281758f4b..5c97a768c 100644 --- a/ui/app/utils/parse-pki-cert-oids.js +++ b/ui/app/utils/parse-pki-cert-oids.js @@ -24,7 +24,7 @@ export const EXTENSION_OIDs = { }; // these are allowed ext oids, but not parsed and passed to cross-signed certs -export const IGNORED_OIDs = { +export const OTHER_OIDs = { // These two extensions are controlled by the parent authority. authority_key_identifier: '2.5.29.35', authority_access_info: '1.3.6.1.5.5.7.1.1', diff --git a/ui/app/utils/parse-pki-cert.js b/ui/app/utils/parse-pki-cert.js index 5aca3e7ae..03db1e21c 100644 --- a/ui/app/utils/parse-pki-cert.js +++ b/ui/app/utils/parse-pki-cert.js @@ -9,7 +9,7 @@ import { Certificate } from 'pkijs'; import { differenceInHours, getUnixTime } from 'date-fns'; import { EXTENSION_OIDs, - IGNORED_OIDs, + OTHER_OIDs, KEY_USAGE_BITS, SAN_TYPES, SIGNATURE_ALGORITHM_OIDs, @@ -131,15 +131,34 @@ export async function verifyCertificates(certA, certB, leaf) { const parsedCertB = jsonToCertObject(certB); if (leaf) { const parsedLeaf = jsonToCertObject(leaf); - const chainA = await parsedLeaf.verify(parsedCertA); - const chainB = await parsedLeaf.verify(parsedCertB); + const chainA = await verifySignature(parsedCertA, parsedLeaf); + const chainB = await verifySignature(parsedCertB, parsedLeaf); // the leaf's issuer should be equal to the subject data of the intermediate certs const isEqualA = parsedLeaf.issuer.isEqual(parsedCertA.subject); const isEqualB = parsedLeaf.issuer.isEqual(parsedCertB.subject); return chainA && chainB && isEqualA && isEqualB; } // can be used to validate if a certificate is self-signed (i.e. a root cert), by passing it as both certA and B - return (await parsedCertA.verify(parsedCertB)) && parsedCertA.issuer.isEqual(parsedCertB.subject); + return (await verifySignature(parsedCertA, parsedCertB)) && parsedCertA.issuer.isEqual(parsedCertB.subject); +} + +export async function verifySignature(parent, child) { + try { + return await child.verify(parent); + } catch (error) { + // ed25519 is an unsupported signature algorithm and so verify() errors + // SKID (subject key ID) is the byte array of the key identifier + // AKID (authority key ID) is a SEQUENCE-type extension that includes the key identifier and potentially other information. + const skidExtension = parent.extensions.find((ext) => ext.extnID === OTHER_OIDs.subject_key_identifier); + const akidExtension = parent.extensions.find((ext) => ext.extnID === OTHER_OIDs.authority_key_identifier); + // return false if either extension is missing + // this could mean a false-negative but that's okay for our use-case + if (!skidExtension || !akidExtension) return false; + const skid = new Uint8Array(skidExtension.parsedValue.valueBlock.valueHex); + const akid = new Uint8Array(akidExtension.extnValue.valueBlock.valueHex); + // Check that AKID includes the SKID, which saves us from parsing the AKID and is unlikely to return false-positives. + return akid.toString().includes(skid.toString()); + } } //* PARSING HELPERS @@ -182,7 +201,7 @@ export function parseExtensions(extensions) { if (!extensions) return null; const values = {}; const errors = []; - const allowedOids = Object.values({ ...EXTENSION_OIDs, ...IGNORED_OIDs }); + const allowedOids = Object.values({ ...EXTENSION_OIDs, ...OTHER_OIDs }); const isUnknownExtension = (ext) => !allowedOids.includes(ext.extnID); if (extensions.any(isUnknownExtension)) { const unknown = extensions.filter(isUnknownExtension).map((ext) => ext.extnID); diff --git a/ui/lib/pki/addon/components/page/pki-issuer-list.hbs b/ui/lib/pki/addon/components/page/pki-issuer-list.hbs index 649fa419f..7ee843815 100644 --- a/ui/lib/pki/addon/components/page/pki-issuer-list.hbs +++ b/ui/lib/pki/addon/components/page/pki-issuer-list.hbs @@ -2,7 +2,7 @@
-
+
{{pkiIssuer.issuerRef}} diff --git a/ui/tests/acceptance/pki/pki-configuration-test.js b/ui/tests/acceptance/pki/pki-configuration-test.js index 67f47d7a6..de0d672e9 100644 --- a/ui/tests/acceptance/pki/pki-configuration-test.js +++ b/ui/tests/acceptance/pki/pki-configuration-test.js @@ -6,7 +6,7 @@ import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; import { setupMirage } from 'ember-cli-mirage/test-support'; -import { click, currentURL, fillIn, visit, isSettled, waitUntil } from '@ember/test-helpers'; +import { click, currentURL, fillIn, visit, isSettled, waitUntil, find } from '@ember/test-helpers'; import { v4 as uuidv4 } from 'uuid'; import authPage from 'vault/tests/pages/auth'; @@ -193,5 +193,30 @@ module('Acceptance | pki configuration test', function (hooks) { "This PKI mount hasn't yet been configured with a certificate issuer. There are existing certificates. Use the CLI to perform any operations with them until an issuer is configured." ); }); + + // test coverage for ed25519 certs not displaying because the verify() function errors + test('it generates and displays a root issuer of key type = ed25519', async function (assert) { + assert.expect(4); + await authPage.login(this.pkiAdminToken); + await visit(`/vault/secrets/${this.mountPath}/pki/overview`); + await click(SELECTORS.issuersTab); + await click(SELECTORS.generateIssuerDropdown); + await click(SELECTORS.generateIssuerRoot); + await fillIn(SELECTORS.configuration.inputByName('type'), 'internal'); + await fillIn(SELECTORS.configuration.inputByName('commonName'), 'my-certificate'); + await click(SELECTORS.configuration.keyParamsGroupToggle); + await fillIn(SELECTORS.configuration.inputByName('keyType'), 'ed25519'); + await click(SELECTORS.configuration.generateRootSave); + + const issuerId = find(SELECTORS.configuration.saved.issuerLink).innerHTML; + await visit(`/vault/secrets/${this.mountPath}/pki/issuers`); + assert.dom(SELECTORS.issuerListItem(issuerId)).exists(); + assert + .dom('[data-test-common-name="0"]') + .hasText('my-certificate', 'parses certificate metadata in the list view'); + await click(SELECTORS.issuerListItem(issuerId)); + assert.strictEqual(currentURL(), `/vault/secrets/${this.mountPath}/pki/issuers/${issuerId}/details`); + assert.dom(SELECTORS.configuration.saved.commonName).exists('renders issuer details'); + }); }); }); diff --git a/ui/tests/helpers/pki/values.js b/ui/tests/helpers/pki/values.js index 1c49bb583..7c945b6f8 100644 --- a/ui/tests/helpers/pki/values.js +++ b/ui/tests/helpers/pki/values.js @@ -180,3 +180,6 @@ export const oldParentIssuerCert = `-----BEGIN CERTIFICATE-----\nMIIDKzCCAhOgAwI export const parentIssuerCert = `-----BEGIN CERTIFICATE-----\nMIIDKzCCAhOgAwIBAgIUBxLeuD3K0hF5dGpaEgZqytTN3lswDQYJKoZIhvcNAQEL\nBQAwHTEbMBkGA1UEAxMSTG9uZy1MaXZlZCBSb290IFgyMB4XDTIzMDEyNTAwMjQz\nM1oXDTIzMDIyNjAwMjUwM1owHTEbMBkGA1UEAxMSTG9uZy1MaXZlZCBSb290IFgy\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAuqkwN4m5dFLwFi0iYs4r\nTO4HWzloF4yCOaNfVksh1cOafVu9vjFwOWgHZFe6h8BOn6biKdFtvGTyIzlMHe5t\nyFmec9pfjX243bH9Ev4n2RTMKs818g9LdoZT6SI7DxHEu3yuHBg9TM87+GB+dA1V\nkRsK5hgtNCSMdgFSljM169sYbNilpk8M7O2hr+AmgRi0c1nUEPCe4JAr0Zv8iweJ\ntFRVHiQJXD9WIVxaWVxqWFsHoXseZS7H76RSdf4jNfENmBguHZMAPhtqlc/pMan8\nu0IJETWjWENn+WYC7DnnfQtNqyebU2LdT3oKO8tELqITygjT2tCS1Zavmsy69VY0\nYwIDAQABo2MwYTAOBgNVHQ8BAf8EBAMCAQYwDwYDVR0TAQH/BAUwAwEB/zAdBgNV\nHQ4EFgQUxgchIBo+1F++IFW0F586I5QDFGYwHwYDVR0jBBgwFoAUxgchIBo+1F++\nIFW0F586I5QDFGYwDQYJKoZIhvcNAQELBQADggEBAI6DdnW8q/FqGqk/Y0k7iUrZ\nYkfMRlss6uBTzLev53eXqFIJ3+EFVfV+ohDEedlYYm2QCELzQcJSR7Q2I22PQj8X\nTO0yqk6LOCMv/4yiDhF4D+haiDU4joq5GX1dpFdlNSQ5fJmnLKu8HYbOhbwUo4ns\n4yGzIMulZR1Zqf/HGEOCYPDQ0ZHucmHn7uGhmV+kgYGoKVEZ8XxfmyNPKuwTAUHL\nfInPJZtbxXTVmiWWy3iraeI4XcUvaD0JtVnsVphYrqrSZ60DjgFsjiyenxePGHXf\nYXV9HIS6OXlvWhJKlSINOTv9fAa+e+JtK7frdvxJNHoTG34PiGXfOV2swTvLJQo=\n-----END CERTIFICATE-----\n`; export const intIssuerCert = `-----BEGIN CERTIFICATE-----\nMIIDKzCCAhOgAwIBAgIUPt5VyO6gyA4hVaMkdpNyBlP+I64wDQYJKoZIhvcNAQEL\nBQAwHTEbMBkGA1UEAxMSTG9uZy1MaXZlZCBSb290IFgxMB4XDTIzMDEyNTAwMjQz\nM1oXDTIzMDIyNjAwMjUwM1owHTEbMBkGA1UEAxMSU2hvcnQtTGl2ZWQgSW50IFIx\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAqsvFU7lzt06n1w6BL+Wa\nf9zd+Z3G90Kv0HAksoLaWYinhkxNIUTU8ar9HLa2WV4EoJbNq91Hn+jFc2SYEXtR\nV+jm0kEhz4C4AoQ4D0s83JcYNssiNbVA04wa5ovD0iA/pzwVz8TnJSfAAuZ3vXFl\nyIbQ3ESozt9hGjo/JOpoBh67E7xkuzw4lnC2rXGHdh9pk1Di+wqREnKU4nuhDLnT\nC/LL+Mkm07F1aMAW3Z/PWFmmsDJHMhZnaYo2LGCwU4A0U1ED0XpwflobVbkzZDmC\nXOPEI8UXG6VcL36zWnzEQnlZKN91MAa+s0E4z40KHKVSblSkjYD1K6n0y787ic2m\nDwIDAQABo2MwYTAOBgNVHQ8BAf8EBAMCAQYwDwYDVR0TAQH/BAUwAwEB/zAdBgNV\nHQ4EFgQUkBK+oGpo5DNj2pCKoUE08WFOxQUwHwYDVR0jBBgwFoAUQsdYFMtsNMYN\nDIhZHMd77kcLLi8wDQYJKoZIhvcNAQELBQADggEBAIf4Bp/NYftiN8LmQrVzPWAe\nc4Bxm/NFFtkwQEvFhndMN68MUyXa5yxAdnYAHN+fRpYPxbjoZNXjW/jx3Kjft44r\ntyNGrrkjR80TI9FbL53nN7hLtZQdizsQD0Wype4Q1JOIxYw2Wd5Hr/PVPrJZ3PGg\nwNeI5IRu/cVbVT/vkRaHqYSwpa+V2cZTaEk6h62KPaKu3ui+omoeitU6qXHOysXQ\nrdGkJl/x831sIKmN0dMiGeoJdHGAr/E2f3ijKbVPsjIxZbm2SSumldOFYWn9cNYD\nI6sizFH976Wpde/GRIvBIzJnlK3xgfy0D9AUvwKyt75PVEnshc9tlhxoSVlKaUE=\n-----END CERTIFICATE-----\n`; export const newlySignedCert = `-----BEGIN CERTIFICATE-----\nMIIDKzCCAhOgAwIBAgIUKapKK5Coau2sfIJgqA9jcC6BkWIwDQYJKoZIhvcNAQEL\nBQAwHTEbMBkGA1UEAxMSTG9uZy1MaXZlZCBSb290IFgyMB4XDTIzMDEyNTIyMjky\nNVoXDTIzMDIyNjIyMjk1NVowHTEbMBkGA1UEAxMSU2hvcnQtTGl2ZWQgSW50IFIx\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAqsvFU7lzt06n1w6BL+Wa\nf9zd+Z3G90Kv0HAksoLaWYinhkxNIUTU8ar9HLa2WV4EoJbNq91Hn+jFc2SYEXtR\nV+jm0kEhz4C4AoQ4D0s83JcYNssiNbVA04wa5ovD0iA/pzwVz8TnJSfAAuZ3vXFl\nyIbQ3ESozt9hGjo/JOpoBh67E7xkuzw4lnC2rXGHdh9pk1Di+wqREnKU4nuhDLnT\nC/LL+Mkm07F1aMAW3Z/PWFmmsDJHMhZnaYo2LGCwU4A0U1ED0XpwflobVbkzZDmC\nXOPEI8UXG6VcL36zWnzEQnlZKN91MAa+s0E4z40KHKVSblSkjYD1K6n0y787ic2m\nDwIDAQABo2MwYTAOBgNVHQ8BAf8EBAMCAQYwDwYDVR0TAQH/BAUwAwEB/zAdBgNV\nHQ4EFgQUkBK+oGpo5DNj2pCKoUE08WFOxQUwHwYDVR0jBBgwFoAUxgchIBo+1F++\nIFW0F586I5QDFGYwDQYJKoZIhvcNAQELBQADggEBAJaems1vgEjxgb3d1y9PYxzN\nLZbuf/+0+BCVa9k4bEsbuhXhEecFdIi2OKS6fabeoEOF97Gvqrgc+LEpNsU6lIRA\nkJ/nHe0CD2hf0aBQsGsOllYy/4QnrPlbowb4KizPknEMWdGcvfnlzzOJzo4/UuMk\nMZ9vn2GrINzfml/sLocOzP/MsPd8bBhXI2Emh2O9tJ4+zeHLhEzcM1gdNk8pp+wP\nEOks0EcN4UBkpEnDZcDTJVgp9XpWy19EEGqsxjBq6rlpIvPW8XHoH1jZSGY1KWBJ\nRGtDcGugwTxO9jYHz/a1qu4BVt5FFcb0L3IOvcr+3QCCeiJQHcVY8QRbO9M4AQk=\n-----END CERTIFICATE-----\n`; +// both certs generated with key type ed25519 +export const unsupportedSignatureRoot = `-----BEGIN CERTIFICATE-----\nMIIBXTCCAQ+gAwIBAgIUcp9CkzsU5Pkv2ZJO8Gp+tJrzuJYwBQYDK2VwMBIxEDAO\nBgNVBAMTB215LXJvb3QwHhcNMjMwNzE4MTYyNzQ3WhcNMjMwODE5MTYyODE3WjAS\nMRAwDgYDVQQDEwdteS1yb290MCowBQYDK2VwAyEAmZ+By07QvgAEX1HRjhltJlgK\nA8il2LYUpH0uw7f2lXCjdzB1MA4GA1UdDwEB/wQEAwIBBjAPBgNVHRMBAf8EBTAD\nAQH/MB0GA1UdDgQWBBTAcYaOaiKhDmYqSe6vg/lAtYspkDAfBgNVHSMEGDAWgBTA\ncYaOaiKhDmYqSe6vg/lAtYspkDASBgNVHREECzAJggdteS1yb290MAUGAytlcANB\nAG9xXZnKNEXRyfa91hm9S80PwlwIMh4MkWetwfPBn3M74cHzDK1okANmweca4RRq\nQHDPT7shx3CuosvL2Ori/ws=\n-----END CERTIFICATE-----`; +export const unsupportedSignatureInt = `-----BEGIN CERTIFICATE-----\nMIICfTCCAWWgAwIBAgIUei2XIhhsP1/ytDciEGfA1C7t/sMwDQYJKoZIhvcNAQEL\nBQAwFjEUMBIGA1UEAxMLZXhhbXBsZS5jb20wHhcNMjMwNzE4MTg1NDA3WhcNMjMw\nODE5MTg1NDM3WjASMRAwDgYDVQQDEwdpbnQtY3NyMCowBQYDK2VwAyEAa9vHnJA3\nnzA/fYiTUg8EhomjMtVp5O2c01nQRXEv72OjgcAwgb0wDgYDVR0PAQH/BAQDAgEG\nMA8GA1UdEwEB/wQFMAMBAf8wHQYDVR0OBBYEFGtjjUwrRGmFmYBHrUE38tSxvVM3\nMB8GA1UdIwQYMBaAFNng9+uArFyIUcD23XdvCSIfYiDPMEYGCCsGAQUFBwEBBDow\nODAZBggrBgEFBQcwAoYNaGFzaGljb3JwLmNvbTAbBggrBgEFBQcwAoYPdmF1bHRw\ncm9qZWN0LmlvMBIGA1UdEQQLMAmCB2ludC1jc3IwDQYJKoZIhvcNAQELBQADggEB\nAAOSNgZjesJG4BgLU8jQmOO7n6W8WcR+dT+ELDC1nLlEZ2BJCDSXXUX8AihIHKxn\nA9W4slABUacyJlAZo/o/wcxyfbA6PUXmHnoqEPZ3zXMwuLN/iRW7/uQvI6TIwnpH\nXETFARLmK8cfGgbhi24STkHTF4ljczkOab7sTUQTHELlo+F2gNtmgnyaBFCGUYor\nX1pkMBcBa9BWRsfhy8E+tBVVUrNNUddwzC/5nMLqT8XqENMndDoG7eeT9Ex6otZy\nzURkcq09FtcmyY2RBYkV4UzyHN7cESMIk/J33ZCNAfHaDGuOqTy5nYU5fTtjJcit\nwEcWiSesrKPCletBpuMpgiU=\n-----END CERTIFICATE-----\n`; diff --git a/ui/tests/helpers/pki/workflow.js b/ui/tests/helpers/pki/workflow.js index 42b7a5bc8..679aec084 100644 --- a/ui/tests/helpers/pki/workflow.js +++ b/ui/tests/helpers/pki/workflow.js @@ -12,6 +12,7 @@ import { SELECTORS as CONFIGURATION } from './pki-configure-create'; import { SELECTORS as DELETE } from './pki-delete-all-issuers'; import { SELECTORS as TIDY } from './page/pki-tidy-form'; import { SELECTORS as CONFIGEDIT } from './page/pki-configuration-edit'; +import { SELECTORS as GENROOT } from './pki-generate-root'; export const SELECTORS = { breadcrumbContainer: '[data-test-breadcrumbs]', @@ -51,6 +52,7 @@ export const SELECTORS = { ...KEYPAGES, }, // ISSUERS + issuerListItem: (id) => `[data-test-issuer-list="${id}"]`, importIssuerLink: '[data-test-generate-issuer="import"]', generateIssuerDropdown: '[data-test-issuer-generate-dropdown]', generateIssuerRoot: '[data-test-generate-issuer="root"]', @@ -70,6 +72,7 @@ export const SELECTORS = { ...CONFIGURATION, ...DELETE, ...TIDY, + ...GENROOT, }, // EDIT CONFIGURATION configEdit: { diff --git a/ui/tests/integration/utils/parse-pki-cert-test.js b/ui/tests/integration/utils/parse-pki-cert-test.js index d467cbd41..e1d0a8a61 100644 --- a/ui/tests/integration/utils/parse-pki-cert-test.js +++ b/ui/tests/integration/utils/parse-pki-cert-test.js @@ -11,14 +11,19 @@ import { fromBase64, stringToArrayBuffer } from 'pvutils'; import { Certificate } from 'pkijs'; import { addHours, fromUnixTime, isSameDay } from 'date-fns'; import errorMessage from 'vault/utils/error-message'; -import { SAN_TYPES } from 'vault/utils/parse-pki-cert-oids'; +import { OTHER_OIDs, SAN_TYPES } from 'vault/utils/parse-pki-cert-oids'; import { certWithoutCN, loadedCert, pssTrueCert, skeletonCert, unsupportedOids, + unsupportedSignatureRoot, + unsupportedSignatureInt, } from 'vault/tests/helpers/pki/values'; +import { verifyCertificates } from 'vault/utils/parse-pki-cert'; +import { jsonToCertObject } from 'vault/utils/parse-pki-cert'; +import { verifySignature } from 'vault/utils/parse-pki-cert'; module('Integration | Util | parse pki certificate', function (hooks) { setupTest(hooks); @@ -227,6 +232,35 @@ module('Integration | Util | parse pki certificate', function (hooks) { ); }); + test('the helper verifyCertificates catches errors', async function (assert) { + assert.expect(5); + const verifiedRoot = await verifyCertificates(unsupportedSignatureRoot, unsupportedSignatureRoot); + assert.true(verifiedRoot, 'returns true for root certificate'); + const verifiedInt = await verifyCertificates(unsupportedSignatureInt, unsupportedSignatureInt); + assert.false(verifiedInt, 'returns false for intermediate cert'); + + const filterExtensions = (list, oid) => list.filter((ext) => ext.extnID !== oid); + const { subject_key_identifier, authority_key_identifier } = OTHER_OIDs; + const testCert = jsonToCertObject(unsupportedSignatureRoot); + const certWithoutSKID = testCert; + certWithoutSKID.extensions = filterExtensions(testCert.extensions, subject_key_identifier); + assert.false( + await verifySignature(certWithoutSKID, certWithoutSKID), + 'returns false if no subject key ID' + ); + + const certWithoutAKID = testCert; + certWithoutAKID.extensions = filterExtensions(testCert.extensions, authority_key_identifier); + assert.false(await verifySignature(certWithoutAKID, certWithoutAKID), 'returns false if no AKID'); + + const certWithoutKeyID = testCert; + certWithoutAKID.extensions = []; + assert.false( + await verifySignature(certWithoutKeyID, certWithoutKeyID), + 'returns false if neither SKID or AKID' + ); + }); + test('it fails silently when passed null', async function (assert) { assert.expect(3); const parsedCert = parseCertificate(certWithoutCN);