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

docs: faq entry for multiple digest algorithm support #547

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rchincha
Copy link
Contributor

Add the rationale (hence the Chesterton's fence) for why we are supporting multiple digest algorithms.

@rchincha rchincha mentioned this pull request Jul 24, 2024
3 tasks
Add the rationale (hence the Chesterton's fence) for why we are
supporting multiple digest algorithms.

Signed-off-by: Ramkumar Chinchani <[email protected]>
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.

This feels like it's missing the reasoning behind #543 and #494, and it's mixing up concerns of image-spec with those of distribution-spec.

@@ -81,3 +81,26 @@ If the client is trying to be defensive to nonconformant registries, and receive
Mounting without having to specify `from`, also known as automatic mount origin discovery, requires the registry to determine whether or not a blob exists in any repository.
If the existence check for the blob is done first, an immediate failure will indicate the lack of presence of a blob.
On the other hand, if the registry needs to perform further work to determine if the blob can be accessed by the mounter, it could create an information disclosure risk, in leaking that presence of a blob with that digest in the registry.

**Q: Why is support for new digest algorithms being added?**
Copy link
Contributor

Choose a reason for hiding this comment

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

Did we add any new algorithms? sha512 was already on the image-spec list of valid algorithms: https://github.com/opencontainers/image-spec/blob/main/descriptor.md#digests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

support for sha512 in the dist-spec?

Copy link
Contributor

Choose a reason for hiding this comment

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

But we don't define the list of supported algorithms in distribution-spec.

The default digest algorithm for OCI ecosystem has been _sha256_ and it has
worked well. So why bother with additional digest algorithms?

There are a couple of reasons (not in any particular order of importance):
Copy link
Contributor

Choose a reason for hiding this comment

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

This is missing that blob uploads may be broken on some registries, particularly for post/patch/put requests. There is also no way to push a manifest with a tag by a client controlled algorithm, which means it's not possible to associate referrers with a tagged manifest unless the default algorithm is used.

Comment on lines +94 to +97
- OCI (v1.1.0 specs) now supports arbitrary artifacts and large artifacts pose
a performance problem especially with strictly serial hash algorithms such as
_sha2_-family. Much faster parallel digest algorithms such as _blake3-256_
are a viable alternative.
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems unrelated to what we're doing now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps not directly, but more a long-term concern.

Comment on lines +91 to +93
- Existing digest algorithms may be broken in the future and it is prudent
that we design and accomodate for additional digest algorithms. Doing this _a
posteriori_ is [much harder](https://lwn.net/Articles/898522/).
Copy link
Contributor

Choose a reason for hiding this comment

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

While true, I don't see this as a key reason to improve support of sha512.

Copy link
Member

Choose a reason for hiding this comment

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

a goal for moving to 256/512 should be to begin the process of testing additional digests...

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that's a good goal, but it's an image-spec goal that we can document with the registered digest algorithms there. For distribution-spec, my focus is on allowing valid image-spec content to be pushed and pulled from a registry.

Comment on lines +99 to +101
Note that the two properties that matter to us are _collision resistance_ (so
content is uniquely addressable) and _preimage resistance_ (so source cannot be
guessed from the output hash)
Copy link
Contributor

Choose a reason for hiding this comment

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

These are image-spec concerns. Distribution-spec should only be concerned with accepting image-spec content.

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.

3 participants