Re: [systemd-devel] systemd's connections to /run/systemd/private ?

2019-08-14 Thread Dave Howorth
On Wed, 14 Aug 2019 12:04:03 -0400
Brian Reichert  wrote:
> On Wed, Aug 14, 2019 at 04:19:46PM +0100, Simon McVittie wrote:
> > On Wed, 14 Aug 2019 at 10:26:53 -0400, Brian Reichert wrote:
> > Doesn't daemonize(1) make stdin, stdout and stderr point
> > to /dev/null, instead of closing them?  
> 
> Looking at the source, yes, it does.
> 
> > Expecting arbitrary subprocesses to cope gracefully with being
> > invoked without the three standard fds seems likely to be a losing
> > battle. I've implemented this myself, in dbus; it isn't a whole lot
> > of code, but it also isn't something that I would expect the
> > authors of all CLI tools to get right.  
> 
> I concede that reopening FD 0,1,2 is a good practice to insulate
> against the issues you cite.
> 
> I agree with your points; I code aggressively, and sometimes forget
> others don't.

I didn't know what you meant by this. Do you mean 'Aggressive
Programming'? Is
https://www.apharmony.com/software-sagacity/2014/10/principles-of-aggressive-programming/
a reasonable summary?

> > smcv
> > 
> > [1] I'm sure there are lots of other executables named daemon or
> > daemonize in other OSs, and perhaps some of them get this wrong?
> 
> -- 
> Brian Reichert
> BSD admin/developer at large  
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] systemd's connections to /run/systemd/private ?

2019-08-14 Thread Brian Reichert
On Wed, Aug 14, 2019 at 04:19:46PM +0100, Simon McVittie wrote:
> On Wed, 14 Aug 2019 at 10:26:53 -0400, Brian Reichert wrote:
> Doesn't daemonize(1) make stdin, stdout and stderr point to /dev/null,
> instead of closing them?

Looking at the source, yes, it does.

> Expecting arbitrary subprocesses to cope gracefully with being invoked
> without the three standard fds seems likely to be a losing battle.
> I've implemented this myself, in dbus; it isn't a whole lot of code,
> but it also isn't something that I would expect the authors of all CLI
> tools to get right.

I concede that reopening FD 0,1,2 is a good practice to insulate
against the issues you cite.

I agree with your points; I code aggressively, and sometimes forget
others don't.

> smcv
> 
> [1] I'm sure there are lots of other executables named daemon or daemonize
> in other OSs, and perhaps some of them get this wrong?
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/systemd-devel

-- 
Brian Reichert  
BSD admin/developer at large
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] systemd's connections to /run/systemd/private ?

2019-08-14 Thread Simon McVittie
On Wed, 14 Aug 2019 at 10:26:53 -0400, Brian Reichert wrote:
> And, if I were to ever use daemonize(1), or any other other canonical
> mechanism for daemonizing code, STDOUT would normally be closed
> under those circumstances, as well.

Doesn't daemonize(1) make stdin, stdout and stderr point to /dev/null,
instead of closing them? That's what glibc daemon(3) does, and seems
a lot better as a way to not break subprocesses' expectations. The man
pages of the daemon(1) and daemonize(1) in Debian[1] also both say they
do this, and it's what is recommended in systemd's own daemon(7).

Expecting arbitrary subprocesses to cope gracefully with being invoked
without the three standard fds seems likely to be a losing battle.
I've implemented this myself, in dbus; it isn't a whole lot of code,
but it also isn't something that I would expect the authors of all CLI
tools to get right. dbus only does this because X11 autolaunching can
lead to dbus-daemon being invoked in a broken execution environment, and
I would have considered the equivalent bug in most tools to be WONTFIX.

Expecting arbitrary *library code* to cope gracefully with being invoked
without the three standard fds seems like a non-starter, because these
fds are process-global state, so library code cannot safely mess with
them unless specifically asked to do so by the calling process.

In general library code can't do this safely anyway, because as soon as
the process has a second thread, it becomes impossible to fix the fds
in one thread without risking breaking another thread - this is one of
the operations that is usually only safe to do right at the beginning
of main(), similar to setenv().

smcv

[1] I'm sure there are lots of other executables named daemon or daemonize
in other OSs, and perhaps some of them get this wrong?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] systemd's connections to /run/systemd/private ?

2019-08-14 Thread Lennart Poettering
On Mi, 14.08.19 10:26, Brian Reichert (reich...@numachi.com) wrote:

> On Wed, Aug 14, 2019 at 11:34:21AM +0200, Lennart Poettering wrote:
> > Hence: your code that closes fd1 like this is simply buggy. Don't do
> > that, you are shooting yourself in the foot.
>
> Buggy or no, this is fifteen-year-old code, and prior cron/service
> mgmt framework implementations had no issue.
>
> And, if I were to ever use daemonize(1), or any other other canonical
> mechanism for daemonizing code, STDOUT would normally be closed
> under those circumstances, as well.

In daemons, stdin/stdout/stderr are generally closed and then opened
as /dev/null again, precisely to avoid problems like that, so that
fd0,1,2 are taken and any future fd allocations are > 2. This is what
BSD/glibc daemon(3) does for example, and most daemons do it like
that.

printf() and friends assume that fd1 is stdout.
fgets() and friends assume that fd0 is stdin.
error() and friends assume that fd2 is stderr.

Most command line programs (such as ls, echo or systemctl assume that
fd0, 1, 2 are defined that way, and they will do their output and
input according to that. if you close these fds before invoking these
programs they will do borked stuff too...

Example:

   telnet www.microsoft.de 80 2>&1-

See, this will connect to port 80, and do so with a socket which is
allocated as fd1, because we called this tool with fd1 closed. Thus it
interferes with stdout, and that one line string that says "Trying
…\nConnected to …\r\nEscape character is '^]'" will be written to the
peer instead of the local tty!

That works with any tool. If you invoke a tool with
stdout/stderr/stdin not connected weird shit happens as soon as the
app allocates the first fd. systemctl is just one of many.

Note that the robustness we added to sd-bus which explicitly moves the
fd >= 3 is not supposed to make things work in cases where systemctl
is called with closed stdin, stdout or stderr. Instead, we made this
change since the sd-bus code is used in NSS clients, i.e. as backend
for gethostbyname(), and thus we really don't know in which kind of
contexts it is used. i.e. it might very well be called inside a
process that is currently rearranging stdin/stdout/stderr and thus
might have closed them temporarily.

Lennart

--
Lennart Poettering, Berlin
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] systemd's connections to /run/systemd/private ?

2019-08-14 Thread Brian Reichert
On Wed, Aug 14, 2019 at 11:34:21AM +0200, Lennart Poettering wrote:
> Hence: your code that closes fd1 like this is simply buggy. Don't do
> that, you are shooting yourself in the foot.

Buggy or no, this is fifteen-year-old code, and prior cron/service
mgmt framework implementations had no issue.

And, if I were to ever use daemonize(1), or any other other canonical
mechanism for daemonizing code, STDOUT would normally be closed
under those circumstances, as well.

I'm wading into hypotheticals here, but daemonized code should, in
turn, be able to invoke whatever code is subsequently wants.

> Lennart
> 
> --
> Lennart Poettering, Berlin

-- 
Brian Reichert  
BSD admin/developer at large
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Antw: Re: Antw: Re: systemd's connections to /run/systemd/private ?

2019-08-14 Thread Reindl Harald


Am 14.08.19 um 14:59 schrieb Ulrich Windl:
 Reindl Harald  schrieb am 14.08.2019 um 12:22 in
> Nachricht <13150bf2-e0c9-063a-9026-ac95c1fda...@thelounge.net>:
>>
>> Am 14.08.19 um 12:10 schrieb Ulrich Windl:
>> Michael Chapman  schrieb am 14.08.2019 um 11:47 
>> in
 That's all true, but the thing we need to check here is that systemd 
 correctly handles junk on the /run/systemd/private socket. The change on 
 the systemctl side certainly tries to prevent incorrect data being sent 
 down the socket -- though it looks like there's several ways in which 
 fd_move_above_stdio() can fail, so this isn't foolproof -- but we need to 
 ensure that some _malicious_ client can't DoS systemd.
>>>
>>> I don't want to contradict in principle, but doesn't "private socket" mean 
>> it's intended to be used by systemd only? Of course being root allows you to 
>> use any socket...
>>
>> may is ask you to read the thread you are responding to?
>> nobody is touching the private socket
> 
> Then why care about "junk on the /run/systemd/private socket."?

to avoid when people like you doing strange stuff coming here to blame
systemd as you did often enough in the past months
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] Antw: Re: Antw: Re: systemd's connections to /run/systemd/private ?

2019-08-14 Thread Ulrich Windl
>>> Reindl Harald  schrieb am 14.08.2019 um 12:22 in
Nachricht <13150bf2-e0c9-063a-9026-ac95c1fda...@thelounge.net>:

> 
> Am 14.08.19 um 12:10 schrieb Ulrich Windl:
> Michael Chapman  schrieb am 14.08.2019 um 11:47 in
>>> That's all true, but the thing we need to check here is that systemd 
>>> correctly handles junk on the /run/systemd/private socket. The change on 
>>> the systemctl side certainly tries to prevent incorrect data being sent 
>>> down the socket -- though it looks like there's several ways in which 
>>> fd_move_above_stdio() can fail, so this isn't foolproof -- but we need to 
>>> ensure that some _malicious_ client can't DoS systemd.
>> 
>> I don't want to contradict in principle, but doesn't "private socket" mean 
> it's intended to be used by systemd only? Of course being root allows you to 
> use any socket...
> 
> may is ask you to read the thread you are responding to?
> nobody is touching the private socket

Then why care about "junk on the /run/systemd/private socket."?

> 
>  Weitergeleitete Nachricht 
> Betreff: Re: [systemd-devel] systemd's connections to /run/systemd/private ?
> Datum: Tue, 13 Aug 2019 17:50:56 -0400
> Von: Brian Reichert 
> An: Zbigniew J??drzejewski-Szmek 
> Kopie (CC): systemd-devel@lists.freedesktop.org 
> This is sufficient to reproduce the effect of increasing the number
> of file descriptors open to /run/systemd/private; at least, on my
> box, in it's current state:
> 
>   sh -c 'exec 1>&-; /usr/bin/systemctl status ntpd.service'
> 
> We have cronjob that closes STDOUT, remaps STDERR to a log file,
> and runs this systemctl command.  In my environment, this one-liner
> will cause that FD count to go up by, 100% reproducible
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org 
> https://lists.freedesktop.org/mailman/listinfo/systemd-devel 




___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] systemd's connections to /run/systemd/private ?

2019-08-14 Thread Michael Chapman
On Wed, 14 Aug 2019, Lennart Poettering wrote:
> Well, a D-Bus connection can remain open indefinitely, and may even
> have incomplete "half" messages queued in them as long as the client
> desires. After the initial authentication is done, clients may thus
> take up resources as long as they want, this is by design of dbus
> really, and is different from HTTP for example, where connections
> usually have a time-out applied. dbus doesn't know timeouts for
> established connections. It knows them for the authentication phase,
> and it knows them for method calls that are flight, but it does not
> know them for the mere existance of an established connection.

I'm sure it's not in the design of DBus that clients can continue to 
consume those resources after they've disconnected.

> PID 1 authenticates clients of the private connection simply by making
> the socket for it inaccessible to anyone who is not privileged. Due to
> that it gets away with not doing any further per-user accounting,
> because it knows the clients are all privileged anyway.
> 
> So, yes, it would be good if we could protect us from any form of
> misuse, but basically, if you have a root client that misbehaves you
> have too accept that...

I understand all that. Nevertheless, Brian identified a bug: after 
receiving certain data on its private socket, the systemd process can leak 
a file descriptor.

All we need to establish is whether that bug still exists in the current 
version of systemd. _Even if_ it isn't a security issue, having one fewer 
bugs is a good thing.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] systemd's connections to /run/systemd/private ?

2019-08-14 Thread Lennart Poettering
On Mi, 14.08.19 19:47, Michael Chapman (m...@very.puzzling.org) wrote:

> On Wed, 14 Aug 2019, Lennart Poettering wrote:
> > Quite frankly, invoking generic UNIX programs with fds < 3 closed is a
> > really bad idea in general. That systemctl nowadays is particularly
> > careful and deals with situations like that is not an invitation to
> > actually invoke things like this. After all UNIX guarantees that
> > open() (and the other calls that allocate an fd) allocated the lowest
> > available one, hence they will necessarily step into
> > stdin/stdout/stderr territory when invoked with any of those fds
> > closed.
> >
> > Hence: your code that closes fd1 like this is simply buggy. Don't do
> > that, you are shooting yourself in the foot. And even if newer
> > systemctl will handle cases like this more gracefully pretty much any
> > other tool you might call will break in some form or another too,
> > since a simple printf() will already spill into the wrong fds in that
> > case.
>
> That's all true, but the thing we need to check here is that systemd
> correctly handles junk on the /run/systemd/private socket. The change on
> the systemctl side certainly tries to prevent incorrect data being sent
> down the socket -- though it looks like there's several ways in which
> fd_move_above_stdio() can fail, so this isn't foolproof -- but we need to
> ensure that some _malicious_ client can't DoS systemd.

Well, a D-Bus connection can remain open indefinitely, and may even
have incomplete "half" messages queued in them as long as the client
desires. After the initial authentication is done, clients may thus
take up resources as long as they want, this is by design of dbus
really, and is different from HTTP for example, where connections
usually have a time-out applied. dbus doesn't know timeouts for
established connections. It knows them for the authentication phase,
and it knows them for method calls that are flight, but it does not
know them for the mere existance of an established connection.

PID 1 authenticates clients of the private connection simply by making
the socket for it inaccessible to anyone who is not privileged. Due to
that it gets away with not doing any further per-user accounting,
because it knows the clients are all privileged anyway.

So, yes, it would be good if we could protect us from any form of
misuse, but basically, if you have a root client that misbehaves you
have too accept that...

Lennart

--
Lennart Poettering, Berlin
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Antw: Re: systemd's connections to /run/systemd/private ?

2019-08-14 Thread Reindl Harald

Am 14.08.19 um 12:48 schrieb Michael Chapman:
> On Wed, 14 Aug 2019, Reindl Harald wrote:
 may is ask you to read the thread you are responding to?
 nobody is touching the private socket
>>>
>>> systemctl will mostly use /run/systemd/private when run as root
>>
>> that's not the point - the point is his talking about "doesn't private
>> socket mean" when the code triggering in the thread the issue don't talk
>> to it directly
> 
> I don't know who specifically you are referring to.
> 
> Brian's example, when run as root, connects to systemd using the private 
> socket. When run as an unprivileged user it will go via the DBus daemon 
> instead.

sh -c 'exec 1>&-; /usr/bin/systemctl status ntpd.service'

that code don#t talk directly to that socket and so any "but doesn't
private socket mean it's intended to be used by systemd only" is
nonsense give it is used by systemd in the form of systemctl
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Antw: Re: systemd's connections to /run/systemd/private ?

2019-08-14 Thread Michael Chapman
On Wed, 14 Aug 2019, Reindl Harald wrote:
> Am 14.08.19 um 12:41 schrieb Michael Chapman:
> > On Wed, 14 Aug 2019, Reindl Harald wrote:
> >> Am 14.08.19 um 12:10 schrieb Ulrich Windl:
> >> Michael Chapman  schrieb am 14.08.2019 um 
> >> 11:47 in
>  That's all true, but the thing we need to check here is that systemd 
>  correctly handles junk on the /run/systemd/private socket. The change on 
>  the systemctl side certainly tries to prevent incorrect data being sent 
>  down the socket -- though it looks like there's several ways in which 
>  fd_move_above_stdio() can fail, so this isn't foolproof -- but we need 
>  to 
>  ensure that some _malicious_ client can't DoS systemd.
> >>>
> >>> I don't want to contradict in principle, but doesn't "private socket" 
> >>> mean it's intended to be used by systemd only? Of course being root 
> >>> allows you to use any socket...
> >>
> >> may is ask you to read the thread you are responding to?
> >> nobody is touching the private socket
> > 
> > systemctl will mostly use /run/systemd/private when run as root
> 
> that's not the point - the point is his talking about "doesn't private
> socket mean" when the code triggering in the thread the issue don't talk
> to it directly

I don't know who specifically you are referring to.

Brian's example, when run as root, connects to systemd using the private 
socket. When run as an unprivileged user it will go via the DBus daemon 
instead.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Antw: Re: systemd's connections to /run/systemd/private ?

2019-08-14 Thread Reindl Harald


Am 14.08.19 um 12:41 schrieb Michael Chapman:
> On Wed, 14 Aug 2019, Reindl Harald wrote:
>> Am 14.08.19 um 12:10 schrieb Ulrich Windl:
>> Michael Chapman  schrieb am 14.08.2019 um 11:47 
>> in
 That's all true, but the thing we need to check here is that systemd 
 correctly handles junk on the /run/systemd/private socket. The change on 
 the systemctl side certainly tries to prevent incorrect data being sent 
 down the socket -- though it looks like there's several ways in which 
 fd_move_above_stdio() can fail, so this isn't foolproof -- but we need to 
 ensure that some _malicious_ client can't DoS systemd.
>>>
>>> I don't want to contradict in principle, but doesn't "private socket" mean 
>>> it's intended to be used by systemd only? Of course being root allows you 
>>> to use any socket...
>>
>> may is ask you to read the thread you are responding to?
>> nobody is touching the private socket
> 
> systemctl will mostly use /run/systemd/private when run as root

that's not the point - the point is his talking about "doesn't private
socket mean" when the code triggering in the thread the issue don't talk
to it directly
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Antw: Re: systemd's connections to /run/systemd/private ?

2019-08-14 Thread Michael Chapman
On Wed, 14 Aug 2019, Reindl Harald wrote:
> Am 14.08.19 um 12:10 schrieb Ulrich Windl:
>  Michael Chapman  schrieb am 14.08.2019 um 11:47 
>  in
> >> That's all true, but the thing we need to check here is that systemd 
> >> correctly handles junk on the /run/systemd/private socket. The change on 
> >> the systemctl side certainly tries to prevent incorrect data being sent 
> >> down the socket -- though it looks like there's several ways in which 
> >> fd_move_above_stdio() can fail, so this isn't foolproof -- but we need to 
> >> ensure that some _malicious_ client can't DoS systemd.
> > 
> > I don't want to contradict in principle, but doesn't "private socket" mean 
> > it's intended to be used by systemd only? Of course being root allows you 
> > to use any socket...
> 
> may is ask you to read the thread you are responding to?
> nobody is touching the private socket

systemctl will mostly use /run/systemd/private when run as root.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Antw: Re: systemd's connections to /run/systemd/private ?

2019-08-14 Thread Michael Chapman
On Wed, 14 Aug 2019, Ulrich Windl wrote:
> >>> Michael Chapman  schrieb am 14.08.2019 um 11:47 in
> Nachricht :
> > On Wed, 14 Aug 2019, Lennart Poettering wrote:
> >> Quite frankly, invoking generic UNIX programs with fds < 3 closed is a
> >> really bad idea in general. That systemctl nowadays is particularly
> >> careful and deals with situations like that is not an invitation to
> >> actually invoke things like this. After all UNIX guarantees that
> >> open() (and the other calls that allocate an fd) allocated the lowest
> >> available one, hence they will necessarily step into
> >> stdin/stdout/stderr territory when invoked with any of those fds
> >> closed.
> >> 
> >> Hence: your code that closes fd1 like this is simply buggy. Don't do
> >> that, you are shooting yourself in the foot. And even if newer
> >> systemctl will handle cases like this more gracefully pretty much any
> >> other tool you might call will break in some form or another too,
> >> since a simple printf() will already spill into the wrong fds in that
> >> case.
> >> 
> >> Lennart
> > 
> > That's all true, but the thing we need to check here is that systemd 
> > correctly handles junk on the /run/systemd/private socket. The change on 
> > the systemctl side certainly tries to prevent incorrect data being sent 
> > down the socket -- though it looks like there's several ways in which 
> > fd_move_above_stdio() can fail, so this isn't foolproof -- but we need to 
> > ensure that some _malicious_ client can't DoS systemd.
> 
> Hi!
> 
> I don't want to contradict in principle, but doesn't "private socket" 
> mean it's intended to be used by systemd only? Of course being root 
> allows you to use any socket...
>
> I could imaging two things of torture testing:
> 1) send random data to the socket and see what happens.
> 2) vary valid messages randomly to see what happens

Any user can connect to it at least. systemd will drop the connection if 
it's from an unprivileged client. The question though is whether junk 
data still pending on that connection will cause systemd to leak the file 
descriptor.

I think we'd need to fully understand why the problem occurred on older 
systemd versions to be able to answer that. Maybe that problem has been 
fixed.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Antw: Re: systemd's connections to /run/systemd/private ?

2019-08-14 Thread Reindl Harald


Am 14.08.19 um 12:10 schrieb Ulrich Windl:
 Michael Chapman  schrieb am 14.08.2019 um 11:47 in
>> That's all true, but the thing we need to check here is that systemd 
>> correctly handles junk on the /run/systemd/private socket. The change on 
>> the systemctl side certainly tries to prevent incorrect data being sent 
>> down the socket -- though it looks like there's several ways in which 
>> fd_move_above_stdio() can fail, so this isn't foolproof -- but we need to 
>> ensure that some _malicious_ client can't DoS systemd.
> 
> I don't want to contradict in principle, but doesn't "private socket" mean 
> it's intended to be used by systemd only? Of course being root allows you to 
> use any socket...

may is ask you to read the thread you are responding to?
nobody is touching the private socket

 Weitergeleitete Nachricht 
Betreff: Re: [systemd-devel] systemd's connections to /run/systemd/private ?
Datum: Tue, 13 Aug 2019 17:50:56 -0400
Von: Brian Reichert 
An: Zbigniew J??drzejewski-Szmek 
Kopie (CC): systemd-devel@lists.freedesktop.org
This is sufficient to reproduce the effect of increasing the number
of file descriptors open to /run/systemd/private; at least, on my
box, in it's current state:

  sh -c 'exec 1>&-; /usr/bin/systemctl status ntpd.service'

We have cronjob that closes STDOUT, remaps STDERR to a log file,
and runs this systemctl command.  In my environment, this one-liner
will cause that FD count to go up by, 100% reproducible
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] Antw: Re: systemd's connections to /run/systemd/private ?

2019-08-14 Thread Ulrich Windl
>>> Michael Chapman  schrieb am 14.08.2019 um 11:47 in
Nachricht :
> On Wed, 14 Aug 2019, Lennart Poettering wrote:
>> Quite frankly, invoking generic UNIX programs with fds < 3 closed is a
>> really bad idea in general. That systemctl nowadays is particularly
>> careful and deals with situations like that is not an invitation to
>> actually invoke things like this. After all UNIX guarantees that
>> open() (and the other calls that allocate an fd) allocated the lowest
>> available one, hence they will necessarily step into
>> stdin/stdout/stderr territory when invoked with any of those fds
>> closed.
>> 
>> Hence: your code that closes fd1 like this is simply buggy. Don't do
>> that, you are shooting yourself in the foot. And even if newer
>> systemctl will handle cases like this more gracefully pretty much any
>> other tool you might call will break in some form or another too,
>> since a simple printf() will already spill into the wrong fds in that
>> case.
>> 
>> Lennart
> 
> That's all true, but the thing we need to check here is that systemd 
> correctly handles junk on the /run/systemd/private socket. The change on 
> the systemctl side certainly tries to prevent incorrect data being sent 
> down the socket -- though it looks like there's several ways in which 
> fd_move_above_stdio() can fail, so this isn't foolproof -- but we need to 
> ensure that some _malicious_ client can't DoS systemd.

Hi!

I don't want to contradict in principle, but doesn't "private socket" mean it's 
intended to be used by systemd only? Of course being root allows you to use any 
socket...

I could imaging two things of torture testing:
1) send random data to the socket and see what happens.
2) vary valid messages randomly to see what happens

(1 may take longer time, but will be more thorough, while 2 while be fast 
without catching all possible mis-interpretation)

Regards,
Ulrich

> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org 
> https://lists.freedesktop.org/mailman/listinfo/systemd-devel 




___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] systemd's connections to /run/systemd/private ?

2019-08-14 Thread Michael Chapman
On Wed, 14 Aug 2019, Lennart Poettering wrote:
> Quite frankly, invoking generic UNIX programs with fds < 3 closed is a
> really bad idea in general. That systemctl nowadays is particularly
> careful and deals with situations like that is not an invitation to
> actually invoke things like this. After all UNIX guarantees that
> open() (and the other calls that allocate an fd) allocated the lowest
> available one, hence they will necessarily step into
> stdin/stdout/stderr territory when invoked with any of those fds
> closed.
> 
> Hence: your code that closes fd1 like this is simply buggy. Don't do
> that, you are shooting yourself in the foot. And even if newer
> systemctl will handle cases like this more gracefully pretty much any
> other tool you might call will break in some form or another too,
> since a simple printf() will already spill into the wrong fds in that
> case.
> 
> Lennart

That's all true, but the thing we need to check here is that systemd 
correctly handles junk on the /run/systemd/private socket. The change on 
the systemctl side certainly tries to prevent incorrect data being sent 
down the socket -- though it looks like there's several ways in which 
fd_move_above_stdio() can fail, so this isn't foolproof -- but we need to 
ensure that some _malicious_ client can't DoS systemd.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] systemd's connections to /run/systemd/private ?

2019-08-14 Thread Lennart Poettering
On Mi, 14.08.19 18:53, Michael Chapman (m...@very.puzzling.org) wrote:

> On Wed, 14 Aug 2019, Michael Chapman wrote:
> > On Wed, 14 Aug 2019, Brian Reichert wrote:
> > > On Thu, Aug 01, 2019 at 07:18:20PM +, Zbigniew J??drzejewski-Szmek 
> > > wrote:
> > > > Yes. (With the caveat that there *are* legitimate reasons to have new
> > > > long-lived fds created, so not every long-lived fd is "wrong".)
> > >
> > > I finally was able to track down what's happening on my system.
> > >
> > > This is sufficient to reproduce the effect of increasing the number
> > > of file descriptors open to /run/systemd/private; at least, on my
> > > box, in it's current state:
> > >
> > >   sh -c 'exec 1>&-; /usr/bin/systemctl status ntpd.service'
> >
> > I can reproduce this on CentOS 7's systemd 219, but not on Fedora 29's
> > systemd 239.
>
> Looking in to this further, my guess is that this is fixed in:
>
>   commit 7fe2903c2315db9d9f501ae255a6b6e4f01ba46c
>   Author: Lennart Poettering 
>   Date:   Fri Feb 9 17:53:28 2018 +0100
>
>   fd-util: move certain fds above fd #2 (#8129)
>
> which was introduced in systemd 238.
>
> What's happening is that systemctl is getting fd 1 for the socket to
> /run/systemd/private, as it's the lowest unused fd, but fd 1 (aka stdout)
> is also where it sends its regular human-readable output. This commit
> ensures that the socket is moved to a file descriptor that can't be
> mistaken for a standard stream.
>
> I'm not sure if systemd >= 238 itself still suffers from the problem where
> any "extra" data on that connection causes it to leak its file descriptor.
> That may still be a problem, or it may have been fixed by a different
> commit.

Quite frankly, invoking generic UNIX programs with fds < 3 closed is a
really bad idea in general. That systemctl nowadays is particularly
careful and deals with situations like that is not an invitation to
actually invoke things like this. After all UNIX guarantees that
open() (and the other calls that allocate an fd) allocated the lowest
available one, hence they will necessarily step into
stdin/stdout/stderr territory when invoked with any of those fds
closed.

Hence: your code that closes fd1 like this is simply buggy. Don't do
that, you are shooting yourself in the foot. And even if newer
systemctl will handle cases like this more gracefully pretty much any
other tool you might call will break in some form or another too,
since a simple printf() will already spill into the wrong fds in that
case.

Lennart

--
Lennart Poettering, Berlin
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] systemd's connections to /run/systemd/private ?

2019-08-14 Thread Michael Chapman
On Wed, 14 Aug 2019, Michael Chapman wrote:
> On Wed, 14 Aug 2019, Brian Reichert wrote:
> > On Thu, Aug 01, 2019 at 07:18:20PM +, Zbigniew J??drzejewski-Szmek 
> > wrote:
> > > Yes. (With the caveat that there *are* legitimate reasons to have new
> > > long-lived fds created, so not every long-lived fd is "wrong".)
> > 
> > I finally was able to track down what's happening on my system.
> > 
> > This is sufficient to reproduce the effect of increasing the number
> > of file descriptors open to /run/systemd/private; at least, on my
> > box, in it's current state:
> > 
> >   sh -c 'exec 1>&-; /usr/bin/systemctl status ntpd.service'
> 
> I can reproduce this on CentOS 7's systemd 219, but not on Fedora 29's 
> systemd 239.

Looking in to this further, my guess is that this is fixed in:

  commit 7fe2903c2315db9d9f501ae255a6b6e4f01ba46c
  Author: Lennart Poettering 
  Date:   Fri Feb 9 17:53:28 2018 +0100

  fd-util: move certain fds above fd #2 (#8129)

which was introduced in systemd 238.

What's happening is that systemctl is getting fd 1 for the socket to 
/run/systemd/private, as it's the lowest unused fd, but fd 1 (aka stdout) 
is also where it sends its regular human-readable output. This commit 
ensures that the socket is moved to a file descriptor that can't be 
mistaken for a standard stream.

I'm not sure if systemd >= 238 itself still suffers from the problem where 
any "extra" data on that connection causes it to leak its file descriptor. 
That may still be a problem, or it may have been fixed by a different 
commit.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] systemd's connections to /run/systemd/private ?

2019-08-14 Thread Michael Chapman
On Wed, 14 Aug 2019, Brian Reichert wrote:
> On Thu, Aug 01, 2019 at 07:18:20PM +, Zbigniew J??drzejewski-Szmek wrote:
> > Yes. (With the caveat that there *are* legitimate reasons to have new
> > long-lived fds created, so not every long-lived fd is "wrong".)
> 
> I finally was able to track down what's happening on my system.
> 
> This is sufficient to reproduce the effect of increasing the number
> of file descriptors open to /run/systemd/private; at least, on my
> box, in it's current state:
> 
>   sh -c 'exec 1>&-; /usr/bin/systemctl status ntpd.service'

I can reproduce this on CentOS 7's systemd 219, but not on Fedora 29's 
systemd 239.

On CentOS 7 I took two `strace -e desc -p 1` runs, comparing:

  # good
  sh -c 'exec 1>/dev/null; systemctl status tmp.mount'

with:

  # bad
  sh -c 'exec 1>&-; systemctl status tmp.mount'

The diff is:

  # diff -u /tmp/good /tmp/bad
  --- /tmp/good 2019-08-14 18:11:20.100792406 +1000
  +++ /tmp/bad  2019-08-14 18:11:24.882886972 +1000
  @@ -6,7 +6,7 @@
   fstat(24, {st_mode=S_IFSOCK|0777, st_size=0, ...}) = 0
   epoll_ctl(4, EPOLL_CTL_ADD, 24, {0, {u32=107062384, u64=94382013391984}}) = 0
   epoll_ctl(4, EPOLL_CTL_MOD, 24, {EPOLLIN|EPOLLOUT, {u32=107062384, 
u64=94382013391984}}) = 0
  -timerfd_settime(3, TFD_TIMER_ABSTIME, {it_interval={0, 0}, 
it_value={1136546, 444853000}}, NULL) = 0
  +timerfd_settime(3, TFD_TIMER_ABSTIME, {it_interval={0, 0}, 
it_value={1136551, 444853000}}, NULL) = 0
   epoll_wait(4, [{EPOLLOUT, {u32=107062384, u64=94382013391984}}], 58, -1) = 1
   epoll_ctl(4, EPOLL_CTL_MOD, 24, {EPOLLIN, {u32=107062384, 
u64=94382013391984}}) = 0
   epoll_wait(4, [{EPOLLIN, {u32=107062384, u64=94382013391984}}], 58, -1) = 1
  @@ -23,13 +23,7 @@
   openat(AT_FDCWD, "/usr/local/lib/systemd/system/tmp.mount.d", 
O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = -1 ENOENT (No such file or 
directory)
   openat(AT_FDCWD, "/usr/lib/systemd/system/tmp.mount.d", 
O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = -1 ENOENT (No such file or 
directory)
   openat(AT_FDCWD, "/run/systemd/generator.late/tmp.mount.d", 
O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = -1 ENOENT (No such file or 
directory)
  -epoll_wait(4, [{EPOLLIN|EPOLLHUP, {u32=107062384, u64=94382013391984}}], 58, 
-1) = 1
  -epoll_ctl(4, EPOLL_CTL_MOD, 24, {0, {u32=107062384, u64=94382013391984}}) = 0
  -timerfd_settime(3, TFD_TIMER_ABSTIME, {it_interval={0, 0}, it_value={0, 1}}, 
NULL) = 0
  -epoll_wait(4, [{EPOLLHUP, {u32=107062384, u64=94382013391984}}, {EPOLLIN, 
{u32=3, u64=3}}], 58, -1) = 2
  -read(3, "\1\0\0\0\0\0\0\0", 8)  = 8
  +epoll_wait(4, [{EPOLLIN, {u32=107062384, u64=94382013391984}}], 58, -1) = 1
   epoll_ctl(4, EPOLL_CTL_DEL, 24, NULL)   = 0
  -close(24)   = 0
  -timerfd_settime(3, TFD_TIMER_ABSTIME, {it_interval={0, 0}, 
it_value={1136622, 194853000}}, NULL) = 0
   epoll_wait(4, strace: Process 1 detached
  

So it looks like systemd is removing the file descriptor from the epoll 
instance in both cases. However, in the "bad" case no EOF was reached, and 
the file descriptor is never closed. Looking at `ss` there is still 387 
bytes in this descriptor's receive queue.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel