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

Explicitly recommend content digest information #556

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

burgerdev
Copy link
Contributor

The spec mandated only the verification of digests in the response headers, not the requested digests. That allowed conformant clients not to validate content at all, leaving the users of these clients exposed to accidental or malicious bad content.

This commit adds a "SHOULD verify" clause to the blob and manifest pull sections. It's not a MUST to keep it somewhat backwards compatible with requirements of 1.1 and prior, but it's not a MAY to convey that "the full implications should be understood and the case carefully weighed" (description in RFC 2119) for a client not to verify digests.

Fixes: #549

cc @sudo-bmitch

Copy link
Contributor

@sudo-bmitch sudo-bmitch left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@rchincha rchincha left a comment

Choose a reason for hiding this comment

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

Good guidance to have.

lgtm

spec.md Outdated Show resolved Hide resolved
spec.md Outdated Show resolved Hide resolved
Copy link
Contributor

@sudo-bmitch sudo-bmitch left a comment

Choose a reason for hiding this comment

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

I've added a few suggested edits to hopefully make updating this easier.

spec.md Outdated Show resolved Hide resolved
spec.md Outdated Show resolved Hide resolved
spec.md Outdated Show resolved Hide resolved
@burgerdev
Copy link
Contributor Author

Thanks for the suggestions, @sudo-bmitch, and sorry for the delay. I'm happy with the change now, wdyt?

@sudo-bmitch
Copy link
Contributor

Thanks for the suggestions, @sudo-bmitch, and sorry for the delay. I'm happy with the change now, wdyt?

The changes LGTM. Please squash the commits and I'll kick off the CI. @burgerdev

The spec mandated only the verification of digests in the response
headers, not the requested digests. That allowed conformant clients not
to validate content at all, leaving the users of these clients exposed
to accidental or malicious bad content.

This commit adds a "SHOULD verify" clause to the blob and manifest pull
sections. It's not a MUST to keep it somewhat backwards compatible with
requirements of 1.1 and prior, but it's not a MAY to convey that "the
full implications should be understood and the case carefully weighed"
(description in RFC 2119) for a client not to verify digests.

This commit also aligns the recommendations for server-returned digests
between manifest and blob - now both can be ignored, but must be
verified if used.

Fixes: opencontainers#549

Co-authored-by: Brandon Mitchell <[email protected]>
Signed-off-by: Markus Rudy <[email protected]>
@burgerdev burgerdev force-pushed the require-digest-verification branch from d9cc775 to 24084d4 Compare December 2, 2024 16:12
Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

LGTM

@mikebrow mikebrow merged commit 75ca125 into opencontainers:main Dec 12, 2024
4 checks passed
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.

proposal: tighten digest verification requirements for clients
6 participants