emscripten-core / emsdk

Emscripten SDK
http://emscripten.org
Other
2.98k stars 679 forks source link

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

Closed pnacht closed 1 year ago

pnacht commented 1 year ago

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.

sbc100 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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