From 213d7d3ac5d4221efc3996d62f45a4f551554554 Mon Sep 17 00:00:00 2001 From: Claire Bontempo Date: Thu, 18 Nov 2021 17:02:48 -0800 Subject: [PATCH 01/12] add catch for node-forge error handling --- ui/app/helpers/parse-pki-cert.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/ui/app/helpers/parse-pki-cert.js b/ui/app/helpers/parse-pki-cert.js index 6da3c745d6f7..cf98181d1803 100644 --- a/ui/app/helpers/parse-pki-cert.js +++ b/ui/app/helpers/parse-pki-cert.js @@ -6,7 +6,14 @@ export function parsePkiCert([model]) { if (!model.certificate) { return; } - const cert = pki.certificateFromPem(model.certificate); + let cert; + // node-forge cannot parse EC (elliptical curve) certs + // so just return model (original response) if pki parse returns an error + try { + cert = pki.certificateFromPem(model.certificate); + } catch (error) { + return model; + } const commonName = cert.subject.getField('CN') ? cert.subject.getField('CN').value : null; const issueDate = cert.validity.notBefore; const expiryDate = cert.validity.notAfter; From 1a6395f8126b8871f6bec661b77659588bd50dd2 Mon Sep 17 00:00:00 2001 From: Claire Bontempo Date: Mon, 22 Nov 2021 11:56:17 -0500 Subject: [PATCH 02/12] update comment --- ui/app/helpers/parse-pki-cert.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/helpers/parse-pki-cert.js b/ui/app/helpers/parse-pki-cert.js index cf98181d1803..9cd63bc81dfb 100644 --- a/ui/app/helpers/parse-pki-cert.js +++ b/ui/app/helpers/parse-pki-cert.js @@ -8,7 +8,7 @@ export function parsePkiCert([model]) { } let cert; // node-forge cannot parse EC (elliptical curve) certs - // so just return model (original response) if pki parse returns an error + // return original response if unable to convert a Forge cert from PEM try { cert = pki.certificateFromPem(model.certificate); } catch (error) { From ec9ce568270106206830c61ecdbf027f23f7bcbb Mon Sep 17 00:00:00 2001 From: Claire Bontempo Date: Mon, 22 Nov 2021 12:13:46 -0500 Subject: [PATCH 03/12] adds changelog --- changelog/13238.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/13238.txt diff --git a/changelog/13238.txt b/changelog/13238.txt new file mode 100644 index 000000000000..7b11758d707d --- /dev/null +++ b/changelog/13238.txt @@ -0,0 +1,3 @@ +```release-note:bug +ui: Fixes node-forge error when parsing EC (elliptical curve) certs +``` \ No newline at end of file From 180384237f6355dd685e34bf1168bfc229368b14 Mon Sep 17 00:00:00 2001 From: Claire Bontempo Date: Mon, 22 Nov 2021 16:20:51 -0500 Subject: [PATCH 04/12] alphabetize attrs and add canParse attr --- ui/app/models/pki-ca-certificate.js | 107 ++++++++++++++-------------- ui/app/models/pki-certificate.js | 62 ++++++++-------- 2 files changed, 85 insertions(+), 84 deletions(-) diff --git a/ui/app/models/pki-ca-certificate.js b/ui/app/models/pki-ca-certificate.js index 8217165630e8..7e638cc0594a 100644 --- a/ui/app/models/pki-ca-certificate.js +++ b/ui/app/models/pki-ca-certificate.js @@ -4,7 +4,6 @@ import { computed } from '@ember/object'; import Certificate from './pki-certificate'; import lazyCapabilities, { apiPath } from 'vault/macros/lazy-capabilities'; -// TODO: alphabetize attrs export default Certificate.extend({ DISPLAY_FIELDS: computed(function() { return [ @@ -28,6 +27,7 @@ export default Certificate.extend({ backend: attr('string', { readOnly: true, }), + // canParse: attr('boolean'), caType: attr('string', { possibleValues: ['root', 'intermediate'], defaultValue: 'root', @@ -35,18 +35,71 @@ export default Certificate.extend({ readOnly: true, }), commonName: attr('string'), + csr: attr('string', { + editType: 'textarea', + label: 'CSR', + masked: true, + }), expiryDate: attr('string', { label: 'Expiration date', }), issueDate: attr('string'), + keyBits: attr('number', { + defaultValue: 2048, + }), + keyType: attr('string', { + possibleValues: ['rsa', 'ec', 'ed25519'], + defaultValue: 'rsa', + }), + maxPathLength: attr('number', { + defaultValue: -1, + }), + organization: attr({ + editType: 'stringArray', + }), + ou: attr({ + label: 'OU (OrganizationalUnit)', + editType: 'stringArray', + }), pemBundle: attr('string', { label: 'PEM bundle', editType: 'file', }), + permittedDnsNames: attr('string', { + label: 'Permitted DNS domains', + }), + privateKeyFormat: attr('string', { + possibleValues: ['', 'der', 'pem', 'pkcs8'], + defaultValue: '', + }), + type: attr('string', { + possibleValues: ['internal', 'exported'], + defaultValue: 'internal', + }), uploadPemBundle: attr('boolean', { label: 'Upload PEM bundle', readOnly: true, }), + + // address attrs + country: attr({ + editType: 'stringArray', + }), + locality: attr({ + editType: 'stringArray', + label: 'Locality/City', + }), + streetAddress: attr({ + editType: 'stringArray', + }), + postalCode: attr({ + editType: 'stringArray', + }), + province: attr({ + editType: 'stringArray', + label: 'Province/State', + }), + fieldDefinition: computed('caType', 'uploadPemBundle', function() { const type = this.caType; const isUpload = this.uploadPemBundle; @@ -98,58 +151,6 @@ export default Certificate.extend({ return groups; }), - type: attr('string', { - possibleValues: ['internal', 'exported'], - defaultValue: 'internal', - }), - ou: attr({ - label: 'OU (OrganizationalUnit)', - editType: 'stringArray', - }), - organization: attr({ - editType: 'stringArray', - }), - country: attr({ - editType: 'stringArray', - }), - locality: attr({ - editType: 'stringArray', - label: 'Locality/City', - }), - province: attr({ - editType: 'stringArray', - label: 'Province/State', - }), - streetAddress: attr({ - editType: 'stringArray', - }), - postalCode: attr({ - editType: 'stringArray', - }), - - keyType: attr('string', { - possibleValues: ['rsa', 'ec','ed25519'], - defaultValue: 'rsa', - }), - keyBits: attr('number', { - defaultValue: 2048, - }), - privateKeyFormat: attr('string', { - possibleValues: ['', 'der', 'pem', 'pkcs8'], - defaultValue: '', - }), - maxPathLength: attr('number', { - defaultValue: -1, - }), - permittedDnsNames: attr('string', { - label: 'Permitted DNS domains', - }), - - csr: attr('string', { - editType: 'textarea', - label: 'CSR', - masked: true, - }), deletePath: lazyCapabilities(apiPath`${'backend'}/root`, 'backend'), canDeleteRoot: and('deletePath.canDelete', 'deletePath.canSudo'), diff --git a/ui/app/models/pki-certificate.js b/ui/app/models/pki-certificate.js index ae57039ec60b..6b657f17ad03 100644 --- a/ui/app/models/pki-certificate.js +++ b/ui/app/models/pki-certificate.js @@ -6,10 +6,6 @@ import fieldToAttrs, { expandAttributeMeta } from 'vault/utils/field-to-attrs'; export default Model.extend({ idPrefix: 'cert/', - - backend: attr('string', { - readOnly: true, - }), //the id prefixed with `cert/` so we can use it as the *secret param for the secret show route idForNav: attr('string', { readOnly: true, @@ -29,55 +25,59 @@ export default Model.extend({ ]; }), - commonName: attr('string'), - expiryDate: attr('string', { - label: 'Expiration date', + altNames: attr('string', { + label: 'DNS/Email Subject Alternative Names (SANs)', }), - issueDate: attr('string'), - role: attr('object', { + backend: attr('string', { readOnly: true, }), - revocationTime: attr('number'), - altNames: attr('string', { - label: 'DNS/Email Subject Alternative Names (SANs)', + caChain: attr('string', { + label: 'CA chain', + masked: true, }), - ipSans: attr('string', { - label: 'IP Subject Alternative Names (SANs)', + canParse: attr('boolean'), + certificate: attr('string', { + masked: true, }), - otherSans: attr({ - editType: 'stringArray', - label: 'Other SANs', - helpText: - 'The format is the same as OpenSSL: ;: where the only current valid type is UTF8', + commonName: attr('string'), + excludeCnFromSans: attr('boolean', { + label: 'Exclude Common Name from Subject Alternative Names (SANs)', + defaultValue: false, }), - ttl: attr({ - label: 'TTL', - editType: 'ttl', + expiryDate: attr('string', { + label: 'Expiration date', }), format: attr('string', { defaultValue: 'pem', possibleValues: ['pem', 'der', 'pem_bundle'], }), - excludeCnFromSans: attr('boolean', { - label: 'Exclude Common Name from Subject Alternative Names (SANs)', - defaultValue: false, - }), - certificate: attr('string', { - masked: true, + ipSans: attr('string', { + label: 'IP Subject Alternative Names (SANs)', }), + issueDate: attr('string'), issuingCa: attr('string', { label: 'Issuing CA', masked: true, }), - caChain: attr('string', { - label: 'CA chain', - masked: true, + otherSans: attr({ + editType: 'stringArray', + label: 'Other SANs', + helpText: + 'The format is the same as OpenSSL: ;: where the only current valid type is UTF8', }), privateKey: attr('string', { masked: true, }), privateKeyType: attr('string'), + revocationTime: attr('number'), + role: attr('object', { + readOnly: true, + }), serialNumber: attr('string'), + ttl: attr({ + label: 'TTL', + editType: 'ttl', + }), fieldsToAttrs(fieldGroups) { return fieldToAttrs(this, fieldGroups); From b3e3f7671eac22a39e4e3c806a3765881e09b246 Mon Sep 17 00:00:00 2001 From: Claire Bontempo Date: Mon, 22 Nov 2021 16:26:29 -0500 Subject: [PATCH 05/12] show alert banner if unable to parse metadata --- ui/app/helpers/parse-pki-cert.js | 15 +++++++++------ ui/app/models/pki-ca-certificate.js | 2 +- ui/app/templates/components/pki-cert-show.hbs | 8 +++++++- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/ui/app/helpers/parse-pki-cert.js b/ui/app/helpers/parse-pki-cert.js index 9cd63bc81dfb..56a897fe6819 100644 --- a/ui/app/helpers/parse-pki-cert.js +++ b/ui/app/helpers/parse-pki-cert.js @@ -8,19 +8,22 @@ export function parsePkiCert([model]) { } let cert; // node-forge cannot parse EC (elliptical curve) certs - // return original response if unable to convert a Forge cert from PEM + // set canParse to false if unable to convert a Forge cert from PEM try { cert = pki.certificateFromPem(model.certificate); } catch (error) { - return model; + return { + can_parse: false, + }; } - const commonName = cert.subject.getField('CN') ? cert.subject.getField('CN').value : null; - const issueDate = cert.validity.notBefore; - const expiryDate = cert.validity.notAfter; + const commonName = cert?.subject.getField('CN') ? cert.subject.getField('CN').value : null; + const expiryDate = cert?.validity.notAfter; + const issueDate = cert?.validity.notBefore; return { + can_parse: true, common_name: commonName, - issue_date: issueDate, expiry_date: expiryDate, + issue_date: issueDate, }; } diff --git a/ui/app/models/pki-ca-certificate.js b/ui/app/models/pki-ca-certificate.js index 7e638cc0594a..c81a10f89022 100644 --- a/ui/app/models/pki-ca-certificate.js +++ b/ui/app/models/pki-ca-certificate.js @@ -27,7 +27,7 @@ export default Certificate.extend({ backend: attr('string', { readOnly: true, }), - // canParse: attr('boolean'), + canParse: attr('boolean'), caType: attr('string', { possibleValues: ['root', 'intermediate'], defaultValue: 'root', diff --git a/ui/app/templates/components/pki-cert-show.hbs b/ui/app/templates/components/pki-cert-show.hbs index cfade36dc251..d0b4aaa10099 100644 --- a/ui/app/templates/components/pki-cert-show.hbs +++ b/ui/app/templates/components/pki-cert-show.hbs @@ -8,7 +8,13 @@ - +{{#unless model.canParse}} + +{{/unless}}
{{#each model.attrs as |attr|}} From 0ef047a0e5b6f855af6ba71b378761905b5c4da9 Mon Sep 17 00:00:00 2001 From: Claire Bontempo Date: Mon, 22 Nov 2021 17:01:16 -0500 Subject: [PATCH 06/12] add alert banner to CA certs --- ui/app/templates/components/config-pki-ca.hbs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ui/app/templates/components/config-pki-ca.hbs b/ui/app/templates/components/config-pki-ca.hbs index 3da44ecdead2..299af82f41bd 100644 --- a/ui/app/templates/components/config-pki-ca.hbs +++ b/ui/app/templates/components/config-pki-ca.hbs @@ -8,6 +8,13 @@ {{/if}} {{#if (or model.certificate model.csr)}} + {{#unless model.canParse}} + + {{/unless}} {{#each model.attrs as |attr|}} {{#if attr.options.masked}} Date: Mon, 22 Nov 2021 17:06:44 -0500 Subject: [PATCH 07/12] change conditional in case canParse is undefined --- ui/app/templates/components/config-pki-ca.hbs | 4 ++-- ui/app/templates/components/pki-cert-show.hbs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ui/app/templates/components/config-pki-ca.hbs b/ui/app/templates/components/config-pki-ca.hbs index 299af82f41bd..e10e54707f68 100644 --- a/ui/app/templates/components/config-pki-ca.hbs +++ b/ui/app/templates/components/config-pki-ca.hbs @@ -8,13 +8,13 @@ {{/if}} {{#if (or model.certificate model.csr)}} - {{#unless model.canParse}} + {{#if not (eq model.canParse true)}} - {{/unless}} + {{/if}} {{#each model.attrs as |attr|}} {{#if attr.options.masked}} -{{#unless model.canParse}} +{{#if not (eq model.canParse true)}} -{{/unless}} +{{/if}}
{{#each model.attrs as |attr|}} From 145367f2edbe2c611032dbd52695befe6f60c1a3 Mon Sep 17 00:00:00 2001 From: Claire Bontempo Date: Mon, 22 Nov 2021 23:12:06 -0500 Subject: [PATCH 08/12] fixes indentation --- ui/app/templates/components/config-pki-ca.hbs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/templates/components/config-pki-ca.hbs b/ui/app/templates/components/config-pki-ca.hbs index e10e54707f68..dd2d8aa8a573 100644 --- a/ui/app/templates/components/config-pki-ca.hbs +++ b/ui/app/templates/components/config-pki-ca.hbs @@ -109,7 +109,7 @@ @allowCopy={{true}} /> - {{else if (and (get model attr.name) (or (eq attr.name "issueDate") (eq attr.name "expiryDate")))}} + {{else if (and (get model attr.name) (or (eq attr.name "issueDate") (eq attr.name "expiryDate")))}} From 80b1fe353e4a10f63526f5e8635b9276ef1164aa Mon Sep 17 00:00:00 2001 From: Claire Bontempo Date: Tue, 23 Nov 2021 09:37:41 -0500 Subject: [PATCH 09/12] fixes if statement syntax --- ui/app/templates/components/config-pki-ca.hbs | 2 +- ui/app/templates/components/pki-cert-show.hbs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/app/templates/components/config-pki-ca.hbs b/ui/app/templates/components/config-pki-ca.hbs index dd2d8aa8a573..01e18f3e70e0 100644 --- a/ui/app/templates/components/config-pki-ca.hbs +++ b/ui/app/templates/components/config-pki-ca.hbs @@ -8,7 +8,7 @@ {{/if}} {{#if (or model.certificate model.csr)}} - {{#if not (eq model.canParse true)}} + {{#if (not (eq model.canParse true))}} -{{#if not (eq model.canParse true)}} +{{#if (not (eq model.canParse true))}} Date: Tue, 23 Nov 2021 11:40:28 -0500 Subject: [PATCH 10/12] add test to check info banner renders --- .../pki/section-cert-test.js | 12 +++++++++++- ui/tests/pages/components/config-pki-ca.js | 12 ++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/ui/tests/acceptance/settings/configure-secret-backends/pki/section-cert-test.js b/ui/tests/acceptance/settings/configure-secret-backends/pki/section-cert-test.js index b680e9964706..5c330db4aae1 100644 --- a/ui/tests/acceptance/settings/configure-secret-backends/pki/section-cert-test.js +++ b/ui/tests/acceptance/settings/configure-secret-backends/pki/section-cert-test.js @@ -1,4 +1,4 @@ -import { currentRouteName, settled, click } from '@ember/test-helpers'; +import { currentRouteName, settled, click, fillIn } from '@ember/test-helpers'; import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; import page from 'vault/tests/pages/settings/configure-secret-backends/pki/section-cert'; @@ -94,6 +94,16 @@ BXUV2Uwtxf+QCphnlht9muX2fsLIzDJea0JipWj1uf2H8OZsjE8= ); }); + test('EC cert config: generate', async function(assert) { + await mountAndNav(assert); + await settled(); + assert.equal(currentRouteName(), 'vault.cluster.settings.configure-secret-backend.section'); + + await page.form.generateCAKeyTypeEC(); + + assert.dom('[data-test-warning]').exists('Info banner renders when unable to parse certificate metadata'); + }); + test('cert config: upload', async function(assert) { await mountAndNav(assert); await settled(); diff --git a/ui/tests/pages/components/config-pki-ca.js b/ui/tests/pages/components/config-pki-ca.js index 8f7bc4ae3851..7e77dfb9e2c4 100644 --- a/ui/tests/pages/components/config-pki-ca.js +++ b/ui/tests/pages/components/config-pki-ca.js @@ -32,6 +32,9 @@ export default { enterCertAsText: clickable('[data-test-text-toggle]'), pemBundle: fillable('[data-test-text-file-textarea="true"]'), commonName: fillable('[data-test-input="commonName"]'), + toggleOptions: clickable('[data-test-toggle-group="Options"]'), + keyType: fillable('[data-test-input="keyType"]'), + keyBits: fillable('[data-test-input="keyBits"]'), issueDateIsPresent: text('[data-test-row-value="Issue date"]'), expiryDateIsPresent: text('[data-test-row-value="Expiration date"]'), @@ -48,6 +51,15 @@ export default { .submit(); }, + async generateCAKeyTypeEC(commonName = 'PKI CA EC') { + return await this.replaceCA() + .commonName(commonName) + .toggleOptions() + .keyType('ec') + .keyBits(256) + .submit(); + }, + async uploadCA(pem) { return await this.replaceCA() .uploadCert() From 34e422ce84024413170724501e3048463a413e48 Mon Sep 17 00:00:00 2001 From: Claire Bontempo Date: Tue, 23 Nov 2021 12:15:18 -0500 Subject: [PATCH 11/12] remove unused var? \\\\ q --- .../settings/configure-secret-backends/pki/section-cert-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/tests/acceptance/settings/configure-secret-backends/pki/section-cert-test.js b/ui/tests/acceptance/settings/configure-secret-backends/pki/section-cert-test.js index 5c330db4aae1..70ca4f5bfbb1 100644 --- a/ui/tests/acceptance/settings/configure-secret-backends/pki/section-cert-test.js +++ b/ui/tests/acceptance/settings/configure-secret-backends/pki/section-cert-test.js @@ -1,4 +1,4 @@ -import { currentRouteName, settled, click, fillIn } from '@ember/test-helpers'; +import { currentRouteName, settled, click } from '@ember/test-helpers'; import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; import page from 'vault/tests/pages/settings/configure-secret-backends/pki/section-cert'; From b89e7f9de3c46f7658dc35c6d9248e1a293d2d28 Mon Sep 17 00:00:00 2001 From: Claire Bontempo Date: Tue, 23 Nov 2021 12:35:49 -0500 Subject: [PATCH 12/12] update verbiage for CA certs --- ui/app/templates/components/config-pki-ca.hbs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/templates/components/config-pki-ca.hbs b/ui/app/templates/components/config-pki-ca.hbs index 01e18f3e70e0..9fe0b04b8767 100644 --- a/ui/app/templates/components/config-pki-ca.hbs +++ b/ui/app/templates/components/config-pki-ca.hbs @@ -11,7 +11,7 @@ {{#if (not (eq model.canParse true))}} {{/if}}