Add secret masking functions to be called in logger and directly #345

mickmcgrath13 · 2022-11-03T15:16:50Z

Description

Add configuration in the bitops-level bitops.config to add masks based on regular expressions.

Todo

add a setting for turning masking on or off (on by default, include a warning if it's turned off)
There's a TODO to look into a library and swap out the regex format if necessary, but I think this is a good start
ops-repo bitops.config.yaml support for custom masking at the ops-repo level
docs

Fixes #208

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update

How Has This Been Tested?

Tested locally with BITOPS_KUBECONFIG_BASE64 (see this pr for the local development setup used)

Logs
without change:

~#~#~#~BITOPS ENVIRONMENT VARIABLES~#~#~#~
	BITOPS_DEFAULT_ROOT_DIR=_default
	BITOPS_DIR=/opt/bitops
	BITOPS_ENVIRONMENT=prod
	BITOPS_ENVROOT=/tmp/tmplcry4xqb/prod
	BITOPS_FAIL_FAST=True
	BITOPS_KUBECONFIG_BASE64=uueagqwugv8a4isbviaw9ah49h9g39vba4ba4v9wvofgairgw

with changes:

~#~#~#~BITOPS ENVIRONMENT VARIABLES~#~#~#~
	BITOPS_DEFAULT_ROOT_DIR=_default
	BITOPS_DIR=/opt/bitops
	BITOPS_ENVIRONMENT=prod
	BITOPS_ENVROOT=/tmp/tmplcry4xqb/prod
	BITOPS_FAIL_FAST=True
	BITOPS_KUBECONFIG_BASE64=*******

Test Configuration: (see below)

BitOps image tag used: 2.1.0
Tool/plugin: n/a

mask config: modified the omnibus bitops.config.yaml to include new mask configuration
command:

docker run --rm --name bitops \
-e AWS_ACCESS_KEY_ID="${AWS_ACCESS_KEY_ID}" \
-e AWS_SECRET_ACCESS_KEY="${AWS_SECRET_ACCESS_KEY}" \
-e AWS_SESSION_TOKEN="${AWS_SESSION_TOKEN}" \
-e AWS_DEFAULT_REGION="${AWS_DEFAULT_REGION}" \
-e BITOPS_ENVIRONMENT="prod" \
-e SKIP_DEPLOY_TERRAFORM="true" \
-e SKIP_DEPLOY_HELM="true" \
-e DEFAULT_FOLDER_NAME="_default" \
-e BITOPS_KUBECONFIG_BASE64="${BITOPS_KUBECONFIG_BASE64}" \
-e PYTHONUNBUFFERED=1 \
-v /path/to/bitops:/opt/bitops \
-v /path/to/operations-repo:/opt/bitops_deployment \
-v /path/to/bitops/prebuilt-config/omnibus/bitops.config.yaml:/opt/bitops/bitops.config.yaml \
-v /opt/bitops/scripts/plugins/aws \
-v /opt/bitops/scripts/plugins/cloudformation \
-v /opt/bitops/scripts/plugins/kubectl \
-v /opt/bitops/scripts/plugins/ansible \
-v /path/to/bitops-plugins/helm:/opt/bitops/scripts/plugins/helm \
-v /path/to/bitops-plugins/terraform:/opt/bitops/scripts/plugins/terraform \
--entrypoint "python3" \
bitovi/bitops:2.1.0 \
/opt/bitops/scripts/main.py deploy

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation

prebuilt-config/omnibus/bitops.config.yaml

scripts/plugins/logging.py

PhillypHenning · 2022-11-03T15:46:57Z

scripts/plugins/logging.py

+}
+
+
+# TODO: move to its own file so it can be used by plugins and before/after hooks


I agree and was thinking of mentioning that this would be best in the utilities module. The trick is that would create a cyclic dependency.

If we really want to make this function available, then I'd recommend we create a bare-utilities.py file that strictly maintains a "no app specific import" rule. This would mean that it would be imported by all other app-specific modules.

@PhillypHenning do you mind creating an issue for this?

PhillypHenning · 2022-11-03T18:16:00Z

@mickmcgrath13 I'm going to merge this unless you stop me within the next hour

PhillypHenning · 2022-11-04T15:23:27Z

@mickmcgrath13

I ran into this issue

#8 0.955   File "/opt/bitops/scripts/plugins/logging.py", line 84, in format
#8 0.955     record.msg = mask_message(record.msg)
#8 0.955   File "/opt/bitops/scripts/plugins/logging.py", line 44, in mask_message
#8 0.955     for config_item in BITOPS_logging_masks:
#8 0.955 TypeError: 'NoneType' object is not iterable

I've added the following to the logging.py file to ensure this doesn't happen;

if BITOPS_logging_masks is None:
        return message

This is fixed in #347

arm4b

Left my opinions about this feature, but it works in optimistic examples and is a nice start! 👍

Outside of heuristics and rules from 3rd party libs we may also want to define a list of secrets, take the ENV var value and replace it in the output too so it catches the real secret string.

From the evident next steps we're missing other secret vars for all the plugins (AWS, etc) in the fleet https://github.com/bitops-plugins/ and it is worth adding some minimal documentation.

arm4b · 2022-11-07T18:04:50Z

scripts/plugins/logging.py

+
+    res_str = message
+    for config_item in BITOPS_logging_masks:
+        # TODO: use a library here?


Yes, totally.

It's good to have something initially working, but IMO it doesn't solve the secrets problem technically with such approach, but rather creates a visibility of solving it which is dangerous in security context.
Trying to mask the secrets this way might be not production ready, and also hard to call user-friendly with the new bitops config.

We might leverage external rules like: https://github.com/Yelp/detect-secrets/tree/master/detect_secrets/plugins and heuristics from the tool that might already having secrets detection.

Ex: https://github.com/Yelp/detect-secrets/blob/master/detect_secrets/plugins/github_token.py#L15

arm4b · 2022-11-07T18:13:04Z

prebuilt-config/omnibus/bitops.config.yaml

+      - # regex to search
+        # looks for `BITOPS_KUBECONFIG_BASE64={string}`
+        search:  (.*BITOPS_KUBECONFIG_BASE64.*\=)(.*\n)
+        # replace the value part
+        replace: '\1*******\n'
+      - # regex to search
+        # looks for `The namespace kube-system exists`
+        search:  (.*The namespace )(kube-system)( exists.*)
+        #replace kube-system
+        replace: '\1*******\3'


Does it really close the #208?

Was the BITOPS_KUBECONFIG_BASE64 the only available secret we're passing or are there any other secret vars from the plugins?

I think you're right that it does not close #208. This PR was to set up a decent approach, but there's certainly more to do for specific secrets. Thx for re-opening #208.

arm4b · 2022-11-07T19:34:32Z

prebuilt-config/omnibus/bitops.config.yaml

+    masks:
+      - # regex to search
+        # looks for `BITOPS_KUBECONFIG_BASE64={string}`
+        search:  (.*BITOPS_KUBECONFIG_BASE64.*\=)(.*\n)


If we're targeting beginners, I wouldn't expose regex setting in the BitOps config as it brings complexity to the user's side. It's enough to list 10 regex rules to make everything look complicated and repell someone using it.

Other options:

Just list the secret vars in bitops core or config. No regex, just BITOPS_KUBECONFIG_BASE64, AWS_SECRET_KEY, etc. and do the rest filtering on the BitOps side.

auto-detect secrets in log via heuristics and rules from libs like https://github.com/Yelp/detect-secrets/tree/master/detect_secrets/plugins

allow specifying extra secret names on top of default/hardcoded

secrets: - BITOPS_KUBECONFIG_BASE64 - AWS_SECRET_ACCESS_KEY - AWS_ACCESS_KEY_ID - AWS_SESSION_TOKEN

Alternatively, allow defining secrets on BitOps Plugin level and process in the code so the secret definition is pluggable.

arm4b · 2022-11-07T19:35:15Z

prebuilt-config/omnibus/bitops.config.yaml

+        # looks for `BITOPS_KUBECONFIG_BASE64={string}`
+        search:  (.*BITOPS_KUBECONFIG_BASE64.*\=)(.*\n)
+        # replace the value part
+        replace: '\1*******\n'


I think it makes sense to omit the "replace" part, defaulting it to just ****.

arm4b · 2022-11-07T19:37:28Z

prebuilt-config/omnibus/bitops.config.yaml

@@ -13,6 +13,18 @@ bitops:
    filename: bitops-run      # log filename
    err: bitops.logs          # error logs filename
    path: /var/logs/bitops    # path to log folder
+    # Define the secrets to mask
+    masks:


mickmcgrath13 · 2022-11-07T20:16:26Z

@armab I've captured your comments here: #208 so we can coordinate the efforts/other tickets

mickmcgrath13 requested review from arm4b and PhillypHenning November 3, 2022 15:16

mickmcgrath13 self-assigned this Nov 3, 2022

mickmcgrath13 force-pushed the 208-mask-sensitive-output branch from 212ee56 to d6be4ca Compare November 3, 2022 15:18

PhillypHenning reviewed Nov 3, 2022

View reviewed changes

prebuilt-config/omnibus/bitops.config.yaml Show resolved Hide resolved

PhillypHenning reviewed Nov 3, 2022

View reviewed changes

scripts/plugins/logging.py Outdated Show resolved Hide resolved

scripts/plugins/logging.py Outdated Show resolved Hide resolved

mickmcgrath13 added 2 commits November 3, 2022 10:31

add masking functions to be called in logger and directly

dd8835e

black formating

63b46d0

mickmcgrath13 force-pushed the 208-mask-sensitive-output branch from d6be4ca to 63b46d0 Compare November 3, 2022 15:31

mickmcgrath13 added 4 commits November 3, 2022 10:38

move logging masks to settings

7a56979

fix linting error

3f517a3

black formating

b87dfd4

more linting errors

5de655e

mickmcgrath13 force-pushed the 208-mask-sensitive-output branch from d9054d6 to 5de655e Compare November 3, 2022 15:45

PhillypHenning reviewed Nov 3, 2022

View reviewed changes

PhillypHenning approved these changes Nov 3, 2022

View reviewed changes

PhillypHenning merged commit d5f625e into main Nov 4, 2022

PhillypHenning deleted the 208-mask-sensitive-output branch November 4, 2022 15:06

arm4b changed the title ~~[WIP] closes #298 - add masking functions to be called in logger and directly~~ Add secret masking functions to be called in logger and directly Nov 7, 2022

arm4b reviewed Nov 7, 2022

View reviewed changes

arm4b mentioned this pull request Nov 7, 2022

Parse passwords / creditentials out of logging output #208

Closed

arm4b mentioned this pull request Nov 8, 2022

Fix error when there's an empty 'mask' in bitops config #350

Merged

5 tasks

arm4b mentioned this pull request Dec 7, 2022

Documentation for Secrets Masking #374

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add secret masking functions to be called in logger and directly #345

Add secret masking functions to be called in logger and directly #345

mickmcgrath13 commented Nov 3, 2022 •

edited

Loading

PhillypHenning Nov 3, 2022

mickmcgrath13 Nov 3, 2022

PhillypHenning commented Nov 3, 2022

PhillypHenning commented Nov 4, 2022

arm4b left a comment

arm4b Nov 7, 2022

arm4b Nov 7, 2022

mickmcgrath13 Nov 7, 2022

arm4b Nov 7, 2022

arm4b Nov 7, 2022

arm4b Nov 7, 2022

mickmcgrath13 commented Nov 7, 2022

		}


		# TODO: move to its own file so it can be used by plugins and before/after hooks

Add secret masking functions to be called in logger and directly #345

Add secret masking functions to be called in logger and directly #345

Conversation

mickmcgrath13 commented Nov 3, 2022 • edited Loading

Description

Todo

Type of change

How Has This Been Tested?

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PhillypHenning commented Nov 3, 2022

PhillypHenning commented Nov 4, 2022

arm4b left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mickmcgrath13 commented Nov 7, 2022

mickmcgrath13 commented Nov 3, 2022 •

edited

Loading