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

do not register sha384 by default #97

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

Conversation

thaJeztah
Copy link
Member

related:

The SHA-384 algorithm is not documented in the OCI image-spec (1, 2), and is not encouraged to be used. Commit 084376b registered all algorithms by default, but also included SHA-384.

This patch disables SHA-384 by default, to discourage its use.

sha.go Outdated
@@ -18,6 +18,5 @@ const (

func init() {
RegisterAlgorithm(SHA256, crypto.SHA256)
RegisterAlgorithm(SHA384, crypto.SHA384)
Copy link
Member

Choose a reason for hiding this comment

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

This is a breaking change, do we need to bump up the major version?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm.. good question. Before 084376b (not yet part of a release), neither of them would work out of the box unless the algorithms were registered, so perhaps some wiggle room, but you may be right that it could be a v2 worth if we decide to remove.

Copy link
Member Author

Choose a reason for hiding this comment

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

(honestly not sure why SHA384 and SHA512 ever made it in at all, but 🤷‍♂️)

@thaJeztah
Copy link
Member Author

Let me move this one to draft; I rebased #98 to not depend on this PR.

@thaJeztah thaJeztah marked this pull request as draft January 30, 2024 11:02
@thaJeztah thaJeztah mentioned this pull request Jan 30, 2024
Copy link
Contributor

@stevvooe stevvooe left a comment

Choose a reason for hiding this comment

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

This needs to be rebased because we made a big move in another PR.

The SHA-384 algorithm is not documented in the OCI image-spec ([1], [2]), and
is not encouraged to be used. Commit 084376b
registered all algorithms by default, but also included SHA-384.

This patch disables SHA-384 by default, to discourage its use.

[1]: https://github.com/opencontainers/image-spec/blob/v1.0.2/descriptor.md#digests
[2]: https://github.com/opencontainers/image-spec/blob/v1.0.2/descriptor.md#registered-algorithms

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah force-pushed the remove_SHA384_default branch from 5dee832 to dead9ba Compare April 24, 2024 08:42
@thaJeztah
Copy link
Member Author

I rebased this, but kept it in draft for now, pending the discussion on this being a breaking change (and warranting a v2 of this module)

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