-
Notifications
You must be signed in to change notification settings - Fork 403
feat(internal-plugin-encryption): validate kms cert #5238
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.
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
base: next
Are you sure you want to change the base?
Changes from 9 commits
35fbb6e
a2d47cf
8a4f4d3
5f6c3a6
a53c931
b760e37
8a2d7ca
72a2c7f
0823d59
6bea5da
4ed3a66
348a17e
66fc296
8c442ae
97aa3d7
23765dd
e7af66e
98f3015
6e7abe3
63478fc
631ec1c
f00e3fe
d07191f
50791f1
d33be45
7d63d1e
ce5786a
ca60dcb
d642cfa
a5483dc
a528a04
056c3bd
5144289
99e3237
54d6bab
fac9faf
4f4a715
d98806c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,8 +48,26 @@ export default { | |
| batcherMaxWait: 150, | ||
|
|
||
| /** | ||
| * PEM encoded CA root bundle used to validate the KMS certificate chain. | ||
| * When omitted, the KMS certificate chain signature is not verified. | ||
| * Whether to validate the KMS certificate chain against `caroots`. Defaults | ||
| * to true as a secure default: when enabled the KMS certificate must | ||
| * validate against a configured `caroots` bundle, and a missing bundle | ||
| * fails closed. Set to false to temporarily opt out of validation, e.g. | ||
| * while upgrading and wiring up the CA root bundle. | ||
| * @type {boolean} | ||
| */ | ||
| shouldValidateKMSCertificate: true, | ||
|
Coread marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a Node consumer upgrades without Useful? React with 👍 / 👎. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When users invoke the root Useful? React with 👍 / 👎. |
||
|
|
||
| /** | ||
| * CA root bundle used to validate the KMS certificate chain, as an array of | ||
| * raw base64-encoded certificates (the DER body, without the | ||
| * -----BEGIN/END CERTIFICATE----- lines). Required when | ||
| * `shouldValidateKMSCertificate` is true. | ||
| * | ||
| * Supplied by the consuming application at build/config time; the SDK does | ||
| * not ship a bundle and does no file/network I/O to obtain one. Cisco | ||
| * first-party clients should source these roots from the Cisco Trusted Root | ||
| * Store Union bundle. See the plugin README, tooling/generate-kms-caroots.js, | ||
| * and https://www.cisco.com/security/pki/trs/readme.html for details. | ||
| * @type {?string[]} | ||
| */ | ||
| caroots: undefined, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -784,24 +784,28 @@ const KMS = WebexPlugin.extend({ | |
| }, | ||
|
|
||
| /** | ||
| * Validates the KMS static public key against the configured CA roots. The | ||
| * enforced `caroots` bundle rejects on failure. When a `carootsReportOnly` | ||
| * bundle is also configured, it is validated in addition to `caroots`, but a | ||
| * failure against it is only reported as a metric so a new bundle can be | ||
| * trialled without risking failure. | ||
| * Validates the KMS static public key against the configured CA roots. | ||
| * Validation is enabled by default (`shouldValidateKMSCertificate`) and fails | ||
| * closed: when enabled the enforced `caroots` bundle must be configured and | ||
| * the chain must validate against it. When a `carootsReportOnly` bundle is | ||
| * also configured, it is validated in addition to `caroots`, but a failure | ||
| * against it is only reported as a metric so a new bundle can be trialled | ||
| * without risking failure. | ||
| * @private | ||
| * @param {Object} kmsStaticPubKey | ||
| * @returns {Promise<Object>} the KMS static public key | ||
| */ | ||
| _validateKMSStaticPubKey(kmsStaticPubKey) { | ||
| const {caroots, carootsReportOnly} = this.config; | ||
| const {caroots, carootsReportOnly, shouldValidateKMSCertificate} = this.config; | ||
|
|
||
| return validateKMS(caroots)(kmsStaticPubKey).then((jwt) => { | ||
| return validateKMS({caroots, validateSignature: shouldValidateKMSCertificate})( | ||
| kmsStaticPubKey | ||
| ).then((jwt) => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the new default validation rejects (for example, an upgraded client initially has no CA roots, or its bundled roots are stale), Useful? React with 👍 / 👎. |
||
| if (!carootsReportOnly) { | ||
| return jwt; | ||
| } | ||
|
|
||
| return validateKMS(carootsReportOnly)(kmsStaticPubKey) | ||
| return validateKMS({caroots: carootsReportOnly, validateSignature: true})(kmsStaticPubKey) | ||
| .catch((reason) => { | ||
| this.logger.warn('kms: report-only certificate validation failed', reason); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| module.exports = { | ||
| root: true, | ||
| ignorePatterns: ['*.d.ts'], | ||
| env: { | ||
| node: true, | ||
| es2021: true, | ||
| }, | ||
| parserOptions: { | ||
| ecmaVersion: 2021, | ||
| sourceType: 'script', | ||
| }, | ||
| extends: ['eslint:recommended'], | ||
| rules: { | ||
| 'no-console': 'off', | ||
| 'no-plusplus': 'off', | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On a normal root install,
yarn caroots:generateruns with only the root workspace's dependency binaries onPATH.@webex/kms-carootsis declared only bypackages/legacy/tools, so itswebex-kms-carootsbin is not available to this script; the new root command exitscommand not foundunless the CLI was installed globally. Add the package as a root dependency or invoke it through its workspace.Useful? React with 👍 / 👎.