Skip to content

Conversation

@mahdiridho
Copy link
Contributor

We have security concern for sensitive data to be pull in log. The logger masking is not fully help for debugging as we don't know the payload identifier when they are masked. So instead of mask the sensitive attributes, we can keep securing them with encryption tool as well as let them to be searchable on debugging.


// Mask sensitive attributes, remove null
const maskSensitiveAttributes = (key: string, value: JSONObject): JSONObject | string | undefined => {
const maskSensitiveAttributes = (key: string, value: any): JSONObject | string | undefined => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't define types as any. It has to be JSONObject

*/
if (!!this.cipherKey) {
const cipher = createCipheriv(
'aes-256-ecb',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to use aes-256-ecb ? We should have the algorithm as an optional setting, with aes-256-cbc as a default.
An IV would need to be created for each encryption.
The response should be an object:

{
  iv: 'xxxxxx',
  encrypted: 'abcdef'
}

When value is an array or an object, the String(value) won't return anything usable. For theses cases, the value should be stringified.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to understand the reason behind using encryption?

We also need to understand if this is PII compliant. Encrypted values are decryptable if you have the encryption key.

If what we want is to be able to filter logs based on a known value, maybe hashing the value (with a non-reversible algorithm) is good enough and improves compliance?

{
"name": "@serverless-guru/logger",
"version": "1.0.7",
"version": "1.0.8",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment for another PR.
This should be automated.

https://github.com/semantic-release/semantic-release

*/
if (!!this.cipherKey) {
const cipher = createCipheriv(
'aes-256-ecb',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to understand the reason behind using encryption?

We also need to understand if this is PII compliant. Encrypted values are decryptable if you have the encryption key.

If what we want is to be able to filter logs based on a known value, maybe hashing the value (with a non-reversible algorithm) is good enough and improves compliance?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants