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

env variable file support #391

Merged
merged 7 commits into from
Dec 25, 2023
Merged

Conversation

cameronaw13
Copy link
Contributor

@cameronaw13 cameronaw13 commented Jun 14, 2023

Update to my previous PR fix #390 that fixes #389, fixes #322

Introduces alternatives to sensitive environment variables allowing users to pass sensitive information within a file to an environment variable to prevent the exposure of passwords and keys in the docker run command or docker compose file.

New alternative env variables include SESSION_SECRET_FILE, WGUI_PASSWORD_FILE, WGUI_PASSWORD_HASH_FILE, SENDGRID_API_KEY_FILE, and SMTP_PASSWORD_FILE

@cameronaw13 cameronaw13 changed the title Docker Secret support env variable file support Jul 10, 2023
@cameronaw13
Copy link
Contributor Author

I'm done with all the commits. Won't add any further modifications unless issues are pointed out.

@ngoduykhanh
Copy link
Owner

I personally don't think it is a good idea to have native support for loading environment variables from files as it makes the application config handling logic complicated. Also, we will have to worry about how to secure the files. To me, loading the config from environment variables directly is good enough, it works across environments such as VM, standalone docker container, Kubernetes, etc.

@cameronaw13
Copy link
Contributor Author

Many environments such as docker swarm and kubernetes offer native support for encryption that the container doesn't need to deal with when passing secret files into containers. But, outside of cluster environments, I would agree that importing secret files doesn't really improve security when it acts more like a bind mount moving the potential attack surface to a different location.

On the other hand, I can't see how my implementation of config logic with file support overcomplicates things. What I've implemented are conditions in the config logic that check if a select few environment variables are empty upon startup (SMTP_PASSWORD for example). If so, then check if the user has provided a file in their alternative variables (SMTP_PASSWORD_FILE). And if both are empty, simply use a default value. To me, this change seems fairly straightforward. Maybe I could change how I implemented this in the jsondb.go file to improve the nesting but everything else seems simple enough.

But, if you see no value in adding this feature anyway due to the niche amount of people that can use it properly, then that's fair.

@qosmio
Copy link

qosmio commented Sep 9, 2023

+1 for merging this feature. I use the secret features of Docker Swarm and K8S for anything requiring passing sensitive information. It's also a pretty standard practice to use the naming connection APP_PASS for literal values and APP_PASS_FILE for extracting values.

Postres and many other projects use the following function as part of their init scripts.

If you wanted to move the logic out of the go code you, I modified the init.sh script and made an inline entry for entrypoint.

version: "3.9"
volumes:
    db:
  
secrets:
  wg_user:
    external: true
  wg_pass:
    external: true

services:
  ui:
    image: ngoduykhanh/wireguard-ui
    cap_add:
      - NET_ADMIN
    environment:
      - WGUI_MANAGE_START=true
      - WGUI_MANAGE_RESTART=true
      - WGUI_USERNAME_FILE=/run/secrets/wg_user
      - WGUI_PASSWORD_FILE=/run/secrets/wg_pass
    volumes:
      - db:/app/db
      - /etc/wireguard:/etc/wireguard
    secrets:
      - wg_user
      - wg_pass
    deploy:
      placement:
        constraints:
          - node.hostname == minipc
      mode: global
    ports:
      # Bypass the routing mesh by setting mode to host
      - target: 5000
        published: 5050
        protocol: tcp
        mode: host
      - target: 51820
        published: 51820
        protocol: udp
        mode: host
    entrypoint:
      - /bin/bash
      - -c
      - |
        file_env() {
          local var="$$1"
          local fileVar="$${var}_FILE"
          local def="$${2:-}"
          local val
          if [ "$${!var:-}" ] && [ "$${!fileVar:-}" ]; then
            printf >&2 'error: both %s and %s are set (but are exclusive)\n' "$$var" "$$fileVar"
            exit 1
          fi
          local val=$${def}
          if [ "$${!var:-}" ]; then
            val="$${!var}"
          elif [ "$${!fileVar:-}" ]; then
            val="$$(< "$${!fileVar}")"
          fi
        
          [[ "$$val" != "" ]] && export "$$var"="$$val"
          unset fileVar
        }
        # Could technically add all variables referenced here, 
        # but for now only listing ones commonly considered sensitive
        env_file_vars=(
          SESSION_SECRET
          WGUI_USERNAME
          WGUI_PASSWORD
          WGUI_PASSWORD_HASH
          WGUI_DNS
          EMAIL_FROM_ADDRESS
          EMAIL_FROM_NAME
          SENDGRID_API_KEY
          SMTP_HOSTNAME
          SMTP_USERNAME
          SMTP_PASSWORD
        )
        
        for i in "$${env_file_vars[@]}"; do
          file_env "$$i"
        done
        
        # extract wg config file path, or use default
        conf=/etc/wireguard/wg0.conf
        
        [ -r db/server/global_settings.json ] && conf="$$(jq -r .config_file_path db/server/global_settings.json)"
        
        # manage wireguard stop/start with the container
        case $$WGUI_MANAGE_START in (1|t|T|true|True|TRUE)
            wg-quick up "$$conf"
            trap 'wg-quick down "$$conf"' SIGTERM # catches container stop
        esac
        
        # manage wireguard restarts
        case $$WGUI_MANAGE_RESTART in (1|t|T|true|True|TRUE)
            [[ -f $$conf ]] || touch "$$conf" # inotifyd needs file to exist
            inotifyd - "$$conf":w | while read -r event file; do
                wg-quick down "$$file"
                wg-quick up "$$file"
            done &
        esac

It's messy... but would allow others to at least use the feature without having to recompile the project or rebuild the image.

@ngoduykhanh
Copy link
Owner

Fair enough. I am merging the PR. Sorry for the late response.

@ngoduykhanh ngoduykhanh merged commit 3024d36 into ngoduykhanh:master Dec 25, 2023
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.

Docker Compose secrets not passing into env vars Secret support for email || maybe email issue
3 participants