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

Wrong permissions on built Fedora artifacts #727

Closed
apyrgio opened this issue Feb 26, 2024 · 7 comments · Fixed by #819
Closed

Wrong permissions on built Fedora artifacts #727

apyrgio opened this issue Feb 26, 2024 · 7 comments · Fixed by #819
Assignees

Comments

@apyrgio
Copy link
Contributor

apyrgio commented Feb 26, 2024

While testing the produced RPMs for the 0.6.0 release, we stumbled upon this issue on Fedora 39 (Fedora 38 is also affected):

image

It seems that the produced RPMs have wrong permissions (current: rw------- / 600, expected: rw-r--r-- / 644), specifically on the following files:

dangerzone/conversion/common.py
dangerzone/conversion/doc_to_pixels.py
dangerzone/conversion/pixels_to_pdf.py
qubes/dz.Convert
qubes/dz.ConvertDev

While rebuilding the RPMs, we see the following warnings in the logs:

*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone-0.6.0.dist-info/LICENSE is executable but has no shebang, removing executable bit
*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone-0.6.0.dist-info/WHEEL is executable but has no shebang, removing executable bit
*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone-0.6.0.dist-info/entry_points.txt is executable but has no shebang, removing executable bit
*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone-0.6.0.dist-info/INSTALLER is executable but has no shebang, removing executable bit
*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone-0.6.0.dist-info/METADATA is executable but has no shebang, removing executable bit
*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone/util.py is executable but has no shebang, removing executable bit
*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone/__init__.py is executable but has no shebang, removing executable bit
*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone/logic.py is executable but has no shebang, removing executable bit
*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone/container-pip-requirements.txt is executable but has no shebang, removing executable bit
*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone/cli.py is executable but has no shebang, removing executable bit
*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone/document.py is executable but has no shebang, removing executable bit
*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone/gui/main_window.py is executable but has no shebang, removing executable bit
*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone/gui/__init__.py is executable but has no shebang, removing executable bit
*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone/gui/updater.py is executable but has no shebang, removing executable bit
*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone/gui/logic.py is executable but has no shebang, removing executable bit
mangling shebang in /usr/lib/python3.12/site-packages/dangerzone/conversion/doc_to_pixels.py from /usr/bin/env python3 to #!/usr/bin/python3
mangling shebang in /usr/lib/python3.12/site-packages/dangerzone/conversion/common.py from /usr/bin/env python3 to #!/usr/bin/python3
mangling shebang in /usr/lib/python3.12/site-packages/dangerzone/conversion/pixels_to_pdf.py from /usr/bin/env python3 to #!/usr/bin/python3
*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone/conversion/errors.py is executable but has no shebang, removing executable bit
*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone/settings.py is executable but has no shebang, removing executable bit
*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone/args.py is executable but has no shebang, removing executable bit
*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone/errors.py is executable but has no shebang, removing executable bit
*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone/isolation_provider/qubes.py is executable but has no shebang, removing executable bit
*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone/isolation_provider/container.py is executable but has no shebang, removing executable bit
*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone/isolation_provider/dummy.py is executable but has no shebang, removing executable bit
*** WARNING: ./usr/lib/python3.12/site-packages/dangerzone/isolation_provider/base.py is executable but has no shebang, removing executable bit
*** WARNING: ./usr/share/applications/press.freedom.dangerzone.desktop is executable but has no shebang, removing executable bit
*** WARNING: ./usr/share/dangerzone/image-id.txt is executable but has no shebang, removing executable bit
*** WARNING: ./usr/share/dangerzone/dangerzone.css is executable but has no shebang, removing executable bit
*** WARNING: ./usr/share/dangerzone/version.txt is executable but has no shebang, removing executable bit
mangling shebang in /etc/qubes-rpc/dz.ConvertDev from /usr/bin/env python3 to #!/usr/bin/python3
mangling shebang in /etc/qubes-rpc/dz.Convert from /bin/sh to #!/usr/bin/sh

We can see here that the files with the wrong permissions are the ones whose shebang has been mangled. The Fedora docs further explain this operation: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_shebang_lines

What's interesting though is that this warning is not present in the CI job that builds RPMs, nor is it reproduced locally: https://github.com/freedomofpress/dangerzone/actions/runs/8041707585/job/21961324246#step:5:207.

@deeplow
Copy link
Contributor

deeplow commented Feb 26, 2024

Interesting and great that we caught this prior to shipping.

@apyrgio
Copy link
Contributor Author

apyrgio commented Feb 26, 2024

One factor that may contribute to this is the difference in the base images. The Fedora image differs if you build the development environment through Podman instead of Docker. In Podman, the Fedora image is grabbed by registry.fedoraproject.org/fedora, whereas in Docker, it's fetched by docker.io. In this particular case, the offending RPMs are built in Docker, whereas our CI and my local environment uses Podman.

Turns out that these two base images do differ from each other, albeit in small ways. Still, using the (seemingly working) Fedora image from registry.fedoraproject.org/fedora does not fix this bug. It does fix another issue (we stumbled again on #675 in our Docker environment for no reason), but the resulting RPM is still corrupted.

@deeplow deeplow self-assigned this Mar 4, 2024
deeplow added a commit that referenced this issue Mar 4, 2024
deeplow added a commit that referenced this issue Mar 4, 2024
@deeplow
Copy link
Contributor

deeplow commented Mar 5, 2024

I have some updates here.

(1) It affects also Fedora 38 (and the Qubes versions)

(2) Shebang WARNING is unrelated

I think the shebang warning line is unrelated. After adding those, the permissions remain wrong. After all it only mentions affecting the executable bit -- not the read permissions. However it does show that there are some differences in the Docker-packaged Fedora version and the one provided by Podman...

(3) More files are affected

It seems that the produced RPMs have wrong permissions (current: rw------- / 600, expected: rw-r--r-- / 644), specifically on the following files:

dangerzone/conversion/common.py
dangerzone/conversion/doc_to_pixels.py
dangerzone/conversion/pixels_to_pdf.py
qubes/dz.Convert
qubes/dz.ConvertDev

Actually, I think we have more affected files:

  • (current: rw------- / 600, expected: rw-r--r-- / 644) - all .py files within /usr/lib/python3.{11,12}/dangerzone/
  • (current: rw------- / 600, expected: rwxr-xr-x / 755) /etc/qubes-rpc/dz/Convert{,Dev}

@deeplow deeplow changed the title Corrupted RPMs due to shebang mangling Wrong permissions on built Fedora artifacts. Mar 5, 2024
@deeplow
Copy link
Contributor

deeplow commented Mar 5, 2024

Turns out that these two base images do differ from each other, albeit in small ways. Still, using the (seemingly working) Fedora image from registry.fedoraproject.org/fedora does not fix this bug. It does fix another issue (we stumbled again on #675 in our Docker environment for no reason), but the resulting RPM is still corrupted.

I tried now building on my linux system with podman, but using the docker.io registry instead (by editing the fedora listing in the shortnames /etc/containers/registries.conf.d/000-shortnames.conf) and the final version worked well. So somehow this is a difference with how Docker and Podman works...

@legoktm
Copy link
Member

legoktm commented Mar 5, 2024

Turns out that these two base images do differ from each other, albeit in small ways. Still, using the (seemingly working) Fedora image from registry.fedoraproject.org/fedora does not fix this bug.

We ran into this as well (freedomofpress/securedrop-workstation#912) but at the time I couldn't find any practical differences in the two images.

@apyrgio
Copy link
Contributor Author

apyrgio commented Mar 5, 2024

Well, turns out that FUSE is the culprit, when it's used to mount directories to the containers. If we move the rpm-build/ directory outside the mounted volume, then the permissions are correct.

apyrgio added a commit to apyrgio/yum-tools-prod that referenced this issue May 28, 2024
Bump the Dangerzone release to 0.6.1-2, to fix an issue with the
permissions in our RPM packages.

Refs freedomofpress/dangerzone#727
@apyrgio
Copy link
Contributor Author

apyrgio commented May 28, 2024

This issue has bit us again. In order to fix it, I propose doing the following:

  1. Detect in our RPM .spec file if the created package contains any files with wrong permissions (0600 / -rw-------). If that's the case, throw an error and give some context to the user.
  2. Fix any incorrect shebangs that need to be mangled in any case.
  3. Make package building take place in a directory that's not mounted by FUSE. Since the dangerzone/ directory is mounted in the container, it has to happen somewhere else. The canonical place for RPM building is ~/rpmbuild, so we plan to go with that.

apyrgio added a commit that referenced this issue May 28, 2024
When building the Dangerzone RPM package, detect if the files bundled in
it have any incorrect permissions. We have seen in the past that
building RPMs from the Dangerzone source, mounted to a macOS Docker
container, can lead to files readable only by the root user (600 /
rw-------).

Refs #727
apyrgio added a commit that referenced this issue May 28, 2024
Switch build directory for the `rpmbuild` command from
`./install/linux/rpm-build` to `~/rpmbuild`. The main reason for this is
that we want a build directory that will not be mounted in the
container, since we've experienced issues with file permissions.

Regarding the choice of directories, we went with `~/rpmbuild` because
it's outside the Dangerzone source, and also because it's the default
choice in Fedora [1].

[1]: https://github.com/rsrchboy/rpmdevtools/blob/3ae1eeafeeda19e7bcb4546300fe551fd879e0ca/rpmdev-setuptree#L60

Closes #727
apyrgio added a commit that referenced this issue May 28, 2024
When building the Dangerzone RPMs, we were seeing the following shebang
warnings:

    + /usr/lib/rpm/redhat/brp-mangle-shebangs
    mangling shebang in /usr/lib/python3.12/site-packages/dangerzone/conversion/doc_to_pixels.py from /usr/bin/env python3 to #!/usr/bin/python3
    mangling shebang in /usr/lib/python3.12/site-packages/dangerzone/conversion/common.py from /usr/bin/env python3 to #!/usr/bin/python3
    mangling shebang in /usr/lib/python3.12/site-packages/dangerzone/conversion/pixels_to_pdf.py from /usr/bin/env python3 to #!/usr/bin/python3
    mangling shebang in /etc/qubes-rpc/dz.ConvertDev from /usr/bin/env python3 to #!/usr/bin/python3
    mangling shebang in /etc/qubes-rpc/dz.Convert from /bin/sh to #!/usr/bin/sh

These warnings are benign in nature, but coupled with #727, they could
lead to incorrect file permissions.

Remove shebangs from the following files, since they are not executed
directly, but are imported instead:

    dangerzone/conversion/common.py
    dangerzone/conversion/doc_to_pixels.py
    dangerzone/conversion/pixels_to_pdf.py

Also, accept the suggestions by Fedora (/bin/sh -> /usr/bin/sh,
/usr/bin/env python3 -> /usr/bin/python3) for the following files:

    qubes/dz.Convert
    qubes/dz.ConvertDev

Refs #727
apyrgio added a commit that referenced this issue May 28, 2024
When building the Dangerzone RPMs, we were seeing the following shebang
warnings:

    + /usr/lib/rpm/redhat/brp-mangle-shebangs
    mangling shebang in /usr/lib/python3.12/site-packages/dangerzone/conversion/doc_to_pixels.py from /usr/bin/env python3 to #!/usr/bin/python3
    mangling shebang in /usr/lib/python3.12/site-packages/dangerzone/conversion/common.py from /usr/bin/env python3 to #!/usr/bin/python3
    mangling shebang in /usr/lib/python3.12/site-packages/dangerzone/conversion/pixels_to_pdf.py from /usr/bin/env python3 to #!/usr/bin/python3
    mangling shebang in /etc/qubes-rpc/dz.ConvertDev from /usr/bin/env python3 to #!/usr/bin/python3
    mangling shebang in /etc/qubes-rpc/dz.Convert from /bin/sh to #!/usr/bin/sh

These warnings are benign in nature, but coupled with #727, they could
lead to incorrect file permissions.

Remove shebangs from the following files, since they are not executed
directly, but are imported instead:

    dangerzone/conversion/common.py
    dangerzone/conversion/doc_to_pixels.py
    dangerzone/conversion/pixels_to_pdf.py

Also, accept the suggestions by Fedora (/bin/sh -> /usr/bin/sh,
/usr/bin/env python3 -> /usr/bin/python3) for the following files:

    qubes/dz.Convert
    qubes/dz.ConvertDev

Refs #727
@apyrgio apyrgio changed the title Wrong permissions on built Fedora artifacts. Wrong permissions on built Fedora artifacts Jun 6, 2024
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 a pull request may close this issue.

3 participants