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.

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

fix: mask secret info from bash logs #2360

Merged
merged 3 commits into from
Aug 18, 2024
Merged

fix: mask secret info from bash logs #2360

merged 3 commits into from
Aug 18, 2024

Conversation

VietND96
Copy link
Member

@VietND96 VietND96 commented Aug 18, 2024

User description

Thanks for contributing to the Docker-Selenium project!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines, applied for this repository.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Motivation and Context

Mask sensitive information in console logs

Few variables output like password, secret, etc. are masked in console logs. For debugging purposes, you can disable it by setting SE_MASK_SECRETS to false

While creating bash script, your can mask the output by using syntax echo "Current value is $(mask ${YOUR_VARIABLE})

SE_MASK_SECRETS_MIN_LENGTH default is 3. It means a long string will be masked to *** to avoid exposing length for brute force attack.

Types of changes

  • 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 change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Bug fix, Documentation


Description

  • Implemented masking of sensitive information such as passwords and secrets in various startup scripts to enhance security.
  • Added a new mask script to handle masking logic, allowing configuration of minimum length and visibility.
  • Updated the Dockerfile to include the mask script in the image.
  • Enhanced documentation to include details about the new masking feature and how to configure it.

Changes walkthrough 📝

Relevant files
Bug fix
10 files
start-selenium-grid-distributor.sh
Mask sensitive information in distributor script logs       

Distributor/start-selenium-grid-distributor.sh

  • Masked SE_JAVA_SSL_TRUST_STORE_PASSWORD in console logs.
  • Masked SE_REGISTRATION_SECRET in console logs.
  • +2/-2     
    start-selenium-grid-eventbus.sh
    Mask sensitive information in event bus script logs           

    EventBus/start-selenium-grid-eventbus.sh

    • Masked SE_JAVA_SSL_TRUST_STORE_PASSWORD in console logs.
    +1/-1     
    start-selenium-grid-hub.sh
    Mask sensitive information in hub script logs                       

    Hub/start-selenium-grid-hub.sh

  • Masked SE_JAVA_SSL_TRUST_STORE_PASSWORD in console logs.
  • Masked SE_REGISTRATION_SECRET and SE_ROUTER_PASSWORD.
  • +3/-3     
    start-selenium-node.sh
    Mask sensitive information in node base script logs           

    NodeBase/start-selenium-node.sh

  • Masked SE_JAVA_SSL_TRUST_STORE_PASSWORD in console logs.
  • Masked SE_REGISTRATION_SECRET when logging is disabled.
  • +4/-2     
    start-selenium-grid-docker.sh
    Mask sensitive information in node docker script logs       

    NodeDocker/start-selenium-grid-docker.sh

    • Masked SE_JAVA_SSL_TRUST_STORE_PASSWORD in console logs.
    +1/-1     
    start-selenium-grid-router.sh
    Mask sensitive information in router script logs                 

    Router/start-selenium-grid-router.sh

  • Masked SE_JAVA_SSL_TRUST_STORE_PASSWORD in console logs.
  • Masked SE_REGISTRATION_SECRET and SE_ROUTER_PASSWORD.
  • +3/-3     
    start-selenium-grid-session-queue.sh
    Mask sensitive information in session queue script logs   

    SessionQueue/start-selenium-grid-session-queue.sh

  • Masked SE_JAVA_SSL_TRUST_STORE_PASSWORD in console logs.
  • Masked SE_REGISTRATION_SECRET.
  • +2/-2     
    start-selenium-grid-sessions.sh
    Mask sensitive information in sessions script logs             

    Sessions/start-selenium-grid-sessions.sh

  • Masked SE_JAVA_SSL_TRUST_STORE_PASSWORD in console logs.
  • Masked SE_REGISTRATION_SECRET.
  • +2/-2     
    start-selenium-standalone.sh
    Mask sensitive information in standalone script logs         

    Standalone/start-selenium-standalone.sh

  • Masked SE_JAVA_SSL_TRUST_STORE_PASSWORD in console logs.
  • Masked SE_ROUTER_PASSWORD when logging is disabled.
  • +4/-2     
    start-selenium-grid-docker.sh
    Mask sensitive information in standalone docker script logs

    StandaloneDocker/start-selenium-grid-docker.sh

    • Masked SE_JAVA_SSL_TRUST_STORE_PASSWORD in console logs.
    +1/-1     
    Enhancement
    2 files
    Dockerfile
    Add mask script to Docker image                                                   

    Base/Dockerfile

    • Added mask script to /usr/local/bin/.
    +1/-0     
    mask
    Add script for masking sensitive information                         

    Base/mask

  • Added a script to mask sensitive information.
  • Configurable masking length and visibility.
  • +14/-0   
    Documentation
    1 files
    README.md
    Document masking of sensitive information in logs               

    README.md

  • Documented masking of sensitive information in logs.
  • Updated Docker run command example.
  • +10/-1   

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    frudisch and others added 3 commits August 18, 2024 12:44
    …to console on selenium grid hub and router startup (#2359)
    Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
    Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
    
    # Conflicts:
    #	Hub/start-selenium-grid-hub.sh
    #	Router/start-selenium-grid-router.sh
    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    New Script
    A new script 'mask' has been added to handle masking of sensitive information. This script should be reviewed for correctness and security.

    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Add input validation to the mask script to ensure it receives the expected arguments

    Add input validation to ensure that the script receives the expected number of
    arguments and that they are not empty.

    Base/mask [1-14]

     #!/usr/bin/env bash
    +
    +if [ $# -eq 0 ]; then
    +    echo "Error: No argument provided" >&2
    +    exit 1
    +fi
     
     SE_MASK_SECRETS_MIN_LENGTH=${SE_MASK_SECRETS_MIN_LENGTH:-3}
     SE_MASK_SECRETS_KEEP_LAST=${SE_MASK_SECRETS_KEEP_LAST:-0}
     if [ "${SE_MASK_SECRETS:-true}" = "true" ]; then
         n=${SE_MASK_SECRETS_KEEP_LAST}
         [[ ${#1} -le ${SE_MASK_SECRETS_MIN_LENGTH} ]] && n=$(( ${#1} - ${SE_MASK_SECRETS_MIN_LENGTH} ))
         a="${1:0:${#1}-n}"
         a="${a:0:${SE_MASK_SECRETS_MIN_LENGTH}}"
         b="${1:${#1}-n}"
         printf "%s%s" "${a//?/*}" "$b"
     else
         printf "%s" "$1"
     fi
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Adding input validation to the mask script is a crucial enhancement that ensures the script behaves correctly and predictably, preventing potential runtime errors.

    8
    Refactor the masking logic into a reusable function to improve code maintainability and reduce duplication

    Consider using a function to handle the masking of sensitive information instead of
    repeating the logic in multiple places.

    NodeBase/start-selenium-node.sh [20-24]

    -if [ "${log_message}" = "true" ]; then
    -  echo "Appending Selenium option: ${option} ${value}"
    -else
    -  echo "Appending Selenium option: ${option} $(mask ${value})"
    -fi
    +log_masked_message() {
    +  local option="$1"
    +  local value="$2"
    +  local log_message="$3"
    +  if [ "${log_message}" = "true" ]; then
    +    echo "Appending Selenium option: ${option} ${value}"
    +  else
    +    echo "Appending Selenium option: ${option} $(mask ${value})"
    +  fi
    +}
    +log_masked_message "${option}" "${value}" "${log_message}"
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Refactoring the masking logic into a reusable function improves code maintainability and reduces duplication, which is beneficial for long-term code management.

    7
    Add error handling for edge cases in the mask script to improve robustness

    Consider adding error handling for edge cases, such as when the input string is
    shorter than SE_MASK_SECRETS_MIN_LENGTH.

    Base/mask [5-14]

     if [ "${SE_MASK_SECRETS:-true}" = "true" ]; then
    -    n=${SE_MASK_SECRETS_KEEP_LAST}
    -    [[ ${#1} -le ${SE_MASK_SECRETS_MIN_LENGTH} ]] && n=$(( ${#1} - ${SE_MASK_SECRETS_MIN_LENGTH} ))
    -    a="${1:0:${#1}-n}"
    -    a="${a:0:${SE_MASK_SECRETS_MIN_LENGTH}}"
    -    b="${1:${#1}-n}"
    -    printf "%s%s" "${a//?/*}" "$b"
    +    if [ ${#1} -lt ${SE_MASK_SECRETS_MIN_LENGTH} ]; then
    +        printf "%s" "$1"
    +    else
    +        n=${SE_MASK_SECRETS_KEEP_LAST}
    +        [[ ${#1} -le ${SE_MASK_SECRETS_MIN_LENGTH} ]] && n=$(( ${#1} - ${SE_MASK_SECRETS_MIN_LENGTH} ))
    +        a="${1:0:${#1}-n}"
    +        a="${a:0:${SE_MASK_SECRETS_MIN_LENGTH}}"
    +        b="${1:${#1}-n}"
    +        printf "%s%s" "${a//?/*}" "$b"
    +    fi
     else
         printf "%s" "$1"
     fi
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Adding error handling for edge cases improves the robustness of the mask script, ensuring it handles unexpected input gracefully.

    7

    @VietND96 VietND96 merged commit 0a4a778 into trunk Aug 18, 2024
    31 checks passed
    @VietND96 VietND96 deleted the mask-secrets branch August 18, 2024 07:33
    @VietND96 VietND96 added this to the 4.23.1 milestone Aug 20, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants