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

Hash-pin docker images, set up dependabot to keep them up-to-date #1244

Closed
wants to merge 2 commits into from

Conversation

pnacht
Copy link
Contributor

@pnacht pnacht commented Jun 26, 2023

Fixes #1243.

As described in the issue, this PR hash-pins the ubuntu:jammy image used in docker/Dockerfile. This prevents unexpected tampering with the image.

Keeping pinned images up-to-date can be a hassle, so I've also set up dependabot, which will periodically send a PR to bump the image's version.

Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>
Signed-off-by: Pedro Kaj Kjellerup Nacht <pnacht@google.com>
@sbc100
Copy link
Collaborator

sbc100 commented Jun 26, 2023

I think this is perhaps somewhat overkill. Aren't there all kinds of docker images on docker hub that reference imagines by name.. i'm not sure emsdk needs to be at the vanguard of this type of security.

If names/branches are not sufficient for a given user of these images, such as user could make a pinned version of the image (and indeed such as user would likely want audit/pin all their docker imagines). If this is a real risk it seems like a problem that should probably solved that the docker level.. or in specific places that care more about security than we do here in emsdk.

@sbc100
Copy link
Collaborator

sbc100 commented Jun 26, 2023

Also, if a malicious actor were able to take over dockerhub itself or the ubuntu account thereon, wouldn't modifying emsdk images be a long was down their list of targets. Surely there are project with many orders of magnitude more users (ones that actually run in production environment) that don't do this kind of pinning?

@pnacht
Copy link
Contributor Author

pnacht commented Jul 4, 2023

Hey @sbc100, sorry for the delay in replying.

The main risk we're mitigating here isn't that the ubuntu:jammy image might be maliciously modified to poison the emsdk container created by these Dockerfiles. That is certainly possible, but I agree that a hacker capable of taking over the ubuntu account would probably have bigger priorities than emsdk.

The main risk is that the ubuntu:jammy image might be maliciously modified to install malware in the host machine or run a crypto-miner in the container. This would be an attack that doesn't care about emsdk; it'd simply affect anyone using FROM ubuntu:jammy.

@sbc100
Copy link
Collaborator

sbc100 commented Jul 5, 2023

Hey @sbc100, sorry for the delay in replying.

The main risk we're mitigating here isn't that the ubuntu:jammy image might be maliciously modified to poison the emsdk container created by these Dockerfiles. That is certainly possible, but I agree that a hacker capable of taking over the ubuntu account would probably have bigger priorities than emsdk.

The main risk is that the ubuntu:jammy image might be maliciously modified to install malware in the host machine or run a crypto-miner in the container. This would be an attack that doesn't care about emsdk; it'd simply affect anyone using FROM ubuntu:jammy.

Doesn't that seem like a fundamental issue that the docker community or the Ubuntu community should solve address though? I'm not sure we want to be concerning ourselves too much with such things.. on the other hand its not really adding much complexity or effort on our end.

Our of interest, how common is this kind of mitigation? Are many other container publishers are doing this? Is this risk/solution documented somewhere?

@pnacht
Copy link
Contributor Author

pnacht commented Jul 24, 2023

Sorry for the delay again.

Doesn't that seem like a fundamental issue that the docker community or the Ubuntu community should solve address though? I'm not sure we want to be concerning ourselves too much with such things.. on the other hand its not really adding much complexity or effort on our end.

Yes, this is the sort of thing that – from a security perspective – would be best handled at the ecosystem level. However, allowing mutable tags makes things more convenient for developers, ensuring they get the security benefit of new versions without having to update the hashes themselves.

That is why I've also set up dependabot to monitor the hashes for you. This way, the project gets the best of both worlds for "free": the consistent behavior from hash-pinning the image and the security updates via dependabot's automatic bumps (though you'll have to merge its PRs whenever a new jammy version is released).

Our of interest, how common is this kind of mitigation? Are many other container publishers are doing this? Is this risk/solution documented somewhere?

The risk isn't explicitly stated in the Docker docs, but mentions to it can be found in item 6 of this Synk post (Snyk is a cybersecurity company that does a lot of work on supply-chain security).

@sbc100
Copy link
Collaborator

sbc100 commented Jul 24, 2023

I think my opinion here is basically that this is overkill for emsdk. If this kind of thing becomes widely adopted/recommended we could reconsider but I don't think we need be the tip of the spear for this kind of thing.

@pnacht
Copy link
Contributor Author

pnacht commented Jul 24, 2023

That's perfectly reasonable. Feel free to close this PR and #1243.

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.

Hash-pin images used in the Dockerfile
2 participants