[04:54] <mborzecki> morning
[04:55] <mborzecki> morning
[04:59] <mardy> mborzecki: hi!
[05:00] <mborzecki> mardy: heya
[05:01] <mborzecki> mardy: have you figured out that mount control interface?
[05:22] <wez> Hi, does snappy plan on moving away from squashfs at some point?  My CPU and CPU fan would appreciate it.
[05:45] <mardy> mborzecki: the only question left is why we accept a list of types for the mount, rather than a single type. But it's a detail, not blocking me
[05:46] <mborzecki> wez: not that i'm aware of, but different compression methods are in use, lzo iirc which should be much more cpu friendly
[05:49] <mardy> wez: you might find this relevant: https://forum.snapcraft.io/t/squashfs-performance-effect-on-snap-startup-time/13920
[06:39] <pedronis> I need to re-review this but it also needs other reviews https://github.com/snapcore/snapd/pull/10420
[07:07] <pstolowski> morning
[07:09] <mborzecki> pstolowski: hey
[07:10] <mvo> good morning pstolowski, mborzecki and pedronis 
[07:10] <mborzecki> pedronis: and good morning to you too
[07:11] <wez> mborzecki, mardy: Thank you both!
[07:12] <mborzecki> mvo: hey, thanks for landing that PR
[07:20] <mardy> from 1 to 10, how horrible would it be if from interfraces/buildin/mount-control.go I called the resolveSpecialVariable() function defined in content.go? :-)
[07:20] <zyga-mbp> good morning
[07:20] <zyga-mbp> mardy: hmmm
[07:20] <mvo> good morning mardy and zyga-mbp 
[07:21] <zyga-mbp> mardy: there are two functions like that and I don't think they are exactly the same (unfortunately)
[07:21] <mardy> for the time being I'll just replicate the method with another name, and then -- if confirmed that I really need it -- I can move it to another file and share it across the two interfaces
[07:21] <mardy> hi zyga-mbp, mvo 
[07:21] <zyga-mbp> mardy: I'm sick today so my memory is rusty but IIRC they don't allow the exact same things
[07:21] <zyga-mbp> but give it a try and ping me for a review, I'll gladly look
[07:22] <mardy> zyga-mbp: currently there's just one function, it's resolveSpecialVariable() in content.go. Now I'm writing a new interface (mount-control), and I think I could use the same function
[07:22] <zyga-mbp> mardy: there's another function that looks and feels like resolve special variable
[07:22] <zyga-mbp> I really don't remember where but there are two
[07:23] <zyga-mbp> mvo: hey :)
[07:25] <pedronis> mvo: do I misremember:  https://github.com/snapcore/snapd/pull/10449#discussion_r661199310 ?
[07:35] <mardy> mvo: I think you once mentioned that protecting filesystem resources via chroot and namespaces is insecured as it can be trivially bypassed; that only applies if plain chroot() is used, right? Or is pivot_root unsecure too?
[07:36] <mardy> (it's not related to what I'm working on, it's just a curiosity)
[07:36] <mvo> pedronis: I think you are right, was mostly wondering what we should do for corner cases like a pi that has no config.txt for some reason and someone sets pi-config.some-option=1. but otoh I'm not even sure a system without config.txt will even boot
[07:36] <mborzecki> mardy: you want pivot_root, chroot can be trivially bypassed
[07:36] <zyga-mbp> mardy: pivot_root breaks apparmor if you think about it 
[07:37] <zyga-mbp> mardy: chroot is IIRC now unprivileged and doesn't really buy much security 
[07:37] <mborzecki> `sshpass -p ubuntu ssh -p 8022 -o ConnectTimeout=10 -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no user1@localhost date -d tomorrow "+%Y-%m-%d %T"` hmm gives `date: extra operand ‘%T’`
[07:37] <mborzecki> but `sshpass -p ubuntu ssh -p 8022 -o ConnectTimeout=10 -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no user1@localhost sh -c 'date -d tomorrow "+%Y-%m-%d %T"'` works as expected
[07:38] <mborzecki> oh how i hate shell quoting
[07:40] <mvo> mardy: not sure I said this about namespacing, but definitely about chroot. let me quickly look at pivot_root
[07:41] <mborzecki> hm nope, still broken
[07:41] <zyga-mbp> mvo: pivot_root affects apparmor path resolution
[07:42] <zyga-mbp> mvo: if you can mount and pivot_root you can break out of the sandbox
[07:43] <mborzecki> hmm hmm, so passing parameters with spaces to nested_exec is broken
[07:43] <mborzecki> and that's because sshpass appears to do the wrong thing
[07:48] <mborzecki> oh well, i'm just dump
[07:48] <mborzecki> dumb
[07:54] <mardy> zyga-mbp: you mean, one could mount a partition like /dev/sda1 and the re-pivot to that, or something else? If one does not expose /dev/sd* (and other block devices) to the chroot, would you still be able to escape?
[07:55] <zyga-mbp> mardy: so first of all, the apparmor profile allows you do do certain things
[07:55] <zyga-mbp> but all that is based on paths
[07:55] <zyga-mbp> pivot_root lets you create arbitrary new place that apparmor thinks is /
[07:55] <zyga-mbp> so if we allow you do do anything in $SNAP_DATA
[07:55] <zyga-mbp> you can put the entire system in $SNAP_DATA
[07:55] <zyga-mbp> and then modify snapd state.json
[07:55] <zyga-mbp> and that's it
[07:56] <zyga-mbp> as for your question
[07:56] <zyga-mbp> again, it all depends
[07:56] <zyga-mbp> if there's a way for you to mount a block device all hope is lost
[07:56] <zyga-mbp> all snaps see device nodes in /dev
[07:57] <zyga-mbp> if apparmor was not blocking mounts, you'd be able to break out easily
[07:57] <zyga-mbp> I'm not sure exactly how powerful you want mount-control to be
[07:57] <zyga-mbp> there are ranges of options here
[07:57] <zyga-mbp> from super-privileged to mildly privileted
[07:57] <zyga-mbp> *privileged
[07:57] <mardy> zyga-mbp: my concern is about distros where we don't have apparmor; would security there be improved, if we setup a chroot (with pivot_root) and didn't mount /dev/ block devices into that?
[07:57] <zyga-mbp> there are now interesting ways to do mount mediation
[07:58] <zyga-mbp> lxd is using some of that
[07:58] <zyga-mbp> mardy: no
[07:58] <zyga-mbp> mardy: without apparmor the sandbox is entirely ineffective today
[07:59] <zyga-mbp> mardy: you can just spin up a service that runs as root, talk to udisks and ask it to mount whatever you want
[07:59] <zyga-mbp> mardy: this is just a random example, I'm sure there are countless other attacks that entirely break out of the sandbox
[08:00] <zyga-mbp> mardy: I mean, without apparmor you can just go to hostfs and modify /boot
[08:00] <zyga-mbp> or drop a new systemd unit
[08:00] <zyga-mbp> or whatever
[08:00] <mardy> zyga-mbp: right, but then udisks itself is a rather privileged interface, I guess?
[08:00] <zyga-mbp> mardy: without apparmor, there is no control over what dbus traffic is allowed
[08:00] <zyga-mbp> mardy: on a Fedora system or on a Debian system where dbus is not mediated by apparmor, you can break out instantly 
[08:06] <pedronis> zyga-mbp: mount-control will be superprivileged, also like system-files it will have allow lists about device and mountpoints
[08:07] <zyga-mbp> pedronis: for a super-privileged interface things are easier
[08:07] <zyga-mbp> if the interface also comes with a list of what is effectively allowed
[08:07] <zyga-mbp> you can synthesize the apparmor profile that captures that
[08:07] <zyga-mbp> there's one complexity though
[08:08] <zyga-mbp> we don't have any logic that would react to the snap performing some mounts in arbitrary places
[08:08] <zyga-mbp> and the mount profile being updapted
[08:08] <zyga-mbp> it's likely that a mount performed by the snap itself
[08:08] <zyga-mbp> will either be undone by any mount changes done by snap-update-ns (and those can happen just because some other snap refreshes)
[08:08] <zyga-mbp> or worse, snap-update-ns can fail, leaving the mount namespace of that snap in a corrupted state
[08:09] <pedronis> I understand but the main use case is actually super privileged, is about mounts int the host, not even the snap namespace
[08:10] <pedronis> anyway another reason to have a allow list, I would expect some mounts to make no sense, or be very fragile because of the namespace
[08:12] <zyga-mbp> pedronis: can you give me an example of what might be mounted?
[08:13] <zyga-mbp> I'm asking because there's one more special bit that is relevant here
[08:13] <zyga-mbp> propagation settins
[08:13] <zyga-mbp> *settings
[08:13] <zyga-mbp> currently snaps cannot technically mount anything that would show up on the host, except for the /mnt directoy
[08:14] <zyga-mbp> to be clear: if a snap is allowed to do any mounts, and then mounts something on the host's /foo/bar, then that will not be really visible on the host's default mount namespace
[08:14] <pedronis> zyga-mbp: we will do also persistent mounts on behalf of snaps, via snapctl likely as there isn't thing that we can easily mediate that allows for that
[08:14] <zyga-mbp> because that mount event will not reach the hosts default mount namespace 
[08:14] <zyga-mbp> pedronis: we could do the same thing that lxd is doing, seccomp based mount mediation
[08:15] <pedronis> I said persistent, I mean fstab or mount units
[08:15] <zyga-mbp> mardy: do you know about how snap-confine sets up /mnt or /media for mount event propagation?
[08:15] <zyga-mbp> pedronis: I see
[08:15] <zyga-mbp> so the actual mount would happen on the host
[08:15] <zyga-mbp> not in the snap
[08:15] <zyga-mbp> (mount namespace)
[08:15] <pedronis> sometimes yes
[08:18] <zyga-mbp> pedronis those are probably least problematic then
[08:18] <zyga-mbp> since they can only affect hostfs and not what snap-update-ns does 
[08:21] <mborzecki> heh, opensuse TW has ssh configuration in /usr/etc/ now
[08:22] <mborzecki> the /etc/ssh/ directory is almost empty, definitely there's no ssh_config there
[08:23] <pedronis> zyga-mbp: afaict only /media (and /run/netns) are setup bidirectional in snap-confine, /mnt isn't
[08:32] <zyga-mbp> mborzecki: WAT? /usr/etc?
[08:32] <mborzecki> zyga-mbp: yeah, see https://build.opensuse.org/request/show/888799
[08:32] <mborzecki> zyga-mbp: they went through /usr/share/ssh at first ;)
[08:32] <zyga-mbp> pedronis: my memory may have been rusty
[08:33] <zyga-mbp> pedronis: you are right, and there's also /run/media depending on distro
[08:33] <zyga-mbp> mborzecki is that a FHS compliant location
[08:34] <mborzecki> hm idk, /usr/local/etc would be, but not quite sure about /usr/etc
[08:34] <mborzecki> not that i care much either
[08:36] <mborzecki> zyga-mbp: and tbh with immutable hosts, i'm not sure people care that much about it still
[08:36] <zyga-mbp> mborzecki: yeah but remember the /snap drama
[08:36] <zyga-mbp> oh my they said
[08:37] <mborzecki> well, i guess it depends on who makes the change ;)
[08:53] <mborzecki> the preseed spread test is failing now that lxd switched to core20
[09:02] <pedronis> mborzecki: didn't you just merge the fix for that?
[09:03] <mborzecki> pedronis: yeah, was about to fix that when i found a pr from sergio that was ready to land
[09:04] <pedronis> mborzecki: I merged your cgroup.freeze PR
[09:04] <mborzecki> pedronis: thanks!
[09:53] <mborzecki> pedronis: i think i've found a bug, the seed system that gets created has the old model, even though the right assertion seems to be passed around
[09:54] <mborzecki> pedronis: seedwriter fetches the model from the db?
[09:55] <mborzecki> so if the new model is just a revision bump, and isn't passed to the db, it will get the old one?
[10:23] <pedronis> mborzecki: it depends what db is used, the db used for the create from current probably doesn't make sense for the remodel case
[10:23] <mborzecki> pedronis: yeah, that's the one that is used
[10:27] <pedronis> mborzecki: there's probably a couple of ways to fix this, I'm having my lunch break. We can chat quickly after the standup?
[10:27] <mborzecki> pedronis: hm i think i can create one through WithStackedBackstore() ?
[10:27] <mborzecki> pedronis: sure, enjoy your lunch
[10:27] <pedronis> mborzecki: yes, that's one of the possible options to fix this
[10:42] <mardy> zyga-mbp: nope, I haven't digged into snap-confine yet
[11:09] <pstolowski> pedronis: i've proposed refresh control spread test - https://github.com/snapcore/snapd/pull/10476 - as well as a simple bugfix in the prerequisite pr
[11:11] <pedronis> pstolowski: thx,  I think I mentioned this already, but I reviewed the wait-chain PRs
[11:11] <pstolowski> pedronis: yes, thanks, i'm updating them right now
[11:13] <pedronis> pstolowski: I'm starting something, I'll look at the simple fix in a bit
[11:17] <pstolowski> sure, ty
[11:43] <mborzecki> pedronis: opened a proposal: https://github.com/snapcore/snapd/pull/10477
[11:48] <mardy> I'm creating a snap using snap pack, and I want to run mount(8) in it. I copied its dependencies to the snap, properly set LD_LIBRARY_PATH, but I get a segfault inside libc when running it. Any idea on what I could be doing wrong?
[11:52] <mborzecki> pstolowski: can you take a look at https://github.com/snapcore/snapd/pull/10477 ? 
[11:52] <pstolowski> mborzecki: sure
[11:52] <mborzecki> mardy: snap run --strace or snap run --gdbserver
[11:53] <mborzecki> unless the backtrace is already useful :)
[11:53] <mvo> mardy: does it have to be "mount(8)" or could you simply use the mount binary? did you try to link statically?
[11:54] <mborzecki> tbh segfault is a bit unexpected, it'd expect it to fail gracefully
[11:54] <mborzecki> as in `cannot mount because reasons` & non 0 exit
[11:58] <mardy> mborzecki: [pid 187371] mprotect(0x7f606bed1000, 4096, PROT_READ) = 0
[11:58] <mardy> [pid 187371] mprotect(0x7f606c110000, 16384, PROT_READ) = 0
[11:58] <mardy> [pid 187371] --- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=0xe} ---
[11:58] <mardy> mvo: I was just being lazy and did not want to build a binary myself, so I wanted to use a shell script for testing
[11:59] <mardy> it's not only mount that it's segfaulting, also "grep" gets terminated in the same way
[12:00] <mborzecki> mardy: hmm looks like some callback through a function pointer in a structure, that address is very low
[12:00] <mborzecki> probably gdb would be more useful if you want to try it (and explote the tooling at the same time)
[12:08] <pstolowski> mborzecki: a simple one https://github.com/snapcore/snapd/pull/10461
[12:09] <mborzecki> pstolowski: sure
[12:13] <mardy> mborzecki: nice! But it does not help much, I see the same 0xe address in the backtrace, and not much else:
[12:14] <mardy> #0  0x000000000000000e in ?? ()
[12:14] <mardy> #1  0x00007f7c9e180119 in ?? ()
[12:14] <mardy> #2  0x0000000000000000 in ?? ()
[12:14] <mborzecki> mardy: hm try running `set sysroot /snap/<your-base>/<revision>` and then `set solib-search-path /snap/<your-base>/<revision>/<usual lib dirs>`
[12:33] <mborzecki> pedronis: so you're saying this should be taken care of in the fetcher we pass to seedwriter?
[12:34] <mardy> mborzecki: didn't help. I guess I'll have to drop "snap pack" and use snapcraft instead
[12:51] <pedronis> mborzecki: I commented a bit more
[12:52] <mborzecki> pedronis: ah, ok, that is clear now
[14:05] <pedronis> degville: this is the phrasing confusion issue I mentioned in the standup: https://github.com/snapcore/snapd/pull/10478/files#r661506853
[14:06] <degville> pedronis: thanks. Taking a look now.
[14:10] <pstolowski> thanks
[14:27] <mardy> I moved to python now, and I'm getting another weirdness :-) If I print the python version by running the program directly (without snappy), I get: 3.8.5 (default, May 27 2021, 13:30:53)
[14:27] <mardy> but when I run the snap (just built with "snap pack"), I get 3.5.2 (default, Jan 26 2021, 13:30:48)
[14:28] <pstolowski> cachio__: can you take a look at https://github.com/snapcore/snapd/pull/10470 ?
[14:28] <mardy> there's no python inside the snap, so where is it finding version 3.5.2 from?
[14:29] <cachio__> pstolowski, sure
[14:32] <pstolowski> need 2nd review for a simple bugfix: https://github.com/snapcore/snapd/pull/10475
[15:28]  * cachio__ lunch
[16:18] <mvo> pedronis: is 10449 something you want to have a final look over or are you okay with the reviews it has?
[16:18] <mvo> pedronis: the "# snap-edit: no" header PR
[17:17] <jdstrand> I haven't read all backscroll, but note that the seccomp backend mediates mount and the device cgroup mediates a lot of what is in /dev (I say a lot because there is some interplay with apparmor, but apparmor relies on the device cgroup currently for hotpluggable things, etc to avoid a policy recomplile in the udev helper)
[17:18] <jdstrand> we could choose to enable those backends on systems without apparmor. we've not historically since, as mentioned, things like dbus could allow breakout (eg, udisks)
[17:20] <jdstrand> since we wouldn't want to make security claims, we've historically chosen to not enable all the available backends since that would complicate testing and support due to combinatorics
[17:21] <jdstrand> (all the available backends on non-strict systems like Fedora or Debian). That said, Debian should be getting close. talk to amurray (or jj) for status
[17:22] <jdstrand> we did do it for tumbleweed. I can't recall if we did it anywhere else...
[17:26] <jdstrand> as for a snap helper to facilitate mounts, that seems quite reasonable, easy to mediate from within the helper and something that could be done on any kernel
[17:40] <sergiusens> jdstrand hey there! just to add to your list, Manjaro should also be using strict confinement these days
[17:40] <sergiusens> mardy your python 3.5.2 probably comes from the base snap (core I believe), set snap.yaml to base: core18|20 for a newer python :-)
[17:42] <sergiusens> mardy or run `python3 -c "import sys; print(sys.executable)"` to find out where python comes from :-)
[22:48] <jdstrand> sergiusens: hi! oh yes, manjaro has been there for a while. there is a list that I did in the forum at some point