git-daemon may unexpectedly run as root
Overriding gitdaemon_flags in /etc/r.conf.local will cause rc.d to
run git-daemon run as root instead of the expected _gitdaemon user.
To reproduce, try an rc.conf.local line such as:
gitdaemon_flags=--listen=127.0.0.1 /git
This happens because the rc script currently depends on git-daemon itself
to switch the user ID, rather than using rc.d's built-in mechanism which
forces a UID with su(1). If the user overrides the flags, no UID switch
will happen.
Anyone exposing git-daemon to the public internet on an OpenBSD system
should check their system. This line in rc.conf.local will force the
daemon to run under its dedicated user account:
gitdaemon_user=_gitdaemon
Note that git-daemon does not support any of the standard exploit
mitigation measures regular OpenBSD daemons provide; there is not
even support for chroot(8).
Fix for the port:
diff 64e903a627aaf6f20b8adcb3028f2aad79137a9e /usr/ports
blob - c5ddeb706f54602a8c1648ec1825eaf8cb1f99ba
file + devel/git/Makefile
--- devel/git/Makefile
+++ devel/git/Makefile
@@ -5,6 +5,7 @@ COMMENT-svn = GIT - subversion interoperability tools
COMMENT-x11 = GIT - graphical tools
V =2.22.0
+REVISION = 0
DISTNAME = git-${V}
PKGNAME-main = ${DISTNAME}
PKGNAME-svn = git-svn-${V}
blob - daf33d41548331295cd05bcb53f075781bce9b90
file + devel/git/pkg/gitdaemon.rc
--- devel/git/pkg/gitdaemon.rc
+++ devel/git/pkg/gitdaemon.rc
@@ -3,7 +3,7 @@
# $OpenBSD: gitdaemon.rc,v 1.3 2018/01/11 19:27:02 rpe Exp $
daemon="${TRUEPREFIX}/bin/git daemon --detach"
-daemon_flags="--user=_gitdaemon"
+daemon_user="_gitdaemon"
. /etc/rc.d/rc.subr
Re: git-daemon may unexpectedly run as root
On Sun, Jul 14, 2019 at 03:46:06PM +0200, Stefan Sperling wrote: > Overriding gitdaemon_flags in /etc/r.conf.local will cause rc.d to > run git-daemon run as root instead of the expected _gitdaemon user. Well, you ought to check the existing daemon flags before changing them. Admittedly, this is suboptimal and users have to carry the options along themselves, but some third-party daemons will not work with `daemon_user`, and I believe git-daemon(1) is one of them. > > To reproduce, try an rc.conf.local line such as: >gitdaemon_flags=--listen=127.0.0.1 /git > > This happens because the rc script currently depends on git-daemon itself > to switch the user ID, rather than using rc.d's built-in mechanism which > forces a UID with su(1). If the user overrides the flags, no UID switch > will happen. Off the top of my head, I know that this is the case for security/hitch and net/tinc as well. I remember discussion about putting `-u $user' into `$daemon' itself to enforce the user for cases where our rc.subr(8) framework is not applicable, but there were objections. > Anyone exposing git-daemon to the public internet on an OpenBSD system > should check their system. This line in rc.conf.local will force the > daemon to run under its dedicated user account: >gitdaemon_user=_gitdaemon This will also (partially?) break git-daemon(1)'s `--inetd' option, I think. From the manual: --user=, --group= Change daemon’s uid and gid before entering the service loop. When only --user is given without --group, the primary group ID for the user is used. The values of the option are given to getpwnam(3) and getgrnam(3) and numeric IDs are not supported. Giving these options is an error when used with --inetd; use the facility of inet daemon to achieve the same before spawning git daemon if needed. Like many programs that switch user id, the daemon does not reset environment variables such as $HOME when it runs git programs, e.g. So switching to "_gitdaemon" at runtime and starting as it through su(1) is not the same. > Note that git-daemon does not support any of the standard exploit > mitigation measures regular OpenBSD daemons provide; there is not > even support for chroot(8). > > Fix for the port: This seems like the right direction and I support setups with less potential to accidentially run things as root, but existing setups might break.
Re: git-daemon may unexpectedly run as root
On 2019/07/14 17:18, Klemens Nanni wrote:
> On Sun, Jul 14, 2019 at 03:46:06PM +0200, Stefan Sperling wrote:
> > Overriding gitdaemon_flags in /etc/r.conf.local will cause rc.d to
> > run git-daemon run as root instead of the expected _gitdaemon user.
> Well, you ought to check the existing daemon flags before changing them.
>
> Admittedly, this is suboptimal and users have to carry the options along
> themselves, but some third-party daemons will not work with `daemon_user`,
> and I believe git-daemon(1) is one of them.
>
> >
> > To reproduce, try an rc.conf.local line such as:
> >gitdaemon_flags=--listen=127.0.0.1 /git
> >
> > This happens because the rc script currently depends on git-daemon itself
> > to switch the user ID, rather than using rc.d's built-in mechanism which
> > forces a UID with su(1). If the user overrides the flags, no UID switch
> > will happen.
> Off the top of my head, I know that this is the case for security/hitch
> and net/tinc as well. I remember discussion about putting `-u $user'
> into `$daemon' itself to enforce the user for cases where our rc.subr(8)
> framework is not applicable, but there were objections.
>
> > Anyone exposing git-daemon to the public internet on an OpenBSD system
> > should check their system. This line in rc.conf.local will force the
> > daemon to run under its dedicated user account:
> >gitdaemon_user=_gitdaemon
> This will also (partially?) break git-daemon(1)'s `--inetd' option,
> I think. From the manual:
You wouldn't be using --inetd in a rcctl-started daemon.
> --user=, --group=
> Change daemon’s uid and gid before entering the service loop. When
> only --user is given without --group, the primary group ID for the
> user is used. The values of the option are given to getpwnam(3) and
> getgrnam(3) and numeric IDs are not supported.
>
> Giving these options is an error when used with --inetd; use the
> facility of inet daemon to achieve the same before spawning git
> daemon if needed.
>
> Like many programs that switch user id, the daemon does not reset
> environment variables such as $HOME when it runs git programs, e.g.
>
> So switching to "_gitdaemon" at runtime and starting as it through su(1)
> is not the same.
>
> > Note that git-daemon does not support any of the standard exploit
> > mitigation measures regular OpenBSD daemons provide; there is not
> > even support for chroot(8).
> >
> > Fix for the port:
> This seems like the right direction and I support setups with less
> potential to accidentially run things as root, but existing setups might
> break.
>
I think this is likely to work for git-daemon but isn't always going
to work in the general case (sometimes a daemon needs to bind to port <1024,
for example). So maybe it's better if we come up with a more general thing
that can be copied and pasted (after all rc.d/gitdaemon is going to be
present on many users' and devs' machines so it's not a bad place for
a reusable example).
Hardcoding a username in $daemon won't always work, somebody might need
to run as a different uid. Another approach might be a custom rc_start
that starts the daemon as root and adds --user=${daemon_user} etc to the
command line.
Re: git-daemon may unexpectedly run as root
On Sun, Jul 14, 2019 at 05:18:39PM +0200, Klemens Nanni wrote: > On Sun, Jul 14, 2019 at 03:46:06PM +0200, Stefan Sperling wrote: > > Overriding gitdaemon_flags in /etc/r.conf.local will cause rc.d to > > run git-daemon run as root instead of the expected _gitdaemon user. > Well, you ought to check the existing daemon flags before changing them. > > Admittedly, this is suboptimal and users have to carry the options along > themselves, but some third-party daemons will not work with `daemon_user`, > and I believe git-daemon(1) is one of them. > > > > > To reproduce, try an rc.conf.local line such as: > >gitdaemon_flags=--listen=127.0.0.1 /git > > > > This happens because the rc script currently depends on git-daemon itself > > to switch the user ID, rather than using rc.d's built-in mechanism which > > forces a UID with su(1). If the user overrides the flags, no UID switch > > will happen. > Off the top of my head, I know that this is the case for security/hitch > and net/tinc as well. I remember discussion about putting `-u $user' > into `$daemon' itself to enforce the user for cases where our rc.subr(8) > framework is not applicable, but there were objections. My position is that while we cannot prevent people from changing their configuration in a risky fashion, the default should be as secure as we can reasonably make it. I've gotten used to not having daemons, even from ports land, start up as root. I don't even think about it anymore, I just take this for granted. I suspect other users have similar expectations. Setting something like a --listen option should not affect this expectation. (Unless the service needs a privileged port, of course, which this one does not.) > > Anyone exposing git-daemon to the public internet on an OpenBSD system > > should check their system. This line in rc.conf.local will force the > > daemon to run under its dedicated user account: > >gitdaemon_user=_gitdaemon > This will also (partially?) break git-daemon(1)'s `--inetd' option, > I think. From the manual: Is there any reason to start git-daemon through rc.d(8) in --inetd mode? git-daemon isn't running persistently in the inetd case. It will be started by inetd on demand, based on configuration parameters in /etc/inetd.conf. I don't see how the rc.d script would be involved here in the first place. > --user=, --group= > Change daemon’s uid and gid before entering the service loop. When > only --user is given without --group, the primary group ID for the > user is used. The values of the option are given to getpwnam(3) and > getgrnam(3) and numeric IDs are not supported. > > Giving these options is an error when used with --inetd; use the > facility of inet daemon to achieve the same before spawning git > daemon if needed. > > Like many programs that switch user id, the daemon does not reset > environment variables such as $HOME when it runs git programs, e.g. > > So switching to "_gitdaemon" at runtime and starting as it through su(1) > is not the same. I don't understand which difference you mean. Could you please try to explain your point better? Are you referring to the environment being different when started as a non-root user? I would trust rc.d(8) to do the right thing with the environment. > > Note that git-daemon does not support any of the standard exploit > > mitigation measures regular OpenBSD daemons provide; there is not > > even support for chroot(8). > > > > Fix for the port: > This seems like the right direction and I support setups with less > potential to accidentially run things as root, but existing setups might > break. Break in what way? Please consider that any setup that runs this thing as root is a hazard. Can you name any potential consequences which the proposed change might introduce which are actually worse than running git-daemon as root? Even if there were people how wanted to run git-daemon as root on purpose I wouldn't see any reason for us to support them in doing so.
Re: git-daemon may unexpectedly run as root
On Sun, Jul 14, 2019 at 10:41:33PM +0100, Stuart Henderson wrote:
> I think this is likely to work for git-daemon but isn't always going
> to work in the general case (sometimes a daemon needs to bind to port <1024,
> for example). So maybe it's better if we come up with a more general thing
> that can be copied and pasted (after all rc.d/gitdaemon is going to be
> present on many users' and devs' machines so it's not a bad place for
> a reusable example).
I see what you mean, but I think it makes sense to have porters deal
with this on a case-by-case basis. Every upstream software has different
requirements and edge cases. In some cases we might decide we want to
limit possibilities provided by upstream (e.g. upstream might be fine
with using a privileged port by default, while we aren't), in some
cases we might trust upstream code to do things right.
Encouraging copy-pasting in this area might result in porters not
considering all their options carefully.
> Hardcoding a username in $daemon won't always work, somebody might need
> to run as a different uid.
My patch still alllows for that. It doesn't tweak $daemon. Rather, it
sets a reasonable default value for $gitdaemon_user which can still be
overridden if desired.
> Another approach might be a custom rc_start
> that starts the daemon as root and adds --user=${daemon_user} etc to the
> command line.
I don't see why we should trust this --user option and give upstream code
a window where it runs as root until it decides to switch UID, if rc.d(8)
can make the entire program start out with an unprivileged UID from the
beginning.
Re: git-daemon may unexpectedly run as root
On Mon, Jul 15, 2019 at 12:01:59AM +0200, Stefan Sperling wrote: > Is there any reason to start git-daemon through rc.d(8) in --inetd mode? No; I was stupidly short-sighted when looking through git-daemon(1) with regard to its options and `--user' in particular. > I don't understand which difference you mean. > Could you please try to explain your point better? The daemon itself won't reset the environment, so starting it as root with `-u _gitdaemon' will preserve stuff like HOME whereas setting `$daemon_user' will cause it to be started with a properly set environment. > Are you referring to the environment being different when started > as a non-root user? I would trust rc.d(8) to do the right thing > with the environment. Yes; having said the above, I do agree here. Just wanted to point out that these two approaches are not the same. > Break in what way? Thought of the inetd thing, please disregard. > Please consider that any setup that runs this thing as root is a hazard. > Can you name any potential consequences which the proposed change might > introduce which are actually worse than running git-daemon as root? Running the daemon on privileged ports as only root may do so, which would work with `--user _gitdaemon' but not `daemon_user=_gitdaemon', but that not a bright idea anyway. Also, git-daemon(1) has `--group' as well: dropping to a group other than the user's primary one will only work then started as root. However, I doubt that people are running such setups. > Even if there were people how wanted to run git-daemon as root on purpose > I wouldn't see any reason for us to support them in doing so. Providing a safe default is good and since my initial doubts were based on my own stupidity alone (--inetd in rc.d does not make sense), OK kn.
Re: git-daemon may unexpectedly run as root
On Mon, Jul 15, 2019 at 12:31:21AM +0200, Stefan Sperling wrote: > On Sun, Jul 14, 2019 at 10:41:33PM +0100, Stuart Henderson wrote: > > I think this is likely to work for git-daemon but isn't always going > > to work in the general case (sometimes a daemon needs to bind to port <1024, > > for example). Yes, that was my point. > > So maybe it's better if we come up with a more general thing > > that can be copied and pasted (after all rc.d/gitdaemon is going to be > > present on many users' and devs' machines so it's not a bad place for > > a reusable example). This could work, but I have not idea of how it would look like, yet. > I see what you mean, but I think it makes sense to have porters deal > with this on a case-by-case basis. Every upstream software has different > requirements and edge cases. In some cases we might decide we want to > limit possibilities provided by upstream (e.g. upstream might be fine > with using a privileged port by default, while we aren't), in some > cases we might trust upstream code to do things right. I concur. > I don't see why we should trust this --user option and give upstream code > a window where it runs as root until it decides to switch UID, if rc.d(8) > can make the entire program start out with an unprivileged UID from the > beginning. As already said, this is not already possible, but it still is a good reason to prefer our framework (as the majority of user setting rc.d scripts in ports already does).
Re: git-daemon may unexpectedly run as root
> > I don't understand which difference you mean. > > Could you please try to explain your point better? > The daemon itself won't reset the environment, so starting it as root > with `-u _gitdaemon' will preserve stuff like HOME whereas setting > `$daemon_user' will cause it to be started with a properly set > environment. How so? What makes it behave differently when both options are started from rc.d? > > Are you referring to the environment being different when started > > as a non-root user? I would trust rc.d(8) to do the right thing > > with the environment. > Yes; having said the above, I do agree here. Just wanted to point out > that these two approaches are not the same. > > > Break in what way? > Thought of the inetd thing, please disregard. > > > Please consider that any setup that runs this thing as root is a hazard. > > Can you name any potential consequences which the proposed change might > > introduce which are actually worse than running git-daemon as root? > Running the daemon on privileged ports as only root may do so, which > would work with `--user _gitdaemon' but not `daemon_user=_gitdaemon', > but that not a bright idea anyway. > > Also, git-daemon(1) has `--group' as well: dropping to a group other > than the user's primary one will only work then started as root. > However, I doubt that people are running such setups. > > > Even if there were people how wanted to run git-daemon as root on purpose > > I wouldn't see any reason for us to support them in doing so. > Providing a safe default is good and since my initial doubts were based > on my own stupidity alone (--inetd in rc.d does not make sense), > OK kn. > -- Antoine
Re: git-daemon may unexpectedly run as root
On Mon, Jul 15, 2019 at 08:05:56AM +0200, Antoine Jacoutot wrote: > How so? What makes it behave differently when both options are started > from rc.d? Both are started from rc.d through `su -l ...', both get their HOME, USER, PATH, etc. set - that means different `daemon_user' values will cause different values in those environment variables. `daemon_user=root daemon_flags=-u _gitdaemon' will therefore start as root, drop privileges, but run with HOME=/root as it is not reset. `daemon_user=_gitdaemon' will therefore start as _gitdaemon, not drop privileges and keep its proper environment, where everything already matches the running user.
Re: git-daemon may unexpectedly run as root
On Mon, Jul 15, 2019 at 11:15:55AM +0200, Klemens Nanni wrote: > On Mon, Jul 15, 2019 at 08:05:56AM +0200, Antoine Jacoutot wrote: > > How so? What makes it behave differently when both options are started > > from rc.d? > Both are started from rc.d through `su -l ...', both get their HOME, > USER, PATH, etc. set - that means different `daemon_user' values will > cause different values in those environment variables. > > `daemon_user=root daemon_flags=-u _gitdaemon' will therefore start as > root, drop privileges, but run with HOME=/root as it is not reset. > > `daemon_user=_gitdaemon' will therefore start as _gitdaemon, not drop > privileges and keep its proper environment, where everything already > matches the running user. Yes I know that, I wrote the framework :-) My question is how will it make it behave different? -- Antoine
