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

Delay between a release (minor release with security fixes) and Docker images available on hub #168

Closed
dgonzalez opened this issue Mar 20, 2018 · 42 comments

Comments

@dgonzalez
Copy link
Member

This delay can allow attackers to exploit a vulnerability in node core on Docker based systems. Last time I remember there was a delay of 1 day (which is not a lot) but still has to be considered as an attack vector.

@lirantal
Copy link
Member

I'm not aware of the delay times but I agree if this is indeed the issue.
I believe this falls under the scope of the core nodejs/security team as this relates to job setup and such. Should we ping nodejs code security for more info on this?

@dgonzalez
Copy link
Member Author

dgonzalez commented Mar 20, 2018 via email

@mhdawson
Copy link
Member

I think it will also involve the @nodejs/docker team as they do the updates which trigger new docker images to be built. It may be that they need to be part of the security release process so that the commits can be landed as soon as possible after the releases are available.

@Starefossen
Copy link
Member

New versions for Official Docker Images needs to go through approval with Docker Inc. hence the 12-24h delay.

@PeterDaveHello
Copy link
Member

@Starefossen is right, most of the update processes in the docker-node repository go fast, but we'll need the approval with Docker in the official-images repository, so it'll be a delay.

Another issue is that the members of @nodejs/docker have no control of our Travis CI, thus we can't stop the invalid or useless builds, and it takes a while to finish the all the CI tasks, that's also the part maybe we could improve.

@SimenB
Copy link
Member

SimenB commented Mar 22, 2018

Sorta related: nodejs/node#14190

Getting a new version into master of docker-node as quickly as possible (and PRing docker hub), maybe even coordinating with the docker hub folks to have them ready to start the merging process beforehand, would be really good, I think.

@dgonzalez
Copy link
Member Author

dgonzalez commented Mar 22, 2018 via email

@pesho
Copy link

pesho commented Mar 22, 2018

@dgonzalez Holding the main release won't work currently, because official Docker images are built from public sources by design. So anyone can see which release tarball is used and access it.

Delaying disclosure of the actual vulnerabilities is a good idea though.

@LaurentGoderre
Copy link
Member

Having a binary for alpine would also help speed up the updating of docker images but there isn't a consensus on this ATM.

nodejs/build#75

@dgonzalez
Copy link
Member Author

@pesho and what about prepare the build on a private repository and at the moment of release PR into the repo and build it?

I am also ok on the delaying disclosure. I am a big fan on disclosing as early as possible but 12-24 hours won't make a huge difference. Said that, the commit history will surely show what was fixed and an avid hacker can use it as an attack vector.

@pesho
Copy link

pesho commented Mar 22, 2018

@pesho and what about prepare the build on a private repository and at the moment of release PR into the repo and build it?

This will help a bit, but not enough. The Travis build will still take about 40 minutes (due to having to build for Alpine), and then it still must be submitted to Docker Inc. and wait for their separate review and approval.

@mhdawson
Copy link
Member

mhdawson commented Apr 27, 2018

There was feedback in the Enterprise user-feedback session today that this does cause anxiety for them. The feedback is that when we say you need up update quickly that activates processes in the and then they are under pressure while they have to wait a day for the release.

@Starefossen do we have any contacts at docker that we would talk to about the review delay for security releases? I can understand that for normal releases but I wonder if there is some way we can expedite by giving them advance notice of when a release will be coming.

@SimenB
Copy link
Member

SimenB commented Apr 27, 2018

There is a process where, if a PR is tagged with "[Security release]" in the title, it will be prioritised by the Docker team. But I'm not sure how formal that process is, or what, if any, guarantees are given about response time.

@tianon and/or @yosifkit might be able to weigh in on this?

@chorrell
Copy link

We're also dealing with timezone differences.

@LaurentGoderre
Copy link
Member

Note that we are working hard to reduce build times and implement automation to speed all of this up.

@tianon
Copy link

tianon commented Apr 27, 2018

I find myself wishing we'd been looped in on this thread sooner. 😅

Would it be possible for us to get an advance (private) heads up of incoming security releases so we can adjust our schedule to process them in a more timely manner? It's really just @yosifkit and I, and we are in Pacific Time doing most of our review during normal working hours Mon-Fri, but we're happy to make adjustments/arrangements if we know a couple days in advance that a security release is coming.

It would definitely help to have pre-built Alpine releases instead of having the image build from source, without question. As part of our review we do both a diff and build test, so the faster the images build, the faster we can get that out of the way (the ccache stuff being worked on in nodejs/docker-node#703 is great for Travis speedups, but really can't help the speed of our process/builds at all).

So to summarize, my (official) recommendations for a timely merge of a security release to an image in the official-images program are:

  1. keep the PR free of unrelated changes -- minimum necessary for the security fix
  2. include [security] as a prefix on the PR title
  3. give us a few days heads up with a timing estimate (privately is fine) so we can make sure we're ready to receive the PR ASAP after it's open

(Going to go work on some language for the README.md of https://github.com/docker-library/official-images to help formalize this recommendation better. Edit: docker-library/official-images#4298)

@mhdawson
Copy link
Member

@ahmadnassri-Telus adding you as an FYI since you were interested in this topic.

@mhdawson
Copy link
Member

@tianon for 1 and 2 when you say PR, is that the PR to the docker repo?

@SimenB
Copy link
Member

SimenB commented Apr 30, 2018

Yes

@mhdawson
Copy link
Member

@tianon we generally announce publicly ~ 1 week in advance. We could add to our process to directly notify you as well. What would be the best way to do that? I'd probably see if we can get an agreement for a private notification as well in case there is an instance were for some reason we can't do the public announcement far enough in advance (although this should be rare or never hopefully).

@tianon
Copy link

tianon commented May 8, 2018

If https://groups.google.com/group/nodejs-sec is the appropriate place to subscribe to see those (which it appears to be), we can just subscribe (no need to make us a special part of the official notification process).

It would still be useful to have whichever https://github.com/nodejs/docker-node maintainer is planning to do the actual bump PR to us give us a poke on IRC or via email (https://github.com/docker-library/official-images/blob/master/MAINTAINERS) to let us know their expected timeframe, but not strictly necessary. (Perhaps adding something like "fyi @tianon @yosifkit" to the security bump PR in that repo so we know it's in progress?)

Getting something down for private notification is probably also a good idea, but if that's as rare as you hope, we could wait and do that when it becomes necessary.

@mhdawson
Copy link
Member

mhdawson commented May 8, 2018

@tianon https://groups.google.com/group/nodejs-sec is the right place to subscribe to. @SimenB is the poke through IRC something you can add to your process. We should also talk about getting the docker-node team more closely tied to the security release process so you have a better insight into when the releases are going to be shipped, with at least some notifications when we publish the release (unless you already get that effectively through the nodejs-sec mailing list).

@SimenB
Copy link
Member

SimenB commented Jun 6, 2018

With a security release coming up, do we have a process ready?

https://nodejs.org/en/blog/vulnerability/june-2018-security-releases/

/cc @tianon as a heads up

@vdeturckheim
Copy link
Member

@SimenB I dont think we made progress here. I don't know much about how we publish the docker images and how this process goes. What is the status and how could we help improve it?

@SimenB
Copy link
Member

SimenB commented Jun 6, 2018

See the messages above from @tianon.
What I think it boils down to is that we should aim to open up a PR to Docker the moment the tarballs are available on https://nodejs.org/dist/. To be able to do that someone close to the build/release has to do the PR.

It also involves pinging the Docker people beforehand so they can be ready to run their tests.

Not sure what happened about holding back disclosure until the new docker images are available? Or if it's needed (or even feasible).

@mhdawson
Copy link
Member

mhdawson commented Jun 6, 2018

@SimenB I've sent you an email to connect you to the individual who will be being doing this release. I suggest we use this release to try and experiment with what helps and the capture what works into the process.

@mhdawson
Copy link
Member

mhdawson commented Jun 6, 2018

In terms of holding back disclosure, I think we still need to understand what that means. Does that mean holding the final announcement with the details for a shortish period of time (ex 1 hour) or something more complicated than that which requires being able to build before the regular binaries are available?

@SimenB
Copy link
Member

SimenB commented Jun 6, 2018

I'm not sure what holding back disclosure would entail either, I just mentioned it since it was brought up in #168 (comment). Just holding back the blog post about the exact details for a short time was my interpretation, though. Again, not sure if it's helpful or feasible.

I agree we should use this opportunity to learn what we can about the optimal process for this.

heads up @nodejs/docker @tianon @yosifkit

@PeterDaveHello
Copy link
Member

CI is still a big issue to us we, as I mentioned, @nodejs/docker has no permission to stop, restart any certain CI tasks, also, Travis CI sometimes hangs, reason unknown.

@tianon
Copy link

tianon commented Jun 6, 2018

Oh boy, this is the first day of DockerCon, this'll be fun 😅

We'll make it work, the most helpful would be more details on the rough timing we'll see so we can be on the lookout for it (especially since we'll be in conference mode). 👍

@SimenB
Copy link
Member

SimenB commented Jun 13, 2018

@mhdawson I think that went reasonably well? Only hitch was waiting on approval for merge in nodejs/docker-node#783. I think an admin in the organisation would be able to just merge it, but it's maybe not a big deal?

@LaurentGoderre
Copy link
Member

I get a lot of notifications on GitHub. Maybe in the future it would be nice to get a direct email?

@SimenB
Copy link
Member

SimenB commented Jun 13, 2018

I don't think security releases should need approval in the first place, but I think only org admins can ignore stuff like that

@chorrell
Copy link

I agree, they should be able to to approve and merge when they need to

@mhdawson
Copy link
Member

@SimenB I agree it went reasonably, not being able to merge was the main surprise and I agree that we should let at least some people approve and merge, especially in the case of a security release.

I took a first cut at documenting the process here: #306

@mhdawson
Copy link
Member

mhdawson commented Jun 13, 2018

As a TSC member I think I'm an org admin, and whatever is in place to prevent a push without a review blocked me as well. Both in the UI and also from pushing manually from the git command line.

@SimenB
Copy link
Member

SimenB commented Jun 13, 2018

Hmm, OK. Not sure how to get around it, then. Maybe the bottom box is checked in this form (screenshot from another repo)?

image

@mhdawson
Copy link
Member

Yup,

image

@SimenB
Copy link
Member

SimenB commented Jun 14, 2018

So in theory removing the "include administrators" would help?

@mhdawson
Copy link
Member

@SimenB I think so.

@sam-github
Copy link
Contributor

There was some discussion of improvements to make to the process, and also some delays that are unavoidable. I know the sec release process includes a heads up to the docker team so they know a release is coming.

If the concerns that are addressable have been addressed, can this be closed now, @mhdawson @dgonzalez @nodejs/docker ?

@mhdawson
Copy link
Member

I think this can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests