FS#12263 - sudo's new umask handling can cause library permission errors

Attached to Project: Pacman
Opened by Brad Conte (B-Con) - Friday, 28 November 2008, 04:10 GMT
Last edited by Dan McGee (toofishes) - Saturday, 03 January 2009, 06:25 GMT
Task Type Bug Report
Category General
Status Closed
Assigned To Dan McGee (toofishes)
Allan McRae (Allan)
Architecture All
Severity Low
Priority Normal
Reported Version 3.2.1
Due in Version 3.2.2
Due Date Undecided
Percent Complete 100%
Votes 5
Private No

Details

Summary and Info:

When a user's umask is set to a value stricter than sudo's default, sudo will now use to that user's umask value instead. When using the -U switch, it seems that sometimes the directory for a package in /var/lib/pacman/local will be set according to the user's umask, which can cause database query errors.

Steps to Reproduce:

Actual example: if the user's umask is 077,recompile kernel26 via makepkg and install via pacman -U kernel26.pkg.tar.gz. The library directory /var/lib/pacman/local/kernel26-2.6.27.6-1 will have permission 700.

This problem also occurs when re-installing Firefox.

Subsequent queries by pacman (such as pacman -Qi pkgname) run by the user will result in "cannot read this directory" errors.

This does not seem to be reliably repeatable, as I've used -U to install packages that did not result in any permission problems. kernel26 and firefox have suffered from this problem on multiple trials, however. It might be solved by explicitly setting the directory permissions after creating a directory in /var/lib/pacman/local/ .

Some forum chatter: http://bbs.archlinux.org/viewtopic.php?pid=454561
This task depends upon

Closed by  Dan McGee (toofishes)
Saturday, 03 January 2009, 06:25 GMT
Reason for closing:  Fixed
Additional comments about closing:  Commit a73ad4f0e3
Comment by Allan McRae (Allan) - Friday, 28 November 2008, 05:56 GMT
Can we actually do anything about this within pacman?
Comment by Brad Conte (B-Con) - Saturday, 29 November 2008, 22:50 GMT
Since the umask only determines the default permissions, I would presume the folder permissions could be explicitly set after folder creation, since we know what permissions we want.
Comment by Dan McGee (toofishes) - Sunday, 30 November 2008, 21:58 GMT
WTF? This is odd.

Makepkg does absolutely nothing with directories in the pacman DB- those are created exclusively within pacman itself, so lets not confuse the issue here.

Are we saying the following code is not working? We explicitly create directories with 755 in addition to changing our umask for all operations when we write DB entries:
http://projects.archlinux.org/?p=pacman.git;a=blob;f=lib/libalpm/be_files.c;h=f5d4e826a466aa63c95a7122cc204746bd6a3d7f;hb=HEAD#l670

If sudo screws this up, I'm not sure what more we can do beyond raising some voices to the upstream devs that this breaks a *lot* of things.
Comment by Brad Conte (B-Con) - Monday, 01 December 2008, 00:10 GMT
Never meant to imply that makepkg was in any way related to the problem, I only mentioned it because I originally discovered the problem installing a package manually. Since I had not yet seen any such problems resulting from run-of-the-mill pacman usage, I thought it might be specifically related to the -U method.

However, I'm just now noticing more packages that suffer from this problem, and they were all installed via pacman -Su.

Here's what I've gathered so far:

Setting umask to 022 solves the problem. With umask 077, this glitch only appears for specific packages. No matter which machine I try it on, and no matter where the package file comes from, the directory permissions will be incorrect for certain packages, but correct for others. I can set the exact same permissions for the .pkg, set a umask of 077, remove the package(s) from the db, and one package will install to the db correctly while another will not. This occurs repeatedly (and is independent of the package version) and the permission glitch persists even if the package was removed first or if the directory in question had the proper permissions set before upgrade.

What gets me is that, reliably, certain packages will always result in a bad db permission, while others will always have a good db permission. If sudo is allowing permission setting correctly for one package, why would it obstruct permissions for another? That kind of flakiness seems to imply either something amis in pacman, or the sudo devs implemented the new umask-inheritance scheme poorly.

Referring to the pacman code Dan linked to, pacman does set the directory permissions explicitly. What's noteworthy is that all created *files* have the correct permissions, it's the *directory* that doesn't. Also, from that code I noted that the directory is the only thing created prior to the manual setting of a new umask (lines 671 and 673). I tried switching those two lines, but it didn't the problem.

For reference, packages that are suspect to this glitch (for me, at least) include: kernel26, firefox, xine-lib, evince, pango, imagemagick, and several others.
Comment by Dan McGee (toofishes) - Monday, 01 December 2008, 00:14 GMT
Perhaps you can send an email to the sudo mailing list asking about this? Maybe even point them to this report (and more specifically the code I linked above). I'm really at a loss what to do here.

If you do an 'su' to root, set a restrictive umask (0077) there, and then perform the same operation, can you reproduce the problem or is this *only* when running under sudo?
Comment by Ben Kuhn (bkuhn) - Tuesday, 30 December 2008, 17:17 GMT
Any progress here?

I usually set my umask to 077. Pacman often (usually?) sets incorrect permissions in /var/lib/pacman/local. As a workaround, I schedule a cron job to chmod a+rx /var/lib/pacman/local.

However, I recently installed a package using Yaourt and the binary ended up with permissions 700. As I understand this is probably a problem with makepkg?

Thanks!
Comment by Allan McRae (Allan) - Wednesday, 31 December 2008, 04:59 GMT
I can not replicate this with a simple testcase (attached).
Comment by Allan McRae (Allan) - Wednesday, 31 December 2008, 05:17 GMT
Comments in the duplicate  FS#12333  indicate that this is specific to particular packages. I can always replicate this when installing mutagen but never with vlc. However, changing my test code above to make the /var/lib/pacman/local/mutagen-1.15-1 directory does not replicate...
Comment by Allan McRae (Allan) - Wednesday, 31 December 2008, 06:57 GMT
I installed almost every package in my cache directory into a temporary directory using the --root option and created a list of packages with good permissions and bad permissions. Anybody see a pattern?
Comment by Heiko Baums (cyberpatrol) - Wednesday, 31 December 2008, 07:50 GMT
Maybe I should post a workaround for people, who come to this bug, probably because they have problems with scripts like yaourt due to this bug, and just want a "working" package management.

Just put the attached script in a directory which is in $PATH, set "alias pacman='pacman-chmod'" in ~/.bashrc and "PacmanBin /usr/local/bin/pacman-chmod" in /etc/yaourtrc.

It's not a solution for this bug, but a dirty workaround which sets the correct file permissions after every installation done by root/with sudo.
Comment by Allan McRae (Allan) - Wednesday, 31 December 2008, 07:56 GMT
Even adding a chmod call at the end of the relevant function (as a poor work around) does not fix this...
Comment by Allan McRae (Allan) - Wednesday, 31 December 2008, 08:18 GMT
THIS IS NOT A SUDO ISSUE! This occurs if I set root's umask to 0077 and try installing affected packages as root.
Comment by Heiko Baums (cyberpatrol) - Wednesday, 31 December 2008, 09:18 GMT
I know, that my script is a poor workaround and doesn't fix this issue. I've already said that. ;-)
Nevertheless it could help people get scripts like yaourt run without errors until this bug is fixed. So I think it can help anyway. ;-)

I set root's umask to 022 and the unprivileged users' umask to 077 and I've got the same issue, even if run pacman as root.
Comment by Allan McRae (Allan) - Wednesday, 31 December 2008, 09:34 GMT
I was not talking about your script but actually trying for a workaround in the pacman code.

What is really confusing is this statement:
I set root's umask to 022 and the unprivileged users' umask to 077 and I've got the same issue, even if run pacman as root.

Are you absolutely sure about that? I can not replicate...
Comment by Allan McRae (Allan) - Wednesday, 31 December 2008, 11:19 GMT
Hmmmm.... wrapping the mkdir call like this:

if((tmp = mkdir(pkgpath, 0755)) != 0) {
fprintf(stderr, "ERROR %d: unable to mkdir; %s\n", errno, strerror(errno));
}

Then I get this for packages that fail to have their permissions set:
ERROR 17: unable to mkdir; File exists

The directory obviously does get created... In fact, if you remove the directory creation line altogether, packages which do not have the right umask install fine but packages whose dir has the right umask fail to create an entry in /var/lib/pacman/local at all.

So, there appears to be something else creating this directory (maybe).
Comment by Allan McRae (Allan) - Wednesday, 31 December 2008, 11:52 GMT
And the common problem with all these packages failing to set the umask is the presence of a changelog. That is getting extracted before the main directory is "created".

Comment by Dan McGee (toofishes) - Wednesday, 31 December 2008, 13:24 GMT
Ahh, interesting. We're finally getting somewhere!

Allan- if you strace pacman (maybe send it to a file), you should be able to grep out all of the mkdir calls. I'm guessing libarchive, when extracting the ChangeLog, is creating these directories automagically and that is where we are hitting the issues?

Install files might play into these as well- actually, they probably don't as we extract it to /tmp/ to do the pre-* functions, don't we. Looks like we need to rethink when we extract the ChangeLog files?
Comment by Dan McGee (toofishes) - Wednesday, 31 December 2008, 13:31 GMT
add.c:291-300 deal with extraction of the changelog and install files. Their extraction path is set directly to the database location, so if this directory structure doesn't exist yet, we will run into problems. We explicitly set the file mode to 0644, but that doesn't control directory creation. Looking at this again, we should see this for both .INSTALL and .CHANGELOG files, no?

since our db_write() call (add.c:803) is after we have finished our entire extract_single_file() loop, the changelog will always get extracted before we write to the database for the first time (and thus creating the directory).
Comment by Allan McRae (Allan) - Wednesday, 31 December 2008, 13:34 GMT
The install scripts are also extracted into the local pacman db during archive extraction, so they cause this too.

The fix is to create the directory during just before the archive extraction. I'm also adding a check for successful directory creation (like a couple of comments above) so something like this will be flagged in the future.
Comment by Dan McGee (toofishes) - Wednesday, 31 December 2008, 13:37 GMT
Keep in mind that "create a directory" blows the db_write() abstraction out of the water, which we have already done anyway with the install/changelog extraction but let's be careful not to stray too far down that road. I don't see many super-clean and awesome ways to do this though.

The one thing to consider is what effect an empty directory (or one with only install/changelog) would have on pacman behavior. I'm not sure how clean we would handle this as a lot of times we prime our lists by just looking at folder names to get pkgname and pkgver, and only look at the files once we know we need more info on the given package.
Comment by Allan McRae (Allan) - Wednesday, 31 December 2008, 13:47 GMT
Hmmm... I had drafted a function to create the directory with the right permissions and split it off from the db_write stuff. I hadn't thought about the need to clean it up if something goes wrong.

Maybe we should skip extraction of install file and changelog while we are extracting the package and either do it after the db_write or even during?
Comment by Dan McGee (toofishes) - Wednesday, 31 December 2008, 13:49 GMT
I was considering that option as well. More wonderful abstraction barriers to punch through though.
Comment by Heiko Baums (cyberpatrol) - Wednesday, 31 December 2008, 15:41 GMT
It seems that you found a possible reason for this bug, but yes, I'm absolutely sure about my root's umask 022 and my unprivileged users' umask 077.

Loading...