[MD] Add encrypt/decrypt module on data source plugin

[noCharger]

Signed-off-by: Louis Chu [email protected]

Description

Add encrypt/decrypt module on data source plugin

Issues Resolved

1. [MD]Refactor server side router with SavedObjectsClientWrapperFactory #1850
2. [MD] Implement Encrypt / Decrypt module #2081

Check List

• New functionality includes testing.
  • All tests pass
    • yarn test:jest
• New functionality has been documented.
• Commits are signed per the DCO using --signoff

[seraphjiang]

why use this for default value?

[noCharger]

why use this for default value?

the default value is provided for simplify the configuration, but we can also remove it

[zengyan-amazon]

shall we use changeme? I think we used to use changeme as default OSD server password to hint admins to replace it

[zengyan-amazon]

meanwhile, WRAPPING_KEY_SIZE is 32, will this default value for with 32 byte key size?

[seraphjiang]

ok with default value, but could we come up better value

[noCharger]

meanwhile, WRAPPING_KEY_SIZE is 32, will this default value for with 32 byte key size?

I'm not sure I understand the last part, but I believe the question is whether the value here means setting the default size. It's config validation, which means that if the size of the config file is less than 32, it will throw an error. And it must be 32, as defined by the encryption algorithm for key wrapping. The generated data key is not 32 bytes in length.

[codecov-commenter]

Codecov Report

Merging #2120 (4d420a2) into feature/multi-datasource (456b5c7) will decrease coverage by 0.00%.
The diff coverage is 40.00%.

❗ Current head 4d420a2 differs from pull request most recent head 092493d. Consider uploading reports for the commit 092493d to get more accurate results

@@                     Coverage Diff                      @@
##           feature/multi-datasource    #2120      +/-   ##
============================================================
- Coverage                     67.47%   67.47%   -0.01%     
============================================================
  Files                          3080     3081       +1     
  Lines                         59246    59261      +15     
  Branches                       9010     9011       +1     
============================================================
+ Hits                          39977    39985       +8     
- Misses                        17080    17087       +7     
  Partials                       2189     2189              

Impacted Files	Coverage Δ
..._source/server/cryptography/cryptography_client.ts	40.00% <40.00%> (ø)
...ic/application/models/sense_editor/sense_editor.ts	64.88% <0.00%> (+0.88%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[noCharger]

@joedimarzio Feel free to drop comments!

[zengyan-amazon]

shall we use changeme? I think we used to use changeme as default OSD server password to hint admins to replace it

[zengyan-amazon]

meanwhile, WRAPPING_KEY_SIZE is 32, will this default value for with 32 byte key size?

[zengyan-amazon]

why throw error ? if you throw error here, the saved object won't be stored, thus you cause error for all saved objects except credentials

[noCharger]

Good catch. Will only validate on credential type and call default contract otherwise. I didn't intend to support encryption for all types using the factory pattern because deciding which fields to encrypt is not scalable. Open to more ideas.

[zengyan-amazon]

better to set username and password to undefined before saving

[noCharger]

better to set username and password to undefined before saving

The Saved object update API currently only accepts partial attributes as input. Dropping the field or marking it as undefined will be ignored, and the existing field will remain unchanged.

TODO: Provide another update API to drop the field #2124

  private dropCredentialMaterialsContent<T = unknown>(credentialMaterials: T) {
    return {
      credentialMaterialsType: credentialMaterials.credentialMaterialsType,
      /**
       * The Saved object update API currently only accepts partial attributes as input.
       * Dropping the field or marking it as undefined will be ignored, and the existing field will remain unchanged.
       * TODO: Provide another update API to drop the field  https://github.com/opensearch-project/OpenSearch-Dashboards/issues/2124
       */
      credentialMaterialsContent: {
        username: '',
        password: '',
      },
    };
  }

[noCharger]

There are multiple places in the AWS Encryption SDK that dataKey / plainText is buffer but recognized as object instead of uint8array during jest test case running

function unwrapDataKey(dataKey) {
    if (dataKey instanceof Uint8Array)
        return dataKey;
    if (exports.supportsKeyObject && dataKey instanceof exports.supportsKeyObject.KeyObject)
        return dataKey.export();
    throw new Error('Unsupported dataKey type');
}

found an known issue - jestjs/jest#4422

Using this solution helped

jestjs/jest#4422 (comment)

[zengyan-amazon]

will it impact other test execution?

[noCharger]

Don't think so. It's considered a bug in the Jest framework from the issue above. Also, I ran 'yarn test:jest' on local, and it appears that nothing was broken.

[zhongnansu]

Np: maybe just pass in one config object as a whole, and parse the config items you need in the crypto related module. Since non of those configs are used else where, in this plugin.ts

[zengyan-amazon]

will it impact other test execution?

[zengyan-amazon]

what is shared auth type?

[noCharger]

As previously discussed, credentials can have different authTypes (Shared / Private / etc.). We will only support shared for current release and will explicitly notify customers that this credential is shared for everyone.

[kgcreative]

would Credential.AuthType = 'shared | private | etc' make more sense and be more long-term sustainable here? (as opposed to SharedAuthType, which feels a bit redundant)

[noCharger]

would Credential.AuthType = 'shared | private | etc' make more sense and be more long-term sustainable here? (as opposed to SharedAuthType, which feels a bit redundant)

use enum instead

[zengyan-amazon]

we need to think a bit more about no auth type, e.g. in case of user choose to use no auth type for a data source, shall we link the data source with a credential which doesn't have any actual credential? or we do not need to let data source to refer to a credential. If we go with the second route, we may have an auth type, or auth/noauth flag in data source

[noCharger]

Yes, we started this conversation with @KrooshalUX. From an engineering standpoint, adding withAuth flag to the data source makes more sense for single responsibility. Will update the logic for credential side.

[zengyan-amazon]

Seems this similar code is being used in create/bulkCreate/update/bulkCreate can we move this to separate functions?

[noCharger]

refactored

[zengyan-amazon]

let's remove this for now, and add it when we implement private/shared credential

[noCharger]

removed

[zengyan-amazon]

let's add a default value

[noCharger]

updated