owenthereal / upterm

Instant Terminal Sharing
https://upterm.dev
Apache License 2.0
833 stars 55 forks source link

`upterm host` should reject any unauthenticated users when `--authorized-key`, `--github-user` or `--gitlab-user` option is present #119

Open lhotari opened 2 years ago

lhotari commented 2 years ago

Problem: I ran into an issue where I had a passed an empty file to --authorized-key option for the upterm host command.

Steps to reproduce

  1. Start a new upterm host session in a docker container:

    # start a new ubuntu docker container
    docker run --rm -it ubuntu:20.04 bash
    # in the container run these commands to start upterm
    apt-get update
    DEBIAN_FRONTEND=noninteractive apt-get -y install --no-install-recommends curl openssh-client ca-certificates
    curl -sL https://github.com/owenthereal/upterm/releases/download/v0.7.6/upterm_linux_amd64.tar.gz | \
    tar zxvf - -C /tmp upterm && \
    install /tmp/upterm /usr/local/bin/ && rm -rf /tmp/upterm
    mkdir -p ~/.ssh && chmod 0700 ~/.ssh
    ssh-keygen -q -t ed25519 -N "" -f ~/.ssh/id_ed25519
    # Auto-generate ~/.ssh/known_hosts by attempting connection to uptermd.upterm.dev
    ssh -i ~/.ssh/id_ed25519 -o 'StrictHostKeyChecking no' uptermd.upterm.dev
    cat <(cat ~/.ssh/known_hosts | awk '{ print "@cert-authority * " $2 " " $3 }' | sort | uniq) >> ~/.ssh/known_hosts
    touch ~/.ssh/authorized_keys
    upterm host -a $HOME/.ssh/authorized_keys -- bash
  2. Use the SSH session string in another terminal. The client connects to the upterm session. This is unexpected since -a $HOME/.ssh/authorized_keys is passed to upterm host.

Expected behavior

upterm host should reject any unauthenticated users when --authorized-key, --github-user or --gitlab-user option is present. An empty authorized keys file shouldn't be an exception.

bavarianbidi commented 7 months ago

:+1: from my side. we've run into the same issue and we are first wondering why we are still able to login.

IMHO we could fail if one of the flags are defined and if len(authorizedKeys) > 0.

https://github.com/owenthereal/upterm/blob/98036a647cc210f976d5f09bd6c137d795283bc1/cmd/upterm/command/host.go#L196-L211