Skip to content

Commit

Permalink
UI/Fix node-forge EC error (#13238)
Browse files Browse the repository at this point in the history
* add catch for node-forge error handling

* update comment

* adds changelog

* alphabetize attrs and add canParse attr

* show alert banner if unable to parse metadata

* add test to check info banner renders
  • Loading branch information
hellobontempo committed Nov 23, 2021
1 parent 34aa9a0 commit 3dbcc65
Show file tree
Hide file tree
Showing 8 changed files with 140 additions and 91 deletions.
3 changes: 3 additions & 0 deletions changelog/13238.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
ui: Fixes node-forge error when parsing EC (elliptical curve) certs
```
20 changes: 15 additions & 5 deletions ui/app/helpers/parse-pki-cert.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,24 @@ export function parsePkiCert([model]) {
if (!model.certificate) {
return;
}
const cert = pki.certificateFromPem(model.certificate);
const commonName = cert.subject.getField('CN') ? cert.subject.getField('CN').value : null;
const issueDate = cert.validity.notBefore;
const expiryDate = cert.validity.notAfter;
let cert;
// node-forge cannot parse EC (elliptical curve) certs
// set canParse to false if unable to convert a Forge cert from PEM
try {
cert = pki.certificateFromPem(model.certificate);
} catch (error) {
return {
can_parse: false,
};
}
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,
};
}

Expand Down
107 changes: 54 additions & 53 deletions ui/app/models/pki-ca-certificate.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 [
Expand All @@ -28,25 +27,79 @@ export default Certificate.extend({
backend: attr('string', {
readOnly: true,
}),
canParse: attr('boolean'),
caType: attr('string', {
possibleValues: ['root', 'intermediate'],
defaultValue: 'root',
label: 'CA Type',
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;
Expand Down Expand Up @@ -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'),
Expand Down
62 changes: 31 additions & 31 deletions ui/app/models/pki-certificate.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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: <oid>;<type>:<value> 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: <oid>;<type>:<value> 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);
Expand Down
9 changes: 8 additions & 1 deletion ui/app/templates/components/config-pki-ca.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@
{{/if}}
</h2>
{{#if (or model.certificate model.csr)}}
{{#if (not (eq model.canParse true))}}
<AlertBanner
@type="info"
@message="There was an error parsing the certificate's metadata. As a result, Vault cannot display the issue and expiration dates. This will not interfere with the certificate's functionality."
data-test-warning
/>
{{/if}}
{{#each model.attrs as |attr|}}
{{#if attr.options.masked}}
<InfoTableRow data-test-table-row
Expand Down Expand Up @@ -102,7 +109,7 @@
@allowCopy={{true}}
/>
</InfoTableRow>
{{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")))}}
<InfoTableRow data-test-table-row={{value}}
@label={{capitalize (or attr.options.label (humanize (dasherize attr.name)))}}
@value={{date-format (get model attr.name) 'MMM dd, yyyy hh:mm:ss a' isFormatted=true}}/>
Expand Down
8 changes: 7 additions & 1 deletion ui/app/templates/components/pki-cert-show.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,13 @@
</h1>
</p.levelLeft>
</PageHeader>

{{#if (not (eq model.canParse true))}}
<AlertBanner
@type="info"
@message="There was an error parsing the certificate's metadata. As a result, Vault cannot display the common name or the issue and expiration dates. This will not interfere with the certificate's functionality."
data-test-warning
/>
{{/if}}
<div class="box is-fullwidth is-sideless is-paddingless is-marginless">
<MessageError @model={{model}} />
{{#each model.attrs as |attr|}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
12 changes: 12 additions & 0 deletions ui/tests/pages/components/config-pki-ca.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"]'),
Expand All @@ -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()
Expand Down

0 comments on commit 3dbcc65

Please sign in to comment.