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

Override default password #334

Closed
pcrockett opened this issue Mar 21, 2021 · 13 comments
Closed

Override default password #334

pcrockett opened this issue Mar 21, 2021 · 13 comments

Comments

@pcrockett
Copy link

pcrockett commented Mar 21, 2021

I like to run x11docker as a second full-screen session using --xorg. There are times when I walk away from my laptop and want to lock my screen. I would like to make it so that I can specify my own password to unlock the screen, rather than using the default password "x11docker".

I see how we are hard-coding the password here. I have managed to manually edit x11docker so it uses my own hard-coded password, however that won't survive updates. Would it be feasible to allow the user to specify a password via a parameter, environment variable, etc?

I was also thinking I could potentially do the work myself and submit a pull request. Do you have any guidance on how you prefer it to be done?

@mviereck
Copy link
Owner

That is a reasonable request. However, I am not sure how to include it the best way.

A simple attempt would be an option --password=TOPSECRET. But it is plain text and appears in the log and (if running in a shell) in bash history. So I did not include this possibility.

Currently hardcoding inside x11docker is the cleanest way.
Here a script x11dockerpassword to ease this task:

#! /bin/bash
# x11dockerpassword: change container user password of x11docker
# Usage:
#   x11dockerpassword NEWPASSWORD

X11dockerbin="$(which x11docker)"
[ -f "$X11dockerbin" ] || {
  echo "x11docker not found" >&2
  exit 1
}

command -v perl >/dev/null || {
  echo "perl not found" >&2
  exit 1
}

Password="${1:-}"
[ -z "$Password" ] && {
  echo "No password specified.
Usage:
  x11dockerpassword NEWPASSWORD" >&2
  exit 1
}
Password="$(perl -e "print crypt(\"$Password\", \"salt\")")"

sed -i "s%Containeruserpassword=.*%Containeruserpassword='$Password'%" "$X11dockerbin" && {
  echo "Container user password changed"
  exit 0
} || {
  echo "Password change failed" >&2
  exit 1
}

Another attempt would be a file ~/config/x11docker/passwd where the password is stored in encrypted format (read-only for current user), and x11docker would check for that file on each start.

@pcrockett
Copy link
Author

pcrockett commented Mar 21, 2021 via email

@mviereck
Copy link
Owner

I may find time to implement it, and if I do, I'll send a PR.

Thank you for your offer! I am aready looking how to implement this, so don't worry.

mviereck added a commit that referenced this issue Mar 21, 2021
--sudouser=nopasswd: Optional argument to allow sudo without password
@mviereck
Copy link
Owner

mviereck commented Mar 21, 2021

I've implemented a new option --password WORD that allows to set a custom container user password.
This generates a file ~/.config/x11docker/passwd containing the encrypted password and with access for current user only.
If this option is set, x11docker exits after creating the file.

From x11docker --help:

     --password WORD   Change container user password and exit.
                       Stored encrypted in ~/.config/x11docker/passwd.

@pcrockett
Copy link
Author

Awesome, thanks! I'll give it a try.

For what it's worth, the way I'll probably use it would look something like this:

read -rs -p "Enter password: " my_pass
x11docker --password "${my_pass}"

I personally don't mind the extra step of prompting for the password to prevent it from showing up in Bash history. Though it would be a slight usability improvement if x11docker could handle that password prompt for me when no WORD is specified.

But in any case, what you have looks like a great solution. Many thanks.

@mviereck
Copy link
Owner

prevent it from showing up in Bash history.

If you write a whitespace before the command, it won't appear in the history.

Though it would be a slight usability improvement if x11docker could handle that password prompt for me when no WORD is specified.

You are right, I'll look at this.

@mviereck
Copy link
Owner

Now x11docker prompts interactively if --password has no argument:

     --password [=WORD]   Change container user password and exit.
                       Interactive input if argument WORD is not provided.
                       Stored encrypted in ~/.config/x11docker/passwd.

@pcrockett
Copy link
Author

pcrockett commented Mar 21, 2021

Great. Just tested it on my machine, and it looks like it's working.

I did encounter two minor bumps:

  1. I am using Podman, and I got a surprising error: "docker is not installed." I didn't think we would need to touch Docker or Podman for this, so I didn't bother specifying which I was using. Adding --podman to the parameter list made the error go away.
  2. I then tried x11docker --podman --password foobar. That surprisingly prompted me for a password. After realizing my mistake and adding an = sign in there, it worked as expected.

Regardless, it was easy to figure out. Not sure if those are even worth paying attention to, just sharing my first-time experience.

@mviereck
Copy link
Owner

I got a surprising error: "docker is not installed."

Does this happen with x11docker --password=foobar?

It would happen with x11docker --password foobar because foobar would be parsed as an image name.

@pcrockett
Copy link
Author

pcrockett commented Mar 21, 2021

Sorry, looks like I was running from memory. Here are my actual results. In particular, I forgot about the realpath errors.

$ x11docker --password=foobar
realpath: '': No such file or directory
x11docker note: Using X server option --xephyr

x11docker note: Option --password: Password changed, exiting.
$ x11docker --password foobar
realpath: '': No such file or directory

x11docker ERROR: docker is not installed.
  To run docker images, you need to install docker.
  docker

  Type 'x11docker --help' for usage information
  Debug options: '--verbose' (full log) or '--debug' (log excerpt).
  Logfile will be: 
  Please report issues at https://github.com/mviereck/x11docker
$ x11docker --podman --password foobar
x11docker note: Option --podman: experimental option.
  Please report issues at: https://github.com/mviereck/x11docker/issues/255

x11docker note: Using X server option --xephyr

Please type in a new container user password (chars are invisible): x11docker note: Option --password: Password changed, exiting.

That last one wasn't a copy / paste mistake; a call to echo would put a newline in there and clean up output a little.

@mviereck
Copy link
Owner

Thank you for the details!

x11docker runs the password change earlier now, before several other checks cause messages (and trouble).
The newline is added, too.

@pcrockett
Copy link
Author

Ah much better. There is just one tiny quirk left:

$ x11docker --password foobar
Please type in a new container user password (chars are invisible): 
x11docker note: Option --password: Password changed, exiting.

So it looks like foobar is being parsed as the image name that you're trying to run, which totally makes sense. But then it doesn't actually try to run the image. I'd expect it to either (a) try running the image, or (b) set foobar as the password without prompting.

At this point I'm just trying to help with feedback in case you care about those small details. But I'm more than happy - I think this issue could be closed. Thanks so much for your time and effort!

@mviereck
Copy link
Owner

I'd expect it to either (a) try running the image, or (b) set foobar as the password without prompting.

I've decided to make --password an always-exit option that just drops all other options, similar to --update or --help. This avoids that users use --password=foobar as a permanent script option. So (a) is not a way to go.

(b) would require additional parsing, basically to allow wrong input. So I don't want to include this. Many other options also take an optional argument that forces the use of =, e.g. option --alsa. = can only be omitted if an option always must have an argument like e.g. --env. The possibility to omit = is already an exception.

I think we can leave it as it is.

Thanks so much for your time and effort!

You're welcome! Thanks for feedback and suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants