=== chihchun_afk is now known as chihchun [05:17] morning [05:25] Hey :-) [05:32] zyga: hey hey === chihchun is now known as chihchun_afk [06:09] Hey mvo :-) [06:09] hey zyga [06:09] it looks like we have failures in the testsuite in fedora / centos right now [06:09] or is it just my PR? [06:10] My stuff was ok last night [06:10] I’m getting breakfast still === chihchun_afk is now known as chihchun [06:18] mvo: hey, more selinux crap? [06:19] btw. https://bugs.launchpad.net/ubuntu/+source/snapd/+bug/1826460 is the same story as https://forum.snapcraft.io/t/selinux-warning-when-running-lxc/11100/ [06:19] i seriously don't know what to think of it, oh and 2.39 breaks totally there [06:20] i reproduced the problem with disco cloud image and installed that weird kernel from the kernel-ppa [06:20] mborzecki: aha, is that the stacked LSM kernel? [06:26] mvo: i don't think so, the kernel seems to have AA disabled, and SELinux enabled and default [06:28] mvo: oh, and once you install the kernel, none of the selinux utils are pulled in [06:33] mborzecki: woah [06:36] Stacking? [06:38] zyga: there is this work to allow selinux and apparmor in parallel, I was wondering if its that kernel [06:38] mvo: disco has it [06:39] It landed at the same time as another lxd related bug with shiftfs [06:43] shitfs? [06:43] ah, that's for uid/gid on fs [06:44] anyways, i don't think it's this kernel === pstolowski|afk is now known as pstolowski [07:01] heyas [07:03] pstolowski: hey [07:04] left a note under https://bugs.launchpad.net/ubuntu/+source/snapd/+bug/1826460/comments/3 [07:04] hey PaweΕ‚ :) [07:05] idk, maybe we could somehow guess the system is using a default policy, meaning, nothing probably works an then stop any attempt to use selinux [07:07] pstolowski: https://github.com/snapcore/snapd/pull/6781 <- 2nd version of static attr fix [07:07] zyga: yes, i saw it, thank you, will get to it shortly [07:07] pstolowski: tha main difference is that the changes are made in reloadConnections, not in setup profiles, because this catches system key regeneration [07:08] thanks! [07:11] still don't get why someone who's not a kernel developer or test would install some kernel package from a random PPA [07:28] mborzecki: I know why [07:28] mborzecki: sergiusens does it because it makes his surface work [07:28] mborzecki: AFAIK people do it to get modern vega working on older releases [07:28] zyga: well, but sergiusens is more than capable of making it work [07:28] mborzecki: some people do it to follow vanilla kernel development [07:29] mborzecki: but not all people are sergiusens :) [07:31] zyga: also, if you provide a kernel package for ubuntu, one would expect that it's soemwhat tailored for the distro :P [07:31] mborzecki: there's an official vanila kernel ppa [07:31] mborzecki: not tailored at all [07:31] zyga: it's like someone provided a kernel package from COPR for fedora, which defaults to AppArmor, that'd be fun [07:31] but what do I know :) [08:05] * zyga could use a 2nd review for https://github.com/snapcore/snapd/pull/6758 [08:06] promise not to push more today ;) [08:12] * zyga does more reviews [08:30] zyga: reviewed static attrs PR [08:37] pstolowski: thank you! [08:40] mvo: morning sir. Working with sergiusens for a bit yesterday we were both surprised that #6532, which landed in master on 7/3, wasn't in 2.38 which was cut 21/3. My memory of (and, granted, attention to) the release is very poor, so I was hoping you'd know why this was this way [08:41] https://github.com/snapcore/snapd/pull/6743 could use a second review if anybody is hankerin' to do one [08:42] Chipaca: uh, that sounds fishy! [08:44] Chipaca: aha, I see now. 2.38 was actually branched earlier, 2019-03-05 and at this point we only cherry-picked things targeted for 2.38 :/ [08:45] mvo: i suspected something like that [08:45] mvo: but didn't have dates :-) [08:45] a shame, but ok [08:45] (sergio was all ready to use that feature :) ) [08:47] Chipaca: it wil be in 2.39 which is not that far away [08:47] yep [08:55] * pstolowski running quick errand, afk for a while [08:58] heh [08:58] Chipaca: I wanted to review https://github.com/snapcore/snapd/pull/6743 but I noticed I already did [08:58] * Chipaca hugs zyga [08:58] zyga: thank you [09:02] Chipaca: wanna do a quick 2nd +1 for https://github.com/snapcore/snapd/pull/6758 ? [09:02] a part of an ongoing refactoring [09:12] zyga: where is NewUserProfileUpdateContext used for, that it's public? [09:13] it's used from main.go [09:13] * zyga checks [09:14] probably in subsequent patches [09:14] Chipaca: in 2 patches from this one: https://github.com/zyga/snapd/commit/1ffaae4478d680566b78ead86f3911c5784893ea [09:14] Chipaca: so not yet, but soon :) [09:15] Chipaca: https://github.com/zyga/snapd/commits/feature/user-mount-ns-20.9-split-rebased - [09:15] zyga: that's in the same package, isn't it? [09:15] yes [09:16] but we may need to put it into a library soon-ish so I'm slowly building towards that [09:16] (for a new binary) [09:16] snap-update-ns and snap-bootstrap-ns [09:16] would share most of the code but would behave differently and bootstrap would do some new things too [09:17] (bootstrap would also not need the horrid C bootstrap logic) [09:17] (as in bootstrap.c, the name overlap is unfortunate) [09:21] zyga: reviewed, mostly. Almost a +1. [09:21] reading [09:23] Chipaca: replied [09:25] if it seems I'm relucant to change it it's partially true and because of the changes stacked on top; I can rework locking if you feel strongly about it but even there I would prefer to append a patch to this series (that was tested and "polished" rather than rebase it, mostly to avoid the extra cost [09:28] re [09:29] pstolowski: hey [09:29] question to you sir: [09:29] is this what you expected? https://www.irccloud.com/pastebin/CoMWt6SB/ [09:29] I would have to add the pair of unrelated snaps for this test to preserve that connection [09:31] zyga: yes I think the lock/unlock pair would be better, but also yes no problem for it to be done further down the series [09:31] zyga: yes, that's what i expected. do you need to add those snaps because something is unhappy about them missing? [09:31] Chipaca: in that model, the unlock state would be simply stored in the context, right? [09:31] zyga: yes [09:31] pstolowski: not unhappy, as the comment states, they are removed on manager startup [09:31] pstolowski: note that the order of manager startup vs adding snaps is essential [09:32] zyga: or you could just embed the locker [09:34] zyga: you could add them to the state after instantiating the manager and before carrying on with the test, no? [09:35] No [09:36] Because manager removes unused connections on startup [09:36] I would have to add the unrelated snaps before starting the manager [09:37] I can do that, just asking if that is what you expected [09:37] zyga: yes, but that happens in Manager - initialize, when you instantiate the manager in the test. you can alter the state afterwards? [09:39] zyga: ah i see [09:39] zyga: perhaps the second test is better to do this [09:39] zyga: the one that creates a task for setup-profiles [09:39] Mhm [09:40] zyga: sorry, you're right about the one i commented on [09:44] zyga: btw https://github.com/snapcore/snapd/pull/6751 is pending some rework right? [09:44] pstolowski: looking [09:44] pstolowski: well, it needs tests [09:44] pstolowski: but I think the place is right [09:45] ok [09:47] pstolowski: some quick fixes for the policy https://github.com/snapcore/snapd/pull/6783 that were caught in 6778 [09:53] mborzecki: man, selinux is comple [09:53] complex* [09:56] zyga: or just different conceptually [09:57] zyga: π•Œπ•Ÿπ••π•–π•£π•€π•₯𝕒π•₯π•–π•žπ•–π•Ÿπ•₯ 𝔸𝕝𝕖𝕣π•₯ [09:58] * mvo hugs zyga for "the computer says NO" [10:03] lol :) === Girtablulu is now known as Girt|Vacation [10:12] pstolowski: updated https://github.com/snapcore/snapd/pull/6781 [10:15] the new thing is https://github.com/snapcore/snapd/pull/6781/commits/dd739571c24e659d0449c0c0ca6314719f301777 [10:19] * Chipaca goes out for a bit [10:22] mvo: I think you should review https://github.com/snapcore/snapd/pull/6781 [10:35] zyga: full of meetings today but will do [10:35] zyga: once I get the chance [10:37] zyga: ty, will look in a bit [10:48] Chipaca: something like this? https://github.com/zyga/snapd/commit/328b4b471e22609d60b4b4e430a4aa648963c714 [10:48] mvo: thank you! [10:50] mvo: about https://github.com/snapcore/snapd/pull/6756 - should I rework it to return an error or leave as-is? [10:57] Chipaca: https://bet365techblog.com/open-sourcing-jingo-a-faster-json-encoder-for-go <- maybe worth prototyping a switch to see what we'd save? [11:16] * zyga quick lunch === chihchun is now known as chihchun_afk [11:38] my body demands coffee [11:39] my mind demands coffee [11:39] I shall make coffee [11:39] though going outside for a bike ride seems lovely too, it's so warm already [11:39] pstolowski, mborzecki: I have 28C outside, you? [11:39] 28-29 [11:40] zyga: 27 outside. 28 inside.. /o\ [11:43] desktop-portal-filechooser fails [11:43] la la la [11:47] out to drop the kids off at school for some extra classes, back in 30 [11:47] mborzecki: hey, can you look at https://github.com/snapcore/snapd/pull/6780#discussion_r278914233 [11:47] mborzecki: in 30 :) === chihchun_afk is now known as chihchun [11:57] jdstrand: I pushed some things to https://github.com/snapcore/snapd/pull/6714 [11:58] hey :) [11:58] jdstrand: (also replied to all the comments there) [11:59] jdstrand: the /tmp thing has left one question in my mind [11:59] jdstrand: on regular systems with classic packages /tmp is not private [12:00] jdstrand: so if an app invoked by another user on this system drops a file that just happens to be readable by my user, well, tough luck, it'd better not contain secrets [12:00] that's just normal dac [12:01] jdstrand: the question is: why is snap /tmp different? could we just make /tmp/snap.$SNAP_NAME/ with 01777 [12:01] the /tmp should still be 1777 root:root [12:01] jdstrand: note, in my question I eliminate the presence of intermediate root:root 0700 base directory [12:02] it doesn't really need to be different. iirc, you are doing 0700 at that point only to bind mount on it and then later chmod 1777 [12:02] hmmm, we don't bind mount on it AFAIK [12:02] or perhaps we do but that's something that we could change [12:03] you are otherwise it wouldn't be known as /tmp in the mount namesapce... ? [12:03] that is, we could mkdir 01777 and then bind mount that over /tmp (so not over /tmp/$RANDOM/tmp but over constructed-snap-rootfs/tmp [12:03] yeah, but we construct the root filesystem in a different way now (on the side) and the very old code that manages /tmp could be adapted to that style too [12:04] we could even mount a tmpfs there and call it a day (no visibility from initial mount namespace) but I think that was ruled out because of space constraints [12:04] I'm not opposed to that changing. the main thing is, the result needs to be root:root 1777 and the getting there needs to be race-free [12:04] jdstrand: can you expand on the race-free aspect? [12:04] zyga: I don't think we want to mount a tmpfs there [12:05] jdstrand: (agreed on tmpfs) [12:05] zyga: we are actively not doing that because a tmpfs can take up to 50% of ram and those cause low memory [12:05] thus* [12:06] zyga: race free> yeah, however we get there, there can't be a possibility of an unprivileged user (or a snap at all) to to symlink attacks, etc [12:06] mhm [12:07] we protect against some of the attacks on mount with our special mount code in snap-update-ns but we don't have a C equivalent AFAIK [12:08] jdstrand: as far as I'm aware only mount is really susceptible to a race attack, out of the primitives we use [12:08] re [12:08] jdstrand: I'm trying to find a solution that would be devoid of edge cases, correct, not insecure (see what I did there :), and usable for people [12:08] zyga: I think you are not taking my point as intended [12:09] zyga: we have a property now where we are race free. with the rename to a fixed dir or the rewrite you are sugggesting, we want the same property [12:10] jdstrand: no disagreement there [12:13] * jdstrand takes deep breath [12:14] why is sid failing to build all of a sudden: https://paste.ubuntu.com/p/k9nYZkrSgP/ [12:15] jdstrand: where are you seeing this? [12:15] in the sbuild test? [12:15] I can't get out from under constant problems with this PR... /o\ [12:16] zyga: I saw it didn't build here: https://travis-ci.org/snapcore/snapd/jobs/524707744 [12:16] jdstrand: hey do you remember user mounts? :D [12:16] * zyga looks [12:16] zyga: it did not have great logs, so I tried in a local qemu [12:16] jdstrand: it's going to be all right, it's just one of those moments where you improve things for many people, not just snap users, by fixing bugs blocking your work [12:16] use -debug with spread and then rewound and did: [12:17] cd /home/gopath/src/github.com/snapcore/snapd [12:17] cd _build && go install -gcflags=all=\"-trimpath=/home/gopath/src/github.com/snapcore/snapd/_build/src\" -asmflags=all=\"... [12:17] as shown by the -debug output [12:17] zyga: yes, that's true. it is just been nuts [12:17] thanks [12:19] in this case, I don't know what bolt is. I do know what libseccomp-golang is, and I'm puzzled why sid is not working with the distro golang-seccomp [12:19] * jdstrand shakes fist [12:19] it seems to not be installed. why? [12:19] how we ignore errors in some places? https://www.irccloud.com/pastebin/4jf6qJKw/ [12:19] [12:19] * jdstrand looks [12:20] jdstrand: I'm trying to make sense of that but I see more magic [12:20] * jdstrand is puzzled. these packages are in the build-depends [12:21] src/github.com/snapcore/snapd/cmd/snap-seccomp/main.go:655:10: undefined: seccomp.ActLog <- this seems to say that golang-seccomp is older? [12:21] jdstrand: build-depends vs vendorized [12:22] I'm wrong, it is installed [12:22] zyga: ah. ok. yes [12:23] package from debian took priority? [12:23] zyga: man I skipped right over that [12:23] ok, I see what to do [12:24] mborzecki: do you think if I just added the 4th field to system-key it wouldn't be a big deal? [12:24] jdstrand: but does that mean we won't be able to build de-vendorized in sid? [12:24] jdstrand: the sbuild test should show that btw, if that breaks next we need to update golang-seccomp in debian [12:24] zyga: I can make it work. it worked before [12:24] jdstrand: system key is flexible, just check how costly it is to compute because snap run does it [12:24] I made a small changge to tyhicks initial patch which apparently broke it [12:25] zyga: adding the 4th field couldn't be cheaper. it is that string match [12:25] zyga: sorry for the delay - 6756 is afaict no longer needed, the change that landed recently checks for nil and returns an error now [12:26] mvo: oh, where was that? [12:26] mvo: please comment in the PR and feel free to close it [12:26] zyga: thanks, will do [12:26] mborzecki: nm, I'm going for it [12:27] jdstrand: i wouldn't mind, it's an implementation aspect, not user facing, but nothing i would block on [12:27] jdstrand: I will be much happer once, one day, I will replace all of snap-seccomp with pure go code [12:27] jdstrand: though once it's collected & parsed in backend init, you could do whatever later on in the code, no need to call snap-seccomp once again [12:27] (unless for compilation obviously)( [12:27] jdstrand: and can remove that from system key for that extra oomph speed for snap startup :) [12:28] mborzecki: note that snap-run does this every time AFIAK [12:28] zyga: yes, it also picks up build ids and so on [12:29] so-many-reviews [12:29] zyga: well, I think you cannot do that without removing libseccomp cause libseccomp is C code and CGO_ENABLED=0, but I don't want to go there [12:29] mborzecki: does it mean snap-run will run snap-seccomp with some query argument each time on every run? [12:29] jdstrand: yes, that's what I meant [12:29] but don't worry, I'm sure we will slowly get there, with enough checks to be confident we are producing the same bpf as before [12:30] zyga: it's part of system-key, so yes [12:30] mborzecki: would be good to time that [12:31] (on a pi for example) [12:32] zyga: I can't say how worried I am about that any more than I have. we should *not* reimplement libseccomp. the world has standardized on it and it is tricky code. we will trade one set of problems that everyone faces and has a desire to fix for another that only you could fix and no one else cares [12:32] zyga: afaik we call apparmor_parser too, woulnd't expect snap-seccomp to be slower than that [12:33] but I don't have time to debate this [12:34] showoffs [12:34] oops, wrong place [12:34] offshows :) [12:34] that was a comment about you and zyga having ~28C [12:35] didn't notice i was scrolled way up [12:35] haha [12:35] zyga: looking at your locking code now [12:35] Chipaca: I pushed a 2nd branch that fixes a typo in the branch name but it's otherwise identical [12:36] zyga: calling Unlock when it's not locked would usually be a panic() [12:36] Chipaca: +1, easy change [12:36] zyga: either implicit or explicit I don't mind :-) [12:37] hmm a bunch of snaps mounted locally, somehow udisks insists on showing 2 of them as normal disks, wtf? [12:37] mborzecki: that's a local patch to udisks or you lost your mtab state [12:38] mborzecki: that seems to happen when something is using and old core [12:38] mborzecki: jdstrand: though once it's collected & parsed in backend init, you could do whatever later on in the code, no need to call snap-seccomp once again [12:38] Chipaca: yes, that would be it [12:39] mborzecki: I recall looking at that and had some trouble. what are you suggesting? [12:39] mborzecki: reading it off the disk? [12:39] my typo-ing rate today is quite high [12:39] seem to be typing phonetically a lot [12:39] jdstrand: iirc version info is kept in seccomp backend struct [12:39] mborzecki: (this was the bit where I said 'there were multiple ways of doing this' in my PR [12:40] mborzecki: that isn't available way out in overlord/snapstate [12:41] mborzecki: I was hoping to extract it from somewhere via interfaces/system_key.go but resorted to a callout to snap-seccomp [12:42] augh. There's a Jesse Lenton somewhere in Winnipeg that's receiving some documents via a secure online thing, but they've given my email for it … so if anybody wants to find out some dirt about some employees of some random corp in canada, hit me up [12:43] mborzecki: afaict, snapd doesn't maintain system-key in memory. it has it on disk and at strategic times, recalculates and compares to waht is on disk [12:43] jdstrand: hm that's probably easier, the cache info is well hidden under interfaces :/ [12:43] yeah [12:43] mborzecki: but I could read it from disk [12:44] mborzecki: I was somewhat hesitant of that though [12:44] I mean, it *should* be ok [12:44] jdstrand: imo calling the compiler again sounds reasonable [12:45] mborzecki: ok, that is where I landed. I'll add the fourth field then I just have the one callout [12:45] jdstrand: unless, there would be like an instance of seccomp.Compiler() set up to use snap-seccomp kept somewhere [12:46] jdstrand: hm otoh, that microoptimization is probably not worth it, so a second call to snap-seccomp sounds ok to me [12:47] mborzecki: again, where I landed with 'picked something that made sense to me' :) [12:47] mborzecki: thank you for discussing this here [12:55] * Chipaca wanders off to make tea === chihchun is now known as chihchun_afk [13:33] Chipaca: thanks for sharing that error talk! [13:33] I'm very interested in error handling in both Go and C [13:33] zyga: there's also a very good one about constants I think you'd enjoy [13:34] zyga: https://www.youtube.com/watch?v=pN_lm6QqHcw [13:34] Chipaca: opened in another tab [13:35] https://github.com/snapcore/snapd/pull/6785 is green [13:35] * zyga uses social engineering tricks to get reviews [13:36] * Chipaca hires a russian bot farm to -1 zyga's PRs and thwart his social engineering campaign [13:36] * Chipaca buys some sweets with the change [13:36] Chipaca: we could move code review to twitter and facebook :D [13:37] Chipaca: so can we start to stop using fmt.Erorrf :) [13:37] zyga: well we shouldn't :-) [13:37] because they hacked fmt.Errorf :-D [13:37] (continue the talk) [13:37] drat, that's like we watch a movie together but you know the ending :D [13:37] * zyga watches [13:37] as long as it's fmt.Errorf("yadda yadda %v", err), or %s err, it'll DTRT [13:38] oh [13:38] tricky [13:38] it groks the argument is the wrapped error? [13:38] zyga: also the error crawls up its butt and explodes it from the inside [13:38] and stores it as well? [13:38] something like that? i haven't tried it :-) [13:39] juju/errors :) [13:39] nice [13:39] Chipaca: so will golang 2.0 switch to spaces and ban tabs? [13:39] or will it be something we can reasonably be on in the next 12 months after it ships? [13:40] all those big 2.0 features are getting worked into 1.x [13:40] that was one of the things that came out of the 2.0 discussions [13:40] so what will 2.0 bring? [13:40] that's not in 1.x? [13:41] interesting [13:41] zyga: actual incompatible things left over? [13:41] using interfaces is so natural in go [13:41] zyga: basically they don't want to be python [13:41] that's a neat idea [13:42] Chipaca: a bit of what I'm hearing starts to feel like objc [13:43] zyga: i haven't used objc in anger, so i don't have a strong opinion of it either way [13:43] zyga: but i also don't know what you mean :-) [13:43] Chipaca: the highly dynamic aspect [13:44] i think the only time i've used objc it was actually c with some weird bits (rather like how i've used c++) [13:45] the errors.As thing is odd, why on earth is it returning a boolean? [13:47] zyga: sounds like you really want that pkg/errors package :) [13:47] mborzecki: not sure yet [13:47] I see their point about how those are weak [13:49] zyga: errors.Is returns a boolean [13:50] Chipaca: yes, but As ? [13:50] Chipaca: I was expecting some form of casting but perhaps I misunderstood that [13:51] https://tip.golang.org/pkg/errors/#As [13:51] zyga: so you tell it the type by passing it a thing, so it sets that thing to the matching one and tells you that it succeeded via the bool [13:51] ah, so it writes to the target [13:52] I thought it just uses that for type information [13:52] that makes sense now [13:52] ye [13:52] though ugly because golang and generics but that's all right [13:56] zyga: (?i) makes the (containing sub)regexp case-insensitive from there to end or (?-i) [13:57] yeah [13:57] I read the replies from Maciej [13:57] mborzecki: +1 [13:57] zyga: you can also do (?i:regexp) and have that subregexp be case-insensitive [13:57] * zyga liked the RE_I flag or whatever that was called in other systems [13:58] zyga: other systems also support these flags [13:58] i mean, this is taken from perl regexp syntax [13:58] perl, python, and pcre all also support making it case insensitive outside of the regexp though [13:58] and yes that is clearer, to me [13:59] but Β―\_(ツ)_/Β― go's purportedly-perl-compatible regexps (which aren't) have that nice predictable non-pathological time characteristics, so that's nice instead [13:59] btw perl's regexp support for unicode is _still_ unmatched [14:02] * Chipaca looks into outsourcing reviews [14:09] Chipaca: those russian trolls, do they do reviews too? :) [14:10] zyga: i'm sure they do [14:10] CHA-CHING! [14:10] mvo, could you please include this pr #6694 to next 2.39 [14:11] mvo, you merge all the PRs with milestone 2.39 right? [14:13] mvo, otherwise I can provide you a list of PRs which I need on 2.39 [14:15] Chipaca: watched both now [14:22] jamesh, hi [14:24] jamesh, I am testing snapd on 19.04 and 1 test is failing https://paste.ubuntu.com/p/YN44WYGCM5/ [14:25] doing xdg-open snap://package [14:25] jamesh, I was researching and found that it fails when the package gnome-settings-daemon is preinstalled [14:26] mvo: can we merge https://github.com/snapcore/snapd/pull/6717 [14:27] jamesh, were you aware of this problem? [14:38] hmmmmm [14:38] zyga: looking [14:38] zyga: scopedTracker is a strange name [14:38] mvo: perdronis changed it [14:38] mvo: do you know why we allow installing ubuntu-core still? [14:38] zyga: yeah [14:38] mvo: so I guess it is his informal +1 [14:38] Chipaca: wuuut? we do? [14:38] mvo: wrt to name [14:39] zyga: yeah [14:39] (insert chipaca emoji) [14:39] zyga: hahaha [14:39] I don't care, it's important to fix the bug more than to have a fun name [14:39] zyga: I'm with you on that [14:39] mvo: yes we do. Needs to be part of a multi-snap install; we block it for single snaps [14:40] mvo: shall I merge it then? [14:41] zyga: yes, looks sane [14:41] zyga: there is as yet no chipaca emoji. But we've got a mate emoji, so the chipaca can't be far behind! [14:41] Chipaca: aha, I see [14:41] mvo: i'll fix [14:41] mvo: merging, thanks! === chihchun_afk is now known as chihchun [14:44] Chipaca: thank you [14:57] mvo: https://github.com/snapcore/snapd/pull/6786 fwiw [15:06] thanks Chipaca [15:06] Chipaca: s/also// [15:07] Chipaca: +1 though a spread test would be nice [15:07] zyga: why no also? [15:07] it reads oddly, not sure ;) [15:07] saying [15:07] ah ok [15:07] saying "daemon: verify snap instructions for multi-snap requests" reads better [15:07] boom [15:08] thanks! [15:08] i also removed the extra space your regexp left behind [15:09] darn, I was thinking about that [15:09] :D [15:09] ok [15:09] last thing of the day: spread test for the mount bug [15:35] jdstrand: I'd love your thoughts on https://forum.snapcraft.io/t/potential-additions-to-account-control-interface/11123/1 [15:37] Odd_Bloke: is the snap you're building only for use on ubuntu core? [15:39] Chipaca: At the moment, I'm just doing exploratory work to see what would be required to strictly confine cloud-init; this was one of the things that I found I had to add a `system-files` interface for, despite us having an interface that sounds like it would include them. === chihchun is now known as chihchun_afk [15:40] Odd_Bloke: account-control is all about core tho [15:40] There is nothing in the docs that indicates that to be the case, AFAICT? [15:41] the docs might need updating; it's an old interface [15:41] # Only allow modifying the non-system extrausers database [15:41] ^ that's from the code [15:41] The docs say "account-control allows managing non-system user accounts." [15:41] yes i can read [15:41] sigh [15:41] Odd_Bloke: sorry, not sure where that came from [15:42] Odd_Bloke: yes the docs might need updating, if I've got this right [15:42] i'm not an expert on this bit of the code, jdstrand was the right person to ask [15:42] but, from what's in the code, this will only work in core [15:45] I responded [15:46] Thanks! [15:47] * cachio lunch [15:49] ok, rewrote the actlog PR: https://github.com/snapcore/snapd/pull/6787 [15:49] it was a more involved pr, but cleaner in the end, so I thing that is a good thing [15:55] zyga: hostname-control might need a similar warning? [15:55] zyga: OTOH if they're only for core, why have them in classic at all? [15:55] Chipaca: TIF [15:55] ! [15:55] * Chipaca goes [15:56] I think hostname-control is useful on classic? [15:56] It gives write access to /etc/hostname and exec on hostnamectl. [15:58] Odd_Bloke: sure. it has no restrictions like that (just rememeber to snap connect it) [15:58] (it is manually connected by default, which can be adjusted with a snap declaration) [15:59] I also had to add a few system-files rules to allow me to manage networking from cloud-init (on classic); is network-control expected to be a core-only thing, or is that worth opening up a topic for? [16:03] Odd_Bloke: it is for anyone. please open a topic [16:03] Will do. [16:03] Odd_Bloke: I'll be doing some profile bug fixes for 2.39 and will add that to the list [16:06] Oh, actually, I hadn't seen network-setup-control until I typed in my topic title, let me see if that makes a difference. [16:07] OK, that's simple enough to just read. [16:23] jdstrand: does https://forum.snapcraft.io/t/snapping-a-font/11122/2?u=zyga sound sensibly? [16:27] zyga: it would help. with fonts it probably isn't bad. with arbitrary content like images, sounds, movies it would be rather heavyweight [16:33] jdstrand: https://forum.snapcraft.io/t/network-control-network-setup-control-doesnt-enable-running-netplan-generate-on-classic/11126 [16:36] jdstrand: fonts contain full VMs nowadays, you know? [16:37] ISTR there were font cache versioning problems recently, too? [16:37] That might make fonts a little trickier? [16:37] (I recall none of the details.) [16:37] Odd_Bloke: yes, at runtime, but not for distributing the font by itself I think [16:37] but yeah [16:37] it compounds the problem [16:38] zyga: wrt fonts etc, couldn't we sanity-check them server-side? [16:38] Chipaca: that's an interesting idea, perhaps we could [16:39] Chipaca: but it would be tricky for one reason [16:39] Chipaca: when you find that you were not checking for something that is evil and you said it was fine before you have to revoke that decision [16:39] Chipaca: in a filter approach you can provide a refresh to the filter package and re-generate content [16:39] zyga: yeah [16:39] Chipaca: thereby neutering the invalid blob [16:40] Chipaca: I think it's all a scale of paranoia [16:40] one could say that some type of content is plausibly ok to ship without a filter [16:40] CM fonts [16:40] \o/ [16:40] Chipaca: some things can be chameleons and can be interpreted as many things at once [16:40] it's complex but for some stuff I'd love to have it [16:40] wallpapers and fonts are a good candidate [16:40] fonts should be shipped only as metafont instructions, launchpad builds it into a snap that's gtg [16:47] jdstrand: https://github.com/snapcore/snapd/pull/6787#pullrequestreview-231254474 [16:48] i now have two (2! which is also 2) PRs that are green and just need another +1 to land [16:49] * Chipaca also has more prs than just those, but these two are Special [16:49] * Chipaca puts the PRs in the short bus [16:54] Chipaca: wanna read https://github.com/snapcore/snapd/pull/6785 ? [16:54] I could bug mvo for +2 and then open the next chunk :) [16:55] (only about 5 left) [16:55] this is a +12 βˆ’13 PR so pretty painless [16:55] jdstrand: any chance you'll get a chance to review 6697 (about daemon-notify) soonish? if not that's fine I can workaround it but it would be better to know ahead of time if that workaround is likely to be necessary [16:59] zyga: why is 1000 the right thing instead of os.Getuid() [16:59] zyga: ? [17:01] ijohnson: the workaround is likely necessary. possibly next week [17:14] Chipaca: because the test uses 1000 a line above :) [17:14] it's a fixed value [17:14] Chipaca: read the 2nd commit plase [17:14] *please [17:14] Chipaca: sorry for the lag, I went upstairs to stretch and found pizza lefttovers :D [17:15] *leftovers [17:16] mvo: pretty please a 2nd review on https://github.com/snapcore/snapd/pull/6785 [17:16] Chipaca: thank you! [17:17] Chipaca: for on this line https://github.com/snapcore/snapd/pull/6785/commits/0917f2460ae1d8dbe19823c32d79773add5addb1#diff-fca81d6060b39fa4cf7450a9d05bd004R384 [17:28] * zyga EODs [18:21] jdstrand: ack thanks