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

Allow passwordless login for user "user" (when using 'sudo xl console'). #168

Closed
wants to merge 1 commit into from

Conversation

adrelanos
Copy link
Member

QubesOS/qubes-issues#1130

Running sudo usermod --password '' user in debian-10 TemplateVM allowed me to login as user "user" using sudo xl console. While untested, I am pretty sure this would work in the next template build too.

@codecov-io
Copy link

Codecov Report

Merging #168 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #168   +/-   ##
=======================================
  Coverage   65.48%   65.48%           
=======================================
  Files           2        2           
  Lines         394      394           
=======================================
  Hits          258      258           
  Misses        136      136

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da3c22b...54f94cc. Read the comment docs.

@@ -51,6 +51,10 @@ if [ "$1" = "install" ] ; then
}
usermod -L -a --groups qubes user

## Allow passwordless login for user "user" (when using 'sudo xl console').
## https://github.com/QubesOS/qubes-issues/issues/1130
usermod --password '' user
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already done in upgrade case below. Maybe it should be unified, like that call below moved outside of if?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, better. For simplicity and less bugs (at cost of some more needless output) I think it is always good to avoid using install and upgrade cases.

@marmarek
Copy link
Member

marmarek commented Aug 6, 2019

Hmm, when I think about it more, it may not be such a good idea to do it this way. It will enable passwordless login to user not only on xl console, but everywhere. User may decide to remove qubes-core-agent-passwordless-root package to have some isolation between accounts (for example sandboxing some services using dedicated users). In that case, having empty password for user would allow anyone to switch to user by calling su - user.
Note the problem apply much less for the qubes-core-agent-passwordless-root case, where user isolation is explicitly not relied upon.

Maybe better do it by configuring PAM to specifically allow passwordless user login on hvc0 console?

@marmarek
Copy link
Member

marmarek commented Aug 6, 2019

Oh, I see you raised exactly the same concern in QubesOS/qubes-issues#2695 (comment)

@adrelanos
Copy link
Member Author

Answered here: QubesOS/qubes-issues#2695 (comment)

@adrelanos
Copy link
Member Author

Ok, then...

Quote @marmarek QubesOS/qubes-core-agent-linux#168 (comment)

Maybe better do it by configuring PAM to specifically allow passwordless user login on hvc0 console?

Yes.

@adrelanos
Copy link
Member Author

This change is just doing what we are currently doing anyhow. But I can see how my approach can be confusing to break it more visibly before starting to fix it.

@HW42
Copy link
Contributor

HW42 commented Sep 10, 2019

Maybe better do it by configuring PAM to specifically allow passwordless user login on hvc0 console?

Depending on how the exact result should look like, it might be easier to just run agetty with --autologin user (or --autologin root). (You probably also want to add --login-pause)

@marmarek
Copy link
Member

Autologin idea sounds much better and safer.

@marmarek
Copy link
Member

This is handled in #228

@marmarek marmarek closed this May 25, 2020
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.

4 participants