=== cpaelzer_ is now known as cpaelzer [05:11] morning [05:57] mvo: hey [05:58] hey mborzecki - good morning [05:59] mvo: when's the release? [06:01] mborzecki: 2.36~pre2 happend last friday but it looks like we need a 2.35.5 :/ [06:01] mborzecki: why? anything important pending? [06:03] mvo: no, just checking if we could squeeze in https://github.com/snapcore/snapd/pull/5979 [06:03] PR #5979: install: don't start disabled services [06:03] mborzecki: mark it for 2.36 please [06:03] mborzecki: and alsp please mark for squash merge to merge the cherry-pick/pr easier [06:04] mvo: marked [06:05] PR snapd#5983 opened: interfaces/home: don't allow snaps to write to $HOME/bin (2.35) [06:06] mborzecki: ta [06:07] zyga: 5974 has some comment improvement suggestions, could you do a followup with those? I will merge and pull into 2.35 so that things move here [06:07] PR snapd#5974 closed: osutil: workaround overlayfs on ubuntu 18.10 [06:07] zyga: in case you didn't already, not sure if all points where addressed [06:25] mborzecki: jdstrand pushed a bunch of PRs that look important but he did not attach a milestone - do you happen to know what he needs? I tagged them as 2.36 [06:25] mborzecki: but I'm not sure if we might even pull things into 2.35 (cc zyga) [06:27] mvo: i recall the one about scrubbing env on profile change, i see it's tagged for 2.36 === chihchun_afk is now known as chihchun [07:13] re [07:13] good morning [07:13] sorry for starting late, very very rough night [07:13] mvo: looking at 5974 [07:14] ah that, I need to digest those as, as we were discussing in on IRC, we were also talking in the pull request [07:15] mvo: I believe all the code there is correct, we can tweak the comment [07:15] mvo: I will look at Jamie's pr's now [07:15] jdrab: https://github.com/snapcore/snapd/pull/5982 this one is very important [07:15] PR #5982: interfaces/many: conditionally use 'unsafe' with docker-support change_profile rules [07:17] PR snapd#5983 closed: interfaces/home: don't allow snaps to write to $HOME/bin (2.35) [07:17] the k8s pull requests are something I was not tracking so I cannot asses those === pstolowski|afk is now known as pstolowski [07:18] mornings [07:18] zyga: thanks [07:20] zyga: pstolowski: hey [07:20] the unsafe PR is holly smokes long :) [07:51] mvo: I left some comments on https://github.com/snapcore/snapd/pull/5980#pullrequestreview-164581442 - I suspect we should rename the fields/methods to match go naming scheme but otherwise this is a very welcome improvement [07:51] PR #5980: interfaces/apparmor: conditionally add explicit deny rules for ptrace [07:51] mvo: please let me know if you want to see those extra changes as additional patches in this PR directly or as a separate PR [07:52] zyga: ta, I have a look in a wee bit [07:52] British English wee bit :) [07:53] zyga: I don't know :) the wee as in little [07:53] exactly [07:53] that's what I meant :) [07:53] are there more meanings than this one? [07:54] not that I know of, I meant that it is specific to British English as a way to say little [07:54] https://en.wiktionary.org/wiki/wee#Adjective https://en.wiktionary.org/wiki/wee#Noun [07:54] oh, I meant to link to Etymology 2 [07:55] lol [07:55] that's new :) [07:55] TIL wee things matter [07:58] pstolowski: is 5975 something we can land for 2.36? [07:58] it's +2 green right now [07:59] zyga: we could yes, but mvo wanted to have a look i think [07:59] * zyga looks at k8s pull request [07:59] mvo: I would vote +1 on that since it makes everyday "snap tasks" much more understandable and accurate [07:59] mvo: but please do see it before merging [08:00] (let's squash merge then if we do) [08:00] sure, please set a tag there [08:04] zyga: yeah, I think its good, looking now (sorry, just had to finish the classic dimension forum post before it escapes my mind again :) [08:04] cool, thank you for doing that - I will gladly read it soon [08:06] good morning [08:07] good morning sir [08:09] zyga: are we there yet? [08:10] no, not quite [08:10] aww [08:23] pstolowski: 5975 looks great, please go ahead and squash merge it so that we can backport to 2.36 [08:23] mvo: great, ty! [08:24] zyga: anything else we might need for 2.35.5 ? if not I will go ahead with this soon [08:29] mvo: I don't know of anything else but let me look at the full list of PRs [08:35] PR snapd#5975 closed: ifacestate/hooks: only create interface hook tasks if hooks exist === chihchun is now known as chihchun_afk [08:40] mvo: merged, shall i prepare PR for 2.36? [08:40] pstolowski: please do [08:40] sure [08:40] pstolowski: thank you [08:42] mvo: perhaps a subset of https://github.com/snapcore/snapd/pull/5696/files [08:42] PR #5696: interfaces/opengl: add additional accesses for cuda [08:42] where we have the extra directory but not the part that jdstrand pointed out (the mknod perm) [08:43] mvo: otherwise nothing else I see for 3.35.x [08:46] PR snapd#5984 opened: ifacestate/hooks: only create interface hook tasks if hooks exist (2.36) === chihchun_afk is now known as chihchun [08:51] zyga: thanks for double checking. I looked at 5980, it looks good, I agree with your comments about naming conventions though [08:51] I can push a patch to change it there and in the other PRs that are built on it [08:52] mvo, zyga ^ pr for 2.36 is up [08:52] pstolowski: did a pass on #5962 [08:52] PR #5962: ifacestate/hotplug: hotplug handlers [08:52] pstolowski: cool, thanks [08:54] heh github, if you search with query https://github.com/snapcore/snapd/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Aopen+%F0%9F%94%8C (using the hotplug emoji) it finds no PRs [08:55] mborzecki: thanks! [09:07] mborzecki: +1 for #5966, with one remaks (perhaps degville can help again) [09:07] PR #5966: snap: overhaul validation error messages [09:10] Yeah, the emojis are funny, but we should probably drop them [09:10] Morning all [09:11] good morning niemeyer [09:12] mvo: Yo [09:12] mborzecki: does searching by the contents of a tag find things? (doesn't work with 'complex' either) [09:13] Chipaca: probably not, seems like freetext search ignores labels [09:20] Chipaca: a while ago you looked into uc16 to ensure things cleanly unmount on stop/reboot, do you think you could you do the same for uc18 again? [09:21] mvo: I think I probably could :-) do you know it doesn't? [09:21] ppisati: did you had any luck getting to the bottom of the 4.15 pi3 b+ ethernet connection issues? [09:21] Chipaca: I suspect it might be :/ [09:22] mvo: ok. Is there a working intel core18 i could look at? [09:22] Chipaca: http://people.canonical.com/~mvo/core18/ [09:22] Chipaca: so http://people.canonical.com/~mvo/core18/core18-amd64-18-beta20180823.img.xz [09:22] thanks [09:22] Chipaca: should work, please do let me know if you notice anything [09:23] Chipaca: thank *you* [09:24] mvo: i think #5969 needs master merged, the base PR already landed [09:24] PR #5969: apparmor: add unit test for probeAppArmorParser and simplify code [09:25] pstolowski: thanks, done [09:27] thx [09:28] mvo: the fix is already queued in bionic [09:29] ppisati: \o/ [09:29] ppisati: great to hear, do you have a rough eta for the snap? I guess its too much work to just do an edge build of the pi-kernel (?) [09:32] mvo: as soon as the new SRU kernel is released - IIRC they cut a new kernel tomorrow, then it's 2 weeks after that [09:32] mvo: but there was a respin of kernels last week, so the window might have slide a bit [09:34] *slid [09:35] ppisati: ok. I wish we had git snapshots in the edge channel :) but I guess that is a wishlist item for another day. might actually be nice for you guys as well to get people testing/validating fixes [09:35] ppisati: thanks for this update! [09:42] Chipaca: i'm looking at snapshot file names, the code does not reverse the file name anywhere, so we sould be safe there wrt. parallel instances and extra _ [09:43] mborzecki: yep, the filename is for the sysadmin, snapshots doesn't infer info from it [09:43] Chipaca: mhm, cool [09:43] mborzecki: only thing where it might be a problem is creating duplicate filenames, which we might not check for [09:43] mborzecki: but we use InstanceName so should be fine, afaik [09:43] Chipaca: yes, instance names are unique [09:43] i mean afaik we use instance name :) [09:44] mvo: how long does the 'resize partition' step take, usually? boot's been stuck there for a while now [09:46] Chipaca: probably entropy :( [09:46] Chipaca: just hammer at some keys [09:46] heh, that was it [09:47] * mvo weeps a bit in the corner about this problem [09:47] heh, wonder what one will do when there's no keyboard attached [09:48] wiggle some gpios [09:48] * Chipaca suddenly wishes for "base: core | core18 [09:51] huh [09:51] mvo: a 'snap install' got aborted by a refresh of core18; is that expected? [09:51] aborted? [09:51] can we even do that? [09:52] might've been just coincidence [09:52] 'snap tasks' say 'server misbehaving' [09:57] mvo: actually [09:57] mvo: perhaps https://github.com/snapcore/snapd/pull/5982#pullrequestreview-164628872 is a candidate for .5 [09:57] mvo: but it is stacked on top of a pile of other things [09:57] so unsure [09:57] PR #5982: interfaces/many: conditionally use 'unsafe' with docker-support change_profile rules [09:58] you marked that PR as targeting 2.36 so perhaps you know better [10:09] mvo: bad news: we're hitting a variant of https://github.com/systemd/systemd/issues/8155 [10:10] zyga: I can wait for jamie to clarify [10:11] Chipaca: uhhh [10:11] mvo: maybe :) [10:11] * Chipaca is still digging [10:11] but, yeah, we just see the nasty [ 364.293811] systemd-shutdown[1]: Failed to wait for process: Protocol error [10:11] which seems to mean that systemd < 238 [10:12] Chipaca: hrm, hrm, or would need to sru [10:12] yeah, systemd --version says 237 [10:12] we could also backport https://github.com/systemd/systemd/pull/8429/files [10:12] PR systemd/systemd#8429: sd-shutdown improvements

[10:13] uh fun [10:13] (not) [10:13] mvo: but all that _really_ does is turn up the logging [10:13] bah, maybe :) [10:13] for our case it might be the only difference [10:13] i wonder if i can't turn up the logging without that [10:13] Chipaca: ok [10:22] mvo: so. we might not need that patch [10:22] mvo: it's just that our systemd-shutdown helper unit isn't getting triggered [10:22] digging continues [10:23] that is: if I set it up by hand, I get [10:23] Chipaca: oh, nice [10:23] Successfully changed into root pivot. [10:23] Returning to initrd... [10:23] snapd system-shutdown helper: started. [10:23] snapd system-shutdown helper: no reboot parameter [10:23] snapd system-shutdown helper: * unable to disassociate loop device /dev/loop0s: No such device or address [10:23] snapd system-shutdown helper: - was able to unmount writable cleanly [10:23] snapd system-shutdown helper: - halting. [10:23] Chipaca: so maybe just a missing symlink to activate it? [10:23] pr'aps [10:23] rebooting to see :-) [10:23] Chipaca: really /dev/loop0s? [10:23] Chipaca: I can look for this is that helps you :) [10:23] what even is that [10:24] $ systemctl is-enabled snapd.system-shutdown.service [10:24] enabled [10:25] mvo: so it's not that :-| [10:25] zyga: ¯\_(ツ)_/¯ [10:26] zyga: /dev/loop0; the s is probably a typo [10:26] zyga: kmsg("* unable to disassociate loop device %ss: %s", [10:26] ah [10:26] :) [10:26] good [10:26] at least [10:26] loop0 is probably the boot snap [10:27] it's probably fine; disassociation is automatic after dunno-what-utils-linux-version [10:28] mvo: so, the unit is enabled; it's not getting activated afaict [10:28] probably changes to systemd-shutdown [10:29] Chipaca: :/ hrm, hrm [10:30] ah [10:30] i think i might know why [10:32] mvo: the snapd snap does not ship sh [10:33] huh? why would we need to ship sh? [10:34] well, we don't, but we then create snapd.system-shutdown.service as if we did [10:34] is the service using shell? [10:35] sudo sed -i -e 's|/snap/snapd/659/bin/sh|/snap/core18/304/bin/dash|' /etc/systemd/system/snapd.system-shutdown.service && sudo systemctl daemon-reload -> works [10:35] zyga: ExecStart=/snap/core18/304/bin/dash -euc 'mount /run -o remount,exec; mkdir -p /run/initramfs; cp /usr/lib/snapd/system-shutdown /run/initramfs/shutdown' [10:35] ohohh [10:35] I see [10:36] Chipaca: oh woah - nice catch! [10:36] I don't remember why it does that instead of having multiple ExecStart= lines [10:36] let me try that instea [10:36] probably more portable [10:36] * Chipaca tries [10:37] of course if I had 'vi' this would be slightly easier :-D [10:39] Chipaca: I thought we added this [10:39] Chipaca: maybe you need a newer core18? [10:39] mvo: I'll let it refresh [10:40] ok [10:40] mvo: which 'this' is this, btw? [10:42] Chipaca: I thought we added "vi" to core18 [10:42] (vim.tiny to be exact) [10:42] mvo: ah, possibly, it seems the core image was ~200 revisions behind [10:44] brb, dog [10:45] ok [10:45] so [10:45] easy fix [10:45] PR snapd#5985 opened: overlord/many: cleanup use of snapName vs. instanceName [10:46] Chipaca: I like the ring of "easy fix" \o/ [10:47] mvo: what creates snapd.system-shutdown.service? [10:48] mvo: found it [10:51] hmm [10:51] mvo: what changes /bin/sh to /snap/snapd//bin/sh ? [10:55] mvo: found it [10:55] mvo: I have uploaded the probert with the fix for db that mwhudson prepared to our PPA, once it's built I'll be triggering a new core18 [10:56] Once that's in would be nice to confirm that the probert crash is gone [10:56] zyga: ^ [11:00] sil2100: nice [11:00] sil2100: I will test with my pi-64bit [11:01] As will once back [11:02] sil2100: nice fix [11:03] sil2100: great that this is under control [11:14] Chipaca: fwiw, this fix sounds a lot like we want it in 2.35.5 just to be one the safe side (if its someting that needs to go into snapd) [11:15] k [11:15] Chipaca: ta [11:15] Chipaca: but don't worry about this, I can do the backporting etc [11:16] mvo: there is no worry; there is only do [11:16] :) [11:16] DO! [11:16] * mvo hugs Chipaca [11:22] sil2100: is that the image from Friday good for testing? [11:23] mvo: release day is like xmas [11:23] everyone has a present [11:23] No, not yet [11:23] I mean, not anymore ;) [11:23] I'll have a new one soonish [11:23] ok [11:23] The packages need to publish though [11:24] I'll grab lunch quickly then [11:26] PR snapd#5986 opened: data/systemd, wrappers: tweak system-shutdown helper for core18 [11:26] mvo, zyga: ^ [11:26] ooh, lunch, a capital idea [11:26] mborzecki: +1 on #5985, would be good to land it fast [11:27] PR #5985: overlord/many: cleanup use of snapName vs. instanceName [11:28] Chipaca: thank you [11:33] Chipaca: reviewed [11:39] mvo: ummm, could you re-build core18 snap? Since currently I guess the main snap recipe is owned by you for now [11:39] probert is good [11:49] off to pick up the kids [11:58] mvo: hi! [12:00] mvo: I didn't tag those as 2.36 because I wasn't sure how suitable they were for it at this point in time and was going to let you decide (they are bigger than whatI (and probably you) were expecting last week [12:00] needless to say, they took more than 'a few hours' [12:01] mvo: I mean, I'm happy if they go into 2.36, but having them just in trunk is ok too [12:01] mvo: but I wanted to talk to you about this: audit: type=1400 audit(1539604887.557:452): apparmor="DENIED" operation="exec" profile="snap.hello-world.env" name="/sbin/apparmor_parser" pid=30864 comm="snap-exec" requested_mask="x" denied_mask="x" fsuid=1000 ouid=0 [12:02] mvo: it seems every snap run invocation is causing this. let me know when you have a moment [12:03] woah? [12:03] so snap-exec runs apparmor_parser [12:03] probably in the quest to compute system key [12:03] or something similar? [12:03] that is what I was thinking [12:03] what is worse [12:03] if apaprmor is updated [12:03] and we have a new apparmor parser [12:03] the system key will not match [12:04] but snapd doesn't care about the upgrade in the system [12:04] so it will always be wrong [12:04] so, release/apparmor.go init(), there is: [12:04] appArmorLevel, appArmorSummary = probeAppArmor() [12:04] until you reboot and snapd considers this [12:04] appArmorParserFeatures = probeAppArmorParser() [12:04] what is weird though, is if this is the issue, then why don't I see denials from probeAppArmor? [12:05] I only see denials for probeAppArmorParser [12:05] I think we allowed the former [12:05] I looked in the template. let me look again [12:05] oh, in that case I don't know [12:05] also, I was really surprised that snap run is doing all these checks [12:05] it isn't like it can do anything with the result [12:06] I think we d [12:06] we spin and wait [12:06] that's the purpose of system key there [12:06] and we are adding a bunch of stat()s and an exec() to the snap run [12:06] hmm [12:06] I definitely didn't mean for apparmor_parser to be called every time that an app was run [12:06] right [12:07] quite the opposite. I didn't even want it run every time a profile was compile [12:07] I think this falls under the idea I had a while ago with "facts" in /var/lib/snapd/facts - that would be written by snapd and interrogated by other parts of the stsack [12:07] I wanted it precisely once when snapd started :) [12:07] and I thought that was what happened with system-key [12:07] mmm, not quite [12:07] you need to be able to compute the system key [12:07] well, clearly [12:07] in snap-run [12:08] and quickly too as you now learned [12:08] we need to short circuit this or move it [12:08] I mean we could add a rule, but blech, I don't want this-- it is only useful for profile generation [12:08] I agree [12:08] hmmm hmm [12:08] snap run can literally do nothing with the information [12:12] * pstolowski lunches [12:12] jdstrand: if you need me for this just ask, otherwise I'll be going through per user mount namespaces [12:14] yeah, I don't see /sys/kernel/security/apparmor/features in the template [12:14] zyga: thanks, I'll discuss with mvo and we can decide from there [12:15] it is definitely denied: $ ls /sys/kernel/security/apparmor/features/ [12:15] ls: cannot open directory '/sys/kernel/security/apparmor/features/': Permission denied [12:17] so, probeAppArmor() is not getting called, but probeAppArmorParser() is... [12:18] which makes no sense cause they are both only called via init()... [12:19] * jdstrand is confused [12:27] perhaps probe apparmor is really lazy? [12:29] huh [12:29] ./interfaces/system_key.go:sk.AppArmorParserFeatures = release.AppArmorParserFeatures() [12:29] ./interfaces/apparmor/backend.go:parserFeatures = release.AppArmorParserFeatures [12:30] so in system_key, we assign '()' but in backend.go we do not [12:31] that is the same with AppAmorFeatures though... [12:32] ! [12:35] re [12:36] maybeWaitForSecurityProfileRegeneration() does call interfaces.SystemKeyMismatch() [12:40] I'm really puzzled that I'm not seeing denials from release.AppArmorFeatures() but I am for release.AppArmorParserFeatures() [12:41] sil2100: sure [12:41] jdstrand: hi, reading backlog (looks like there is a lot of it :) [12:41] mvo: yes, halp :) [12:42] (insert 5th element quote0 [12:43] I wonder what ioutil.ReadDir() is doing === chihchun is now known as chihchun_afk [12:43] maybe it doesn't trigger the apparmor denial [12:44] but then that would be a bug in system-key detection [12:46] hmm, it is just an os.Open [12:51] jdstrand: ls /sys/kernel/security/apparmor/features/ triggers the denial, but does stat on its own? [12:52] Chipaca: it would not, no [12:53] stat is not mediated anywhere AFAIK [12:53] jdstrand: maybe whatever's triggering the denial is not one of the required features that we stat in probeAppArmor [12:53] yes, that is what it is Chipaca [12:53] ah [12:53] probeAppArmor only does an isDirectory() which is just a stat [12:54] yup [12:54] and a rather dumb stat at that :) [12:54] aaah [12:54] ok, so mystery solved on the why probeAppArmor() doesn't trigger denials [12:54] man, that's magic [12:54] mvo: ^ [12:54] jdstrand: I just finished reading backlog - indeed, we compute the system-key in snap run, sorry that I did not realize during the review about the implication of the apparmor parser test [12:54] so, the question is how to fix probeAppArmorParser [12:54] PR snapd#5987 opened: cmd: refactor IPC and lifecycle of the helper process [12:55] jdstrand: let me quickly look at this (its 5min until our standup) [12:55]