Prevent insecure tag-based signing

gal-legit commented 1 year ago

As demonstrated in this blog post, signing and verifying container images using tags is insecure. The blog post demonstrates the issue using cosign, but the same principles apply using Notary.

Problem

The following race condition illustrates the problem:

# Legitimate user
docker build -t hello-world:v1
docker push hello-world:v1

# Malicious user: the same flow, but with a malicious image and a continuous push to ensure winning the race
docker build -t hello-world:v1
while true; do
  docker push hello-world:v1
done

# Legitimate user signs by tag, thus, signs the malicious image instead of the original one
notation sign hello-world:v1

The same logic applies to the verification, although the attacker needs to have more precise timing since continuous push would cause the verification to fail:

notation verify hello-world:v1
# at this point, the attacker pushes the malicious image
docker run --rm -t hello-world:v1

Solution

To solve the issue, we need to extract the digest from docker push, e.g.:

push_output=$(docker push hello-world:v1)
digest=$(echo $push_output | tail -1 | cut -d ' ' -f3)
notation sign hello-world@${digest}

similarly, we would take the digest from the output of notation verify and use it to run the docker.

Action Items:

Warn users when using tags for signing (preferably, remove support for tag-based signing completely after a compatibility period)
Update documentation to educate users on the correct way to sign & verify.

FeynmanZhou commented 1 year ago

Thank you for providing this feedback.

Since Notation v1.0.0 RC.1, digest will be used to sign by default. If users are using a tag, Notation will automatically resolve tag to digest then sign it. See https://notaryproject.dev/docs/quickstart/

yizha1 commented 1 year ago

@gal-legit Thanks for the suggestions. With notation v1.0.0 rc.1, there is a warning message to the users when they use tag instead of digest for notation sign and notation verify commands. The CLI specs and quick-start were also updated to emphasis it. Let us know anything else we could improve.

https://notaryproject.dev/docs/quickstart/
spec for notation sign https://github.com/notaryproject/notation/blob/v1.0.0-rc.1/specs/commandline/sign.md
spec for notation verify https://github.com/notaryproject/notation/blob/v1.0.0-rc.1/specs/commandline/verify.md

github-actions[bot] commented 1 week ago

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.

notaryproject / specifications