allow binding gitea to privileged port, gated behind environment variable

[clinta]

This is in reference to some issues documented in #1190 and hopefully a more acceptable method of accomplishing what #2183 was trying to do.

This allows the gitea service to run on a privileged port if the container is run with the environment variable ALLOW_PRIVILEGED_PORT is set to 1.

This means the security of the image is unchanged for users who do not run the container with this environment variable, but it gives those who choose to the ability to run gitea on a privileged port.

This is important in our environment because we run docker with a custom network plugin and with docker's iptables integration disabled.

[techknowlogick]

As the project aims to eventually have the docker run rootless and so I am hesitant to approve this PR as it’d soon be have to be removed.

A possibility for you is building a docker imagine using the official one as a base then injecting these changes.

However I am just one approver and others may feel differently so I’ll leave this PR as open.

With that being said we do appreciate your contribution to the project

[zeripath]

Wouldn't these changes be required to make Gitea run rootless under docker in any case?

I've restarted the build - it looks like it failed for an unrelated reason.

[techknowlogick]

@zeripath no because currently the binary is not run as root (hence the changes being made in this PR), these changes are only required to bind to a privileged port. Thanks to the -p option in docker you can use docker to re-map ports (which is the way we recommended) but sadly it won't work for this specific user.

[clinta]

Is the goal to have no daemons run as root in the container, or is the goal to have nothing run as root in the container, even for setup purposes.

I think the latter is going to make a pretty bad experience for the user, because without a setup script to make sure the git user owns the data directory, the user will have to manually do that before starting the container. This is even more difficult if a user turns on UID namespaces and can't know what the UID of the git user will be in the container.

If you want to avoid forcing the user to manually set permissions on the gitea volume, I think there will always need to be some setup commands running as root in the entrypoint. If those commands move out of the s6 run command and into /usr/bin/entrypoint to facilitate running s6 as an unprivileged user, this setcap line can move with it.

[zeripath]

Hmm... I think this is a relatively minor change. I'm not sure what they status of the rootless pr is, it looks a bit dead at present.

In any case this PR needs some documentation. @clinta could you add changes to the documentation too... It might be that instead of adding the option to the docker we should just add documentation on how to set capabilities.

[clinta]

@zeripath, I added the documentation. I don't think a documentation only fix is reasonable, because s6 changes permissions on the gitea binary, the setcap needs to be done after this step, but before the gitea binary is started.

My ugly workaround right now is overriding the docker command with /bin/bash -c "apk --no-cache add libcap && echo setcap 'cap_net_bind_service=+ep' /app/gitea/gitea >> /etc/s6/gitea/setup ; exec /usr/bin/entrypoint", but I don't think that's something you'd want documented as a recommended way to run gitea on a low port.

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@8d3bb86). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #6081   +/-   ##
=========================================
  Coverage          ?   38.86%           
=========================================
  Files             ?      345           
  Lines             ?    49508           
  Branches          ?        0           
=========================================
  Hits              ?    19241           
  Misses            ?    27485           
  Partials          ?     2782

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d3bb86...eb57c94. Read the comment docs.

[zeripath]

Hmm is there anyway not to have setcap unless you actually need it?

[clinta]

Hmm is there anyway not to have setcap unless you actually need it?

That's what the environment variable is for. If you do not run the container with ALLOW_PRIVILEGED_PORT=1 setcap is never run.

[zeripath]

I was meaning not to download it in the first place. We shouldn't have setcap downloaded or installed in the docker if we don't need it.

[clinta]

Downloading packages at runtime instead of build time is an unexpected behavior for docker. The normal expectation is that a specific docker image contains all the dependencies at a specific version. Having a package downloaded at runtime means you could run a docker image one day, then restart the same image and get a different version of a piece of software. It breaks the concept of an image being an immutable snapshot of all dependent software.

[techknowlogick]

Rootless containers (as in you don't need to have root permissions to run the container), are starting to be more commonplace with platforms such as Openshift and k8s. The rootless image PR failed due to our SSH implementation not being as robust as openssh, however there is a PR that improves our SSH implementation. That will also allow us to remove s6.

[clinta]

Is there a solution for managing the permissions on the data volume without root? The setup that is currently done here:

gitea/docker/etc/s6/gitea/setup

Lines 42 to 46 in eb57c94

	# only chown if current owner is not already the gitea ${USER}. No recursive check to save time
	if ! [[ $(ls -ld /data/gitea | awk '{print $3}') = ${USER} ]]; then chown -R ${USER}:git /data/gitea; fi
	if ! [[ $(ls -ld /app/gitea | awk '{print $3}') = ${USER} ]]; then chown -R ${USER}:git /app/gitea; fi
	if ! [[ $(ls -ld /data/git | awk '{print $3}') = ${USER} ]]; then chown -R ${USER}:git /data/git; fi
	chmod 0755 /data/gitea /app/gitea /data/git

[clinta]

Since #4749 has been closed as stale, is the rootless work still ongoing?

[techknowlogick]

Yes, as mentioned rootless is blocked by improvements needed for internal SSH server.

[clinta]

Is the permissions setup in gitea/docker/etc/s6/gitea/setup considered a blocker too?

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

[stale]

This pull request has been automatically closed because of inactivity. You can re-open it if needed.