/srv/irclogs.ubuntu.com/2020/06/08/#snappy.txt

=== KindTwo is now known as KindOne
=== KindTwo is now known as KindOne
=== KindTwo is now known as KindOne
mborzeckimorning05:12
mborzeckimvo: hi, there's a typo in https://github.com/snapcore/snapd/pull/8822 that static checks choke on06:03
mupPR #8822: release: 2.45.1 <Created by mvo5> <https://github.com/snapcore/snapd/pull/8822>06:03
mvomborzecki: meh, ok06:05
mvomborzecki: thank you, will fix06:05
zygahey guys06:12
mborzeckizyga: hey06:29
zygahey06:29
mborzeckizyga: how is your back?06:29
zygathinking about how to say it06:29
zygaf*** uP?06:29
zyganot good for sure06:30
mborzeckizyga: sad to hear that06:31
mvohey zyga - good morning06:39
zygahey mvo06:39
pstolowskimorning07:03
mvohey pstolowski - good morning07:09
mupPR snapd#8804 closed: tests: port xdg-settings test to tests.session <Test Robustness> <Created by zyga> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8804>07:23
mupPR snapd#8815 closed: tests: port snap-handle-link test to tests.session <Test Robustness> <Created by zyga> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8815>07:23
zygathanks mvo!07:32
mvozyga: thank *you*07:32
zygamvo: MRI on Wednesday at 15:0007:40
zygaso before then I'm working from bed07:40
mvozyga: ok07:41
mupPR snapd#8825 opened: tests: move a few more tests to snapstate_install_test <Test Robustness> <Created by stolowski> <https://github.com/snapcore/snapd/pull/8825>07:43
zygahttps://github.com/snapcore/snapd/pull/882607:44
mupPR #8826: tests: assorted small patches <Simple πŸ˜ƒ> <Created by zyga> <https://github.com/snapcore/snapd/pull/8826>07:44
zygapstolowski: +107:47
zygaheh07:48
mupPR snapd#8826 opened: tests: assorted small patches <Simple πŸ˜ƒ> <Created by zyga> <https://github.com/snapcore/snapd/pull/8826>07:48
zygaI've seen so many tasks that never get a ping back from travis07:48
zygamvo: perhaps it is time to drop travis from the "required" list?07:48
zygathe only thing left is the CLA check07:49
pstolowskizyga: ty!07:50
mvozyga: I guess we could remove it totally, yes?07:50
zygamvo: I would keep it for a while because IIRC mborzecki wanted to do some changes to the CLA checker07:51
mborzeckianyone remember whether time-control only works if you use timedatectl?07:51
zygaand having a 2nd guess would be better07:51
zygamborzecki: as in the interface?07:51
zygamborzecki: let me look07:51
zygamborzecki: it's literally defined as such but perhaps that's a bit too tight07:52
zygamborzecki: what kind of extra permissions do you think it should grant?07:53
mborzeckizyga: there's https://forum.snapcraft.io/t/access-to-bin-date-in-strict-confinement/18041 but the reporter gets EPERM (so likely not caused by AA)07:54
zygammm07:55
zygahow does date set time07:55
zygaclock_settime or something like that?07:55
mborzeckizyga: strace tells met it's clock_settime07:55
zygaso a seccomp filter is probably what needs changing07:56
zygaIMO it's sensible07:56
mborzeckiyup07:56
mborzeckiok, let me open a PR and mark it for security review07:57
zygathanks07:58
zygamaybe tweak the spread test in the PR as well07:58
zygaI think we have one07:58
zygamvo: there was a question about https://bugs.launchpad.net/ubuntu/+source/snapd/+bug/1881350 and release to groovy08:20
mupBug #1881350: snap seeding fails with libc6-lse <snapd:Fix Committed by zyga> <glibc (Ubuntu):New> <snapd (Ubuntu):New> <https://launchpad.net/bugs/1881350>08:20
zygado you think it's better to do 2.45.1 or just a distro patch?08:20
pedronispstolowski: mborzecki: hi, when you have time, could you re-review #8702 (it follows my suggestions, I think, now)08:23
mupPR #8702: overlord/configstate: add system.kernel.printk.console-loglevel option <Created by EthanHsieh> <https://github.com/snapcore/snapd/pull/8702>08:23
mborzeckipedronis: sure, will do08:23
pstolowskipedronis: sure08:23
pedronisthx08:23
mvozyga: I uploaded to groovy already08:23
zygaah, fantastic08:24
mvozyga: and it's there already (i.e. autopkgtest passed)08:24
zygamvo: my daughter doesn't recognize me :)09:10
zygamvo: she's scared when she sees me now09:10
mvozyga: haha - oh boy09:11
mvozyga: but you *do look different'09:11
mborzeckizyga: shaved your beard?09:12
zygayep09:12
mborzeckiso we no longer poke jamie, but rather set 'needs security review' tag?09:19
mborzeckizyga: https://github.com/snapcore/snapd/pull/882709:20
mupPR #8827:  interfaces/builtin/time-control: allow POSIX clock API <Needs security review> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8827>09:20
zygamborzecki: yes09:20
zygadone09:21
zygamborzecki: any weird things happen when you change the time?09:21
zygaperhaps that test should be invoked via a time namespace?09:21
zygajust to make sure it affects the system less09:22
mupPR snapd#8827 opened:  interfaces/builtin/time-control: allow POSIX clock API <Needs security review> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8827>09:24
mborzeckizyga: it gets restored in the test, the ntp should slowly update the clock09:24
mborzeckizyga: btw. do any of the systems have a kernel that supports time ns already?09:24
zygamborzecki: yeah, I think 20.04+ should be fine09:24
mborzeckiiirc it's in 5.609:24
zygait was a added a while ago09:24
zygaor maybe I read the patch a while ago :D09:24
mborzeckiit's automatically a part of new uts ns right?09:25
pedronismborzecki: mmh, that PR is not using the file I thought it should use09:26
zygamborzecki: no, I don’t think it is09:27
mborzeckipedronis: 99-snapd.conf?09:27
zygaUts is a separate ns09:27
pedronismborzecki: yes, actually the code is confusing09:27
pedroniswhy is it mentioning 10-console-messages.conf09:27
pedronisI see it has both09:27
pedronisI asked a question there now09:28
mborzeckizyga: https://kernelnewbies.org/Linux_5.6#Time_Namespaces hmm my version if unshare is unaware of CLONE_NEWTIME09:28
pedronismborzecki: I'm not sure why it bothers with 10-console-messages.conf at al09:29
pedronisit should just write an empty 99-snapd.conf, no?09:29
pedronisor remove it09:29
mborzeckipedronis: aiui it reloads the old/default setting09:29
mborzeckiso it's applied now, rather than on the next reboot09:29
pedronisI see, feels fragile09:32
pedronismaybe we should just sysctl --system all the time09:32
pedronisif there's a change09:33
pedronismborzecki: I commented about that09:34
mborzeckipedronis: hm yes, that's an option, i think one could argue if you tweak something at runtime, it could get overwritten09:34
pedronismvo: what's the state of this: https://bugs.launchpad.net/snapd/+bug/1867415 ?09:36
mupBug #1867415: Hardlinking snaps wastes 400 MB tmpfs RAM in live CDs <rls-ff-incoming> <snapd:New for mvo> <snapd (Ubuntu):Fix Released> <https://launchpad.net/bugs/1867415>09:36
mvopedronis: this is fixed, let me update the bug09:39
mvopedronis: oh, let me see, someone claims it's not09:40
pedronismvo: yes, exactly, that's why I'm asking09:40
mvopedronis: I will try to reproduce now09:40
* mvo is downloading09:41
mupPR snapd#8828 opened: tests: clean up up the use of configcoreSuite in the configcore tests <Test Robustness> <Created by stolowski> <https://github.com/snapcore/snapd/pull/8828>09:54
abeatotianon, hey, are all interfaces used by the docker snap actually needed? I was looking into a way to restrict what the snap can do09:56
zygais the store under load?09:56
zygaerror: cannot get snap sections: cannot sections: got unexpected HTTP status code 403 via GET to "https://api.snapcraft.io/api/v1/snaps/sections"09:56
zygapedronis, mborzecki: tracking broken out as a separate PR https://github.com/snapcore/snapd/pull/882909:57
mupPR #8829: sandbox/cgroup: add tracking helpers <Created by zyga> <https://github.com/snapcore/snapd/pull/8829>09:57
zygaI chose to bundle ConfirmSystemdServiceTracking in the same PR as that function itself is tiny and completes the picture as to how this is meant to be used09:58
zygathis is half of the large PR now and contains really just the logic to create a transient scope, along with unit tests09:58
zygaI didn't add the spread tests as those would pull in virtually everything from the original09:58
mupPR snapd#8829 opened: sandbox/cgroup: add tracking helpers <Created by zyga> <https://github.com/snapcore/snapd/pull/8829>09:59
zygathe design was reviewed by jamie and he gave a +1 on the original PR10:01
pedroniszyga: mvo: this has a lot of back and forth by you but no priority or status != new: https://bugs.launchpad.net/ubuntu/+source/snapd/+bug/187300410:03
mupBug #1873004: lxd interaction blocked until snapd was restarted <lxd (Ubuntu):New> <snapd (Ubuntu):New> <https://launchpad.net/bugs/1873004>10:03
zygapedronis: yes, I think this is a duplicate of an existing issue that was investigated but not fixed earlier10:04
zygapedronis: I will look at launchpad to find the other report10:04
zygapedronis: it _looks_ like snapd starts before we mount snaps, and if core is mounted after snapd, this will happen10:04
zyga(no interfaces)10:04
zygaah wait10:05
zygawrong bug10:05
mvooh, is that a dupe of the one andy had on friday ?10:05
zygaI looked at the other tap10:05
zyga*tab10:05
zygano no, sorry let me read the correct link10:05
zygahmm10:06
zygahmm10:10
zygamvo: I don't have any more ideas for that bug10:11
zygathe frequency seems to suggest it is related to core reloading10:12
zygaer, refreshing10:12
pedronismborzecki: bit confused by the rename to UpdateBootScript, not sure how it helps with confusion with InstallBootConfig10:18
* zyga prepares the next segment 10:29
* zyga takes a break from writing and reads https://github.com/snapcore/snapd/pull/8675/files10:38
mupPR #8675: osutil: add disks pkg for associating mountpoints with disks/partitions <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8675>10:38
mborzeckipedronis: i'm thinking about https://github.com/snapcore/snapd/pull/8775#discussion_r434505499 bot end up named grub.cfg, so maybe grub.cfg and recovery/grub.cfg?10:44
mupPR #8775: [RFC] bootloader, boot: boot scripts, edition <Needs Samuele review> <UC20> <β›” Blocked> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8775>10:44
pedronismborzecki: should we have a chat before the standup about the grub scripts?10:45
mborzeckipedronis: now or later?10:45
pedronismborzecki: can do in 5 min?10:46
mborzeckipedronis: sounds fine10:46
pedronismborzecki: going to the standup10:53
* mvo hugs pstolowski for 882810:54
pstolowskimvo: thanks for the review & suggestions!11:14
* zyga needs a break11:21
zygaman, you have kids and they are at home11:21
zygabut when *everyone* just comes to the same room11:21
zygaand the dog too11:21
zygaand then everyone starts moving11:21
zygait's a bit too much11:21
pedroniszyga: I commented/asked some question in #857311:34
mupPR #8573: overlord/snapstate: inhibit startup while unlinked <Created by zyga> <https://github.com/snapcore/snapd/pull/8573>11:34
mupPR snapd#8830 opened: bootloader: introduce bootloarder assets, import grub.cfg with an edition marker <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8830>11:39
zygaThank you!11:43
mupPR snapd#8831 opened: tests: use configcoreSuite in journalSuite and remove some duplicated code <Test Robustness> <Created by stolowski> <https://github.com/snapcore/snapd/pull/8831>11:44
pedronispstolowski: I did a pass on #881211:53
mupPR #8812: o/snapstate: service-control task handler (4/N) <Needs Samuele review> <Services βš™οΈ> <Created by stolowski> <https://github.com/snapcore/snapd/pull/8812>11:53
pstolowskipedronis: great, thank you!11:53
pedronissome high-level questions/comments11:53
mupPR snapd#8722 closed: tests: check that host settings like hostname are settable on core <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8722>11:54
pedroniszyga: does #8829 need a security review?11:57
mupPR #8829: sandbox/cgroup: add tracking helpers <Created by zyga> <https://github.com/snapcore/snapd/pull/8829>11:57
zygapedronis: I think jamie could indicate if he needs to review it again but my understanding it he gave this a +1 already11:57
zygaif he has the time for a quick look to decide if a deeper look is necessary11:57
zygathat would be okay11:57
zygajdstrand: ^11:57
mupPR snapd#8832 opened: travis.yml: removed, all our checks run in GH actions now <Simple πŸ˜ƒ> <Skip spread> <Created by mvo5> <https://github.com/snapcore/snapd/pull/8832>11:59
clmsyis it possible to try core20 out without updating kernel or its a no go12:11
clmsyi set the gadget to use 20 and left the kernel snap 4.4.x (core16 kernel) i guess its normal that i cant use to build an image this way12:12
pedronisclmsy: core20 needs a new model assertion syntax and new initramfs, because of  the latter an old kernel will not work as is12:22
=== mbriza2 is now known as mbriza
clmsythx pedronis!12:27
ograpopey, gitea just exploded in my face ... seems it sets a versioned SNAP_USER_DATA everywhere in its app.ini instead of using the /current link so after an update it cant access its data anymore ...12:34
ogra(fixing app.ini manually to use the symlink everywhere foxes the app and it starts again)12:37
ogra*fixes12:37
zygauh12:43
pedroniszyga: dbusutil/dbustest/stug.bo needs a blankline before package  atm the copyright is the doc comment of the package12:53
zygaah, thanks12:53
zygaI didn't check that12:53
pedronisI have a check, but it's a bit hacky, maybe we should add to run-checks anyway12:54
pedronismborzecki: I did a pass on #8830, mostly naming12:54
mupPR #8830: bootloader: introduce bootloarder assets, import grub.cfg with an edition marker <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8830>12:54
mborzeckipedronis: thanks!12:54
popeyogra i am on vacation, do feel free to provide feedback upstream (which publishes the snap)13:22
mupPR snapd#8833 opened: sandbox/cgroup: remove redundant pathOfProcPidCgroup <Simple πŸ˜ƒ> <Created by zyga> <https://github.com/snapcore/snapd/pull/8833>13:25
ograpopey, will do, thanks ... enjoy your vac.13:27
mupPR snapd#8834 opened: dbusutil/dbustest: add separate license from package <Simple πŸ˜ƒ> <Skip spread> <Created by zyga> <https://github.com/snapcore/snapd/pull/8834>13:30
cachiomvo, I created a PR for skip13:40
cachio#883513:40
mupPR #8835: POC:  skip binary to stip tests in an easy way <Proof of concept> <β›” Blocked> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/8835>13:40
mvocachio: thank you! looking now13:41
cachiomvo, this is to address this https://github.com/snapcore/spread/pull/7213:41
mupPR spread#72: Run condition <Blocked> <Reviewed> <Created by sergiocazzolato> <https://github.com/snapcore/spread/pull/72>13:41
mborzeckipedronis: cmatsuoka: i can take a look at making some of the exported gadget code private tomorrow morning13:43
mborzeckithe updaters, device lookup, filesystem image cuold all be private13:44
mupPR snapd#8835 opened: POC:  skip binary to stip tests in an easy way <Proof of concept> <β›” Blocked> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/8835>13:45
mborzeckiwe did not agree on what to do with the code what was introduced for the tool that could build images, or did we?13:45
cmatsuokamborzecki: ack, thanks!13:45
mupPR snapcraft#3164 closed: cli: remove enable-ci command <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3164>13:46
mupPR snapd#8836 opened: sandbox/cgroup: add tests for ParsePids <Simple πŸ˜ƒ> <Created by zyga> <https://github.com/snapcore/snapd/pull/8836>14:00
zygahmmm14:19
zyganot that many spread tests are in flight14:19
zygamvo: I wonder if it would help if we also ran unit tests in self-hosted runners14:19
zygaafter all, the worker is really mostly idle apart from waiting for network IO from spread14:20
mvozyga: works for me14:20
zygamvo: I somewhat worry about RAM usage though, not sure how much our test suite takes14:20
* zyga checks14:20
zygamvo: I'll experiment with some PRs later14:20
zygamvo: I think now we're waiting for unit tests to pass to run spread and have more spread capacity than unit test capacity14:21
mvozyga: yeah, makes sense14:21
zygahmm, actually, my view was stale 32 spread jobs in flight14:22
zygaso perhaps that would not win much14:22
zygaexactly at capacity14:22
* zyga breaks for lunch14:41
pedronismborzecki: I commented on the name of the assets get function again14:44
mborzeckinice, i like assets.Internal14:49
mupPR snapd#8398 closed: usersession/userd: add "slack" to the white list of URL schemes handled by xdg-open <Created by troyready> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8398>14:50
mupPR snapd#8800 closed: usersession/userd: add "slack" to the white list of URL schemes handled by xdg-open - 2.45 <Created by jdstrand> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8800>14:50
zygamborzecki: what is the word "edition" used for, is it like "version"?14:56
mborzeckizyga: yeah, in the same feel as edition in gadget.yaml14:57
zygaah14:57
zygaI forgot we have that14:57
ijohnsonzyga: what versions of opensuse would you say are supported by snapd?15:07
zyga15.1 and tumbleweed15:07
zygabut .45 is not out there yet as I had issues with auth15:07
zyga(the package is ready for a while, I just need to return to it)15:08
jdstrandzyga: with PR 8829, is that broken out from PR 7825 unmodified? if so and you didn't change anything (significant, ie, algorithm or implementation-wise) in this part of the code, then I don't need to re-review15:09
mupPR #8829: sandbox/cgroup: add tracking helpers <Created by zyga> <https://github.com/snapcore/snapd/pull/8829>15:09
mupPR #7825: many: use transient scope for tracking apps and hooks <Needs security review> <Security-High> <Created by zyga> <https://github.com/snapcore/snapd/pull/7825>15:09
zygajdstrand: there are way more tests15:09
jdstrandtests are great :)15:09
jdstrandothers can review those15:09
jdstrandvariable renames, equivalent/simple refactoring, also not needed in this go code15:10
zygajdstrand: and there's a new helper that checks for the cgroup of a service (using an existing helper but technically the function is new) - it is on line https://github.com/snapcore/snapd/pull/8829/files#diff-4706d268ffa8aa3999d09cac05b3ced2R11015:10
mupPR #8829: sandbox/cgroup: add tracking helpers <Created by zyga> <https://github.com/snapcore/snapd/pull/8829>15:10
jdstrands/not needed/don't need a security review/15:10
zygayou may want to eye that if you want, I included it here because in all cases we call either the new CreateTransientScope or the new ConfirmSystemdServiceTracking15:11
jdstrandthat function is fine and using a reviewed-already api15:11
jdstrandzyga: well, if you feel that my cursory glance missed something and you want me to review, please add the needs security review tag15:11
zygajdstrand: I think this is okay, I really don't think there's anything security related here15:12
zygaand there's no new design15:12
zygajust broken out code15:12
zygatests15:12
zygaand some moving around to different packages to fit the structure better15:12
jdstrandyeah, that is what it looked like15:12
jdstrandI mean, yes, we need to have the design and code for this feature reviewed, but we did that extensively in PR 782515:13
mupPR #7825: many: use transient scope for tracking apps and hooks <Needs security review> <Security-High> <Created by zyga> <https://github.com/snapcore/snapd/pull/7825>15:13
zygayes, this is my understanding as well15:13
zygamborzecki: https://github.com/snapcore/snapd/pull/8830#pullrequestreview-42636289515:35
mupPR #8830: bootloader: introduce bootloarder assets, import grub.cfg with an edition marker <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8830>15:35
zygaman I was up for like 30 seconds15:40
zygaand it hurts for a few minutes now15:40
roadmrouch zyga  :(15:41
zygayeah, this is not a good time15:41
mupPR snapcraft#3162 closed: tests: remove scenario usage from lifecycle order <maintenance> <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3162>15:41
mupPR snapcraft#3163 closed: cli: remove the hidden inspect command <maintenance> <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3163>15:41
* cachio lunch15:45
=== KindTwo is now known as KindOne
pedroniszyga: I wrote a quick mail about /usr/lib/snapd16:05
zygapedronis: checking16:05
zygapedronis: I like this a lot16:08
zygalet me think a little about it and draft something for tomorrow16:08
* zyga goes to rest for a while, putting the laptop away16:21
mupPR snapd#8827 closed:  interfaces/builtin/time-control: allow POSIX clock API <Needs security review> <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/8827>16:30
Saviqanyone else seeing snaps unstable on travis today? I'm getting a lot of failed lxd snap installs…16:35
tianonabeato: I guess that depends on what you want to "lock down" -- "privileged", "home", and "removable-media" are definitely optional, but the others are all related to core functionality of Docker you'd have to go out of your way to not use IIRC16:42
zygaSaviq: we actually moved away from travis *today*16:43
abeatotianon, I actually tried removing "privileged", but get into some problems when starting dockerd16:43
abeatoit was some permission for sysfs16:44
Saviqzyga: lucky16:44
zygaSaviq: maybe store load?16:44
Saviqcould be, there's very little info on what went wrong, suggesting to look at the journal…16:45
tianonabeato: ahh, I can reproduce -- that's definitely related to the mitigations in 19.03.11 for CVE-2020-13401 (open /proc/sys/net/ipv6/conf/docker0/accept_ra: permission denied)16:45
abeatotianon, yeah, it was that16:46
tianonabeato: that one's technically something that has to be updated in the snapd "docker-support" profiles, IIRC (not something we can fix in the Docker snap directly, AFAIK)16:46
abeatotianon, I see, is it something that will be fixed eventually?16:47
ijohnsontianon: o/16:50
ijohnsontianon: I am just now running into the same issue for `/proc/sys/net/ipv6/conf/docker0/accept_ra`16:50
tianonabeato: I *think* it'll require a PR to https://github.com/snapcore/snapd/blob/269ced7c4f31bbd912f73cba822f522244da16d5/interfaces/builtin/docker_support.go, since the denial is likely apparmor16:50
ijohnsontianon: is the right fix to just allow that in the policy?16:50
tianon:D16:50
tianonyeah16:51
ijohnsonok, let me see maybe the docker snap should just be using network-control instead?16:51
tianonit has that already, doesn't it?16:51
ijohnsonI don't remember if network-control provides for that access or not16:51
tianonah, it has network and network-bind but not network-control16:51
ijohnsontianon: yeah I think network-control should give access to that sysfs thing16:52
tianonhttps://github.com/snapcore/snapd/blob/269ced7c4f31bbd912f73cba822f522244da16d5/interfaces/builtin/network_control.go#L84-L93 it does appear so :)16:52
ijohnsontianon: you will need to add network-control to the plugs for dockerd, then we will probably need to get the assertion for the docker snap updated to allow auto-connection of network-control16:52
ijohnsontianon: can you start a new forum topic on forum.snapcraft.io about granting network-control to the docker snap and ping jdstrand ?16:53
tianonsure thing :)16:53
ijohnsonthanks tianon16:53
abeatotianon, ijohnson thanks16:54
mupPR snapd#8834 closed: dbusutil/dbustest: separate license from package <Simple πŸ˜ƒ> <Skip spread> <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/8834>16:55
ijohnsontianon: fyi I just finished testing adding the network-control interface to the docker snap locally and it fixed that issue so that should be all that's needed, we shouldn't need interface changes to docker-support I think16:56
ijohnsontianon: but let's see how the forum topic goes16:56
tianonnice :D16:56
tianonhttps://forum.snapcraft.io/t/auto-connect-docker-to-network-control/18054?u=tianon :)16:58
ijohnsontianon: great, and just to be clear the docker snap does allow capability net_raw in the apparmor policy, so I think it is theoretically possible for that CVE to affect the snap17:02
ijohnsontianon: though I'm not sure how well the docker snap supports specifying different capability sets for the container17:02
* ijohnson has never tried it17:02
tianonijohnson: yeah, the snap is absolutely affected by that CVE -- not sure if it goes all the way to the host level, but certainly cross-container (and likely *is* able to send RAs to the host)17:03
ijohnsontianon: ok, yeah so we should try to get this fixed asap then, also how does the mitigation work, as I tried to reproduce the issue where the docker snap couldn't write to that sysfs file in a VM but it worked fine, presumably because the networking in the VM was different?17:04
ijohnsonerr the networking setup in a VM is different from a real machine is what I meant17:05
jdstrandroadmr: hey, can you pull 20200608-1642UTC ?17:05
ijohnsonbecause I see it on a real machine but not on VM's17:05
tianonijohnson: the mitigation disables accepting IPv6 RA (by writing to that sysfs value) on brdige devices Docker creates (which wouldn't be changed in a VM vs on a physical host); your VM might have had IPv6 completely disabled, in which case the mitigation wouldn't be necessary (that sysfs file wouldn't exist at all)17:07
tianonijohnson: https://github.com/moby/moby/compare/v19.03.10...v19.03.11 is pretty small (just the mitigation patch)17:08
tianonijohnson: reading that patch, your VM might have something like "failed to read ipv6 net.ipv6.conf.<bridge>.accept_ra" as a warning in the dockerd logs17:08
ijohnsonhmm it seems that my VM has ipv6 enabled, but still the docker snap is able to run17:14
ijohnsonperhaps that accept_ra was already disabled in the VM17:14
tianonhmm17:15
tianonmaybe17:15
tianonbut it writes it whether it's disabled or not17:15
ijohnsonyes, I wonder if there's not a default bridge set somehow?17:16
ijohnsonhmm so on a fresh focal VM, I have /proc/sys/net/ipv6/conf/default/accept_ra set as "1"17:16
=== KindTwo is now known as KindOne
tianonijohnson: given the current positive direction of that thread, I should upload an update with network-control to edge right? :)18:24
ijohnsontianon: yes please18:24
ijohnsontianon: I assume when jdstrand or some other store admin gets a chance they will process the auto-connection18:24
ijohnsontianon: but also iirc you should be able to just push a new version with network-control to edge, and that will be installable, but folks won't have it magically work without running manually `snap connect docker:network-control` but after the auto-connection is issued then that will magically work18:25
tianonijohnson: right :)18:26
ijohnsontianon: while not really relevant anymore since you got auto-connect, I debugged a bit more in the VM and I think the bug is only when docker snap is upgraded and the old version of that bridge didn't have the ipv6 disabled, I think that when this version of the docker snap is installed fresh it just creates the bridge with that disabled18:27
ijohnsonbut maybe I'm missing something else18:27
ijohnsons/ipv6 disabled/ipv6 RA packet things/18:27
tianoninteresting18:27
tianonwell, FWIW, the commit is pushed; as soon as launchpad gets around to it, it should be in edge :)18:28
ijohnsongreat thanks!18:29
jdstrandtianon: we have the votes now but normally we wait 7 days for reviewers to comment. is this an emergency?18:47
tianonjdstrand: I'd defer that to ijohnson -- I don't think it's an emergency, but it is preventing folks from using the "docker" snap without privileged enabled18:48
jdstrandtianon: the current docker snap or one in edge/etc?18:49
jdstrandand by current, I mean 'stable'18:49
ijohnsonjdstrand: tianon: I don't know that I'd call it an emergency, but I do think it is urgent since the current docker snap on stable is somewhat broken for existing installations and I don't think it's reasonable to revert to the previous one, as the previous docker snap suffers from that critical CVE18:49
tianonstable18:49
ijohnsonjdstrand: but folks can "un-break" their installs by refreshing to edge and then manually connecting network-control18:50
jdstrandijohnson: so, stable already has network-control?18:50
ijohnsonjdstrand: the critical CVE being https://nvd.nist.gov/vuln/detail/CVE-2020-1340118:50
ijohnsonjdstrand: no, only edge has network-control18:50
ijohnsonas per tianon's commit today18:50
jdstrandijohnson: but stable and edge have the fix, and you want to promate something with network-control to stable?18:50
ijohnsonwell actually edge may not have it yet, I have not looked at the lp builders status to see if they built it or not18:50
jdstrandpromote*18:51
jdstrandand by fix, I mean the cve fix18:51
ijohnsonah18:51
ijohnsonso that CVE is currently fixed on all channels of the docker snap18:51
jdstrandok, but it regressed in certain ituations18:51
ijohnsonyes18:52
jdstrandgotcha. ok, I'll fast track it18:52
ijohnsonthanks jdstrand18:52
jdstrandijohnson, tianon: done18:57
ijohnsonnice thank you18:57
tianonthank you!18:57
tianonnow if I could just figure out why the snap has intermittent build failures on launchpad that I can't reproduce locally...  /o\18:58
roadmrjdstrand: hi! certainly - will put it in the queue.19:27
mupPR snapd#8826 closed: tests: assorted small patches <Simple πŸ˜ƒ> <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/8826>19:41
zygajdstrand: not urgent but small: mvo requested a 3rd review of a test you wrote that I modify: https://github.com/snapcore/snapd/pull/880319:48
mupPR #8803: tests: port interfaces-many-core-provided to tests.session <Test Robustness> <Created by zyga> <https://github.com/snapcore/snapd/pull/8803>19:48
roadmrjdstrand: fun, looks like those 2.45 base declaration changes broke a few store tests :)20:49
jdstrandroadmr: oh? was network-status one of the special ones that was used in the testsuite?21:01
roadmrjdstrand: yep, looks like it21:01
roadmrjdstrand: f.ex I had it in an EXPECTED_DENIED_CONNECTION_INTERFACES list but it's not in that category anymore21:02
roadmr(because its deny-connection: true got removed)21:02
jdstrandroadmr: it looks like you could use mir instead21:03
roadmrjdstrand: thanks! I'll for sure replace with that if needed21:03
jdstrandroadmr: yes, network-status is no longer denied, for sure21:03
roadmrjdstrand: some of the tests just check the interface categories generically so should cope with me moving network-status out of the EXPECTED_DENIED list21:04
* jdstrand nods21:04
roadmrjdstrand: it's also no longer in EXPECTED_APP_PROVIDED_SLOT_INTERFACES as it's now provided by core21:06
roadmrjdstrand: so all the changes seem to make sense; I'll probably tag you in the MP for a sanity check21:06
roadmrnone of this looks controversial in any case21:06
jdstrandhappy to look at it, but your assessment is correct21:07
jdstrandafaics21:07
jdstrand(from way over here :)21:07
roadmrjdstrand: osram-dp2i-avahi is indeed in a brand store O_o they should file a support case21:08
jdstrandroadmr: ok, I responded to say to file a support case21:09
roadmrmakes sense21:09
roadmrthanks!21:10
mupPR snapd#8825 closed: tests: move a few more tests to snapstate_install_test <Test Robustness> <Created by stolowski> <Merged by cmatsuoka> <https://github.com/snapcore/snapd/pull/8825>21:22
roadmrjdstrand: https://code.launchpad.net/~roadmr/software-center-agent/+git/software-center-agent/+merge/385315 for when you have a min - no rush, I'm out of runway today but will check back tomorrow :)21:33
mupPR snapd#8831 closed: tests: use configcoreSuite in journalSuite and remove some duplicated code <Test Robustness> <Created by stolowski> <Merged by cmatsuoka> <https://github.com/snapcore/snapd/pull/8831>21:57
mupPR snapcraft#3165 opened: Update cmake plugin to use more modern flags <Created by GamePad64> <https://github.com/snapcore/snapcraft/pull/3165>22:22

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!