[Toybox] cp --preserve=a doesn't preserve security context of directories.

enh enh at google.com
Sat Aug 19 11:23:48 PDT 2023


On Sat, Aug 19, 2023 at 10:48 AM Rob Landley <rob at landley.net> wrote:
>
> On 8/18/23 13:54, enh wrote:
> > On Fri, Aug 18, 2023 at 11:45 AM Rob Landley <rob at landley.net> wrote:
> >>
> >> On 8/16/23 15:26, enh wrote:
> >> >> I long ago came to the conclusion I can't make a system secure, all I can do is
> >> >> annoy attackers into choosing a less vexing target. But I don't want to the the
> >> >> same to users or developers, so it's always a balancing act.
> >> >
> >> > meh, if your selinux labels are wrong, stuff stops working. you can
> >> > either fix it yourself or `setenforce 0` if you _know_ what you're
> >> > doing isn't compatible with selinux rules for actual shipping systems
> >> > and don't care because you're just testing a thing.
>
> Wasn't that deprecated? https://lwn.net/Articles/831748/ Hmmm... ok, there are
> multiple mechanisms to disable different amounts of it. And one of them was
> being used too much, so they took it away.
>
> >> ...> fwiw, i wouldn't assume it's actually ever been tested? i'd imagine
> >> > most [OS] developers are doing `adb sync` instead anyway. any `cp -r`
> >> > action is most likely just a quick test in /data/local/tmp --- which
> >> > is so useful _because_ it's the wild west where many of the usual
> >> > rules don't apply (but only the root or shell users can do anything
> >> > with it).
>
> In theory tar extract is also doing some variant of this dance, and they should
> probably agree. It's all DIRTREE_COMEAGAIN to set the directory after the contents.
>
> The problem is I don't have this domain expertise, it's nontrivial to _build_
> this domain expertise, and non-obvious how to _borrow_ this domain expertise.
> Which is why it's stayed on the todo list for so long, but gotta tackle it at
> some point.
>
> (The correct answer is "do it wrong and wait for people to complain, because
> then they BRING TEST CASES". But it's now heavily used enough that move fast and
> break things is... less obviously appropriate?)
>
> >> Speaking of tested, what does a good selinux test _look_ like here? The ls -Z
> >> stuff is using regexes. I have a Fedora 36 ISO image that says:
> >>
> >> $ ls -Z .
> >>  unconfined_u:object_r:user_home_t:s0 Desktop
> >>  unconfined_u:object_r:user_home_t:s0 Documents
> >>  unconfined_u:object_r:user_home_t:s0 Downloads
> >> unconfined_u:object_r:audio_home_t:s0 Music
> >>  unconfined_u:object_r:user_home_t:s0 Pictures
> >>  unconfined_u:object_r:user_home_t:s0 Public
> >>  unconfined_u:object_r:user_home_t:s0 Templates
> >>  unconfined_u:object_r:user_home_t:s0 Videos
> >>
> >> And I don't know what any of that means? (I always delete all the directories
> >> except "Downloads" immediately on any new install, and only keep that one
> >> because every web browser uses it.)
> >
> > selinux labels are [insert usual disclaimer about my level of
> > knowledge/understanding here] basically just arbitrary strings. i
> > don't think they "mean" anything more than "enh" or the corresponding
> > integer uid "means" anything. they're useful because you can then say
> > things like "this process can read but not write files with this
> > label" or whatever.
>
> The labels don't, the rules do. The labels just say which rules trigger on each
> filesystem object. If I set the labels last, it may drop the suid bit on an
> object. If I set the label earlier, they may trigger an selinux rule that vetoes
> a later change I try to make to the filesystem object. (In _theory_ your selinux
> rules could default forbid and have labels _grant_ access, but that's now how
> the Red Hat guys wrote their ruleset back before I gave up on them.)
>
> I _think_ the right order is to set the label and then chown to apply the suid
> bit last, but my devuan laptop hasn't got this plumbing, and I don't think my
> root filesystem even supports xattrs, so I can't currently test that outside of
> mkroot or a KVM instance (and Fedora's livecd is... awkward).

that order certainly matches what we do in adbd for a push/sync of a
file to the device (note also the explicit mention of setuid):

            if (fchown(fd.get(), uid, gid) == -1) {
                struct stat st;
                std::string real_path;

                // Only return failure if parent directory does not
have S_ISGID bit set,
                // if S_ISGID is set then file will inherit groupid
from directory
                if (!Realpath(path, &real_path) ||
lstat(Dirname(real_path).c_str(), &st) == -1 ||
                    (S_ISDIR(st.st_mode) && (st.st_mode & S_ISGID) == 0)) {
                    SendSyncFailErrno(s, "fchown failed");
                    goto fail;
                }
            }

#if defined(__ANDROID__)
            // Not all filesystems support setting SELinux labels.
http://b/23530370.
            selinux_android_restorecon(path, 0);
#endif

            // fchown clears the setuid bit - restore it if present.
            // Ignore the result of calling fchmod. It's not supported
            // by all filesystems, so we don't check for success. b/12441485
            fchmod(fd.get(), mode);


> I don't know how big Android's selinux policy ruleset is (I kinda noped out of
> selinux conceptually when Fedora passed 50k rules active in the default install)
> or how many different policies (selectable rulesets) it runs in various
> contexts, so I have no idea what any of the labels I'd be copying _DO_.

certainly big enough that i gave up even trying to understand it years
ago. (it feels a bit like the dependency injection stuff that's so
popular in some circles too --- it doesn't compose clearly, so i find
it quite hard to reason about on the occasions i'm foolish enough to
even try.)

> What
> little I know is that it's a programming language complicated enough that people
> built an IDE for it 15 years ago:
>
> https://lwn.net/Articles/221112/
>
> And it has not gotten simpler since:
>
> > the policies typically are comprised of thousands of policy statements; this
> > makes policy development and analysis very difficult. The complexity of
> > resulting SELinux policies means that for example, safety guarantees cannot be
> > given, defeating the main purpose for SELinux in the first place
>
> https://www.site.uottawa.ca/~afelty/dist/mcetech17.pdf
>
> It's a pity Dan Walsh's blog was on livejournal. I'm not voluntarily linking to
> Russian servers in 2023. But he did a lot of "ruleset reduction" work some years
> back.
>
> One problem with getting old is I'm never quite sure if something that was a big
> deal 10 years ago ever got fixed (like the btrfs guys just merged the fix for
> the directory loop stuff). Memorials to old long-fixed problems is how Jorg
> Schilling happened.
>
> But staying current on everything... KVM has dedicated annual conferences and I
> have an old blog entry I cut and paste "-drive format=raw,file=block.img" from
> every time qemu complains "-hda block.img was being used too much so we nerfed
> it". (And said blog entry is now 2 years old, and I still haven't memorized the
> long version yet...)
>
> >> (I still haven't managed to build a vanilla Android system that boots under
> >> vanilla kvm. Did
> >> https://www.xda-developers.com/microdroid-stripped-down-android-virtual-machines/
> >> ever turn into a thing?)
> >
> > https://source.android.com/docs/core/virtualization/microdroid
>
> I made it as far as "binder RPC" before giving up, not having found a "how do I
> use this" section.
>
> (Yes, glass houses and stones, I totally need to redo the toybox main web page.)
>
> > "cuttlefish" is probably what you want though? i run the riscv64
> > cuttlefish, and you can see instructions at
> > https://github.com/google/android-riscv64/#can-i-try-it --- the x86-64
> > cuttlefish is probably a lot more useful for you (and definitely a lot
> > faster!).
>
> Cool! Let's see:
>
> $ source build/envsetup.sh && lunch
> Lunch menu .. Here are the common combinations:
>      1. aosp_arm-eng
>      2. aosp_arm64-eng
>      3. aosp_barbet-userdebug
>      4. aosp_bluejay-userdebug
>      5. aosp_bluejay_car-userdebug
>      6. aosp_bramble-userdebug
>      7. aosp_bramble_car-userdebug
>      8. aosp_cf_arm64_auto-userdebug
>      9. aosp_cf_arm64_phone-userdebug
>      10. aosp_cf_riscv64_phone-userdebug
>      11. aosp_cf_x86_64_auto-userdebug
>      12. aosp_cf_x86_64_foldable-userdebug
>      13. aosp_cf_x86_64_only_phone_hsum-userdebug
>      14. aosp_cf_x86_64_pc-userdebug
>      15. aosp_cf_x86_64_phone-userdebug
>      16. aosp_cf_x86_64_tv-userdebug
>      17. aosp_cf_x86_phone-userdebug
>      18. aosp_cf_x86_tv-userdebug
>      19. aosp_cheetah-userdebug
>      20. aosp_cheetah_hwasan-userdebug
>      21. aosp_cloudripper-userdebug
>      22. aosp_coral-userdebug
>      23. aosp_coral_car-userdebug
>      24. aosp_felix-userdebug
>      25. aosp_flame-userdebug
>      26. aosp_flame_car-userdebug
>      27. aosp_lynx-userdebug
>      28. aosp_oriole-userdebug
>      29. aosp_oriole_car-userdebug
>      30. aosp_panther-userdebug
>      31. aosp_panther_hwasan-userdebug
>      32. aosp_raven-userdebug
>      33. aosp_raven_car-userdebug
>      34. aosp_ravenclaw-userdebug
>      35. aosp_redfin-userdebug
>      36. aosp_redfin_car-userdebug
>      37. aosp_redfin_vf-userdebug
>      38. aosp_slider-userdebug
>      39. aosp_sunfish-userdebug
>      40. aosp_sunfish_car-userdebug
>      41. aosp_tangorpro-userdebug
>      42. aosp_trout_arm64-userdebug
>      43. aosp_trout_x86_64-userdebug
>      44. aosp_whitefin-userdebug
>      45. aosp_x86-eng
>      46. aosp_x86_64-eng
>      47. arm_krait-eng
>      48. arm_v7_v8-eng
>      49. armv8-eng
>      50. armv8_cortex_a55-eng
>      51. armv8_kryo385-eng
>      52. car_ui_portrait-userdebug
>      53. car_x86_64-userdebug
>      54. db845c-userdebug
>      55. gsi_car_arm64-userdebug
>      56. gsi_car_x86_64-userdebug
>      57. hikey-userdebug
>      58. hikey64_only-userdebug
>      59. hikey960-userdebug
>      60. hikey960_tv-userdebug
>      61. hikey_tv-userdebug
>      62. poplar-eng
>      63. poplar-user
>      64. poplar-userdebug
>      65. qemu_trusty_arm64-userdebug
>      66. rb5-userdebug
>      67. riscv64-eng
>      68. sdk_car_arm-userdebug
>      69. sdk_car_arm64-userdebug
>      70. sdk_car_md_x86_64-userdebug
>      71. sdk_car_portrait_x86_64-userdebug
>      72. sdk_car_x86-userdebug
>      73. sdk_car_x86_64-userdebug
>      74. sdk_pc_x86_64-userdebug
>      75. silvermont-eng
>      76. uml-userdebug
>      77. yukawa-userdebug
>      78. yukawa_sei510-userdebug
>
> Which would you like? [aosp_arm-eng]
> Pick from common choices above (e.g. 13) or specify your own (e.g. aosp_barbet-eng):
>
> So entry #15 then? (There's a User Mode Linux version?)

yeah, aosp_cf_x86_64_phone-userdebug is the one i typically use. (and
probably the most widely-used one, too, and i always recommend staying
on the best-trodden paths!)

> I wonder what "Build sandboxing disabled due to nsjail error." means...

it's part of our hermetic build stuff. we try to really enforce that
stuff hard where possible. this is just telling you that we couldn't,
so some unreproducibility might creep in.

> It flashed up an error "You are building on a machine with 7.7 GB of ram..." and
> then erased it and went back to the previous screen (thanks curses!) before I
> could read the rest, but seems to be building anyway? (I haven't moved the ram
> chips out of the old laptop to the new one, in part because I hadn't realized
> how big an impact the extra memory has on battery life, especially suspend time.)
>
> And I killed it when everything froze converting Android.bp files to soong, with
> a single process already 4 gigs into swap and still going. Try again when I've
> got the extra memory installed I guess...

yeah, there have been some big regressions in amount of memory needed
over the last year or two, and -- because the typical build machine
has 128 cores and 128GiB anyway -- insufficient motivation to invest
the effort in bringing that down. i _think_ you can build with 32GiB,
but that less than that will just OOM kill eventually? (though it's
possible you can just keep re-running over and over and still make
progress? i don't recall, and have never tried it myself.)

hmm, i seem to have updated the docs to say 64GiB last time a member
of the public hit this:

* Google recommends at least 64 GB of RAM and doesn't test with less.
Lower amounts lead to builds being OOM killed.

https://source.android.com/docs/setup/start/requirements

> >> Anyway, I've been poking at the whole tests-under-mkroot thing so I can run
> >> tests as root under a known environment for things like "yes the host system and
> >> this filesystem are capable of doing selinux but haven't got any weird rules
> >> that make stuff go 'boing' by themselves", but setting up a hand-crafted test
> >> environment doesn't help if I don't know what success looks like.
> >>
> >> I'm hoping I can "setprop" something, cp -a, and then "getprop" to see that it
> >> got propagated successfully? I suppose I can just copy one of the Fedora labels...
> >
> > i think you mean chcon(1)?
>
> Actually I meant setfattr since toybox has that. (But getfattr is still in
> pending, for reasons I need to re-familiarize myself with.) Except:
>
>   $ touch walrus
>   $ ./setfattr -n boing -v whap walrus
>   setfattr: walrus: Operation not supported
>
> Since sudo doesn't make a difference I _think_ the problem is my devuan root
> filesystem isn't compiled with xattr support? Another reason I'm trying to get
> the test suite to work in mkroot where I can compile and mount stuff with known
> options...
>
> As for chcon, the man page says "chcon - change file security context". Does
> that work when I'm using a debian system that hasn't got selinux enabled? (I
> _thought_ security context was "which ruleset currently applies to this
> process", but I guess this is just different vocabulary for changing a file's
> label?) If so...
>
>        -r, --role=ROLE
>               set role ROLE in the target security context
>
>        -t, --type=TYPE
>               set type TYPE in the target security context
>
>        -l, --range=RANGE
>               set range RANGE in the target security context
>
> I hope there's a tutorial on it somewhere. (I wonder what the rules are for who
> can change labels...)

(yeah, i think as you implied earlier, that that depends on the
selinux policies in place for that system.)

> > but otherwise, "yes". (though i don't have
> > a good trick for "give me two distinct labels" in the same way we grep
> > /etc/passwd or whatever...)
>
> I want to test "adding label did not veto ensuing chmod" and/or "adding label
> did not drop existing suid bit". Which one is "safer" depends on your threat
> model, but I lean towards label-after-chmod if it'll work because that
> (hopefully) doesn't care what the label means to any selinux ruleset. But
> testing either involves setting up a VM where I can add a label.
>
> And, no matter what I do there's going to be an selinux policy that breaks it.
> The question is does the current _android_ policy break it...

if you're doing things in the same order that adbd does, you're (a) in
good company and (b) as protected as possible from changes here ---
whoever wanted to change the way this works would have to care enough
to modify adbd too. (not that there's a unit test that i'm aware of;
they'd probably have to wait for someone to notice and complain, which
-- given the rarity of setuid binaries [there's exactly one on
Android, right, and that only on debug/eng builds?] -- could take a
while.)

> Then again, given that tar setting xattrs is using setfscreatecon(), or at least
> an unrolled version of it so I don't need -lselinux:
>
>           if ((FLAG(selinux) && !(FLAG(t)|FLAG(O)))
>               && strstart(&pp, "security.selinux="))
>           {
>             i = strlen(pp);
>             sefd = xopen("/proc/self/attr/fscreate", O_WRONLY|WARN_ONLY);
>             if (sefd==-1 ||  i!=write(sefd, pp, i))
>               perror_msg("setfscreatecon %s", pp);
>
> I'm not sure that merely setting the label will let selinux know about the
> file's new security context anyway? (Who knows! A maze of twisty API codepaths,
> all different...)
>
> *shrug* I did the simple thing. It's probably wrong, but I can wait for people
> to complain...
>
> >> Rob
>
> Rob


More information about the Toybox mailing list