FS#198 - image: commit a16a8814ead80984ce4ef7bed756434119b3aafa breaks dnsmasq

[aparcar]

arjendekorte:

Commit a16a8814ead80984ce4ef7bed756434119b3aafa causes dnsmasq to fail to read /etc/hosts and /etc/ethers. Message in the log mentions permissions denied. Notable observation is that permissions of /rom/etc/hosts after installation are 640, where 644 was used previously.

It also causes that when LuCI is built in a sysupgrade image to fail with

Forbidden
You don't have permission to access /cgi-bin/luci on this server.

Similarly as for dnsmasq, many files used by LuCI have permission 640 with this patch, most likely causing it to lack sufficient permissions to run.

Both worked fine up to this commit. Reverting 'include/image-legacy.mk' and 'include/image-legacy.mk' to the status of commit 6c1542787d135de358710254e71fe926f8676954 makes both work fine again.

This was found by running 'git bisect'. Note that .config is generated with the following command before each recompile with

./scripts/config/conf --defconfig=config.seed Config.in

using the attached 'config.seed' file. Using Netgear WNDR4300 for testing.

[aparcar]

NeoRaider:

I tried your config.seed (with the device changed, as I don't have a WNDR4300), and unfortunately, I can't reproduce your issues.

Is it possible that the filesystem you build on is hampering with the permissions of the files in some way? One possible reason might be directory default ACLs, I know I've seen a similar issue in the past that had something to do with POSIX ACLs.

[aparcar]

arjendekorte:

I just tried again. This is what I get:

Sat Oct 1 22:46:44 2016 daemon.err dnsmasq[2705]: failed to load names from /etc/hosts: Permission denied Sat Oct 1 22:46:44 2016 daemon.info dnsmasq[2705]: read /tmp/hosts/dhcp - 4 addresses Sat Oct 1 22:46:44 2016 daemon.info dnsmasq[2705]: read /tmp/hosts/odhcpd - 0 addresses Sat Oct 1 22:46:44 2016 daemon.err dnsmasq-dhcp[2705]: failed to read /etc/ethers: Permission denied

The weird thing is, the permissions on these files:

ls -l /etc/hosts /etc/ethers

-rw-r--r-- 1 root root 0 Oct 1 19:18 /etc/ethers -rw-r----- 1 root root 110 Oct 1 19:18 /etc/hosts

and

ls -l /rom/etc/hosts /rom/etc/ethers

ls: /rom/etc/ethers: No such file or directory -rw-r----- 1 root root 110 Oct 1 19:18 /rom/etc/hosts

I can understand the reason why /etc/hosts is not readable, dnsmasq is running as dnsmasq:dnsmasq and so is unable to read it. But what I don't understand is, why /etc/ethers can't be read. According to the permissions, it should be readable by dnsmasq. Even after rebooting, it can't be read. So either dnsmasq is not looking at the right location, or it is started before it is created.

Last, changing the permissions on /etc/hosts to 644 still doesn't make it readable for dnsmasq. The same errors show up at (re)starting dnsmasq.

[aparcar]

NeoRaider:

Have you looked at the permissions of the containing directories / and /etc ?

[aparcar]

arjendekorte:

That was indeed the problem.

The account I use for building images runs with umask setting 0027, which means that the files from Github feeds all have 0640 / 0750 permissions. It turns out many files are simply copied to the build directory without changing permissions. This means that if a file or directory on the build system has 0640 / 0750 permission, the permissions in the image will also not be world readable. Previously, this was not an issue, as the permissions were always changed to a 0755 / 0644 (with the exception of some files), but now this no longer is the case, it breaks packages.

This cause my /etc directory to have permission 0750 and likewise LuCI was throwing an error, as lots of files didn't have world readable permission. I'm not sure I like this new behavior. At least people should be warned that they need to checkout the Github feeds with a umask setting not more strict than 0022, in order to prevent this from causing problems in the images.

[aparcar]

jow-:

We could implement a build-prereq test for this condition.

[aparcar]

arjendekorte:

A build-prereq test might be an option. However, it is not sufficient to check umask at the time of building, you'd need to check the umask from the moment of the initial git checkout. Permissions will not be updated automagically if umask changes later on.

Alternatively, it is possible to check the permissions on the contents of the package/ and feeds/ directory, although I'm a little concerned about the delay this may cause.

[aparcar]

jow-:

True, but it will catch most instances of this particular issue I think - and it will prompt users to investigate potential issues.

[aparcar]

hnyman:

The new umask prereq check stopped me: I had umask 0002, which now fails the new check for exactly 0022.

I checked and as far as I can see, 0002 is the default in Ubuntu, so the change will likely hit a lot of users. The definition in Ubuntu is defined in /etc/login.defs as a combination of UMASK 0022 and USERGROUPS_ENAB YES that changes the actual UMASK to 0002. Link to a short explanation: https://ubuntuforums.org/showthread.php?t=2154180&p=12691326#post12691326 and https://blueprints.launchpad.net/ubuntu/+spec/umask-to-0002 . Based on the latter link, some other distros also effectively default to 0002

I am not sure if the requirement to have exactly 0022 is the optimal solution. Does the group bit have real impact here?

Could the grep value be changed from "0022" to either "00.2" or "00[02]2" ?

[aparcar]

NeoRaider:

Hmm, even before my change, differing umasks would affect the contents of all ipk packages, there was just a fixup for the base images.

I have not checked, but I'd expect the umask will have a direct effect on some files and directories in packages, including the group bit - maybe not a fatal effect, but it will cause differences and thus make reproducible builds more difficult to achieve.

If we don't want to require one specific umask, we'll need to review the practice of copying files from the source tree while preserving mode. I think git only saves a single executable bit, so it would probably make sense to provide a copy command (or change the existing $(CP) in a way) that makes the resulting mode depend on this bit only.

[aparcar]

arjendekorte:

+1 for changing the existing $(CP) command.

Rather than just implicitly using whatever permissions files in the source tree have, they should be explicitly set to the required permissions. At present, the defaults should probably be 755 for directories and executable files and 644 for ordinary files.

As is already the case and when required, installations commands could use more restrictive permissions.