Skip to content
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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 6 commits into from
Nov 4, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dd8835e
add masking functions to be called in logger and directly
mickmcgrath13 Nov 3, 2022
63b46d0
black formating
mickmcgrath13 Nov 3, 2022
7a56979
move logging masks to settings
mickmcgrath13 Nov 3, 2022
3f517a3
fix linting error
mickmcgrath13 Nov 3, 2022
b87dfd4
black formating
mickmcgrath13 Nov 3, 2022
5de655e
more linting errors
mickmcgrath13 Nov 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions prebuilt-config/omnibus/bitops.config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

secrets:

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

Choose a reason for hiding this comment

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

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:

  1. 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.
  2. auto-detect secrets in log via heuristics and rules from libs like https://github.com/Yelp/detect-secrets/tree/master/detect_secrets/plugins
  3. 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.

# replace the value part
replace: '\1*******\n'
Copy link
Member

Choose a reason for hiding this comment

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

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

- # regex to search
# looks for `The namespace kube-system exists`
search: (.*The namespace )(kube-system)( exists.*)
#replace kube-system
replace: '\1*******\3'
Comment on lines +18 to +27
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

default_folder: _default
plugins:
aws:
Expand Down
53 changes: 40 additions & 13 deletions scripts/plugins/logging.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import logging
import re
import sys

from .settings import (
BITOPS_logging_level,
BITOPS_logging_color,
BITOPS_logging_filename,
BITOPS_logging_path,
BITOPS_logging_masks,
)


Expand All @@ -20,6 +22,31 @@
COLOR_SEQ = "\033[1;%dm"
BOLD_SEQ = "\033[1m"

COLORS = {
"DEBUG": BLUE,
"INFO": GREEN,
"WARNING": YELLOW,
"ERROR": RED,
"CRITICAL": MAGENTA,
}


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

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@PhillypHenning do you mind creating an issue for this?

def mask_message(message):
"""
Given a message, applies a mask according to the
bitops level bitops.config.yaml's bitops.logging.mask property
"""
if message is None:
return message

res_str = message
for config_item in BITOPS_logging_masks:
# TODO: use a library here?
Copy link
Member

Choose a reason for hiding this comment

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

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

res_str = re.sub(rf"{config_item.search}", config_item.replace, str(res_str))
# String after replacement
return res_str


def formatter_message(message, use_color=BITOPS_logging_color):
"""
Expand All @@ -30,19 +57,11 @@ def formatter_message(message, use_color=BITOPS_logging_color):
message = message.replace("$RESET", RESET_SEQ).replace("$BOLD", BOLD_SEQ)
else:
message = message.replace("$RESET", "").replace("$BOLD", "")
return message


COLORS = {
"DEBUG": BLUE,
"INFO": GREEN,
"WARNING": YELLOW,
"ERROR": RED,
"CRITICAL": MAGENTA,
}
return message


class ColoredFormatter(logging.Formatter):
class BitOpsFormatter(logging.Formatter):
"""
Class that controls the formatting of logging text, adds colors if enabled.
Settings are contained within "bitops.config.yaml:bitops.logging".
Expand All @@ -60,21 +79,29 @@ def format(self, record):
if self.use_color and levelname in COLORS:
levelname_color = COLOR_SEQ % (30 + COLORS[levelname]) + levelname + RESET_SEQ
record.levelname = levelname_color

# mask the output
record.msg = mask_message(record.msg)

return logging.Formatter.format(self, record)


formatter = BitOpsFormatter(
formatter_message("%(asctime)s %(name)-12s %(levelname)-8s %(message)s")
)


logger = logging.getLogger()
logger.setLevel(BITOPS_logging_level)

handler = logging.StreamHandler(sys.stdout)
handler.setLevel(BITOPS_logging_level)
formatter = ColoredFormatter(
formatter_message("%(asctime)s %(name)-12s %(levelname)-8s %(message)s")
)


handler.setFormatter(formatter)
logger.addHandler(handler)


if BITOPS_logging_filename is not None:
# This assumes that the user wants to save output to a filename

Expand Down
6 changes: 6 additions & 0 deletions scripts/plugins/settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,9 @@
if bitops_build_configuration.bitops.timeout is not None
else 600
)

BITOPS_logging_masks = (
bitops_build_configuration.bitops.logging.masks
if bitops_build_configuration.bitops.logging.masks is not None
else None
)
7 changes: 4 additions & 3 deletions scripts/plugins/utilities.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from munch import DefaultMunch
from .doc import get_doc
from .logging import logger
from .logging import logger, mask_message
from .settings import (
BITOPS_fast_fail_mode,
BITOPS_config_file,
Expand Down Expand Up @@ -363,9 +363,10 @@ def run_cmd(command: Union[list, str]) -> subprocess.CompletedProcess:
# TODO: parse output for secrets
# TODO: specify plugin and output tight output (no extra newlines)
# TODO: can we modify a specific handler to add handler.terminator = "" ?
sys.stdout.write(combined_output)
sys.stdout.write(mask_message(combined_output))

# This polls the async function to get information about the status of the process execution.
# This polls the async function to get information
# about the status of the process execution.
# Namely the return code which is used elsewhere.
process.communicate()
except Exception as exc:
Expand Down