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

Restrict umask to 027 except for sudo/root broken #185

Open
adrelanos opened this issue Jan 6, 2024 · 19 comments
Open

Restrict umask to 027 except for sudo/root broken #185

adrelanos opened this issue Jan 6, 2024 · 19 comments

Comments

@adrelanos
Copy link
Member

Currently umask is set to 027 (read, write for owner and group only).
(Group is OK because Debian uses usergroups by default, UPG (UserPrivateGroups)).

This however should not be the case for files created by root as this is super confusing, causing issues when creating files in /etc (or /usr) as these won't be readable by applications normally suppose to be able to read these running as non-root. That part is broken.

Therefore unfortunately this feature has to be reverted until this can be fixed.

This was discussed before here:

The following unfortunately has to be removed.

/usr/share/pam-configs/umask-security-misc

Name: Restrict umask to 027 (by package security-misc)
Default: yes
Priority: 100
Session-Type: Additional
Session-Interactive-Only: yes
Session:
	[success=1 default=ignore]	pam_succeed_if.so uid eq 0
	optional	pam_umask.so umask=027

I cannot even just out-comment it to make it easier to tinker as folder /usr/share/pam-configs does not support comments. Populating /etc/pam.d from /usr/share/pam-configs is a Debian / Ubunut feature.


Command for testing:

touch a && ls -la a && rm a

I tried various stuff to fix it:

  • delete Session-Interactive-Only: yes
        [success=2 default=ignore]      pam_succeed_if.so debug uid eq 0
        [success=1 default=ignore]      pam_succeed_if.so debug use_uid uid eq 0
	optional	pam_umask.so debug umask=027

This issue is only happening in non-Qubes. Not reproducible in Qubes Debian. This might be because Qubes login session work differently. This is probably also why I did not spot this issue beforehand.


Contributing: I don't necessarily need the config snippet for /usr/share/pam-configs as I might be able to figure that out but the config snipped required for /etc/pam.d would very useful.

adrelanos added a commit that referenced this issue Jan 6, 2024
because broken because this also happens for root while it should not

#185
@adrelanos
Copy link
Member Author

Disabled just now in git.

@monsieuremre
Copy link
Contributor

What happens when you run the command umask with the old config:

  • When directly root
  • When sudoer using sudo
  • When sudoer not using sudo
  • When non-root user

Expected is:

0022
0022
0027
0027

I don't get how this works normally on a qubes vm but not normal debian. What is the content of /etc/pam.d/common-session under qubes? Does it differ from the normal one?

@monsieuremre
Copy link
Contributor

This however should not be the case for files created by root as this is super confusing, causing issues when creating files in /etc (or /usr) as.

Why don't we just set this same umask for the root account too? I don't think it too much of a stretch to assume that a system admin that creates and maintains configurations files can set the file permissions manually.

But if we want to keep this thing, we can just do this basically.

  • Create a file /etc/profile.d/30_security-misc.sh with the following content:
if [ "$(id -u)" -eq 0 ]
then
    umask 077
else
    umask 022
fi

This would set the umask value for all non root accounts to 0077 and root to 0022. In addition to this, we should not get rid of the pam module. Keep it and set it also to 0077. It gets overriden by /etc/profile anyway, and I think (not being certain) /etc/profile does not set the umask value for system daemons whereas the pam module does that too.

I can create a pull if this works.

@adrelanos
Copy link
Member Author

Root creating files not readable by "others" by default is a problem because it breaks many instructions on how to create any config files or scripts as root that exist for Linux. Not only the file creation needs to be done but also file permissions need to changed after that. For example adding an APT source would be ignored by APT because the sandboxed user under which APT is running cannot read the file.

By extension it breaks many scripts / installers because these are basically just automation of "touch /etc/file", "echo something > /usr/bin/script" and so forth because these are as if root typed these commands by hand. This can involve many files and the installer doesn't inform the user about each file. Fixing file permissions after that would be very hard even for system administrators.

For example, this broke derivative-maker creating an APT sources list file which then APT could not read, which was a confusing and time consuming bug to diagnose. The actual fix was just 1 line to fix the permissions so that doesn't really demonstrate the effort needed to fix it. Pretty much all tools, installers assume default umask except puppet, salt and alike.

This is similar to #147 which broke a lot things which nobody could foresee. Most recent example is that it broke Calamares.


/etc/profile.d isn't the right mechanism because it is ignored in more situations than I can remember now. Some only come up after an issue is happening. But situations in which /etc/profile.d is ignored include

  • if someone is using a different default shell such as Kicksecure using zsh. For zsh it is /etc/zprofile.d/ folder.
  • Also when logging in over SSH this might be ignored.
  • X11 also ignores this, for that /etc/X11/Xsession.d folder would need to be used.
  • Not sure what Wayland uses at this time but weirdly it might be /etc/profile.d.
  • Under Wayland, how you'd create a file in /etc?
  • systemd units ignore all of this but that is probably considered a feature, not a bug.
  • Probably others.

If we cannot do this reliably so the user can trust that this works, we better shouldn't do a half implementation because a user trusting this to work could end up with a security issue when this is ignored in some environments.

PAM seems to be the most suitable mechanism because it's used for all sorts of logins.

@monsieuremre
Copy link
Contributor

Also when logging in over SSH this might be ignored.
Remote root login should already be disabled on a workstation, so not relevant IMO.

if someone is using a different default shell such as Kicksecure using zsh. For zsh it is /etc/zprofile.d/ folder.
Then we link zprofile to profile. Or we just do zprofile. As you said, this package is meant for kicksecure. Anything else should be not an issue.

systemd units ignore all of this but that is probably considered a feature, not a bug.
We want them to. We want root daemons to have a umask of 0077. Your argument only applies to human system admins that set up config files.

X11 also ignores this, for that /etc/X11/Xsession.d folder would need to be used.
Once again, this is what we want.

We won't drop pam. Pam will enforce 0077 for everyone, including all these in the list. Then for the human root, we will have 0022. This is not a bug, this is what we want. The only valid concern in the list is ssh, which is also not valid, because we block this functionality anyway, as we should. A workstation should have a remote root login functionality, especially a secure one. But if desired, umask for ssh can be set separately too.

@adrelanos
Copy link
Member Author

I still think this needs to be done with PAM as this is the universal way to set this without a lot of application specific exceptions. And the PAM way hasn't been exhausted yet.

This is just my general development philosophy to first exhaust the seemingly proper fix before adding lots of workarounds, hacks to accomplish the same. Because all of the exceptions, bug reports coming later are otherwise becoming a unmanageable mess.

monsieuremre:

Also when logging in over SSH this might be ignored.
Remote root login should already be disabled on a workstation, so not relevant IMO.

It's a universal OS. Not a desktop-only (workstation) OS.

systemd units ignore all of this but that is probably considered a feature, not a bug.
We want them to. We want root daemons to have a umask of 0077. Your argument only applies to human system admins that set up config files.

If changing the defaults for systemd units (how to even do that?) then this could lead to a lot unforeseeable issues.

So the scope in your opinion is to change default umask for both human and daemons?

related:

@monsieuremre
Copy link
Contributor

So the scope in your opinion is to change default umask for both human and daemons?

Make the umask 0077 for all daemons and humans, then manually set the umask 0022 for the root account for human logins.

@adrelanos
Copy link
Member Author

daemons: Did you ever see a daemon that uses insecure umask / creates files with insecure permissions? -> Please check for / report bug(s) upstream.

@monsieuremre
Copy link
Contributor

daemons: Did you ever see a daemon that uses insecure umask / creates files with insecure permissions? -> Please check for / report bug(s) upstream.

I raise the same logic. Have you ever seen a daemon break because of the system umask because it doesn't set its own umask explicitly whenever its important? No? If yes, we gotta open an issue there.

So this is not a blocker.

@adrelanos
Copy link
Member Author

adrelanos commented Jan 25, 2024 via email

@monsieuremre
Copy link
Contributor

I am rejecting the burden of proof reversal in this case.

I don't mean to imply any burden of proof. I just wanted to state that it should not be a problem.

And as an example to how it makes sense. Like root privileged services having temp files that are not set with good permissions is just one example. A universal solution is more inclusive than individually setting private tmp folder config for services manually one by one.

@adrelanos
Copy link
Member Author

Any specific examples?

@monsieuremre
Copy link
Contributor

I am not sure what you mean by examples. But I want to at least say this once again, we should set the umask for the root account as well. I do not understand the appeal. I think setting the umask for the root is the most important of them all. Any use cases like system administration requires declaring file permissions when creating them. Any files that are installed with a package are also set with their own umasks not with roots umask. Debian also recommends settings 0077 for the root account in their securing guide. The same goes for redhat too. I think we are just better of with setting 0077 globally, and I don't think any breakage will occur at all.

@monsieuremre
Copy link
Contributor

I mean sir, these problems are caused because the files are touched without specifying the file permissions, which it should have been done especially the source stuff. The apt problem is caused because there is no file at all, not because the permissions are set wrong. The tor browser is caused because the package should set file permissions but it does not. Like this can be solved within the package easily. Debian packages set file permissions dynamically.

I have been using 0077 umask for all accounts and I for one have not experienced any issue so far. Of course, anecdotal evidence, means nothing. But still.

@adrelanos
Copy link
Member Author

I don't think any breakage will occur at all.

I am afraid, that's insufficient. Also

wasn't expected to cause a lot of breakage as referenced there. And if something breaks in complicated, time-consuming ways, nobody is available to pick up the pieces such as:

Hence, I am careful with changes.

So what's the way forward? Work with upstream.

That is, a feature request against Debian and/or other Linux distributions.

Purpose of such a feature request:

  • get valuable input what might happen

Non-purpose of such a feature request:

  • Unfortunately, probability this actually being changed is low. But does not matter much. If,
    • It gets changed: great, done upstream in Debian, much better, not required downstream in Kicksecure.
    • If it does not get changed: The feature gets rejected with reasons and these will be helpful for consideration how it could be handled in Kicksecure.

I'll be sending feature request Change Debian's default umask to a more secure value such as umask 0077. to Debian soon. If you wish to contribute, please do the same for your favorite (base) distribution(s).

The tor browser is caused because the package should set file permissions but it does not.

The packaging did everything correctly. The user didn't set correct permissions for custom configuration files in /etc. Read right for others (chmod o+r) is required. Hence, application running as non-root couldn't read the file.

The apt problem is caused because there is no file at all, not because the permissions are set wrong.

Happened because file was not readable because not readable by others.

@adrelanos
Copy link
Member Author

@monsieuremre
Copy link
Contributor

wasn't expected to cause a lot of breakage as referenced there. And if something breaks in complicated, time-consuming ways, nobody is available to pick up the pieces such as:

Yeah the cause of the problem there is noexec tho, not libpam-tmpdir. Libpam-tmpdir just makes sure that everyone can only acces their own temp files only. And if it breaks anything (it won't), than that program is very poorly written at best at malicious at worst. There is no legit reason to access other user's tmp files.

@adrelanos
Copy link
Member Author

adrelanos commented Feb 17, 2024 via email

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

No branches or pull requests

2 participants