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

add automation scripts to generate node keys #299

Merged
merged 1 commit into from
May 7, 2024

Conversation

DaMandal0rian
Copy link
Contributor

@DaMandal0rian DaMandal0rian commented Mar 29, 2024

User description

We currently generate node keys manually, and store in flat files which is cumbersome especially as the network expands.

These scripts do the following:

  • generates keys with subkey and stores in hashicorp vault
  • restores keys and adds to .env file to be used as part of automation with ansible and terraform

Type

enhancement


Description

  • Introduced two new automation scripts for node key management:
    • generate_node_keys.sh for generating and storing node keys in Hashicorp Vault.
    • restore_node_keys.sh for retrieving node keys from Hashicorp Vault and storing them in a .env file.
  • These scripts facilitate the automation of node key generation, storage, and retrieval, enhancing the deployment process for various node types.

Changes walkthrough

Relevant files
Enhancement
generate_node_keys.sh
Automate Node Key Generation and Storage in Hashicorp Vault

scripts/generate_node_keys.sh

  • Added a script to automate the generation of node keys using subkey
    and store them in Hashicorp Vault.
  • Implemented functions to check for subkey installation, generate node
    keys, and add them to Hashicorp Vault.
  • Automated node key generation for various node types (bootstrap, rpc,
    domain, farmer, nova-bootstrap).
  • +72/-0   
    restore_node_keys.sh
    Automate Restoration of Node Keys from Hashicorp Vault     

    scripts/restore_node_keys.sh

  • Added a script to restore node keys from Hashicorp Vault and store
    them in a .env file.
  • Implemented functions to check for jq installation, retrieve key-value
    pairs from Hashicorp Vault, and store them in .env file.
  • Ensured the script exits if no node name is provided as an argument.
  • +60/-0   

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

    @github-actions github-actions bot added the enhancement New feature or request label Mar 29, 2024
    Copy link

    PR Description updated to latest commit (600cf1e)

    Copy link

    PR Review

    ⏱️ Estimated effort to review [1-5]

    3, because the PR introduces new scripts with several functions for generating and storing node keys, which involves external dependencies like subkey and jq, and interacts with Hashicorp Vault. The logic within the scripts is straightforward, but careful attention is needed to ensure security practices are followed, especially in handling sensitive data like keys and tokens.

    🧪 Relevant tests

    No

    🔍 Possible issues

    Possible Security Concern: The script uses base64 -d <<< ${VAULT_TOKEN} to decode the Vault token. If the environment variable is not properly secured, this could expose sensitive information.

    Error Handling: There is minimal error handling for external commands (e.g., curl, subkey generate-node-key). Failures in these commands may not be caught, leading to incomplete or incorrect execution without clear indication to the user.

    🔒 Security concerns

    Sensitive Information Exposure: The scripts handle sensitive information, such as node keys and Vault tokens. It's crucial to ensure that these are securely managed throughout the script's execution, especially when making API calls or storing keys in files.

    Code feedback:
    relevant filescripts/generate_node_keys.sh
    suggestion      

    Consider adding error handling for the curl command used to install subkey and the subkey generate-node-key command. This ensures the script halts or takes corrective action if these critical steps fail. [important]

    relevant linecurl https://getsubstrate.io -sSf | bash -s -- --fast

    relevant filescripts/generate_node_keys.sh
    suggestion      

    It's recommended to securely handle the output of base64 -d <<< ${VAULT_TOKEN} to prevent potential leakage of sensitive information. Consider storing the decoded token in a variable and ensure it's not inadvertently logged or exposed. [important]

    relevant linelocal vault_token=$(base64 -d <<< ${VAULT_TOKEN})

    relevant filescripts/restore_node_keys.sh
    suggestion      

    To enhance security, validate the format of the key_value retrieved from Hashicorp Vault before proceeding to use it. This can prevent potential command injection or other security issues if the value format is not as expected. [important]

    relevant linelocal key_value=$(echo "${response}" | jq -r '.data.data.key')

    relevant filescripts/restore_node_keys.sh
    suggestion      

    Add error handling for the jq installation command to ensure the script does not proceed if jq cannot be installed successfully. This is critical for the correct execution of the script. [medium]

    relevant linesudo apt-get install -y jq


    ✨ Review tool usage guide:

    Overview:
    The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

    The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

    • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
    /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...
    
    [pr_reviewer]
    some_config1=...
    some_config2=...
    

    See the review usage page for a comprehensive guide on using this tool.

    Copy link

    PR Code Suggestions

    CategorySuggestions                                                                                                                                                       
    Best practice
    Quote variables to prevent globbing and word splitting.

    It's recommended to quote variables to prevent globbing and word splitting. This is
    particularly important in the curl command where ${vault_token} is used. To improve the
    script's robustness, consider quoting ${vault_token}.

    scripts/generate_node_keys.sh [37]

    -curl -X POST -H "X-Vault-Token: ${vault_token}" -d "{\"data\": {\"key\": \"${key_id}=${key_value}\"}}" "${vault_endpoint}"
    +curl -X POST -H "X-Vault-Token: \"${vault_token}\"" -d "{\"data\": {\"key\": \"${key_id}=${key_value}\"}}" "${vault_endpoint}"
     
    Check if the .env file exists to prevent unintentional overwrites.

    To ensure that the .env file does not unintentionally expose sensitive information or get
    overwritten with each execution, it's advisable to check if the file exists and prompt the
    user before appending to it.

    scripts/restore_node_keys.sh [34-35]

    +if [ -f ".env" ]; then
    +  echo ".env file exists. Appending..."
    +else
    +  echo "Creating .env file..."
    +fi
     echo "${node_name}=${key_value}" >> ".env"
     echo "peer_id=${key_id}" >> ".env"
     
    Add error handling for curl command and data retrieval.

    To improve error handling, consider adding checks for the success of the curl command and
    the presence of the expected key_value in the response from Hashicorp Vault. This can
    prevent the script from proceeding with invalid or empty data.

    scripts/restore_node_keys.sh [21-22]

     local response=$(curl -s -H "X-Vault-Token: ${vault_token}" "${vault_endpoint}")
    +if [ $? -ne 0 ]; then
    +  echo "Failed to retrieve data from Vault."
    +  exit 1
    +fi
     local key_value=$(echo "${response}" | jq -r '.data.data.key')
    +if [ -z "${key_value}" ]; then
    +  echo "No key_value found in response."
    +  exit 1
    +fi
     
    Enhancement
    Improve portability by handling differences in the base64 command.

    The script uses base64 -d to decode the VAULT_TOKEN, which might not work on macOS due to
    differences in the base64 command options. To make the script more portable, consider
    using an if-statement to detect the operating system and adjust the command accordingly.

    scripts/generate_node_keys.sh [35]

    -local vault_token=$(base64 -d <<< ${VAULT_TOKEN})
    +if [[ "$OSTYPE" == "darwin"* ]]; then
    +  local vault_token=$(base64 -D <<< ${VAULT_TOKEN})
    +else
    +  local vault_token=$(base64 -d <<< ${VAULT_TOKEN})
    +fi
     
    Security
    Enhance security by inspecting scripts before execution.

    Instead of using curl to pipe a script directly into bash for installing subkey, consider
    downloading the script first, inspecting it, and then executing it. This approach enhances
    security by allowing review before execution.

    scripts/generate_node_keys.sh [7]

    -curl https://getsubstrate.io -sSf | bash -s -- --fast
    +curl https://getsubstrate.io -sSf -o get_substrate.sh
    +# Inspect the script or perform any necessary actions
    +bash get_substrate.sh --fast
     

    ✨ Improve tool usage guide:

    Overview:
    The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

    • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
    /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...
    
    [pr_code_suggestions]
    some_config1=...
    some_config2=...
    

    See the improve usage page for a comprehensive guide on using this tool.

    @DaMandal0rian DaMandal0rian force-pushed the node-key-automation branch 2 times, most recently from a1fd981 to 01ba2be Compare April 1, 2024 17:54
    - generates keys with subkey and stores in hashicorp vault
    - restores keys and adds to .env file as part of automation
    @DaMandal0rian DaMandal0rian merged commit db0530d into main May 7, 2024
    1 check passed
    @DaMandal0rian DaMandal0rian deleted the node-key-automation branch May 7, 2024 13:47
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants