Add protection to disable Gitea when run as root #17168

techknowlogick · 2021-09-28T03:44:52Z

techknowlogick commented

2021-09-28 03:44:52 +00:00

This PR shouldn't be backported as it could potentially be breaking for users who are running Gitea as root.

wxiaoguang approved these changes 2021-09-28 03:50:12 +00:00

lafriks (Migrated from github.com) approved these changes 2021-09-28 09:31:12 +00:00

6543 (Migrated from github.com) approved these changes 2021-09-28 12:34:03 +00:00

6543 (Migrated from github.com) left a comment

🎉

:tada:

delvh reviewed 2021-09-28 13:58:05 +00:00

modules/setting/setting.go

delvh commented

2021-09-28 13:57:25 +00:00

	// The following is a purposefully undocumented option. Please do not run Gitea as root. It will only cause future headaches.

```suggestion // The following is a purposefully undocumented option. Please do not run Gitea as root. It will only cause future headaches. ```

modules/setting/setting.go

						
				@ -905,2 +906,4 @@

					// Please don't use root as a bandaid to "fix" something that is broken, instead the broken thing should instead be fixed properly.

					unsafeAllowRunAsRoot := Cfg.Section("").Key("I_AM_BEING_UNSAFE_RUNNING_AS_ROOT").MustBool(false)

					RunMode = Cfg.Section("").Key("RUN_MODE").MustString("prod")

					// Does not check run user when the install lock is off.

delvh commented

2021-09-28 13:49:31 +00:00

Can we guarantee for every supported OS that the root account will be called "root"?

modules/setting/setting.go

						
				@ -914,1 +923,4 @@

						log.Critical("You are running Gitea using the root user, and have purposely chosen to skip built-in protections around this. You have been warned against this.")

					}

					SSH.BuiltinServerUser = Cfg.Section("server").Key("BUILTIN_SSH_SERVER_USER").MustString(RunUser)

delvh commented

2021-09-28 13:50:27 +00:00

What has the media player to do with the wording of this message? 😱

What has the media player to do with the wording of this message? :scream:

modules/setting/setting.go

						
				@ -914,2 +924,4 @@

					}

					SSH.BuiltinServerUser = Cfg.Section("server").Key("BUILTIN_SSH_SERVER_USER").MustString(RunUser)

delvh commented

2021-09-28 13:55:48 +00:00

When we enter this if, I think we can shutdown Gitea immediately here as otherwise Gitea could still be run as root.

			log.Fatal("Gitea is not supposed to be run as root. Sorry. If you need to use privileged TCP ports please instead use setcap and the `cap_net_bind_service` permission")
			os.Exit(1)

When we enter this if, I think we can shutdown Gitea immediately here as otherwise Gitea could still be run as root. ```suggestion log.Fatal("Gitea is not supposed to be run as root. Sorry. If you need to use privileged TCP ports please instead use setcap and the `cap_net_bind_service` permission") os.Exit(1) ```

delvh reviewed 2021-09-28 13:59:37 +00:00

modules/setting/setting.go

delvh commented

2021-09-28 13:59:37 +00:00

	// Please don't use root as a bandaid to "fix" something that is broken, instead the broken thing should instead be fixed properly.

```suggestion // Please don't use root as a bandaid to "fix" something that is broken, instead the broken thing should instead be fixed properly. ```

lafriks (Migrated from github.com) reviewed 2021-09-28 16:22:27 +00:00

modules/setting/setting.go

						
				@ -914,2 +924,4 @@

					}

					SSH.BuiltinServerUser = Cfg.Section("server").Key("BUILTIN_SSH_SERVER_USER").MustString(RunUser)

lafriks (Migrated from github.com) commented

2021-09-28 16:22:27 +00:00

log.Fatal does call os.Exit by design

techknowlogick reviewed 2021-09-28 16:22:31 +00:00

modules/setting/setting.go

						
				@ -914,1 +923,4 @@

						log.Critical("You are running Gitea using the root user, and have purposely chosen to skip built-in protections around this. You have been warned against this.")

					}

					SSH.BuiltinServerUser = Cfg.Section("server").Key("BUILTIN_SSH_SERVER_USER").MustString(RunUser)

techknowlogick commented

2021-09-28 16:22:31 +00:00

They also disable running of root, and the messaging I used is similar to theirs.

techknowlogick reviewed 2021-09-28 16:23:50 +00:00

modules/setting/setting.go

						
				@ -914,2 +924,4 @@

					}

					SSH.BuiltinServerUser = Cfg.Section("server").Key("BUILTIN_SSH_SERVER_USER").MustString(RunUser)

techknowlogick commented

2021-09-28 16:23:50 +00:00

I thought Fatal will exit Gitea with status 1 already?

Edit: The page didn't update w/ lafrik's post before I submitted mine ?

I thought `Fatal` will exit Gitea with status 1 already? Edit: The page didn't update w/ lafrik's post before I submitted mine ?

techknowlogick reviewed 2021-09-28 16:43:02 +00:00

modules/setting/setting.go

						
				@ -905,2 +906,4 @@

					// Please don't use root as a bandaid to "fix" something that is broken, instead the broken thing should instead be fixed properly.

					unsafeAllowRunAsRoot := Cfg.Section("").Key("I_AM_BEING_UNSAFE_RUNNING_AS_ROOT").MustBool(false)

					RunMode = Cfg.Section("").Key("RUN_MODE").MustString("prod")

					// Does not check run user when the install lock is off.

techknowlogick commented

2021-09-28 16:43:02 +00:00

We can't entirely (Windows is the case where it is certain to not be "root"), however windows is out of scope of this PR. This purpose of this PR wasn't to be exhaustive of all the possibilities, just to prevent me from being lazy and running things as root where I could mess up file permissions ?

delvh reviewed 2021-09-28 19:29:35 +00:00

modules/setting/setting.go

						
				@ -914,2 +924,4 @@

					}

					SSH.BuiltinServerUser = Cfg.Section("server").Key("BUILTIN_SSH_SERVER_USER").MustString(RunUser)

delvh commented

2021-09-28 19:29:35 +00:00

Okay, I didn't know that. I did not expect a method called log.Fatal to automatically shutdown the program.
But I can see why it does that.

Okay, I didn't know that. I did not expect a method called `log.Fatal` to automatically shutdown the program. But I can see why it does that.

zeripath reviewed 2021-09-29 05:25:13 +00:00

modules/setting/setting.go

						
				@ -905,2 +906,4 @@

					// Please don't use root as a bandaid to "fix" something that is broken, instead the broken thing should instead be fixed properly.

					unsafeAllowRunAsRoot := Cfg.Section("").Key("I_AM_BEING_UNSAFE_RUNNING_AS_ROOT").MustBool(false)

					RunMode = Cfg.Section("").Key("RUN_MODE").MustString("prod")

					// Does not check run user when the install lock is off.

zeripath commented

2021-09-29 05:25:13 +00:00

Part of me wonders if this should be log.Critical

zeripath reviewed 2021-09-29 05:31:19 +00:00

modules/setting/setting.go

						
				@ -905,2 +906,4 @@

					// Please don't use root as a bandaid to "fix" something that is broken, instead the broken thing should instead be fixed properly.

					unsafeAllowRunAsRoot := Cfg.Section("").Key("I_AM_BEING_UNSAFE_RUNNING_AS_ROOT").MustBool(false)

					RunMode = Cfg.Section("").Key("RUN_MODE").MustString("prod")

					// Does not check run user when the install lock is off.

zeripath commented

2021-09-29 05:31:19 +00:00

I'd stick a GOOS check here and exclude dozers from this check.
Then just use os.GetUID() == 0 instead of checking username.

I'd stick a GOOS check here and exclude dozers from this check. Then just use os.GetUID() == 0 instead of checking username.

6543 (Migrated from github.com) reviewed 2021-10-05 23:28:58 +00:00

modules/setting/setting.go

6543 (Migrated from github.com) commented

2021-10-05 23:28:57 +00:00

		log.Critical("You are running Gitea using the root user, and have purposely chosen to skip built-in protections around this. You have been warned against this.")

```suggestion log.Critical("You are running Gitea using the root user, and have purposely chosen to skip built-in protections around this. You have been warned against this.") ```

6543 (Migrated from github.com) reviewed 2021-10-05 23:41:19 +00:00

modules/setting/setting.go

						
				@ -905,2 +906,4 @@

					// Please don't use root as a bandaid to "fix" something that is broken, instead the broken thing should instead be fixed properly.

					unsafeAllowRunAsRoot := Cfg.Section("").Key("I_AM_BEING_UNSAFE_RUNNING_AS_ROOT").MustBool(false)

					RunMode = Cfg.Section("").Key("RUN_MODE").MustString("prod")

					// Does not check run user when the install lock is off.