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

'sudo xl console', login asks for password for user 'user' #1130

Closed
adrelanos opened this issue Aug 18, 2015 · 16 comments · Fixed by marmarek/old-qubes-core-agent-linux#23
Closed
Labels
C: core diagnosed Technical diagnosis has been performed (see issue comments). P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists.

Comments

@adrelanos
Copy link
Member

Works passwordless for user 'root' but not for user 'user. Both, Fedora and Debian templates are affected.

@unman
Copy link
Member

unman commented Aug 23, 2015

@adrelanos
I think this is expected behaviour. You could delete password for user, but wouldn't that break the passwordless su? It could be done through pam I guess.
In what use case would you want console access where you didnt want to open a term? The only times I've used xl console is where there's some boot/X issue and then root is what I want.

@adrelanos
Copy link
Member Author

You could delete password for user, but wouldn't that break the passwordless su?

Just now tested sudo passwd user -d. Now passwordless login as both, user or root, functional. Also sudo is still functional.

In what use case would you want console access where you didnt want to open a term? The only times I've used xl console is where there's some boot/X issue and then root is what I want.

Exactly in that use case. But also in exactly these problematic cases it's very good to avoid further confusion and not to add more hurdles [or riddles] for the one trying to provide debug information.

Can someone advice please, where the code is that allows passwordless login for the root user?

@unman
Copy link
Member

unman commented Aug 23, 2015

If you look in /etc/shadow you'll see root has no password set.
Then in /etc/pam/d.common--auth pam_unix.so is set nullok_secure:
this allows access with a blank password to a tty in /etc/securetty.
Finally console will appear in /etc/securetty

@adrelanos
Copy link
Member Author

We could get away with this by modifying https://github.com/marmarek/qubes-core-agent-linux/blob/master/debian/qubes-core-agent.preinst.

useradd --user-group --create-home --shell /bin/bash user
->
useradd --password "" --user-group --create-home --shell /bin/bash user

@marmarek
Copy link
Member

I'm ok with this change. Are you going to commit this?

Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

@adrelanos
Copy link
Member Author

Yes, pull request attached above.

I was wondering if this should be fixed in existing images during upgrade as well by running sudo passwd user -d. Do we care about this? But I am not sure I am getting lost in complexity here. The command would have to be protected by a run-only-once status file mechanism, so users who decided to use protections by user passwords / sudo would not always end up having their user password deleted - do we care about those? And if one wanted to go crazy about this, a protected file mechanism to opt-out beforehand would be required.

@marmarek
Copy link
Member

Theoretically we could check for:

  1. The current value ("!")
  2. Previous package version (so it gets run only on upgrade from before
    3.0.15).

But I think we could also simply ignore this.

Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

@adrelanos
Copy link
Member Author

Theoretically we could check for:

  1. The current value ("!")

Done in above pull request.

@marmarek marmarek added T: enhancement Type: enhancement. A new feature that does not yet exist or improvement of existing functionality. C: core labels Sep 1, 2015
@marmarek marmarek added this to the Release 3.1 milestone Sep 1, 2015
@adrelanos
Copy link
Member Author

Please reopen.

Can someone confirm, that newly created Debian templates that never upgraded qubes-core-agent before, can login sudo xl console vm-name with user user?

Looking at qubes-core-agent-linux preinst it creates user user without password only if that user does not yet exist.

But qubes-builder-debian prepare-chroot-debian uses useradd -g user $USER_OPTS -m user so the user will already be existing, if I am not mistaken.

Below.

usermod -p '' root

We should add.

usermod -p '' user

@unman
Copy link
Member

unman commented Apr 16, 2017

@andrewdavidwong Confirmed this issue still arises in 3.2 milestone for Fedora template

@schnurentwickler
Copy link

schnurentwickler commented May 28, 2018

In current release (Qubes 4.0) the usage of sudo xl console -t pv VMname in dom0 does give me access to console in standard fedora-26, but only passwordless into root. The account user wants a password I do not know or have set / deleted.
In fedora-28-minimal without the packages qubes-core-agent-passwordless-root and polkit, both root and user account are not able to login via sudo xl console.
Root access via xl console should not depend on passwordless-root and polkit, should it?
I thought the passwordless-root package only set up sudo usage for an user shell.

But to get control qvm-run -u root fedora-28 xterm does give me a root shell.

@adrelanos
Copy link
Member Author

QubesOS/qubes-core-agent-linux#167

QubesOS/qubes-core-agent-linux#168

@adrelanos
Copy link
Member Author

QubesOS/qubes-core-agent-linux#169

QubesOS/qubes-core-agent-linux#170

After/if these are merged, I will suggest to move user/root password manipulation from qubes-core-agent to qubes-core-agent-passwordless-root instead. That would simplify https://github.com/tasket/Qubes-VM-hardening since then removal of qubes-core-agent-passwordless-root package would result in what is expected and also nicely encapsulate that functionality into the correct package. But that's a separate ticket. My idea was to first document / make obvious what we are doing to everyone and myself, and then be well prepared to suggest the fixes for the things I actually care about.

This is related to https://github.com/tasket/Qubes-VM-hardening and the recently implemented root/su/sudoers/pam/securetty hardening in https://github.com/Whonix/security-misc.

//cc @tasket

@andrewdavidwong andrewdavidwong added the P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. label Jul 14, 2019
@marmarek
Copy link
Member

marmarek commented Aug 8, 2019

Comment from QubesOS/qubes-core-agent-linux#171 (comment):

What about using auth sufficient pam_listfile.so item=tty sense=allow file=/etc/security/passwordless-tty.conf and /etc/security/passwordless-tty.conf containing just hvc0 (and maybe tty1?)?

This would allow passwordless login on xl console to any user (including root and user), without setting empty password. Should also not interfere with qubes-core-agent-linux-passwordless-root package (or lack of it).

@marmarek
Copy link
Member

From QubesOS/qubes-core-agent-linux#168 (comment):

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)

I like this idea very much, much better than the current empty password.

@DemiMarie
Copy link

QubesOS/qubes-core-agent-linux@fb28a48 and preceding commits fixed this for R4.1. Needs backporting to R4.0. Closing, but marking as “backport needed” so that R4.0 will get the fix.

@DemiMarie DemiMarie added the backport pending On closed issues, indicates fix released for newer version will be backported to older version. label Feb 16, 2022
@andrewdavidwong andrewdavidwong added diagnosed Technical diagnosis has been performed (see issue comments). T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists. and removed T: enhancement Type: enhancement. A new feature that does not yet exist or improvement of existing functionality. labels Feb 17, 2022
@marmarek marmarek removed the backport pending On closed issues, indicates fix released for newer version will be backported to older version. label May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: core diagnosed Technical diagnosis has been performed (see issue comments). P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants