Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to 19.03.15 #13

Merged
merged 1 commit into from
Feb 13, 2021
Merged

Update to 19.03.15 #13

merged 1 commit into from
Feb 13, 2021

Conversation

tianon
Copy link
Contributor

@tianon tianon commented Dec 7, 2020

No description provided.

@tianon
Copy link
Contributor Author

tianon commented Dec 7, 2020

Arg, @anonymouse64 looks like we need some new AppArmor adjustments for the containerd CVE mitigation... 😞

docker: Error response from daemon: /run/containerd/s/e15c35ef67476c11609c7576eee3549e767321cf8f56507989f5fe9ab1455439: mkdir /run/containerd/s: permission denied: unknown.

(That /run/containerd/s prefix is explicitly hard-coded intentionally to avoid unix socket path length issues and potential security issues with it being mutable.)

@anonymouse64
Copy link
Contributor

fun... I will file a PR to snapd then

@anonymouse64
Copy link
Contributor

@tianon do you have a specific CVE I can reference in the commit message?

@tianon
Copy link
Contributor Author

tianon commented Dec 7, 2020

Yes! That'd be CVE-2020-15257, documented in GHSA-36xw-fx78-c5r4.

@anonymouse64
Copy link
Contributor

PR is at canonical/snapd#9764, after a little bit there will be a snapd snap you can download and install locally to test with this new version of the docker snap, could you validate that it fixes the issue?

@tianon
Copy link
Contributor Author

tianon commented Dec 7, 2020

Yeah, absolutely - I should be able to hack it into the pipeline here to ensure it works in GitHub's pristine environment (my local environment doesn't seem to catch these issues reliably, which is part of why I set up GitHub Actions in the first place 😄).

@anonymouse64
Copy link
Contributor

BTW, it would be really cool if there was an equivalent github action here which built the snap from a PR and made it available for download so you can easily test a PR locally if you wanted 😉 it's as simple as something like this: https://github.com/canonical/etrace/blob/master/.github/workflows/snap.yml

@tianon
Copy link
Contributor Author

tianon commented Dec 8, 2020

BTW, it would be really cool if there was an equivalent github action here which built the snap from a PR and made it available for download so you can easily test a PR locally if you wanted it's as simple as something like this: https://github.com/canonical/etrace/blob/master/.github/workflows/snap.yml

Great idea - gonna rebase this on top of #14 once the current build test is finished. 👍

@tianon
Copy link
Contributor Author

tianon commented Dec 9, 2020

(moving back here to keep that snapd issue less noisy 😅)

So, in the context of canonical/snapd#9764 (comment), if we put something like assumes: [snapd2.48.2] in our file, does the store make sure we can't promote until that dependency is promoted, or is that something we have to monitor ourselves?

@anonymouse64
Copy link
Contributor

Yes but unfortunately there is already a snapd 2.48 that does not have the fix, we can only do assumes for major versions not for patch versions like 2.48.2, so definitely using 2.48 in the assumes is better than not doing that but it's not 100% perfect unfortunately

@tianon tianon force-pushed the 19.03.14 branch 2 times, most recently from 9521dc3 to 34d7faf Compare January 6, 2021 23:00
@tianon
Copy link
Contributor Author

tianon commented Jan 6, 2021

Hmm, what triggers snapd to be refreshed from https://snapcraft.io/ after it's installed via APT?

(It seems snap refresh in my GitHub Actions didn't do it, so I'm not sure what would trigger that to occur and how common the situation I've currently got in this action is going to be for actual users? It seems APT gave me 2.48, which is new enough to satisfy my new assumes: but not new enough to actually satisfy the real dependency even though 2.48.2 does appear to be in the stable channel on snapcraft.io. 😞 😕)

@anonymouse64
Copy link
Contributor

Well, the apt version is 2.48.2+20.04, and the snapd version is 2.48.2, so the apt version "wins" since "+20.04" is higher than "".

I'd recommend always installing the snapd snap to be sure you have the most up to date snapd.

@tianon
Copy link
Contributor Author

tianon commented Jan 6, 2021

Well, this is on Xenial, so I think doesn't even have 2.48.2 (yet?).

How common do you think this is going to be for users? In other words, how many users are likely to get broken if we move forward with this? 😬 😇

@tianon
Copy link
Contributor Author

tianon commented Jan 6, 2021

(I suppose the follow-up question is then whether we need to wait for 2.49+ so we can have a proper dependency relationship?)

@anonymouse64
Copy link
Contributor

anonymouse64 commented Jan 6, 2021 via email

@tianon
Copy link
Contributor Author

tianon commented Jan 7, 2021

Well, this includes the mitigation for CVE-2020-15257 (GHSA-36xw-fx78-c5r4), which seems especially relevant for Snaps - they typically run with the host network namespace, right?

@anonymouse64
Copy link
Contributor

Yes snaps run in the same network namespace as the host. I guess I leave it up to you how critical it is to fix this bug soon.

@tianon tianon changed the title Update to 19.03.14 Update to 19.03.15 Feb 2, 2021
@tianon tianon force-pushed the 19.03.14 branch 3 times, most recently from 35835c8 to 66681bb Compare February 2, 2021 18:59
@tianon tianon mentioned this pull request Feb 3, 2021
@anonymouse64
Copy link
Contributor

@tianon on an unrelated note I realized I was mistaken earlier, you can use assumes: [snapd2.48.2] here, assumes does work with minor version numbers like that

@tianon tianon force-pushed the 19.03.14 branch 2 times, most recently from 0301979 to cb0b108 Compare February 10, 2021 19:39
@@ -33,7 +33,7 @@ grade: stable

base: core18
confinement: strict
assumes: [snapd2.40]
assumes: [snapd2.48.3]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Soooo, this is complicated. We need the latest snapd (which includes canonical/snapd#9764, necessary to make this bump even work), which in all the distro builds (https://packages.ubuntu.com/search?keywords=snapd) is 2.48.3 but in https://snapcraft.io/snapd is 2.48.2.1. 😬

I guess it's probably rare that someone is going to be updating docker but not updating snapd, so we're probably safe to just go with 2.48.2 for now?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for the purposes of that pr, then yes assumes: [snapd2.48.2] is sufficient, snapd2.48.2.1 would be best, but that is currently broken (but will eventually be fixed by canonical/snapd#9923)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh wait sorry I forgot that pr didn't make it into 2.48.2, gosh this is all so confusing to keep track of

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well I can say for sure for sure that snapd2.49 will have that pr in it, 2.49 is in beta now and should move to candidate/stable within the next 2 weeks (2.49 was blocked by the CVE)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and checking in on the upgrade situation, we still have a sizable ( more than 10% of total userbase ) install base of 2.48.2 on core/snapd snaps (across all devices, not necessarily docker snap users), so if we do include 2.48.2 in the assumes it's possible that folks are still on 2.48.2 and as such they won't be required to update snapd before installing docker.

at this point I'm feeling like since we pushed out something that triggered refreshes to deal with that CVE, we should just make this wait until 2.49 :-|

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, nice! I'm honestly a little confused why all the suites of Ubuntu got 2.48.3 but the store got 2.48.2.1 which appears to be the same underlying version? I would've expected that to be the other way around (if anything), TBH.

I think the likelihood of folks updating docker but not having updated snapd is terribly low, but it would be nice to have that actually validated (without having to wait another week or two to get there). I guess we could just plan to tell anyone who complains to update their snapd? I don't get to see https://snapcraft.io/snapd/metrics, but I have to imagine the number of (data reporting) users who didn't get the bump by now is pretty tiny. I'd worry more about users who aren't consuming snapd from the Snap, but their mitigation path is pretty straightforward. 😅

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In all likelihood, I think you're correct in that the number of affected docker snap users will likely be very small, so we are probably good to just go with assumes: [snapd2.48.2], and then after 2.49 is out maybe we can revisit and bump it again to 2.49 to be super extra sure

(This reverts cf4a338.)
@tianon tianon merged commit 5b33686 into canonical:main Feb 13, 2021
@tianon tianon deleted the 19.03.14 branch February 13, 2021 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants