Re: inetd(8): continue or exit on error?
On Fri, Jun 02, 2023 at 01:21:15PM +0200, tlaro...@polynum.com wrote:
> > I have not read most of the traffic yet, but I feel, fairly strongly,
> > that inetd should _not_ exit, except (maybe) if the config is broken
> > during its initial startup. It's a critical service.
>
> So I will put together the result of the exchanges in the thread and of
> my reading of the source:
>
> Not stopping on an error was logical with the old syntax since _all the
> directives were independent_: failing to read a line for a service
> shouldn't have the side effect of failing to serve the other services.
>
> But this assumption does not anymore with the new syntax: the feature of
> the implicit address---one does not need to specify an address: it will
> then be what is the default at the moment---makes the config a whole,
> and failing on a line may define the default address with something
> completely different from what was intended: what will be the result of
> runnin login on the external interface instead of an internal one?
>
> => a failure in the config now must discard the whole config.
I don't understand. You read the config, you check it, if it's bad you
complain to syslog and if it's good you install it. There's still no
reason to exit.
> If the [-r] mode is asked for (r for "resilient"), in case of failure to
> validate the config, inetd falls back to "/etc/inetd.fallback.conf" that
> is also parses and, if checking successful, served. If even this
> fallback fails, inetd(8) does not exit but serves the "no-op" config: it
> does nothing simply waiting for the instruction to reload the config
> ("the config" is always the one passed, explicitely or implicitely, on
> the call).
That sounds way more complicated than necessary.
--
David A. Holland
dholl...@netbsd.org
Re: inetd(8): continue or exit on error?
Le Fri, Jun 02, 2023 at 07:01:40AM +, David Holland a écrit :
> On Mon, May 29, 2023 at 10:11:09AM +0200, tlaro...@polynum.com wrote:
> > There are infelicities in /usr/src/usr.sbin/inetd/parse.c and I will
> > send a PR with patches attached.
> >
> > The question is what to do in case of a config file not found (this is
> > the initial problem: the realpath() return status is not tested and a
> > structure is inconditionnally added to a linked list with an unreachable
> > config file).
> >
> > It seems to me, since these are services, that the failure to load a
> > config is critical enough (since the server may be then servicing what
> > was not intended to be serviced; the reverse is less problematic)
> > to exit at least on this error.
> >
> > What do others think?
>
> I have not read most of the traffic yet, but I feel, fairly strongly,
> that inetd should _not_ exit, except (maybe) if the config is broken
> during its initial startup. It's a critical service.
So I will put together the result of the exchanges in the thread and of
my reading of the source:
Not stopping on an error was logical with the old syntax since _all the
directives were independent_: failing to read a line for a service
shouldn't have the side effect of failing to serve the other services.
But this assumption does not anymore with the new syntax: the feature of
the implicit address---one does not need to specify an address: it will
then be what is the default at the moment---makes the config a whole,
and failing on a line may define the default address with something
completely different from what was intended: what will be the result of
runnin login on the external interface instead of an internal one?
=> a failure in the config now must discard the whole config.
Hence to address this and the need for essential services the following
will be done (in the process in fact):
Two modes:
Server mode: inetd [-rl] [-f [-d]] [rooted_config]
Checker mode: inetd -c [-d] [rooted_config]
inetd always checks the config it is asked to serve first and does not
serve anything before validating the config. If default mode, a failure
to check the config causes an exit of the daemon.
Disentangling the parsing from the executing/serving, there is a check
mode that just parses the config and returns EXIT_SUCCESS if the config
is OK, EXIT_FAILURE if not (unfortunately, there is no EXIT_RTFM to
indicate that the call was nonsense...).
Checker mode:
So inetd just exits with the status in check mode. In check mode, if -d
is given, debug message about the parsing (and the errors) are sent to
stderr and, if successful, the parsed config in new syntax is sent to
stdout. Nothing is printed if [-d] is not given.
Server mode:
Inetd always checks the config and if the convid is invalid it exits
with error (default mode).
If the [-r] mode is asked for (r for "resilient"), in case of failure to
validate the config, inetd falls back to "/etc/inetd.fallback.conf" that
is also parses and, if checking successful, served. If even this
fallback fails, inetd(8) does not exit but serves the "no-op" config: it
does nothing simply waiting for the instruction to reload the config
("the config" is always the one passed, explicitely or implicitely, on
the call).
Still in resilient mode, if the server is instructed to reload the
config, it does not stop serving the old but first checks the new. If
the new is valid, it switches to these new instructions. If the new is
not valid, it continues to serve the current served one.
In daemon mode but in foreground mode, messages are logged via syslog.
When in foreground, messages go to stderr.
A new control is added:
Sending USR1 signal instruct inetd to switch to the fallback config (it
is not "sticky": when instructed to reload via HUP, it will reload "the
config" not the fallback one).
Compatibility:
I was unhappy about:
-d run in foreground and add debug messages
-f run in foreground
that seems to me not very Unix like.
Hence the [-f [-d]] but for compatibility in the switch it is a
fallthrough: case d will fallthrough f so the behavior will be the same.
-c takes also -d; no problem since -cfd will invoke the checker, not the
daemon: "inetd is human and always grasps to the first opportunity to do
less work".
--
Thierry Laronde
http://www.kergis.com/
http://kertex.kergis.com/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89 250D 52B1 AE95 6006 F40C
Re: inetd(8): continue or exit on error?
On Mon, May 29, 2023 at 10:11:09AM +0200, tlaro...@polynum.com wrote: > There are infelicities in /usr/src/usr.sbin/inetd/parse.c and I will > send a PR with patches attached. > > The question is what to do in case of a config file not found (this is > the initial problem: the realpath() return status is not tested and a > structure is inconditionnally added to a linked list with an unreachable > config file). > > It seems to me, since these are services, that the failure to load a > config is critical enough (since the server may be then servicing what > was not intended to be serviced; the reverse is less problematic) > to exit at least on this error. > > What do others think? I have not read most of the traffic yet, but I feel, fairly strongly, that inetd should _not_ exit, except (maybe) if the config is broken during its initial startup. It's a critical service. At the moment it's no longer the case that remote machines are likely to become completely inaccessible and require a site visit if inetd craps out, but that could change again in the future. There's at least one variant inetd out there that does not exit once started, except for SIGTERM. I know this, because I wrote/patched it :-) -- David A. Holland dholl...@netbsd.org
Re: inetd(8): continue or exit on error?
Le Mon, May 29, 2023 at 07:49:44AM -0400, Mouse a écrit : > >> I'm not sure inetd(8) has any business calling realpath in the first > >> place. > > I agree. > > > It has to call realpath(3) since in order to not include several > > times the same file, it makes strings comparaisons about names. > > If I as an admin write a config that tries to include the same file > twice, whether via the same or different paths, I expect it to include > the same file twice. If that leads to errors, that's on me. (It might > or might not _always_ lead to an error, depending on how the include > file syntax is defined to interact with the stateful aspects of the > config language and what's in the config files.) I tend to agree that since, with globbing, the order of the inclusion of files is not guaranteed, reloading several times the same file, as long as there is no loop, is not less legitimate (but for globbing, there could be an added defined behavior to sort lexicographically the files list so that carefully choosen names can guarantee a defined order of inclusion; and this will be compatible with "present" behavior, since no order was guaranteed; hence any order imposed now will do). But, at first, I will add a flag to check the present syntax and keep the present theorical behavior (in fact, due to bugs, it doesn't work), and will correct the bugs and make the program exit on error. And only in a second time, perhaps review the way things are processed (perhaps allowing multiple inclusions of the same file---do what the admin requested; as you wrote, it is his work so his responsability as long as, syntactically, the program can parse what he asked, deliver). > If you really want to avoid including the same file twice even if > that's what the config says to do, I'd say it should do so with > dev/ino > comparisons, not pathname comparisons. Yes. Since the operation of calling realpath then open is definitively not atomic, hence the success of realpath(3) been no guarantee of the opening of the file, and since realpath has to go in kernel space also, so there is no process time gained, even if I maintain the not multiple inclusions for the first step, I will make the comparisons with lower level identifiers. -- Thierry Laronde http://www.kergis.com/ http://kertex.kergis.com/ Key fingerprint = 0FF7 E906 FBAF FE95 FD89 250D 52B1 AE95 6006 F40C
Re: inetd(8): continue or exit on error?
> Date: Mon, 29 May 2023 10:11:09 +0200 > From: tlaro...@polynum.com > > The question is what to do in case of a config file not found (this is > the initial problem: the realpath() return status is not tested and a > structure is inconditionnally added to a linked list with an unreachable > config file). I'm not sure inetd(8) has any business calling realpath in the first place. Can we just remove it and let it continue to use paths exactly as written in the config file?
Re: inetd(8): continue or exit on error?
>> I'm not sure inetd(8) has any business calling realpath in the first >> place. I agree. > It has to call realpath(3) since in order to not include several > times the same file, it makes strings comparaisons about names. If I as an admin write a config that tries to include the same file twice, whether via the same or different paths, I expect it to include the same file twice. If that leads to errors, that's on me. (It might or might not _always_ lead to an error, depending on how the include file syntax is defined to interact with the stateful aspects of the config language and what's in the config files.) If you really want to avoid including the same file twice even if that's what the config says to do, I'd say it should do so with dev/ino comparisons, not pathname comparisons. /~\ The ASCII Mouse \ / Ribbon Campaign X Against HTMLmo...@rodents-montreal.org / \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B
Re: inetd(8): continue or exit on error?
Le Mon, May 29, 2023 at 11:20:32AM +, Taylor R Campbell a écrit : > > Date: Mon, 29 May 2023 10:11:09 +0200 > > From: tlaro...@polynum.com > > > > The question is what to do in case of a config file not found (this is > > the initial problem: the realpath() return status is not tested and a > > structure is inconditionnally added to a linked list with an unreachable > > config file). > > I'm not sure inetd(8) has any business calling realpath in the first > place. Can we just remove it and let it continue to use paths exactly > as written in the config file? It has to call realpath(3) since in order to not include several times the same file, it makes strings comparaisons about names. Hence a file has to have some canonical form, not maskerading with differing forms or it will not be able to detect it is the same file or to detect a loop. -- Thierry Laronde http://www.kergis.com/ http://kertex.kergis.com/ Key fingerprint = 0FF7 E906 FBAF FE95 FD89 250D 52B1 AE95 6006 F40C
Re: inetd(8): continue or exit on error?
Le Mon, May 29, 2023 at 11:19:22AM +, Taylor R Campbell a écrit : > > Date: Mon, 29 May 2023 13:13:33 +0200 > > From: tlaro...@polynum.com > > > > I'm for: exit on any error. If we provide a way to check, that's the > > responsability of the administrator to check his config before trying to > > run the thing. > > Yes please. Should provide an option to check a configuration file > without starting inetd(8), make `service inetd check' do this, and > make `service inetd reload' (maybe also `service inetd restart') do > check first. This should be standard practice for all daemons. So: OK. I take this one (inetd) and will do it sometime in the week (at worst, you should here from me on Monday the 4th). -- Thierry Laronde http://www.kergis.com/ http://kertex.kergis.com/ Key fingerprint = 0FF7 E906 FBAF FE95 FD89 250D 52B1 AE95 6006 F40C
Re: inetd(8): continue or exit on error?
> Date: Mon, 29 May 2023 13:13:33 +0200 > From: tlaro...@polynum.com > > I'm for: exit on any error. If we provide a way to check, that's the > responsability of the administrator to check his config before trying to > run the thing. Yes please. Should provide an option to check a configuration file without starting inetd(8), make `service inetd check' do this, and make `service inetd reload' (maybe also `service inetd restart') do check first. This should be standard practice for all daemons.
Re: inetd(8): continue or exit on error?
Le Mon, May 29, 2023 at 11:43:33AM +0100, David Brownlee a écrit : > On Mon, 29 May 2023 at 11:38, Michael van Elst wrote: > > > > tlaro...@polynum.com writes: > > > > >If inetd is not running, if the administrator doesn't look at the logs, > > > > That's why people monitor services and logs and use manual or > > automated procedures to validate and deploy configuration changes. > > I have a slight preference towards 'exit on error', but both options > have completely valid use cases. > Could add a command line flag to determine whether to exit on error. > Is there any prior art in other BSDs/Linux? > One aspect to bear in mind is that inetd has been around ~forever, and > conventions have changed over time. > > > >At least, wouldn't it be worth to add a flag simply to parse and > > >validate the syntax without running the daemon? > > > > It's always a good thing to be able to validate a configuration. > > This absolutely sounds like a nice idea - could then be chained in > rc.d so 'inetd reload' could check the file and abort with an error > rather than reloading (similar to the recent sshd changes) > So seems others are OK at least for the checking without running (that seems to me the bare minimum to provide due to the complexity of the thing and the security implications). But I will once more plead for exit on error due to this feature (from the man page): ---8<--- To avoid the need to repeat listen addresses over and over again, listen addresses are inherited from line to line, and the listen address can be changed without defining a service by including a line containing just a listen-addr followed by a colon. --->8--- If one such line fails, all that will be parsed after (necessarily from another file) will potentially not listen where it should! Imagine what it can be if telnet is listening on 22! (One solution: clear the definition of the default address defhost when including another file.) I'm for: exit on any error. If we provide a way to check, that's the responsability of the administrator to check his config before trying to run the thing. I will also modify the man page, because including several times the same config files has not an undefined behavior (well, at the moment: yes, but only because of a blunder in the code): the file is only included once; every other appearance is skipped (it doesn't really work at the moment due to the bugs). Thing that I will not do now but could be done: when globbing is used and this results in several files, since the order of inclusion is random, verify that the files are orthogonal and exit with error if they are not. -- Thierry Laronde http://www.kergis.com/ http://kertex.kergis.com/ Key fingerprint = 0FF7 E906 FBAF FE95 FD89 250D 52B1 AE95 6006 F40C
Re: inetd(8): continue or exit on error?
On Mon, 29 May 2023 at 11:38, Michael van Elst wrote: > > tlaro...@polynum.com writes: > > >If inetd is not running, if the administrator doesn't look at the logs, > > That's why people monitor services and logs and use manual or > automated procedures to validate and deploy configuration changes. I have a slight preference towards 'exit on error', but both options have completely valid use cases. Could add a command line flag to determine whether to exit on error. Is there any prior art in other BSDs/Linux? One aspect to bear in mind is that inetd has been around ~forever, and conventions have changed over time. > >At least, wouldn't it be worth to add a flag simply to parse and > >validate the syntax without running the daemon? > > It's always a good thing to be able to validate a configuration. This absolutely sounds like a nice idea - could then be chained in rc.d so 'inetd reload' could check the file and abort with an error rather than reloading (similar to the recent sshd changes) Thanks David
Re: inetd(8): continue or exit on error?
tlaro...@polynum.com writes: >If inetd is not running, if the administrator doesn't look at the logs, That's why people monitor services and logs and use manual or automated procedures to validate and deploy configuration changes. >At least, wouldn't it be worth to add a flag simply to parse and >validate the syntax without running the daemon? It's always a good thing to be able to validate a configuration.
Re: inetd(8): continue or exit on error?
Le Mon, May 29, 2023 at 11:47:28AM +0200, tlaro...@polynum.com a écrit :
>
> And some log messages are problematic too:
>
> DPRINTCONF("Syntax error; Exiting '%s'", CONFIG);
>
> while it never exits: the function returns an invalid status code, and
> the process goes on...
For this, I mean "Quitting '%s'" is what it does. "Exiting" has an
implicit meaning when it comes to a program. A program exits, but you
don't "exit" a file.
--
Thierry Laronde
http://www.kergis.com/
http://kertex.kergis.com/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89 250D 52B1 AE95 6006 F40C
Re: inetd(8): continue or exit on error?
Le Mon, May 29, 2023 at 09:03:07AM -, Michael van Elst a écrit :
> tlaro...@polynum.com writes:
>
> >It seems to me, since these are services, that the failure to load a
> >config is critical enough (since the server may be then servicing what
> >was not intended to be serviced; the reverse is less problematic)
> >to exit at least on this error.
>
> inetd will service what is configured. Skipping an unparsable directive
> may have unwanted side effects, but so will a syntactically correct but
> otherwise wrong directive.
>
> The impact of not providing some services in case of a syntax error
> can easily be as problematic or dangerous as a wrongly configured service
> that the parser is unable to detect.
>
> If you want to protect against bad configurations, you could separate
> each service, e.g. chose a syntax without side effects or even use
> a config file per service.
We can not achieve "semantical" correctness: be able to "understand"
what the user wanted to do. But, at least, if a config file is not
reachable or if a directive is unparsable, there is obviously something
wrong.
If inetd is not running, if the administrator doesn't look at the logs,
he will very probably be reachable by phone or by email, and users will
be sure telling him that something is wrong...
At least, wouldn't it be worth to add a flag simply to parse and
validate the syntax without running the daemon?
And some log messages are problematic too:
DPRINTCONF("Syntax error; Exiting '%s'", CONFIG);
while it never exits: the function returns an invalid status code, and
the process goes on...
--
Thierry Laronde
http://www.kergis.com/
http://kertex.kergis.com/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89 250D 52B1 AE95 6006 F40C
Re: inetd(8): continue or exit on error?
tlaro...@polynum.com writes: >It seems to me, since these are services, that the failure to load a >config is critical enough (since the server may be then servicing what >was not intended to be serviced; the reverse is less problematic) >to exit at least on this error. inetd will service what is configured. Skipping an unparsable directive may have unwanted side effects, but so will a syntactically correct but otherwise wrong directive. The impact of not providing some services in case of a syntax error can easily be as problematic or dangerous as a wrongly configured service that the parser is unable to detect. If you want to protect against bad configurations, you could separate each service, e.g. chose a syntax without side effects or even use a config file per service.
inetd(8): continue or exit on error?
There are infelicities in /usr/src/usr.sbin/inetd/parse.c and I will send a PR with patches attached. The question is what to do in case of a config file not found (this is the initial problem: the realpath() return status is not tested and a structure is inconditionnally added to a linked list with an unreachable config file). It seems to me, since these are services, that the failure to load a config is critical enough (since the server may be then servicing what was not intended to be serviced; the reverse is less problematic) to exit at least on this error. What do others think? -- Thierry Laronde http://www.kergis.com/ http://kertex.kergis.com/ Key fingerprint = 0FF7 E906 FBAF FE95 FD89 250D 52B1 AE95 6006 F40C
