diff --git a/changelog/_10416.txt b/changelog/_10416.txt new file mode 100644 index 0000000000..bf061e9250 --- /dev/null +++ b/changelog/_10416.txt @@ -0,0 +1,9 @@ +```release-note:bug +ui/pki: Fixes certificate parsing of the `key_usage` extension so details accurately reflect certificate values. +``` +```release-note:bug +ui/pki: Fixes creating and updating a role so `basic_constraints_valid_for_non_ca` is correctly set. +``` +```release-note:improvement +ui/pki: Adds support to configure `server_flag`, `client_flag`, `code_signing_flag`, and `email_protection_flag` parameters for creating/updating a role. +``` \ No newline at end of file diff --git a/ui/app/models/pki/role.js b/ui/app/models/pki/role.js index 3c6c6f8d9a..1f1957bce8 100644 --- a/ui/app/models/pki/role.js +++ b/ui/app/models/pki/role.js @@ -30,7 +30,7 @@ export default class PkiRoleModel extends Model { 'generateLease', 'noStore', 'noStoreMetadata', - 'addBasicConstraints', + 'basicConstraintsValidForNonCa', ]; if (this.version.isCommunity) { const entFields = ['noStoreMetadata']; @@ -163,7 +163,7 @@ export default class PkiRoleModel extends Model { detailsLabel: 'Add basic constraints', subText: 'Mark Basic Constraints valid when issuing non-CA certificates.', }) - addBasicConstraints; + basicConstraintsValidForNonCa; /* End of overriding default options */ /* Overriding OpenApi Domain handling options */ diff --git a/ui/app/utils/parse-pki-cert-oids.js b/ui/app/utils/parse-pki-cert-oids.js index af2bec8ab0..fb51bcbc14 100644 --- a/ui/app/utils/parse-pki-cert-oids.js +++ b/ui/app/utils/parse-pki-cert-oids.js @@ -33,6 +33,15 @@ export const OTHER_OIDs = { subject_key_identifier: '2.5.29.14', }; +// The order of KEY_USAGE_BITS is critical because the index corresponds +// to the bit position in a certificate. +// The byte value "10101000" contains +// Bit position: 0 1 2 3 4 5 6 7 +// Byte: 1 0 1 0 1 0 0 0 +// | | | +// | | KeyAgreement (index 4) +// | KeyEncipherment (index 2) +// DigitalSignature (index 0) export const KEY_USAGE_BITS = [ 'DigitalSignature', 'ContentCommitment', diff --git a/ui/app/utils/parse-pki-cert.js b/ui/app/utils/parse-pki-cert.js index 88608cc99c..9e36b881e4 100644 --- a/ui/app/utils/parse-pki-cert.js +++ b/ui/app/utils/parse-pki-cert.js @@ -310,17 +310,19 @@ export function parseExtensions(extensions) { } if (values.key_usage) { - // KeyUsage is a big-endian bit-packed enum. Unused right-most bits are - // truncated. So, a KeyUsage with CertSign+CRLSign would be "000001100", - // with the right two bits truncated, and packed into an 8-bit, one-byte - // string ("00000011"), introducing a leading zero. unused indicates that - // this bit can be discard, shifting our result over by one, to go back - // to its original form (minus trailing zeros). - // - // We can thus take our enumeration (KEY_USAGE_BITS), check whether the - // bits are asserted, and push in our pretty names as appropriate. - const unused = values.key_usage.valueBlock.unusedBits; + // KeyUsage is a big-endian bit-packed enum encoded as an ASN.1 BIT STRING. + // The rightmost bits may be unused padding. For example, a KeyUsage with + // CertSign (bit 5) and CRLSign (bit 6) would be encoded as: + // Bits: 0 0 0 0 0 1 1 0 = 0x06 + // Unused bits: 1 (the rightmost bit, bit 8, is padding) + // This means there is no data stored beyond bit 7. + // The byte value has the data bits in their correct positions. + // We can take our enumeration (KEY_USAGE_BITS), check whether each + // bit is asserted using bitwise masks, and push the pretty names as appropriate. + const keyUsage = new Uint8Array(values.key_usage.valueBlock.valueHex); + // DEBUG tip! To inspect key_usage in binary: + // console.log(keyUsage[0].toString(2).padStart(8, '0')) const computedKeyUsages = []; for (const enumIndex in KEY_USAGE_BITS) { @@ -328,19 +330,23 @@ export function parseExtensions(extensions) { const byteIndex = parseInt(enumIndex / 8); const bitIndex = parseInt(enumIndex % 8); const enumName = KEY_USAGE_BITS[enumIndex]; - const mask = 1 << (8 - bitIndex); // Big endian. + // start with 1 (binary 00000001) and << is a "left shift" which moves the 1 bit left by N (7 - bitIndex) positions + // so each `mask` sets the bit position as 1 for the corresponding KEY_USAGE_BITS index + // For example the `mask` for KeyEncipherment is: 00100000 + const mask = 1 << (7 - bitIndex); // Big endian. if (byteIndex >= keyUsage.length) { // DecipherOnly is rare and would push into a second byte, but we // don't have one so exit. break; } - let enumByte = keyUsage[byteIndex]; - const needsAdjust = byteIndex + 1 === keyUsage.length && unused > 0; - if (needsAdjust) { - enumByte = parseInt(enumByte << unused); - } - + const enumByte = keyUsage[byteIndex]; + // bitwise AND (&) operator compares the bit positions between two numbers. + // the resulting number has 1 if BOTH numbers have 1 in that bit position. + // 00001000 (mask) + // & 10101000 (enumByte) + // ---------- + // 00001000 (result) => If this equals `mask` isSet is true const isSet = (mask & enumByte) === mask; if (isSet) { computedKeyUsages.push(enumName); diff --git a/ui/lib/pki/addon/components/pki-key-usage.hbs b/ui/lib/pki/addon/components/pki-key-usage.hbs index f167e4fa32..eeeba0ee04 100644 --- a/ui/lib/pki/addon/components/pki-key-usage.hbs +++ b/ui/lib/pki/addon/components/pki-key-usage.hbs @@ -23,6 +23,20 @@ data-test-key-usage-ext-key-usage-checkboxes />
+
+ +
+ + {{#each this.keyUsageFlags as |field|}} + {{#let (get @model.allByKey field) as |attr|}} + + {{/let}} + {{/each}} + { + keyUsageFlags = ['clientFlag', 'serverFlag', 'codeSigningFlag', 'emailProtectionFlag']; keyUsageFields = KEY_USAGE_FIELDS; extKeyUsageFields = EXT_KEY_USAGE_FIELDS; diff --git a/ui/tests/helpers/pki/pki-helpers.ts b/ui/tests/helpers/pki/pki-helpers.ts index 5799d1244d..67bb78e3b5 100644 --- a/ui/tests/helpers/pki/pki-helpers.ts +++ b/ui/tests/helpers/pki/pki-helpers.ts @@ -213,6 +213,7 @@ hU9YsNh6bCDmnBDBsDMOI7h8lBRQwTiWVoSD9YNVvFiY29YvFbJQGdh+pmBtf7E+ d8SYWhRdxmH3qcHNPcR1iw== -----END CERTIFICATE-----`; const certWithoutCN = `-----BEGIN CERTIFICATE-----\nMIIDUDCCAjigAwIBAgIUEUpM5i7XMd/imZkR9XvonMaqPyYwDQYJKoZIhvcNAQEL\nBQAwHDEaMBgGCSqGSIb3DQEJARYLZm9vQGJhci5jb20wHhcNMjMwMTIzMjMyODEw\nWhcNMzMwMTIwMjMyODEwWjAcMRowGAYJKoZIhvcNAQkBFgtmb29AYmFyLmNvbTCC\nASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAPGSdeqLICZcoUzxk88F8Tp+\nVNI+mS74L8pHyb9ZNZfeXPo0E9L5pi+KKI7rkxAtBGUecG1ENSxDDK9p6XZhWHSU\nZ6bdjOsjcIlfiM+1hhtDclIVxIDnz2Jt1/Vmnm8DXwdwVATWiFLTnfm288deNwsT\npl0ehAR3BadkZvteC6t+giEw/4qm1/FP53GEBOQeUWJDZRvtL37rdx4joFv3cR4w\nV0dukOjc5AGXtIOorO145OSZj8s7RsW3pfGcFUcOg7/flDxfK1UqFflQa7veLvKa\nWE/fOMyB/711QjSkTuQ5Rw3Rf9Fr2pqVJQgElTIW1SKaX5EJTB9mtGB34UqUXtsC\nAwEAAaOBiTCBhjAdBgNVHQ4EFgQUyhFP/fm+798mErPD5VQvEaAZQrswHwYDVR0j\nBBgwFoAUyhFP/fm+798mErPD5VQvEaAZQrswDgYDVR0PAQH/BAQDAgWgMCAGA1Ud\nJQEB/wQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjASBgNVHRMBAf8ECDAGAQH/AgEK\nMA0GCSqGSIb3DQEBCwUAA4IBAQCishzVkhuSAtqxgsZdYzBs3GpakGIio5zReW27\n6dk96hYCbbe4K3DtcFbRD1B8t6aTJlHxkFRaOWErSXu9WP3fUhIDNRE64Qsrg1zk\n3Km430qBlorXmTp6xhYHQfY5bn5rT2YY7AmaYIlIFxRhod43i5GDbBP+e+d/vTqR\nv1AJflYofeR4LeATP64B6a4R+QQVoxI43+pyH3ka+nRHwJBR9h8SMtJoqBy7x9pl\nYlBDa8lSn05doA3+e03VIzitvBBWI4oX1XB0tShSLk6YJXayIwe0ZNVvfYLIRKCp\nb4DUwChYzG/FwFSssUAqzVFhu3i+uU3Z47bsLVm0R5m7hLiZ\n-----END CERTIFICATE-----`; +const certWithAllKeyUsage = `-----BEGIN CERTIFICATE-----\nMIIDjjCCAnagAwIBAgIUYykdVqVIR0kyYMzGIPeqijtsypowDQYJKoZIhvcNAQEL\nBQAwEjEQMA4GA1UEAxMHbXktcm9vdDAeFw0yNTEwMjcyMDUxMjNaFw0yNTEwMjcy\nMjUxNTNaMBYxFDASBgNVBAMTC2FsbGtleXVzYWdlMIIBIjANBgkqhkiG9w0BAQEF\nAAOCAQ8AMIIBCgKCAQEAxmTxzVAon27u1utDb6QCVftjJaMhop/XcIMea8kus0Kj\n1ASE6vITOq6OV0piofdJ1wL7PsZNMJFYHZQVKwXi+UlSxChJQdeqFfSJ6n/V6B7g\nO9cADMJNJOgrujyYQKDWDE9YxQJV4RoikthS0jMDuOSTYcX9s9KywGm5l9p+He0I\nHWZHhBpS3QIrn0InUjANtV5CD8aTqLIu5A3maX1Sba7hvH2h9gzXRhWxmjSAtUOp\ngyYXBbyL4bx7wELKCnIudUDBE4G7imzGbAR0nIRidZJ8vn3Cx9JyK4WMvmEC99wo\n+rDNzaVMsH1WPW9OAlaVX8y6b8Z1kQtCTpYk+fQlFwIDAQABo4HXMIHUMA8GA1Ud\nDwEB/wQFAwMH/4AwaQYDVR0lBGIwYAYEVR0lAAYIKwYBBQUHAwEGCCsGAQUFBwMC\nBggrBgEFBQcDAwYIKwYBBQUHAwQGCCsGAQUFBwMFBggrBgEFBQcDBgYIKwYBBQUH\nAwcGCCsGAQUFBwMIBggrBgEFBQcDCTAdBgNVHQ4EFgQUnakdzwExzVyymSbc6GPP\nyfNHPvEwHwYDVR0jBBgwFoAUBq3YXv2yL2unSF36GmuqeaW/SQkwFgYDVR0RBA8w\nDYILYWxsa2V5dXNhZ2UwDQYJKoZIhvcNAQELBQADggEBAL5f52isY1Oqn20Cez3R\ncgOuMvVBTKw3UAzjJhoSmZEVAhn1TcDjIXrb0dUhleacRdkV7whzL8BAsPV3lB2R\nw9fnWQct4CAlXSgli2w6ONcsfX8ehQ0tiEhjrfJEBu//6Zgj9K5FwMlZ/R0qok5s\nh2hGNU+j8AmE+MWlpdY2+hnvV800ENcHZLbokcRDu+WDGOGdbZcxBZE4Iyg2Ec+7\nSBIBbsq5T1IbQfMmS+udtxEIbb/n+XAGhRdo1hO0A8y4AYcurv+5uLXNSyVh61Xt\nBRjcpUQ6wtgJvGP2xNPSllPQ2UelueKfLqZi6dZWWd/+T6DG6gBbfZzSbFk+W/jt\nbS8=\n-----END CERTIFICATE-----`; // CROSS-SIGNING: const newCSR = { @@ -241,6 +242,7 @@ export const CERTIFICATES = { unsupportedOids, unsupportedPem, certWithoutCN, + certWithAllKeyUsage, newCSR, oldParentIssuerCert, parentIssuerCert, diff --git a/ui/tests/integration/components/pki/pki-issuer-cross-sign-test.js b/ui/tests/integration/components/pki/pki-issuer-cross-sign-test.js index 8073697c89..fafcdab864 100644 --- a/ui/tests/integration/components/pki/pki-issuer-cross-sign-test.js +++ b/ui/tests/integration/components/pki/pki-issuer-cross-sign-test.js @@ -337,7 +337,7 @@ module('Integration | Component | pki issuer cross sign', function (hooks) { assert .dom('[data-test-cross-sign-alert-message]') .hasText( - 'certificate contains unsupported subject OIDs: 1.2.840.113549.1.9.1, certificate contains unsupported extension OIDs: 2.5.29.37' + 'certificate contains unsupported subject OIDs: 1.2.840.113549.1.9.1, certificate contains unsupported extension OIDs: 2.5.29.37, unsupported key usage value on issuer certificate: DigitalSignature, KeyEncipherment' ); for (const field of FIELDS) { diff --git a/ui/tests/integration/components/pki/pki-key-usage-test.js b/ui/tests/integration/components/pki/pki-key-usage-test.js index 3d29f8f6eb..44e66da8e6 100644 --- a/ui/tests/integration/components/pki/pki-key-usage-test.js +++ b/ui/tests/integration/components/pki/pki-key-usage-test.js @@ -18,11 +18,61 @@ module('Integration | Component | pki-key-usage', function (hooks) { hooks.beforeEach(function () { this.store = this.owner.lookup('service:store'); this.model = this.store.createRecord('pki/role'); + // add fields that openapi normally hydrates + // ideally we pull this from the openapi schema in the future + const openapifields = [ + { + name: 'clientFlag', + type: 'boolean', + options: { + editType: 'boolean', + helpText: + 'If set, certificates are flagged for client auth use. Defaults to true. See also RFC 5280 Section 4.2.1.12.', + fieldGroup: 'default', + defaultValue: true, + }, + }, + { + name: 'serverFlag', + type: 'boolean', + options: { + editType: 'boolean', + helpText: + 'If set, certificates are flagged for server auth use. Defaults to true. See also, RFC 5280 Section 4.2.1.12.', + fieldGroup: 'default', + defaultValue: true, + }, + }, + { + name: 'codeSigningFlag', + type: 'boolean', + options: { + editType: 'boolean', + helpText: + 'If set, certificates are flagged for code signing use. Defaults to false. See also RFC 5280 Section 4.2.1.12.', + fieldGroup: 'default', + }, + }, + { + name: 'emailProtectionFlag', + type: 'boolean', + options: { + editType: 'boolean', + helpText: + 'If set, certificates are flagged for email protection use. Defaults to false. See also RFC 5280 Section 4.2.1.12.', + fieldGroup: 'default', + }, + }, + ]; + this.model._allByKey = {}; + openapifields.forEach((f) => { + this.model._allByKey[f.name] = f; + this.model[f.name] = f.options.defaultValue; + }); this.model.backend = 'pki'; }); test('it should render the component', async function (assert) { - assert.expect(6); await render( hbs`
@@ -38,6 +88,10 @@ module('Integration | Component | pki-key-usage', function (hooks) { assert.dom(PKI_ROLE_FORM.keyAgreement).isChecked('Key Agreement is true by default'); assert.dom(PKI_ROLE_FORM.keyEncipherment).isChecked('Key Encipherment is true by default'); assert.dom(PKI_ROLE_FORM.any).isNotChecked('Any is false by default'); + assert.dom(GENERAL.inputByAttr('clientFlag')).isChecked(); + assert.dom(GENERAL.inputByAttr('serverFlag')).isChecked(); + assert.dom(GENERAL.inputByAttr('codeSigningFlag')).isNotChecked(); + assert.dom(GENERAL.inputByAttr('emailProtectionFlag')).isNotChecked(); assert.dom(GENERAL.inputByAttr('extKeyUsageOids')).exists('Extended Key usage oids renders'); }); diff --git a/ui/tests/integration/components/pki/pki-role-form-test.js b/ui/tests/integration/components/pki/pki-role-form-test.js index 0b1d1bbad5..76d3376078 100644 --- a/ui/tests/integration/components/pki/pki-role-form-test.js +++ b/ui/tests/integration/components/pki/pki-role-form-test.js @@ -56,7 +56,7 @@ module('Integration | Component | pki-role-form', function (hooks) { assert .dom(GENERAL.fieldByAttr('noStoreMetadata')) .doesNotExist('noStoreMetadata is not shown b/c not enterprise'); - assert.dom(GENERAL.inputByAttr('addBasicConstraints')).exists(); + assert.dom(GENERAL.inputByAttr('basicConstraintsValidForNonCa')).exists(); assert.dom(GENERAL.button('Domain handling')).exists('shows form-field group add domain handling'); assert.dom(GENERAL.button('Key parameters')).exists('shows form-field group key params'); assert.dom(GENERAL.button('Key usage')).exists('shows form-field group key usage'); @@ -127,7 +127,7 @@ module('Integration | Component | pki-role-form', function (hooks) { .includesText('Name is required.', 'show correct error message'); await fillIn(GENERAL.inputByAttr('name'), 'test-role'); - await click('[data-test-input="addBasicConstraints"]'); + await click('[data-test-input="basicConstraintsValidForNonCa"]'); await click(GENERAL.button('Domain handling')); await click('[data-test-input="allowedDomainsTemplate"]'); await click(GENERAL.button('Policy identifiers')); diff --git a/ui/tests/integration/utils/parse-pki-cert-test.js b/ui/tests/integration/utils/parse-pki-cert-test.js index 8ca5d396f8..7d3f28fa01 100644 --- a/ui/tests/integration/utils/parse-pki-cert-test.js +++ b/ui/tests/integration/utils/parse-pki-cert-test.js @@ -17,6 +17,7 @@ import { CERTIFICATES } from 'vault/tests/helpers/pki/pki-helpers'; const { certWithoutCN, + certWithAllKeyUsage, loadedCert, pssTrueCert, skeletonCert, @@ -83,6 +84,18 @@ module('Integration | Util | parse pki certificate', function (hooks) { ); }); + test('it parses a certificate with every key usage bit set', async function (assert) { + // certificate contains every key usage constraint + // Key Usage is manually parsed and relies on the order of KEY_USAGE_BITS + // so this test ensures that order is preserved. + const { key_usage } = parseCertificate(certWithAllKeyUsage); + assert.strictEqual( + key_usage, + 'DigitalSignature, ContentCommitment, KeyEncipherment, DataEncipherment, KeyAgreement, CertSign, CRLSign, EncipherOnly, DecipherOnly', + 'it contains expected key usage values' + ); + }); + test('it parses a certificate with use_pass=true and exclude_cn_from_sans=false', async function (assert) { assert.expect(2); const parsedPssCert = parseCertificate(pssTrueCert); @@ -123,6 +136,7 @@ module('Integration | Util | parse pki certificate', function (hooks) { [ 'certificate contains unsupported subject OIDs: 1.2.840.113549.1.9.1', 'certificate contains unsupported extension OIDs: 2.5.29.37', + 'unsupported key usage value on issuer certificate: DigitalSignature, KeyEncipherment', ], 'it contains expected error messages' ); @@ -204,7 +218,10 @@ module('Integration | Util | parse pki certificate', function (hooks) { ({ extValues, extErrors } = unsupportedExt); assert.propEqual( this.getErrorMessages(extErrors), - ['certificate contains unsupported extension OIDs: 2.5.29.37'], + [ + 'certificate contains unsupported extension OIDs: 2.5.29.37', + 'unsupported key usage value on issuer certificate: DigitalSignature, KeyEncipherment', + ], 'it returns extension errors' ); assert.ok( @@ -271,14 +288,14 @@ module('Integration | Util | parse pki certificate', function (hooks) { common_name: null, country: null, exclude_cn_from_sans: false, - key_usage: null, + key_usage: 'DigitalSignature, KeyEncipherment', locality: null, max_path_length: 10, not_valid_after: 1989876490, not_valid_before: 1674516490, organization: null, ou: null, - parsing_errors: [{}, {}], + parsing_errors: [{}, {}, {}], postal_code: null, province: null, subject_serial_number: null, @@ -295,6 +312,7 @@ module('Integration | Util | parse pki certificate', function (hooks) { [ 'certificate contains unsupported subject OIDs: 1.2.840.113549.1.9.1', 'certificate contains unsupported extension OIDs: 2.5.29.37', + 'unsupported key usage value on issuer certificate: DigitalSignature, KeyEncipherment', ], 'it returns correct errors' );