host-user-contaminated QA check
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?
Signature · 6 lines
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.
Quoted 5 lines · On Thu, 02 Feb 2017 11:38:00 +0100 Patrick Ohly <[email protected]> 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
Quoted 9 lines · Seebs
Quoted 5 lines · On Thu, 02 Feb 2017 11:38:00 +0100 Patrick Ohly <[email protected]> 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.
Quoted 7 lines
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).
Signature · 6 lines
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.
Quoted 20 lines · On Thu, 02 Feb 2017 17:39:07 +0100 Patrick Ohly <[email protected]> wrote:
Quoted 10 lines · Seebs
Quoted 6 lines · On Thu, 02 Feb 2017 11:38:00 +0100 Patrick Ohly <[email protected]> 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.
Quoted 8 lines
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.)
Quoted 6 lines
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
Quoted 8 lines · Seebs
Quoted 3 lines
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.
Signature · 6 lines
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.
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..
Quoted 32 lines · Patrick Ohly
Quoted 8 lines · Seebs
Quoted 3 lines
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 OhlyThe 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
[email protected]
http://lists.openembedded.org/mailman/listinfo/openembedded-core
Signature · 4 lines
Christopher Larson kergoth at gmail dot com Founder - BitBake, OpenEmbedded, OpenZaurus Senior Software Engineer, Mentor Graphics
Patrick Ohly <[email protected]>
Quoted 11 lines · 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.
Quoted 4 lines
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
Signature · 3 lines
SIGMA Chemnitz GmbH Registergericht: Amtsgericht Chemnitz HRB 1750 Am Erlenwald 13 Geschaeftsfuehrer: Grit Freitag, Frank Pyritz 09128 Chemnitz
Quoted 10 lines · On Thu, 02 Feb 2017 18:17:29 +0100 Patrick Ohly <[email protected]> wrote:
Quoted 8 lines · Seebs
Quoted 3 lines
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.
Quoted 6 lines
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
Quoted 19 lines · Enrico Scholz
Patrick Ohly <[email protected]>
Quoted 11 lines · 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.
Quoted 4 lines
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? ;-)
Quoted 5 lines
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.
Signature · 6 lines
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.
Quoted 16 lines · Seebs
Quoted 13 lines · On Thu, 02 Feb 2017 18:17:29 +0100 Patrick Ohly <[email protected]> wrote:
Quoted 8 lines · Seebs
Quoted 3 lines
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?
Quoted 8 lines
Quoted 3 lines
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"'.
Quoted 2 lines
("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.
Signature · 6 lines
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.
Quoted 4 lines · On Thu, 02 Feb 2017 20:43:49 +0100 Patrick Ohly <[email protected]> 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