| opendevreview | Joel Capitao proposed openstack/diskimage-builder master: DNM Testing on KVM https://review.opendev.org/c/openstack/diskimage-builder/+/936024 | 14:44 |
|---|---|---|
| JayF | This has 3x +1, including from another gentoo expert, would very much appreciate if we could land it: https://review.opendev.org/c/openstack/diskimage-builder/+/923985 | 21:59 |
| clarkb | JayF: I have two concerns/questions (in the bits of the change that affect more than gentoo potentially) | 22:13 |
| clarkb | can you take a look and sanity check them to make sure there aren't any obvious issues? | 22:13 |
| JayF | clarkb: responded; tldr it's all fine | 22:15 |
| JayF | clarkb: I was also WTF when I saw the config gentoo (and upstream) puts in /etc/sudoers wants a directory that doesn't exist | 22:16 |
| clarkb | ok any concern about permissions? as noted it should noop everywhere else | 22:16 |
| JayF | you are right about perms on /etc/sudoers.d, I suepct, though | 22:16 |
| JayF | can I just follow it up? | 22:16 |
| clarkb | but you might end up being able to add extra sudoers content to that dir if it is 777 or whatever | 22:16 |
| JayF | would rather avoid another round-trip on this patch as I'm waiting on it for something else: ) | 22:17 |
| JayF | will push said followup right now if you agree | 22:17 |
| clarkb | yes, I think a followup is fine since as noted mkdir -p should noop if it already exists. I'll quickly test it doesn't change permissions or anything first then approve | 22:17 |
| JayF | if mkdir -p changed permissions ... whooo boy that would break a lot of folks :) | 22:17 |
| JayF | not in this case, but in a bunch of em | 22:18 |
| clarkb | ya manpage even says it will ignore the -m perms setting | 22:18 |
| clarkb | but since this is security related i want to be sure | 22:18 |
| JayF | interesting; you can't chown something to root that you own as your own user | 22:19 |
| JayF | not sure I can wrap my head around why that's a bad idea | 22:19 |
| clarkb | I've approved it mkdir -p seems to noop in all cases where there dir already exists that I threw at it | 22:21 |
| JayF | I'm glad I'm following this up | 22:21 |
| JayF | there are similarly flavored permissions issues in this element | 22:22 |
| JayF | e.g. the sudoers file we put in is getting default umask | 22:22 |
| clarkb | that one gets chowned though | 22:22 |
| clarkb | sorry chmod'd | 22:22 |
| JayF | oh, you're eright | 22:22 |
| JayF | I kept reading EOF as fi | 22:22 |
| clarkb | the bigger concern is that if the dir is writable anyone could add new sudoers content I think | 22:23 |
| JayF | interesting that /etc/sudoers is not even writable by root on this machine | 22:23 |
| clarkb | though maybe suod will only respect files that are properly owned and permed | 22:23 |
| JayF | sudo will 100% do ^ that | 22:24 |
| JayF | because that's part of why you use visudo | 22:24 |
| JayF | bad syntax or bad perms can lock you completely outta a system | 22:24 |
| JayF | we should still fix this, to be clear, but I think it's more of a cleanup than a sec risk | 22:24 |
| clarkb | ack | 22:24 |
| JayF | so something weird about this | 22:39 |
| JayF | in the devuser element | 22:39 |
| JayF | we're not sudo'ing to add the file in /etc/sudoers.d/ | 22:39 |
| * JayF looks up docs on if there's something special about install.d/ | 22:40 | |
| JayF | it's documented as running in-chroot but does not indicate you have root perms | 22:41 |
| JayF | so how does that work/ | 22:41 |
| JayF | so weird, it works in the build without sudo for any of it | 22:52 |
| JayF | which means we must be effective UID 0 for that part of the build but I can't tell how | 22:52 |
| opendevreview | Jay Faulkner proposed openstack/diskimage-builder master: Followup: Ensure devuser-created dir has sane perms https://review.opendev.org/c/openstack/diskimage-builder/+/936206 | 22:54 |
| opendevreview | Merged openstack/diskimage-builder master: [gentoo] Fix+Update CI for 23.0 profile https://review.opendev.org/c/openstack/diskimage-builder/+/923985 | 23:17 |
| opendevreview | Jay Faulkner proposed openstack/diskimage-builder master: Update default Ubuntu to noble (latest LTS) https://review.opendev.org/c/openstack/diskimage-builder/+/936209 | 23:36 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!