host-user-contaminated QA check

host-user-contaminated QA check

[Patrick Ohly]

Hello!

Recently the host-user-contaminated QA check triggered for the trousers
recipe in meta-security:

WARNING: trousers-0.3.14+gitAUTOINC+4b9a70d578-r0 do_package_qa: QA
Issue: trousers: /trousers/etc/tcsd.conf is owned by uid 1000, which is
the same as the user running bitbake. This may be due to host
contamination [host-user-contaminated]

However, that's a false positive in this case. UID 1000 got assigned to
the "tss" user in the target sysroot during the build, and tcsd.conf is
correctly and intentionally owned by that user because tcsd checks
ownership and refuses to start when owned by someone else (including
root). It just happened that the UID was the same.

This is likely to affect all recipes with files owned by dynamically
created users, in particular when the host system assigns UIDs from the
same range as the target system (quick poll: who else has 1000 as his
UID on his main Linux box? ;-)

The current solution is to suppress the QA check for affected recipes.
But I wonder whether we can do better.

Why do we make the real user ID on the build system visible at all when
running under pseudo? The uid and user name have no meaning there, as
the user won't exist on the target system. Instead we could map the
owner of all files to root:root by default, i.e. in those cases where no
other ownership is recorded in the pseudo database.

The usual reason for host-user-contaminated is when do_install does a
"cp -a". When mapping the real owner to root, that command will end up
doing the right thing: create a file owned by root on the target.

Because the host user cannot leak into the target anymore, the entire QA
check can be disabled.

The only downsides of this approach that I can think of is that it hides
such sloppy use of "cp" where "install" would be better, and it might be
slightly confusing at first when working under devshell.

Any thoughts?

Seebs, I suppose this wouldn't be hard to implement in pseudo, would it?

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: host-user-contaminated QA check

[Seebs]

On Thu, 02 Feb 2017 11:38:00 +0100
Patrick Ohly <patrick.ohly@intel.com> wrote:

> Why do we make the real user ID on the build system visible at all
> when running under pseudo? The uid and user name have no meaning
> there, as the user won't exist on the target system. Instead we could
> map the owner of all files to root:root by default, i.e. in those
> cases where no other ownership is recorded in the pseudo database.

We could. Honestly, the underlying reason we don't is at least in part
that that makes the behavior differ more from the behavior of "sudo";
with sudo, you see actual ownerships. But that's less applicable here.

I would be more inclined to report a Definitely Absolutely Not Okay
user ID, like 65533. (65534 and 65535 have both been used as Magic
Cookies in the past, I think.)

-s

Re: host-user-contaminated QA check

[Patrick Ohly]

On Thu, 2017-02-02 at 10:21 -0600, Seebs wrote:
> On Thu, 02 Feb 2017 11:38:00 +0100
> Patrick Ohly <patrick.ohly@intel.com> wrote:
> 
> > Why do we make the real user ID on the build system visible at all
> > when running under pseudo? The uid and user name have no meaning
> > there, as the user won't exist on the target system. Instead we could
> > map the owner of all files to root:root by default, i.e. in those
> > cases where no other ownership is recorded in the pseudo database.
> 
> We could. Honestly, the underlying reason we don't is at least in part
> that that makes the behavior differ more from the behavior of "sudo";
> with sudo, you see actual ownerships. But that's less applicable here.
> 
> I would be more inclined to report a Definitely Absolutely Not Okay
> user ID, like 65533. (65534 and 65535 have both been used as Magic
> Cookies in the past, I think.)

I had considered that approach myself, too. It would make the QA check
reliable and in that sense solve the problem.

But I find mapping to root:root more attractive because it makes
packaging simpler (less worries about accidentally copying the original
uid) and the builds faster (no need to run the QA check).

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: host-user-contaminated QA check

[Seebs]

On Thu, 02 Feb 2017 17:39:07 +0100
Patrick Ohly <patrick.ohly@intel.com> wrote:

> On Thu, 2017-02-02 at 10:21 -0600, Seebs wrote:
> > On Thu, 02 Feb 2017 11:38:00 +0100
> > Patrick Ohly <patrick.ohly@intel.com> wrote:
> > 
> > > Why do we make the real user ID on the build system visible at all
> > > when running under pseudo? The uid and user name have no meaning
> > > there, as the user won't exist on the target system. Instead we
> > > could map the owner of all files to root:root by default, i.e. in
> > > those cases where no other ownership is recorded in the pseudo
> > > database.
> > 
> > We could. Honestly, the underlying reason we don't is at least in
> > part that that makes the behavior differ more from the behavior of
> > "sudo"; with sudo, you see actual ownerships. But that's less
> > applicable here.
> > 
> > I would be more inclined to report a Definitely Absolutely Not Okay
> > user ID, like 65533. (65534 and 65535 have both been used as Magic
> > Cookies in the past, I think.)
> 
> I had considered that approach myself, too. It would make the QA check
> reliable and in that sense solve the problem.
> 
> But I find mapping to root:root more attractive because it makes
> packaging simpler (less worries about accidentally copying the
> original uid) and the builds faster (no need to run the QA check).

Hmm. I think I would rather have the QA check, because if a file's
supposed to be non-root, and ends up root instead, that could cause
subtle problems, but we'd no longer have a way to *detect* those
problems.

-s

Re: host-user-contaminated QA check

[Patrick Ohly]

On Thu, 2017-02-02 at 11:12 -0600, Seebs wrote:
> > But I find mapping to root:root more attractive because it makes
> > packaging simpler (less worries about accidentally copying the
> > original uid) and the builds faster (no need to run the QA check).
> 
> Hmm. I think I would rather have the QA check, because if a file's
> supposed to be non-root, and ends up root instead, that could cause
> subtle problems, but we'd no longer have a way to *detect* those
> problems.

But that's not the kind of the problem detected by the QA check, is it?

It warns when the owner of the file is the same as the user who did the
build, but because root isn't (normally) used for building, files
accidentally owned by root on the target won't trigger the warning.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: host-user-contaminated QA check

[Christopher Larson]

[-- Attachment #1: Type: text/plain, Size: 1801 bytes --]

It's worth noting that host-user-contaminated also triggers on pseudo bugs,
which I've seen before. If we change the behavior, what user would files
that pseudo loses track of entirely be owned by? Or perhaps some of
pseudo's log messages should be made errors..

On Thu, Feb 2, 2017 at 10:17 AM, Patrick Ohly <patrick.ohly@intel.com>
wrote:

> On Thu, 2017-02-02 at 11:12 -0600, Seebs wrote:
> > > But I find mapping to root:root more attractive because it makes
> > > packaging simpler (less worries about accidentally copying the
> > > original uid) and the builds faster (no need to run the QA check).
> >
> > Hmm. I think I would rather have the QA check, because if a file's
> > supposed to be non-root, and ends up root instead, that could cause
> > subtle problems, but we'd no longer have a way to *detect* those
> > problems.
>
> But that's not the kind of the problem detected by the QA check, is it?
>
> It warns when the owner of the file is the same as the user who did the
> build, but because root isn't (normally) used for building, files
> accidentally owned by root on the target won't trigger the warning.
>
> --
> Best Regards, Patrick Ohly
>
> The content of this message is my personal opinion only and although
> I am an employee of Intel, the statements I make here in no way
> represent Intel's position on the issue, nor am I authorized to speak
> on behalf of Intel on this matter.
>
>
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>



-- 
Christopher Larson
kergoth at gmail dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Senior Software Engineer, Mentor Graphics

[-- Attachment #2: Type: text/html, Size: 2674 bytes --]

Re: host-user-contaminated QA check

[Seebs]

On Thu, 02 Feb 2017 18:17:29 +0100
Patrick Ohly <patrick.ohly@intel.com> wrote:

> On Thu, 2017-02-02 at 11:12 -0600, Seebs wrote:
> > > But I find mapping to root:root more attractive because it makes
> > > packaging simpler (less worries about accidentally copying the
> > > original uid) and the builds faster (no need to run the QA check).

> > Hmm. I think I would rather have the QA check, because if a file's
> > supposed to be non-root, and ends up root instead, that could cause
> > subtle problems, but we'd no longer have a way to *detect* those
> > problems.

> But that's not the kind of the problem detected by the QA check, is
> it?
> 
> It warns when the owner of the file is the same as the user who did
> the build, but because root isn't (normally) used for building, files
> accidentally owned by root on the target won't trigger the warning.

Right. But the purpose of that is to detect files which didn't get
their ownership correctly set. If we change to a default which we can't
detect, then we can't detect "files which were supposed to have an
ownership but didn't get it".

The idea here is that, although there's some performance cost, we
*intend* to require that every file installed have its ownership
determined in some way by the recipe, and if you don't do this but copy
in files you didn't set ownership on somehow, we want to detect that.

("Created under pseudo" is enough to count as "ownership determined by
recipe", it doesn't have to be an explicit chown.)

I think that, if we default to root:root, we'll end up with recipe
errors going unnoticed, when they could have been caught. And if we
default to -3:-3 or something similar, I think we'll catch errors we're
currently missing. For instance, what happens if a recipe copies host
/etc/services in, preserving ownership? Right now, we get a plausible
answer, but that's still actually host contamination!

-s

Re: host-user-contaminated QA check

[Patrick Ohly]

On Thu, 2017-02-02 at 13:11 -0600, Seebs wrote:
> On Thu, 02 Feb 2017 18:17:29 +0100
> Patrick Ohly <patrick.ohly@intel.com> wrote:
> 
> > On Thu, 2017-02-02 at 11:12 -0600, Seebs wrote:
> > > > But I find mapping to root:root more attractive because it makes
> > > > packaging simpler (less worries about accidentally copying the
> > > > original uid) and the builds faster (no need to run the QA check).
> 
> > > Hmm. I think I would rather have the QA check, because if a file's
> > > supposed to be non-root, and ends up root instead, that could cause
> > > subtle problems, but we'd no longer have a way to *detect* those
> > > problems.
> 
> > But that's not the kind of the problem detected by the QA check, is
> > it?
> > 
> > It warns when the owner of the file is the same as the user who did
> > the build, but because root isn't (normally) used for building, files
> > accidentally owned by root on the target won't trigger the warning.
> 
> Right. But the purpose of that is to detect files which didn't get
> their ownership correctly set. If we change to a default which we can't
> detect, then we can't detect "files which were supposed to have an
> ownership but didn't get it".

Got it - that's the same concern I had with 'it hides
such sloppy use of "cp"'.

> ("Created under pseudo" is enough to count as "ownership determined by
> recipe", it doesn't have to be an explicit chown.)

One could argue that an implicit "created during build -> owned by root"
follows the same logic. But as the check as it is now did find a real
issue and also others in the past (the pseudo bugs that Chris
mentioned), let's keep it.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: host-user-contaminated QA check

[Seebs]

On Thu, 02 Feb 2017 20:43:49 +0100
Patrick Ohly <patrick.ohly@intel.com> wrote:

> One could argue that an implicit "created during build -> owned by
> root" follows the same logic. But as the check as it is now did find
> a real issue and also others in the past (the pseudo bugs that Chris
> mentioned), let's keep it.

I am somewhat inclined to change pseudo's default to "if there's no
entry, report something definitely invalid", or make that an available
option, because that would avoid false positives, and allow a more
rigorous QA check.

-s

Re: host-user-contaminated QA check

[Enrico Scholz]

Patrick Ohly <patrick.ohly-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
writes:

> Recently the host-user-contaminated QA check triggered for the trousers
> recipe in meta-security:
>
> WARNING: trousers-0.3.14+gitAUTOINC+4b9a70d578-r0 do_package_qa: QA Issue: trousers: /trousers/etc/tcsd.conf is owned by uid 1000, which is the same as the user running bitbake. This may be due to host contamination [host-user-contaminated]
>
> However, that's a false positive in this case. UID 1000 got assigned to
> the "tss" user in the target sysroot during the build, and tcsd.conf is
> correctly and intentionally owned by that user because tcsd checks
> ownership and refuses to start when owned by someone else (including
> root). It just happened that the UID was the same.
>
> This is likely to affect all recipes with files owned by dynamically
> created users, in particular when the host system assigns UIDs from the
> same range as the target system (quick poll: who else has 1000 as his
> UID on his main Linux box? ;-)

Usually, this can not happen.  There is reserved a range for dynamically
created users (standard says 100-499, some distributions use 100-999).

In this case, there is probably some '--system' flag missing when the
'tss' user is created (--> packaging bug).



Enrico
-- 
SIGMA Chemnitz GmbH       Registergericht:   Amtsgericht Chemnitz HRB 1750
Am Erlenwald 13           Geschaeftsfuehrer: Grit Freitag, Frank Pyritz
09128 Chemnitz

Re: host-user-contaminated QA check

[Patrick Ohly]

On Thu, 2017-02-02 at 18:49 +0100, Enrico Scholz wrote:
> Patrick Ohly <patrick.ohly-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> writes:
> 
> > Recently the host-user-contaminated QA check triggered for the trousers
> > recipe in meta-security:
> >
> > WARNING: trousers-0.3.14+gitAUTOINC+4b9a70d578-r0 do_package_qa: QA Issue: trousers: /trousers/etc/tcsd.conf is owned by uid 1000, which is the same as the user running bitbake. This may be due to host contamination [host-user-contaminated]
> >
> > However, that's a false positive in this case. UID 1000 got assigned to
> > the "tss" user in the target sysroot during the build, and tcsd.conf is
> > correctly and intentionally owned by that user because tcsd checks
> > ownership and refuses to start when owned by someone else (including
> > root). It just happened that the UID was the same.
> >
> > This is likely to affect all recipes with files owned by dynamically
> > created users, in particular when the host system assigns UIDs from the
> > same range as the target system (quick poll: who else has 1000 as his
> > UID on his main Linux box? ;-)
> 
> Usually, this can not happen.  There is reserved a range for dynamically
> created users (standard says 100-499, some distributions use 100-999).
> 
> In this case, there is probably some '--system' flag missing when the
> 'tss' user is created (--> packaging bug).

That's a good point. I hadn't considered that.

In that case the QA check has found a real problem, albeit reported it
in a way that it wasn't obvious what was going on - probably the message
should get extended. I therefore retract my earlier proposal.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.