subject:"Re\: \[systemd\-devel\] Github systemd issue 6237"

On Mon, 10.07.17 17:45, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:

> On Mon, Jul 10, 2017 at 06:40:00PM +0200, Lennart Poettering wrote:
> > On Mon, 10.07.17 18:36, Lennart Poettering (lenn...@poettering.net) wrote:
> > 
> > > > After all (as other people said) systemd has no such requirements
> > > > itself. It is true that such user names are confusing and
> > > > non-portable, but if the local admin has or wants to have such an
> > > > account for whatever reason, we don't really care.
> > > 
> > > I don't think things are that simple. We do our user name validation
> > > in two places: for User=/Group= and for sysusers.d drop-ins. In both
> > > cases the setting may have the effect of registering users in the
> > > system user database (in the first case if DynamicUser= is used, in
> > > the latter case if the user doesn't exist yet), and I am pretty sure
> > > we shouldn't register users in the system user databases that aren't
> > > portable.
> > 
> > Or to say this differently: User=/Group=/sysusers.d shouldn't be
> > something you can create users with that for example ArchLinux'
> > useradd command wouldn't allow you to create.
> 
> I can see it both ways, but yeah, it never came up before and
> personally I never had the need (or even whim) to create a user that
> systemd would reject. So I'd like to #6300 to go in, and apart
> from that I'm happy with the status quo, and I merged #6321 now.

BTW, one more reference point to the discussion: shadow-utils upstream
enforces this regex apparently:

[a-z_][a-z0-9_-]*$?

The trailing $ thing appears to be a more recent addition, some
Windows thing. A minimum length of 1 is enforced, but apparently no
max length limit (neither _SC_LOGIN_NAME_MAX nor UT_NAMESIZE-1).

Fedora/RH deviate from that though and explicitly patch this out, replacing this
with the more relaxed regex mentioned earlier:

https://src.fedoraproject.org/cgit/rpms/shadow-utils.git/tree/shadow-4.1.5.1-goodname.patch

It appears our rules are hence pretty close to shadow-util's original
ones with the exeption of the max size limit and the Windows $ thing,
which really shouldn't apply to our system service users I figure.

Lennart

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

Re: [systemd-devel] Github systemd issue 6237

2017-07-10 Thread Zbigniew Jędrzejewski-Szmek

On Mon, Jul 10, 2017 at 06:40:00PM +0200, Lennart Poettering wrote:
> On Mon, 10.07.17 18:36, Lennart Poettering (lenn...@poettering.net) wrote:
> 
> > > After all (as other people said) systemd has no such requirements
> > > itself. It is true that such user names are confusing and
> > > non-portable, but if the local admin has or wants to have such an
> > > account for whatever reason, we don't really care.
> > 
> > I don't think things are that simple. We do our user name validation
> > in two places: for User=/Group= and for sysusers.d drop-ins. In both
> > cases the setting may have the effect of registering users in the
> > system user database (in the first case if DynamicUser= is used, in
> > the latter case if the user doesn't exist yet), and I am pretty sure
> > we shouldn't register users in the system user databases that aren't
> > portable.
> 
> Or to say this differently: User=/Group=/sysusers.d shouldn't be
> something you can create users with that for example ArchLinux'
> useradd command wouldn't allow you to create.

I can see it both ways, but yeah, it never came up before and
personally I never had the need (or even whim) to create a user that
systemd would reject. So I'd like to #6300 to go in, and apart
from that I'm happy with the status quo, and I merged #6321 now.

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

Re: [systemd-devel] Github systemd issue 6237

On Mon, 10.07.17 15:29, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:

> > On current Fedora, the current regex useradd enforces appears to be
> > this:
> > 
> > [a-zA-Z0-9._][a-zA-Z0-9._-]{0,30}[a-zA-Z0-9._-$]?
> > 
> > If I read things correctly at least... (the trailing $ appears to be a
> > special exception they added to be nice to Windows, dunno). And this
> > regex appears not to be configurable.
> 
> Maybe the logic should be reversed: instead of trying to *enforce*
> the most-strict name that works everywhere, just check that the name
> is between 1 and 31 characters is length and not numeric, and relax
> the restrictions on the exact characters in the user name, and *warn*
> if the user name has "strange" characters:
>Warning: user name "foo.bar" contains a dot which conflicts with chown 
> user.group syntax
>Warning: user name "0day" is not portable
>Warning: user name "-user" looks like an option ;)
> etc.
> 
> After all (as other people said) systemd has no such requirements
> itself. It is true that such user names are confusing and
> non-portable, but if the local admin has or wants to have such an
> account for whatever reason, we don't really care.

I don't think things are that simple. We do our user name validation
in two places: for User=/Group= and for sysusers.d drop-ins. In both
cases the setting may have the effect of registering users in the
system user database (in the first case if DynamicUser= is used, in
the latter case if the user doesn't exist yet), and I am pretty sure
we shouldn't register users in the system user databases that aren't
portable.

I mean, again, systemd is the one defining both interfaces:
User=/Group= as well as sysusers.d, and I am pretty sure we should
make sure when they are used they are used in a reasonably safe and
sound and portable way. We want that people can write unit files and
run them everywhere and they work reasonably well. I think it's not
too much to ask from people to follow some simple basic rules when
making use of User=/Group=. We aren't really taking away anything
there, we aren#t really strict on something that was previously less
strict, simply because User=/Group= isn't a sysvinit concept, it's
genuinely a systemd concept.

Hence, it's not really a matter of consuming weird stuff local admins
created. It's a lot more than that: we *create* that stuff, possibly
leaving it around for good, and unit files are supposed to be
portable between systems.

Compare this with logind: we do not validate the user name passed to
us from PAM for new login sessions the same way as we do for
User=/Group=/sysusers.d. In the logind/PAM case the interface isn't
designed by us, PAM doesn't validate its input particularly carefully,
and we should probably just accept what PAM accepts as we don't create
anything here: we just consume what's listed in the user database.

Logging more verbosely about this is good, and maybe making this fatal
instead of just a warning might be good too, but just permitting it
sounds wrong to me.

Lennart

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

Re: [systemd-devel] Github systemd issue 6237

On Mon, 10.07.17 18:36, Lennart Poettering (lenn...@poettering.net) wrote:

> > After all (as other people said) systemd has no such requirements
> > itself. It is true that such user names are confusing and
> > non-portable, but if the local admin has or wants to have such an
> > account for whatever reason, we don't really care.
> 
> I don't think things are that simple. We do our user name validation
> in two places: for User=/Group= and for sysusers.d drop-ins. In both
> cases the setting may have the effect of registering users in the
> system user database (in the first case if DynamicUser= is used, in
> the latter case if the user doesn't exist yet), and I am pretty sure
> we shouldn't register users in the system user databases that aren't
> portable.

Or to say this differently: User=/Group=/sysusers.d shouldn't be
something you can create users with that for example ArchLinux'
useradd command wouldn't allow you to create.

Lennart

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

Re: [systemd-devel] Github systemd issue 6237

2017-07-10 Thread Carlos Silva

On Mon, Jul 10, 2017 at 4:03 PM, Lennart Poettering
 wrote:
> On current Fedora, the current regex useradd enforces appears to be
> this:
>
> [a-zA-Z0-9._][a-zA-Z0-9._-]{0,30}[a-zA-Z0-9._-$]?

So, it *does* allow for usernames starting with numbers...
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Github systemd issue 6237

2017-07-10 Thread Zbigniew Jędrzejewski-Szmek

On Mon, Jul 10, 2017 at 05:03:09PM +0200, Lennart Poettering wrote:
> On Mon, 10.07.17 22:23, Michael Chapman (m...@very.puzzling.org) wrote:
> 
> > > Well, it took 3 years or so, until someone noticed the strict rules we
> > > enforce. I seriously doubt that naming system users in such unsafe
> > > ways is really that wide-spread usage.
> > 
> > That _could_ be because people that have previously used such a username
> > hadn't looked in their logs and noticed that the User= directive wasn't
> > being applied. :-)
> 
> Well: if you write a unit file, it's probably a good idea to test it
> once before deploying it. I mean, it's not that we silently skip stuff
> that was OK on sysvinit or so — that's because sysvinit of course never
> supported dropping user IDs for you, you always had to do it yourself.
> 
> If your service is dropping privs on its own it's of course completely
> between you and that service to name the user any way you like. But if
> you tell systemd to do this for you and for the service, we do some
> basic checking and validation, that's all. Or in other words: if I
> write a unit file on my laptop here, and it works, then systemd is
> supposed to give you the guarantee that it also works correctly on
> my server system I deploy this on, too (to some degree at least), even
> if it runs a slightly different Linux distro.
> 
> > So be it. I'm fine with us remaining in disagreement... I just wish I
> > understood exactly what the security implications are in allowing such
> > usernames. I know my colleagues are going to ask me about this, and being
> > able to point at something and say "oh yeah, it breaks this specifically"
> > would be really handy.
> 
> I am pretty sure it would be very welcome if somebody would do a
> proper investigation and collect the precise restrictions different
> systems in different versions enforce on naming users. And not just
> across distro versions and their user name regexes, but also across
> application software packages running on top of those distros. I am
> not aware of any sufficiently comprehensive list of this.
> 
> On current Fedora, the current regex useradd enforces appears to be
> this:
> 
> [a-zA-Z0-9._][a-zA-Z0-9._-]{0,30}[a-zA-Z0-9._-$]?
> 
> If I read things correctly at least... (the trailing $ appears to be a
> special exception they added to be nice to Windows, dunno). And this
> regex appears not to be configurable.

Maybe the logic should be reversed: instead of trying to *enforce*
the most-strict name that works everywhere, just check that the name
is between 1 and 31 characters is length and not numeric, and relax
the restrictions on the exact characters in the user name, and *warn*
if the user name has "strange" characters:
   Warning: user name "foo.bar" contains a dot which conflicts with chown 
user.group syntax
   Warning: user name "0day" is not portable
   Warning: user name "-user" looks like an option ;)
etc.

After all (as other people said) systemd has no such requirements
itself. It is true that such user names are confusing and
non-portable, but if the local admin has or wants to have such an
account for whatever reason, we don't really care.

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

Re: [systemd-devel] Github systemd issue 6237

On Mon, 10.07.17 22:23, Michael Chapman (m...@very.puzzling.org) wrote:

> > Well, it took 3 years or so, until someone noticed the strict rules we
> > enforce. I seriously doubt that naming system users in such unsafe
> > ways is really that wide-spread usage.
> 
> That _could_ be because people that have previously used such a username
> hadn't looked in their logs and noticed that the User= directive wasn't
> being applied. :-)

Well: if you write a unit file, it's probably a good idea to test it
once before deploying it. I mean, it's not that we silently skip stuff
that was OK on sysvinit or so — that's because sysvinit of course never
supported dropping user IDs for you, you always had to do it yourself.

If your service is dropping privs on its own it's of course completely
between you and that service to name the user any way you like. But if
you tell systemd to do this for you and for the service, we do some
basic checking and validation, that's all. Or in other words: if I
write a unit file on my laptop here, and it works, then systemd is
supposed to give you the guarantee that it also works correctly on
my server system I deploy this on, too (to some degree at least), even
if it runs a slightly different Linux distro.

> So be it. I'm fine with us remaining in disagreement... I just wish I
> understood exactly what the security implications are in allowing such
> usernames. I know my colleagues are going to ask me about this, and being
> able to point at something and say "oh yeah, it breaks this specifically"
> would be really handy.

I am pretty sure it would be very welcome if somebody would do a
proper investigation and collect the precise restrictions different
systems in different versions enforce on naming users. And not just
across distro versions and their user name regexes, but also across
application software packages running on top of those distros. I am
not aware of any sufficiently comprehensive list of this.

On current Fedora, the current regex useradd enforces appears to be
this:

[a-zA-Z0-9._][a-zA-Z0-9._-]{0,30}[a-zA-Z0-9._-$]?

If I read things correctly at least... (the trailing $ appears to be a
special exception they added to be nice to Windows, dunno). And this
regex appears not to be configurable.

Lennart

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

Re: [systemd-devel] Github systemd issue 6237

On Mon, 10.07.17 15:58, Lennart Poettering (lenn...@poettering.net) wrote:

> On Mon, 10.07.17 15:16, Jan Synacek (jsyna...@redhat.com) wrote:
> 
> > On Mon, Jul 10, 2017 at 12:42 PM, Lennart Poettering
> >  wrote:
> > > Now, because this is so weakly defined, we hence do not follow POSIX
> > > rules, but filter out more that might be dangerous. Specifically:
> > >
> > > 1. We do not permit empty usernames
> > > 2. We don't permit the first character to be numeric
> > >(This also filters out fully numeric user names)
> > > 3. We do not permit dots in usernames, neither at the beginning nor in
> > >the middle.
> > > 4. We do not permit "-" at the beginning of usernames (something which
> > >POSIX explicitly suggests, btw)
> > > 5. We require that the user name fits in the utmp user name field, so
> > >that we can always log properly about it.
> > 
> > Is this documented somewhere? If not, it would be great to have it
> > documented. I'm pretty sure that this exact paragraph would be ok.
> 
> There's a longer (and not entirely complete) comment about this in the
> sources, but other than that it's not explicitly documented.
> 
> If you prep a patch that adds this to the User=/Group= man page, this
> would certainly be welcome. However, it should be reworded, as we
> shouldn't say "We" there, and probably drop explicit references to
> POSIX and utmp there, and instead just dryly state the accepted
> character set + minimum and maximum string lengths.

I have posted a PR documenting this just now:

https://github.com/systemd/systemd/pull/6321

Lennart

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

Re: [systemd-devel] Github systemd issue 6237

2017-07-10 Thread Oliver Neukum

Am Montag, den 10.07.2017, 12:57 +0200 schrieb Reindl Harald:
> 
> Am 10.07.2017 um 12:55 schrieb Lennart Poettering:
> > 
> > 
> > The "nobody" user has special semantics on Linux: it's where things
> > are mapped to that can't be mapped otherwise. It's used by user
> > namspacing, by NFS and others. It's really not a good idea, to permit
> > random services to create and access files under that ID
> 
> and run it as root is a better idea?
> seriously?

This is moot. If you specify a user that is considered wrong for whatever
reason the service must fail. Whether we disagree about who decide what is
a valid user name also does not matter. You cannot substitute users for
any reason.

Regards
Oliver

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

Re: [systemd-devel] Github systemd issue 6237

On Mon, 10.07.17 15:16, Jan Synacek (jsyna...@redhat.com) wrote:

> On Mon, Jul 10, 2017 at 12:42 PM, Lennart Poettering
>  wrote:
> > Now, because this is so weakly defined, we hence do not follow POSIX
> > rules, but filter out more that might be dangerous. Specifically:
> >
> > 1. We do not permit empty usernames
> > 2. We don't permit the first character to be numeric
> >(This also filters out fully numeric user names)
> > 3. We do not permit dots in usernames, neither at the beginning nor in
> >the middle.
> > 4. We do not permit "-" at the beginning of usernames (something which
> >POSIX explicitly suggests, btw)
> > 5. We require that the user name fits in the utmp user name field, so
> >that we can always log properly about it.
> 
> Is this documented somewhere? If not, it would be great to have it
> documented. I'm pretty sure that this exact paragraph would be ok.

There's a longer (and not entirely complete) comment about this in the
sources, but other than that it's not explicitly documented.

If you prep a patch that adds this to the User=/Group= man page, this
would certainly be welcome. However, it should be reworded, as we
shouldn't say "We" there, and probably drop explicit references to
POSIX and utmp there, and instead just dryly state the accepted
character set + minimum and maximum string lengths.

Lennart

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

Re: [systemd-devel] Github systemd issue 6237

2017-07-10 Thread Jan Synacek

On Mon, Jul 10, 2017 at 12:42 PM, Lennart Poettering
 wrote:
> Now, because this is so weakly defined, we hence do not follow POSIX
> rules, but filter out more that might be dangerous. Specifically:
>
> 1. We do not permit empty usernames
> 2. We don't permit the first character to be numeric
>(This also filters out fully numeric user names)
> 3. We do not permit dots in usernames, neither at the beginning nor in
>the middle.
> 4. We do not permit "-" at the beginning of usernames (something which
>POSIX explicitly suggests, btw)
> 5. We require that the user name fits in the utmp user name field, so
>that we can always log properly about it.

Is this documented somewhere? If not, it would be great to have it
documented. I'm pretty sure that this exact paragraph would be ok.

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Github systemd issue 6237

2017-07-10 Thread Michael Chapman


On Mon, 10 Jul 2017, Lennart Poettering wrote:

On Mon, 10.07.17 21:15, Michael Chapman (m...@very.puzzling.org) wrote:


Now, I do think that systemd has the duty to complain about any system
user names outside of the safe range. Not only for security reasons,
but also for portability and compatibility reasons: I think we should
ensure that unit files remain portable, and hence we should try to
filter out early stuff that's unlikely going to work outside of the
local scope.


I'm curious as to what you consider portability and compatibility
here.


I want that units written on a system A are likely to work on a system
B. And this means that making use of concepts that are valid on A but
knowingly invalid on B is something we should complain loudly about.

Sure, there are always limitations to make things portable. But this
specific issue is an easy one, and a widely understood one (again:
google for it).


But there are less obviously bad usernames, because -- as you point out --
they're _actually in use already_. I myself already have systems with
usernames that begin with a digit; I don't want those systems to suddenly
break just because I update the Linux release to something that runs
systemd. (In practice they probably won't break, since I'm unlikely to write
system units for these users. But the principle of the matter
stands.)


Well, it took 3 years or so, until someone noticed the strict rules we
enforce. I seriously doubt that naming system users in such unsafe
ways is really that wide-spread usage.


That _could_ be because people that have previously used such a username 
hadn't looked in their logs and noticed that the User= directive wasn't 
being applied. :-)



Sorry, but I really can't see how forbidding usernames like "joe.hacker" or
"0day" improves security. As you said, they're perfectly valid
usernames.


Did I say that? I really don't think they are "perfectly valid"! They
are questionable on all levels. And if people use them for regular
users that's fine for them, but for system users I think stricter
requirements need to apply.

But anyway, I doubt we have to continue this here, we have different
understandings of security. I think validation is a good thing, and
filtering out dangerous strings early is a good thing.

People can always shoot themselves in the foot, and you have every
right to, but I really doubt this easy, well understood superficial
check is the right place to insist that the right to shooting yourself
in the foot is more important than the intention to secure things
down.

Lennart


So be it. I'm fine with us remaining in disagreement... I just wish I 
understood exactly what the security implications are in allowing such 
usernames. I know my colleagues are going to ask me about this, and being 
able to point at something and say "oh yeah, it breaks this specifically" 
would be really handy.

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

Re: [systemd-devel] Github systemd issue 6237

On Mon, 10.07.17 21:15, Michael Chapman (m...@very.puzzling.org) wrote:

> > Now, I do think that systemd has the duty to complain about any system
> > user names outside of the safe range. Not only for security reasons,
> > but also for portability and compatibility reasons: I think we should
> > ensure that unit files remain portable, and hence we should try to
> > filter out early stuff that's unlikely going to work outside of the
> > local scope.
> 
> I'm curious as to what you consider portability and compatibility
> here.

I want that units written on a system A are likely to work on a system
B. And this means that making use of concepts that are valid on A but
knowingly invalid on B is something we should complain loudly about.

Sure, there are always limitations to make things portable. But this
specific issue is an easy one, and a widely understood one (again:
google for it).

> But there are less obviously bad usernames, because -- as you point out --
> they're _actually in use already_. I myself already have systems with
> usernames that begin with a digit; I don't want those systems to suddenly
> break just because I update the Linux release to something that runs
> systemd. (In practice they probably won't break, since I'm unlikely to write
> system units for these users. But the principle of the matter
> stands.)

Well, it took 3 years or so, until someone noticed the strict rules we
enforce. I seriously doubt that naming system users in such unsafe
ways is really that wide-spread usage.

> Sorry, but I really can't see how forbidding usernames like "joe.hacker" or
> "0day" improves security. As you said, they're perfectly valid
> usernames.

Did I say that? I really don't think they are "perfectly valid"! They
are questionable on all levels. And if people use them for regular
users that's fine for them, but for system users I think stricter
requirements need to apply.

But anyway, I doubt we have to continue this here, we have different
understandings of security. I think validation is a good thing, and
filtering out dangerous strings early is a good thing.

People can always shoot themselves in the foot, and you have every
right to, but I really doubt this easy, well understood superficial
check is the right place to insist that the right to shooting yourself
in the foot is more important than the intention to secure things
down.

Lennart

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

Re: [systemd-devel] Github systemd issue 6237

2017-07-10 Thread Michael Chapman


On Mon, 10 Jul 2017, Lennart Poettering wrote:

On Thu, 06.07.17 13:21, Michael Chapman (m...@very.puzzling.org) wrote:


On Thu, 6 Jul 2017, Zbigniew Jędrzejewski-Szmek wrote:

On Thu, Jul 06, 2017 at 01:43:32AM +0200, Reindl Harald wrote:

well, it even don't look but pretend it can't while it does which is
the worst type of operations possible - as long as "adduser" of the
underlying OS accepts and create "0pointer" systemd has *no business
at all* to pretend it can't


Then it's good the that it doesn't ;)

# adduser 0pointer

adduser: Please enter a username matching the regular expression configured
via the NAME_REGEX configuration variable.  Use the `--force-badname'
option to relax this check or reconfigure NAME_REGEX.


I know you really only brought this up to counter Reindl's comment, but I
think it's important to point out that adduser's behaviour here is due to
its default configuration -- not due to any fundamental "problems" with
particular usernames. It's not clear why adduser's developers thought it was
a good default.

I guess what I'm saying is that saying "systemd should not support usernames
that start with a digit, since adduser doesn't" is problematic for at least
two reasons. First, adduser can be reconfigured by the sysadmin to allow
such usernames; and second, systemd places *fewer* restrictions on usernames
than adduser's default configuration. systemd allows usernames containing
uppercase letters and underscores, for instance.


Note one major difference between "adduser" and the unit file setting
"Unit=". The former is a tool you can create regular users with, while
the latter strictly applies to system users, as that's what system
services run as. And yes, different rules apply for system users than
for regular users.

And "0foobar" remains unportable and a bad idea, even if the user
bends his local system in the right way to make it accept it.


To summarize my thoughts on this matter, I think it's fine to restrict
usernames, but only for _very_ good reason. Specifically, we should not
justify such restrictions simply because they exist in one form or another
in other utilities. valid_user_group_name() currently disallows dots, for
instance, and while I recognize that using dots in a username can sometimes
be problematic, it is not in and of itself invalid. If other software can't
handle dots in usernames, that's their problem. libc can, and that's all
that's required to support it in order to use it in User= on most
systems.


I am sorry, but you and I have very different understanding of
computer security. I do believe it is essential to validate all input,
and stick to safe input wherever we can.


That is a misrepresentation of my viewpoint.

I _do_ think systemd should validate all input. I think my other posts in 
this thread make this clear: I want to see systemd complain noisily when 
unit validation fails.


However, I do not think systemd should validate input more than it needs 
to. Just because a particular value may (and _only_ may) cause problems 
downstream of systemd does not mean that systemd should outright forbid 
it. If it doesn't cause problems in systemd, it's not our business to 
prevent its use.



I understand that you'd like to remove input validation from the
systemd codebase, and I welcome you to patch your local systemd
version for it, but please understand that in systemd upstream this is
not how things can work. Sorry.

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

Re: [systemd-devel] Github systemd issue 6237

2017-07-10 Thread Michael Chapman

On Mon, 10 Jul 2017, Lennart Poettering wrote:

On Thu, 06.07.17 09:36, Michael Chapman (m...@very.puzzling.org) wrote:

User=0day fails a syntactic validation, not a semantic validation. systemd
never even checks to see whether the user exists when the unit is loaded.
And nor should it! The user must be allowed to not exist at unit-load time.

Contrary to some of the comments in this thread, there is no point in
systemd's operation where it goes "oh look, that user actually exists but
I'm going to pretend it doesn't".

Well, that's where we disagree: POSIX allows you to create a user
named "1000" that maps to a UID "1001". I find it very hard to swallow
that people actually defend that as a good concept, and claim that
systemd was negatively impacting security here by refusing this and
other ambiguous mappings.

I do think that User=1000 should always be treated as a UID, regardless of
whether there's a "1000" username on the system or not.

We need to validate the input we get, that's security 101. And POSIX
is *not* a good standard to strictly follow here and use for
validating user names, because it's *very* underdefined:

- It permits fully numeric names
- It makes no size restrictions (strictly reading the spec even permits
zero-length usernames!)
- It permits dots, which is conflicting with traditional chown syntax
- It permits dots at the beginning of usernames (which is dangerous
security-wise as this permits users to hide home directories)
- It permits naming users "-", which is often used as special "does
not apply" like value

Now, because this is so weakly defined, we hence do not follow POSIX
rules, but filter out more that might be dangerous. Specifically:

1. We do not permit empty usernames
2. We don't permit the first character to be numeric
(This also filters out fully numeric user names)
3. We do not permit dots in usernames, neither at the beginning nor in
the middle.
4. We do not permit "-" at the beginning of usernames (something which
POSIX explicitly suggests, btw)
5. We require that the user name fits in the utmp user name field, so
that we can always log properly about it.

I think 1, 4 and 5 are good things.

For 2 and 3, however, I'm not so sure.

Note that this isn't even as strict as other systems go. For example,
we do permit uppercase characters, and we do permit underscores.

Are these random rules we came up here? Nope. This actually matches
the various requirements enforced by the regex strings used by various
distros in one way or another. Moreover, if you type "valid linux user
name characters" into google, among the top links you'll find stuff
like this:

https://stackoverflow.com/questions/6949667/what-are-the-real-rules-for-linux-usernames-on-centos-6-and-rhel-6

or this:

https://unix.stackexchange.com/questions/157426/what-is-the-regex-to-validate-linux-users

Which generally suggest similar rules.

Now, I do think that systemd has the duty to complain about any system
user names outside of the safe range. Not only for security reasons,
but also for portability and compatibility reasons: I think we should
ensure that unit files remain portable, and hence we should try to
filter out early stuff that's unlikely going to work outside of the
local scope.

I'm curious as to what you consider portability and compatibility here.

There are some "obviously bad" usernames, like the empty string. We should
definitely prevent these from being used, and I do hope we can get systemd
to complain noisily should somebody try to use them.

But there are less obviously bad usernames, because -- as you point out --
they're _actually in use already_. I myself already have systems with
usernames that begin with a digit; I don't want those systems to suddenly
break just because I update the Linux release to something that runs
systemd. (In practice they probably won't break, since I'm unlikely to
write system units for these users. But the principle of the matter
stands.)

For these "less obviously bad" usernames, I think we should just rely on
getpwnam() doing its own validation -- that is, in systemd's child process
when it's spawning commands. After all, getpwnam() may have any kinds of
restrictions due to the system's NSS config... or it may not. We're not in
any place to second-guess what its limitations are.

So long as _we_ never treat a User= value incorrectly (which we don't, we
decide whether a it's a UID, a username, or an invalid value very early
on) everything should just work fine.

Obviously we should be encouraging software vendors and distributors to
use nice usernames. The portability and compatibility comes from them. But
if a sysadmin wants to run one of their own services as User=0day I don't
think we should be getting in the way of that -- they should know what the
portability issues are with that, if any.

We should also not pretend all was good if other tools make less
careful restrictions and permit usernames that

Re: [systemd-devel] Github systemd issue 6237

2017-07-10 Thread Reindl Harald




Am 10.07.2017 um 12:42 schrieb Lennart Poettering:

(I do accept though that it's a valid discussion whether systemd's
current behaviour of warning and skipping invalid User= rvalues is the
best choice, instead of erroring out completely.)


and *that* is the real point of the whole issue - if one runs "adduser" 
and the user got accepted and  place it into a unit file you can't 
expect that he notices that it get ignored by a random message in the syslog


and that is proven not to happen otherwise a ton of bugs and typos in 
systemd-units whould not have made it into stable updates when at least 
packagers would read their logs properly


well and with your sort of reactions to bugreports like 
https://bugzilla.redhat.com/show_bug.cgi?id=1072368 it's no wonder that 
some single warning get missed in the wood of messages

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

Re: [systemd-devel] Github systemd issue 6237

2017-07-10 Thread Reindl Harald

Am 10.07.2017 um 12:55 schrieb Lennart Poettering:

On Thu, 06.07.17 10:34, Reindl Harald (h.rei...@thelounge.net) wrote:

Am 06.07.2017 um 09:59 schrieb Jonathan de Boyne Pollard:

Reindl Harald:
  > at least fall back to “nobody”

Jonathan de Boyne Pollard:
  > That idea is wrong.
  >
  > https://news.ycombinator.com/item?id=14681377#14682059

Reindl Harald:
  > better than a stupid [...]

Not really, no.  It's the same category of error, in fact: substituting
an account other than the one that the system administrator explicitly
said to drop privileges to.

yes, it's both nonsense, but when i only have the option to choose between
two types of nonsense i take the one which don't give root permissions

The "nobody" user has special semantics on Linux: it's where things
are mapped to that can't be mapped otherwise. It's used by user
namspacing, by NFS and others. It's really not a good idea, to permit
random services to create and access files under that ID

and run it as root is a better idea?
seriously?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Github systemd issue 6237

On Thu, 06.07.17 10:34, Reindl Harald (h.rei...@thelounge.net) wrote:

> 
> 
> Am 06.07.2017 um 09:59 schrieb Jonathan de Boyne Pollard:
> > Reindl Harald:
> >  > at least fall back to “nobody”
> > 
> > Jonathan de Boyne Pollard:
> >  > That idea is wrong.
> >  >
> >  > https://news.ycombinator.com/item?id=14681377#14682059
> > 
> > Reindl Harald:
> >  > better than a stupid [...]
> > 
> > Not really, no.  It's the same category of error, in fact: substituting
> > an account other than the one that the system administrator explicitly
> > said to drop privileges to.
> 
> yes, it's both nonsense, but when i only have the option to choose between
> two types of nonsense i take the one which don't give root permissions

The "nobody" user has special semantics on Linux: it's where things
are mapped to that can't be mapped otherwise. It's used by user
namspacing, by NFS and others. It's really not a good idea, to permit
random services to create and access files under that ID.

Lennart

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

Re: [systemd-devel] Github systemd issue 6237

On Thu, 06.07.17 09:36, Michael Chapman (m...@very.puzzling.org) wrote:

> User=0day fails a syntactic validation, not a semantic validation. systemd
> never even checks to see whether the user exists when the unit is loaded.
> And nor should it! The user must be allowed to not exist at unit-load time.
>
> Contrary to some of the comments in this thread, there is no point in
> systemd's operation where it goes "oh look, that user actually exists but
> I'm going to pretend it doesn't".

We need to validate the input we get, that's security 101. And POSIX
is *not* a good standard to strictly follow here and use for
validating user names, because it's *very* underdefined:

Now, because this is so weakly defined, we hence do not follow POSIX
rules, but filter out more that might be dangerous. Specifically:

Note that this isn't even as strict as other systems go. For example,
we do permit uppercase characters, and we do permit underscores.

https://stackoverflow.com/questions/6949667/what-are-the-real-rules-for-linux-usernames-on-centos-6-and-rhel-6

or this:

https://unix.stackexchange.com/questions/157426/what-is-the-regex-to-validate-linux-users

Which generally suggest similar rules.

We should also not pretend all was good if other tools make less
careful restrictions and permit usernames that cause ambiguities. Yes,
a lot of software is very sloppy with validating input, and yes, some
adduser/useradd implementations allow you to create a user called
"1000", but I am very sure that their sloppiness should not leak into
our codebase.

Also, do note that system users are different concepts than regular
users: system users are concepts required for system services which
are usually put together by developers, packagers and administrators
who hopefully understand these issues to some point and pick good
names instead.

And of course, note that systemd is open source. if you don't like the
restrictions we make, you can patch them out (or even use some other
project). But again, I think it's our duty to help build a more secure
ecosystem (and we do that by validating our input, by running systemd
services in sandboxes and so on), and hence these restrictions really
need to stay.

(I do accept though that it's a valid discussion whether systemd's
current behaviour of warning and skipping invalid User= rvalues is the
best choice, instead of erroring out completely.)

Lennart

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

Re: [systemd-devel] Github systemd issue 6237

On Thu, 06.07.17 13:21, Michael Chapman (m...@very.puzzling.org) wrote:

> On Thu, 6 Jul 2017, Zbigniew Jędrzejewski-Szmek wrote:
> > On Thu, Jul 06, 2017 at 01:43:32AM +0200, Reindl Harald wrote:
> > > well, it even don't look but pretend it can't while it does which is
> > > the worst type of operations possible - as long as "adduser" of the
> > > underlying OS accepts and create "0pointer" systemd has *no business
> > > at all* to pretend it can't
> > 
> > Then it's good the that it doesn't ;)
> > 
> > # adduser 0pointer
> > 
> > adduser: Please enter a username matching the regular expression configured
> > via the NAME_REGEX configuration variable.  Use the `--force-badname'
> > option to relax this check or reconfigure NAME_REGEX.
> 
> I know you really only brought this up to counter Reindl's comment, but I
> think it's important to point out that adduser's behaviour here is due to
> its default configuration -- not due to any fundamental "problems" with
> particular usernames. It's not clear why adduser's developers thought it was
> a good default.
> 
> I guess what I'm saying is that saying "systemd should not support usernames
> that start with a digit, since adduser doesn't" is problematic for at least
> two reasons. First, adduser can be reconfigured by the sysadmin to allow
> such usernames; and second, systemd places *fewer* restrictions on usernames
> than adduser's default configuration. systemd allows usernames containing
> uppercase letters and underscores, for instance.

Note one major difference between "adduser" and the unit file setting
"Unit=". The former is a tool you can create regular users with, while
the latter strictly applies to system users, as that's what system
services run as. And yes, different rules apply for system users than
for regular users.

And "0foobar" remains unportable and a bad idea, even if the user
bends his local system in the right way to make it accept it.

> To summarize my thoughts on this matter, I think it's fine to restrict
> usernames, but only for _very_ good reason. Specifically, we should not
> justify such restrictions simply because they exist in one form or another
> in other utilities. valid_user_group_name() currently disallows dots, for
> instance, and while I recognize that using dots in a username can sometimes
> be problematic, it is not in and of itself invalid. If other software can't
> handle dots in usernames, that's their problem. libc can, and that's all
> that's required to support it in order to use it in User= on most
> systems.

I am sorry, but you and I have very different understanding of
computer security. I do believe it is essential to validate all input,
and stick to safe input wherever we can.

I understand that you'd like to remove input validation from the
systemd codebase, and I welcome you to patch your local systemd
version for it, but please understand that in systemd upstream this is
not how things can work. Sorry.

Lennart

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

Re: [systemd-devel] Github systemd issue 6237

2017-07-08 Thread Reindl Harald




Am 08.07.2017 um 08:29 schrieb Michael Chapman:

On Sat, 8 Jul 2017, Kai Krakow wrote:

Am Sat, 8 Jul 2017 08:05:44 +0200
schrieb Kai Krakow :


Am Sat, 8 Jul 2017 11:39:02 +1000 (AEST)
schrieb Michael Chapman :


On Sat, 8 Jul 2017, Kai Krakow wrote:
[...]

The bug here is that a leading number will "convert" to the number
and it actually runs with the UID specified that way: 0day = 0,
7days = 7.


No, this is not the case. Only all-digit User= values are treated
as


Then behavior is "correct".


Or in other words: The original bug description is wrong. The bug isn't
with non-existent users. That works fine.


That is correct.

There's a lot of misinformation floating around with this issue -- 
there's a tendency amongst some parts of the Linux community to jump on 
the systemd-bashing bandwagon without fully understanding the problems. 
The best thing we can do to counter this is to ascertain the facts, 
decide what if anything needs to be fixed, and discuss the best way to 
move forward with that


why in the world do you not read the bugreport itself?
https://github.com/systemd/systemd/issues/6237

the decription is *not* about non-existent users so top spread FUD about 
"systemd-bashing bandwagon without fully understanding the problems" 
when you even don't read the bugreport you are talking about

_

how does that sound like talking about non-existing users?

I searched google and found that it was not right to named a linux user 
with 0day, it should satisfy "^[a-z][-a-z0-9]*\$ , but when I use xinted 
to start the service, it can handle the previlege rightly with linux 
user 0day

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

Re: [systemd-devel] Github systemd issue 6237

2017-07-08 Thread Michael Chapman


On Sat, 8 Jul 2017, Kai Krakow wrote:

Am Sat, 8 Jul 2017 08:05:44 +0200
schrieb Kai Krakow :


Am Sat, 8 Jul 2017 11:39:02 +1000 (AEST)
schrieb Michael Chapman :


On Sat, 8 Jul 2017, Kai Krakow wrote:
[...]

The bug here is that a leading number will "convert" to the number
and it actually runs with the UID specified that way: 0day = 0,
7days = 7.


No, this is not the case. Only all-digit User= values are treated
as


Then behavior is "correct".


Or in other words: The original bug description is wrong. The bug isn't
with non-existent users. That works fine.


That is correct.

There's a lot of misinformation floating around with this issue -- there's 
a tendency amongst some parts of the Linux community to jump on the 
systemd-bashing bandwagon without fully understanding the problems. The 
best thing we can do to counter this is to ascertain the facts, decide 
what if anything needs to be fixed, and discuss the best way to move 
forward with that.

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

Re: [systemd-devel] Github systemd issue 6237

2017-07-08 Thread Kai Krakow

Am Sat, 8 Jul 2017 08:05:44 +0200
schrieb Kai Krakow :

> Am Sat, 8 Jul 2017 11:39:02 +1000 (AEST)
> schrieb Michael Chapman :
> 
> > On Sat, 8 Jul 2017, Kai Krakow wrote:
> > [...]  
> > > The bug here is that a leading number will "convert" to the number
> > > and it actually runs with the UID specified that way: 0day = 0,
> > > 7days = 7.
> > 
> > No, this is not the case. Only all-digit User= values are treated
> > as  
> 
> Then behavior is "correct".

Or in other words: The original bug description is wrong. The bug isn't
with non-existent users. That works fine.


-- 
Regards,
Kai

Replies to list-only preferred.

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

Re: [systemd-devel] Github systemd issue 6237

2017-07-08 Thread Kai Krakow

Am Sat, 8 Jul 2017 11:39:02 +1000 (AEST)
schrieb Michael Chapman :

> On Sat, 8 Jul 2017, Kai Krakow wrote:
> [...]
> > The bug here is that a leading number will "convert" to the number
> > and it actually runs with the UID specified that way: 0day = 0,
> > 7days = 7.  
> 
> No, this is not the case. Only all-digit User= values are treated as

Then behavior is "correct".


-- 
Regards,
Kai

Replies to list-only preferred.

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

Re: [systemd-devel] Github systemd issue 6237

2017-07-07 Thread Reindl Harald




Am 07.07.2017 um 21:55 schrieb Kai Krakow:

Am Tue, 4 Jul 2017 21:23:01 + (UTC)
schrieb Alexander Bisogiannis :


On Tue, 04 Jul 2017 17:21:01 +, Zbigniew Jędrzejewski-Szmek wrote:


If you need root permissions to create a unit, then it's not a
security issue. An annoyance at most.


The fact that you need to be root to create a unit file is irrelevant.

Systemd is running a service as a different user to what is defined
in the unit file.
This is a bug and a local security issue, especially because it will
run said service as root.

It might not warrant a CVE, although in my line of work this is
considered a security issue, but it is a bug and needs fixing.

The fix is to refuse to run the service, period.


There's nothing to fix because it already works that way: If you give
it a valid user name that does not exists, the system refuses to start
the unit with "user not found"


and if you give a invalid username it has to do the same - PERIOD

systemd is directly after the kernel the most important and lowest level 
stuff on a setup and hence can't be handeled like some random stuff

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

Re: [systemd-devel] Github systemd issue 6237

2017-07-07 Thread Michael Chapman


On Sat, 8 Jul 2017, Kai Krakow wrote:
[...]

The bug here is that a leading number will "convert" to the number and
it actually runs with the UID specified that way: 0day = 0, 7days = 7.


No, this is not the case. Only all-digit User= values are treated as UIDs.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Github systemd issue 6237

2017-07-07 Thread Kai Krakow

Am Tue, 4 Jul 2017 21:23:01 + (UTC)
schrieb Alexander Bisogiannis :

> On Tue, 04 Jul 2017 17:21:01 +, Zbigniew Jędrzejewski-Szmek wrote:
> 
> > If you need root permissions to create a unit, then it's not a
> > security issue. An annoyance at most.  
> 
> The fact that you need to be root to create a unit file is irrelevant.
> 
> Systemd is running a service as a different user to what is defined
> in the unit file. 
> This is a bug and a local security issue, especially because it will
> run said service as root.
> 
> It might not warrant a CVE, although in my line of work this is 
> considered a security issue, but it is a bug and needs fixing.
> 
> The fix is to refuse to run the service, period.

There's nothing to fix because it already works that way: If you give
it a valid user name that does not exists, the system refuses to start
the unit with "user not found".

If you give it an invalid user name (leading digits, disallowed
characters), then it complains with a warning and continues to run as
if you specified no user (thus it runs as root).

The bug here is that a leading number will "convert" to the number and
it actually runs with the UID specified that way: 0day = 0, 7days = 7.
But this is not really a security concern as only root can create units
that contain a user - except you open exploits for that: But then you
have other problems then that.

Conclusion: Not a security issue. If you trick an admin into accepting
unit files without validating the contents, you are having other issues
than an issue with systemd.

> Is there any other place I can go to open a bug, or do I need to go
> to the upstream "vendor" bugzila?

Maybe open a new issue and suggest that the current "conversion" should
be upgraded from a warning to a fatal error. Give examples of behavior
you get and behavior you expect. Also give counter examples of behavior
that works as you expect. Don't try to troll, after all it's the
developers forum and it only works if people stay with the facts.
Otherwise it becomes unusable, nobody wants that.

Best way to get it into one of the next releases is to prepare a pull
request that fixes the issue.

-- 
Regards,
Kai

Replies to list-only preferred.

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

Re: [systemd-devel] Github systemd issue 6237

2017-07-06 Thread Zbigniew Jędrzejewski-Szmek

On Wed, Jul 05, 2017 at 08:10:15PM +1000, Michael Chapman wrote:
> On Wed, 5 Jul 2017, Colin Guthrie wrote:
> >Reindl Harald wrote on 04/07/17 19:50:
> >>>When new configuration options are added, the same unit file can
> >>>almost always be used with older systemd, and it'll just warn & ignore
> >>>the parts it doesn't understand. Similarly, various configuration
> >>>options might be unavailable on some architectures and with some
> >>>compilation options. The current behaviour of warn provides
> >>>for "soft degradation" in those cases.
> >>
> >>frankly a new option on the left side is a completly different thing
> >>than a invalid value - just silently continue with invalid values of
> >>existing options is playing a danergous game in a crucial component like
> >>systemd
> >
> >It's a rare thing :p but I have to agree with you here!
> >
> >I'd say if "User=-notauser" then silently failing and using root is
> >acceptable as per the usual semantics of "- prefix suppresses errors",
> >but "User=notauser" should fail IMO.
> 
> I'm pretty sure you'll find that it does. Specifically, it will fail
> when the child process for the command being executed attempts to
> map the username to a UID.
> 
> The issue being discussed here is that systemd considers "0day" to
> be _syntactically_ invalid for a username. See the
> valid_user_group_name() function in basic/user-util.c.
> 
> (In my opinion, we shouldn't be this restrictive. POSIX permits
> usernames that start with a digit, and just because certain other
> utilities can't deal with them doesn't mean systemd need forbid
> them.)
> 
> So the directive fails the syntactic check for User=, just like
> Zbigniew's example of User="my name is pretty!".
> 
> I do think we ought to have a list of "critical" directives, such
> that any syntactic error in those directives causes the unit load
> state to be "error". For better or worse, people simply don't look
> at logs, so they'll never notice that important directives in their
> units are being ignored.

https://github.com/systemd/systemd/pull/6300

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

Re: [systemd-devel] Github systemd issue 6237

2017-07-06 Thread Reindl Harald




Am 06.07.2017 um 09:59 schrieb Jonathan de Boyne Pollard:

Reindl Harald:
 > at least fall back to “nobody”

Jonathan de Boyne Pollard:
 > That idea is wrong.
 >
 > https://news.ycombinator.com/item?id=14681377#14682059

Reindl Harald:
 > better than a stupid [...]

Not really, no.  It's the same category of error, in fact: substituting 
an account other than the one that the system administrator explicitly 
said to drop privileges to.


yes, it's both nonsense, but when i only have the option to choose 
between two types of nonsense i take the one which don't give root 
permissions

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

Re: [systemd-devel] Github systemd issue 6237

2017-07-06 Thread Jonathan de Boyne Pollard

Reindl Harald:
> at least fall back to “nobody”

Jonathan de Boyne Pollard:
> That idea is wrong.
>
> https://news.ycombinator.com/item?id=14681377#14682059

Reindl Harald:
> better than a stupid [...]

Not really, no.  It's the same category of error, in fact: substituting an
account other than the one that the system administrator explicitly said to drop
privileges to.___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Github systemd issue 6237

2017-07-06 Thread Oliver Neukum

Am Mittwoch, den 05.07.2017, 20:10 +1000 schrieb Michael Chapman:
> I'm pretty sure you'll find that it does. Specifically, it will fail when 
> the child process for the command being executed attempts to map the 
> username to a UID.
> 
> The issue being discussed here is that systemd considers "0day" to be 
> _syntactically_ invalid for a username. See the valid_user_group_name() 
> function in basic/user-util.c.

What a valid username is, is defined in POSIX. If systemd wants
to support only a subset of those, fair enough, but then it must
totally fail anything that uses such a name. Falling back to
another user is not acceptable. It being root is especially bad.

Regards
Oliver

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

Re: [systemd-devel] Github systemd issue 6237


On Thu, 6 Jul 2017, Zbigniew Jędrzejewski-Szmek wrote:

On Thu, Jul 06, 2017 at 01:43:32AM +0200, Reindl Harald wrote:

well, it even don't look but pretend it can't while it does which is
the worst type of operations possible - as long as "adduser" of the
underlying OS accepts and create "0pointer" systemd has *no business
at all* to pretend it can't


Then it's good the that it doesn't ;)

# adduser 0pointer

adduser: Please enter a username matching the regular expression configured
via the NAME_REGEX configuration variable.  Use the `--force-badname'
option to relax this check or reconfigure NAME_REGEX.


I know you really only brought this up to counter Reindl's comment, but I 
think it's important to point out that adduser's behaviour here is due to 
its default configuration -- not due to any fundamental "problems" with 
particular usernames. It's not clear why adduser's developers thought it 
was a good default.


I guess what I'm saying is that saying "systemd should not support 
usernames that start with a digit, since adduser doesn't" is problematic 
for at least two reasons. First, adduser can be reconfigured by the 
sysadmin to allow such usernames; and second, systemd places *fewer* 
restrictions on usernames than adduser's default configuration. systemd 
allows usernames containing uppercase letters and underscores, for 
instance.


To summarize my thoughts on this matter, I think it's fine to restrict 
usernames, but only for _very_ good reason. Specifically, we should not 
justify such restrictions simply because they exist in one form or another 
in other utilities. valid_user_group_name() currently disallows dots, for 
instance, and while I recognize that using dots in a username can 
sometimes be problematic, it is not in and of itself invalid. If other 
software can't handle dots in usernames, that's their problem. libc can, 
and that's all that's required to support it in order to use it in User= 
on most systems.


But whether or not usernames are restricted, it's very important to alert 
the sysadmin to the fact their unit file isn't being interpreted the way 
they wrote it.___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Github systemd issue 6237

2017-07-05 Thread Zbigniew Jędrzejewski-Szmek

On Thu, Jul 06, 2017 at 01:43:32AM +0200, Reindl Harald wrote:
> 
> 
> Am 06.07.2017 um 01:36 schrieb Michael Chapman:
> >Note that the semantic validations you're talking about here --
> >things like "does the user exist?" -- are _not_ preemptive. They
> >are fatal: the child process will exit unsuccessfully as the
> >command is executed if the settings will not be able to be
> >applied.
> >
> >User=0day fails a syntactic validation, not a semantic validation.
> >systemd never even checks to see whether the user exists when the
> >unit is loaded. And nor should it! The user must be allowed to not
> >exist at unit-load time.
> >
> >Contrary to some of the comments in this thread, there is no point
> >in systemd's operation where it goes "oh look, that user actually
> >exists but I'm going to pretend it doesn't"
> 
> well, it even don't look but pretend it can't while it does which is
> the worst type of operations possible - as long as "adduser" of the
> underlying OS accepts and create "0pointer" systemd has *no business
> at all* to pretend it can't

Then it's good the that it doesn't ;)

# adduser 0pointer

adduser: Please enter a username matching the regular expression configured
via the NAME_REGEX configuration variable.  Use the `--force-badname'
option to relax this check or reconfigure NAME_REGEX.

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

Re: [systemd-devel] Github systemd issue 6237




Am 06.07.2017 um 01:36 schrieb Michael Chapman:
Note that the semantic validations you're talking about here -- things 
like "does the user exist?" -- are _not_ preemptive. They are fatal: the 
child process will exit unsuccessfully as the command is executed if the 
settings will not be able to be applied.


User=0day fails a syntactic validation, not a semantic validation. 
systemd never even checks to see whether the user exists when the unit 
is loaded. And nor should it! The user must be allowed to not exist at 
unit-load time.


Contrary to some of the comments in this thread, there is no point in 
systemd's operation where it goes "oh look, that user actually exists 
but I'm going to pretend it doesn't"


well, it even don't look but pretend it can't while it does which is the 
worst type of operations possible - as long as "adduser" of the 
underlying OS accepts and create "0pointer" systemd has *no business at 
all* to pretend it can't

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

Re: [systemd-devel] Github systemd issue 6237


On Thu, 6 Jul 2017, Felipe Sateler wrote:

On Tue, 04 Jul 2017 18:39:15 +, Zbigniew Jędrzejewski-Szmek wrote:


Essentially, User=0day is the same as Usre=0day and the same as User="my
name is pretty!".


I think this is the root of the disagreement. Systemd tries to allow
units written for version X to run on versions earlier than X. I think
that is a good idea, and I don't think anyone is claiming it isn't. Which
is why invalid lvalues should be warned about but ignored, and I don't
think anyone is disputing this.

OTOH, invalid rvalues are a different thing: systemd recognizes the
setting it is trying to apply, but it can't. I think the argument that
systemd should fail the unit here is strong. Or at least for a certain
subset of the settings.



I do agree that we might want to completely reject unit files when some
crucial lines fail to parse, for example ExecStart or WorkingDirectory
or User, but it's not as obvious as one would think at first.


Indeed. Marking invalid rvalues as failing the unit does bring backwards-
compatibility issues. Suppose systemd X+1 adds a new @ shortcut for
SystemCallFilter: should systemd X fail a unit that uses it? The problem
would be the same as with User=0day, as the service would run with higher
privileges than expected.

A possibility would be to make systemd have its preemptive validations[1]
be fatal. Setting `User=idontexist` would then be equal to `User=whoa!`,
because neither of the usernames (should) exist.

[1] That is, validations for things it does not control itself. systemd
does not control the username format, the uid range, allowed directories,
etc, but rather validates against an external standard. That is, assume
that things that don't validate would fail at application time with a non-
ignorable EINVAL. This is different from things it defines itself, such
as @ settings for SystemCallFilter.


Note that the semantic validations you're talking about here -- things 
like "does the user exist?" -- are _not_ preemptive. They are fatal: the 
child process will exit unsuccessfully as the command is executed if the 
settings will not be able to be applied.


User=0day fails a syntactic validation, not a semantic validation. systemd 
never even checks to see whether the user exists when the unit is loaded. 
And nor should it! The user must be allowed to not exist at unit-load 
time.


Contrary to some of the comments in this thread, there is no point in 
systemd's operation where it goes "oh look, that user actually exists but 
I'm going to pretend it doesn't".___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Github systemd issue 6237




Am 05.07.2017 um 20:34 schrieb Jonathan de Boyne Pollard:

Reindl Harald:


at least fall back to "nobody"


That idea is wrong.

https://news.ycombinator.com/item?id=14681377#14682059


better than a stupid "i fall back to root because i think i make the 
rules and not the underlying operating system where this user in fact 
exists"

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

Re: [systemd-devel] Github systemd issue 6237

2017-07-05 Thread Jonathan de Boyne Pollard

Reindl Harald:

> 
> at least fall back to "nobody"
> 

That idea is wrong.

https://news.ycombinator.com/item?id=14681377#14682059___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Github systemd issue 6237

2017-07-05 Thread Felipe Sateler

On Tue, 04 Jul 2017 18:39:15 +, Zbigniew Jędrzejewski-Szmek wrote:

> Essentially, User=0day is the same as Usre=0day and the same as User="my
> name is pretty!".

I think this is the root of the disagreement. Systemd tries to allow 
units written for version X to run on versions earlier than X. I think 
that is a good idea, and I don't think anyone is claiming it isn't. Which 
is why invalid lvalues should be warned about but ignored, and I don't 
think anyone is disputing this.

OTOH, invalid rvalues are a different thing: systemd recognizes the 
setting it is trying to apply, but it can't. I think the argument that 
systemd should fail the unit here is strong. Or at least for a certain 
subset of the settings.

> 
> I do agree that we might want to completely reject unit files when some
> crucial lines fail to parse, for example ExecStart or WorkingDirectory
> or User, but it's not as obvious as one would think at first.

Indeed. Marking invalid rvalues as failing the unit does bring backwards-
compatibility issues. Suppose systemd X+1 adds a new @ shortcut for 
SystemCallFilter: should systemd X fail a unit that uses it? The problem 
would be the same as with User=0day, as the service would run with higher 
privileges than expected.

A possibility would be to make systemd have its preemptive validations[1] 
be fatal. Setting `User=idontexist` would then be equal to `User=whoa!`, 
because neither of the usernames (should) exist.

[1] That is, validations for things it does not control itself. systemd 
does not control the username format, the uid range, allowed directories, 
etc, but rather validates against an external standard. That is, assume 
that things that don't validate would fail at application time with a non-
ignorable EINVAL. This is different from things it defines itself, such 
as @ settings for SystemCallFilter.

-- 
Saludos,
Felipe Sateler

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

Re: [systemd-devel] Github systemd issue 6237




Am 05.07.2017 um 12:32 schrieb Michael Chapman:

In Wed, 5 Jul 2017, Reindl Harald wrote:

 The issue being discussed here is that systemd considers "0day" to be
 _syntactically_ invalid for a username. See the valid_user_group_name()
 function in basic/user-util.c.


yes and hence it should FAIL the service and not behave silently like 
the left side of a param is unknown on a older version - a invalid 
VALUE in a config has to fail until it's makred with a dash to silent 
be ignored in case of errors


That's really not what the dash means.

In the various Exec*= directives, a dash means that the command is 
allowed to fail. Similarly, in WorkingDirectory= the dash means the 
directory need not exist.


I do not believe there are any cases where a dash is used to side-step 
_syntactic_ validation, nor do I think there should be.


Really, you should just think of the dashes as being part of the syntax 
for the directives that support them.


As far as directives like User= go, there _may_ be a use for dash to 
mean "do not change UIDs if the username turns out to not actually 
exist"... but I would strongly advise against it


better than silently ignore a directive and run code as root which was 
never intended to run as root because of questionable "we know better 
what is valid then the underlying system which allowed to create that 
user" - do it proper or fail but not start code as root and pretend you 
are right


such issues closed as "not a bug" as well as 
https://github.com/systemd/systemd/issues/5644 with comments like "Yeah, 
it's a UNIX pitfall, but "rm -rf /foo/.*" will work the exact same way, 
no?" which is not true for years make me afraid because it is a "to 
begin with we are always right" attitude

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

Re: [systemd-devel] Github systemd issue 6237

In Wed, 5 Jul 2017, Reindl Harald wrote:

Am 05.07.2017 um 12:10 schrieb Michael Chapman:

 On Wed, 5 Jul 2017, Colin Guthrie wrote:
>  Reindl Harald wrote on 04/07/17 19:50:
> > >  When new configuration options are added, the same unit file can
> > >  almost always be used with older systemd, and it'll just warn & 
> > >  ignore

> > >  the parts it doesn't understand. Similarly, various configuration
> > >  options might be unavailable on some architectures and with some
> > >  compilation options. The current behaviour of warn provides
> > >  for "soft degradation" in those cases.
> > 
> >  frankly a new option on the left side is a completly different thing

> >  than a invalid value - just silently continue with invalid values of
> >  existing options is playing a danergous game in a crucial component 
> >  like

> >  systemd
> 
>  It's a rare thing :p but I have to agree with you here!
> 
>  I'd say if "User=-notauser" then silently failing and using root is

>  acceptable as per the usual semantics of "- prefix suppresses errors",
>  but "User=notauser" should fail IMO.

 I'm pretty sure you'll find that it does. Specifically, it will fail when
 the child process for the command being executed attempts to map the
 username to a UID.

 The issue being discussed here is that systemd considers "0day" to be
 _syntactically_ invalid for a username. See the valid_user_group_name()
 function in basic/user-util.c.

yes and hence it should FAIL the service and not behave silently like the 
left side of a param is unknown on a older version - a invalid VALUE in a 
config has to fail until it's makred with a dash to silent be ignored in case 
of errors

That's really not what the dash means.

In the various Exec*= directives, a dash means that the command is allowed 
to fail. Similarly, in WorkingDirectory= the dash means the directory need 
not exist.

I do not believe there are any cases where a dash is used to side-step 
_syntactic_ validation, nor do I think there should be.

Really, you should just think of the dashes as being part of the syntax 
for the directives that support them.

As far as directives like User= go, there _may_ be a use for dash to mean 
"do not change UIDs if the username turns out to not actually exist"... 
but I would strongly advise against it.

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

Re: [systemd-devel] Github systemd issue 6237


On Wed, 5 Jul 2017, Colin Guthrie wrote:

Reindl Harald wrote on 04/07/17 19:50:

When new configuration options are added, the same unit file can
almost always be used with older systemd, and it'll just warn & ignore
the parts it doesn't understand. Similarly, various configuration
options might be unavailable on some architectures and with some
compilation options. The current behaviour of warn provides
for "soft degradation" in those cases.


frankly a new option on the left side is a completly different thing
than a invalid value - just silently continue with invalid values of
existing options is playing a danergous game in a crucial component like
systemd


It's a rare thing :p but I have to agree with you here!

I'd say if "User=-notauser" then silently failing and using root is
acceptable as per the usual semantics of "- prefix suppresses errors",
but "User=notauser" should fail IMO.


I'm pretty sure you'll find that it does. Specifically, it will fail when 
the child process for the command being executed attempts to map the 
username to a UID.


The issue being discussed here is that systemd considers "0day" to be 
_syntactically_ invalid for a username. See the valid_user_group_name() 
function in basic/user-util.c.


(In my opinion, we shouldn't be this restrictive. POSIX permits usernames 
that start with a digit, and just because certain other utilities can't 
deal with them doesn't mean systemd need forbid them.)


So the directive fails the syntactic check for User=, just like 
Zbigniew's example of User="my name is pretty!".


I do think we ought to have a list of "critical" directives, such that any 
syntactic error in those directives causes the unit load state to be 
"error". For better or worse, people simply don't look at logs, so 
they'll never notice that important directives in their units are being 
ignored.

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

Re: [systemd-devel] Github systemd issue 6237