Validate digest length on parsing #1231
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Collaborator
|
This assumes that digests have fixed size output. Do we want to force this on all future hash implementations? Are there any variable length hashes that we may want to support in the future? I think the answer is yes, all digest implementations have a fixed size hash output. |
|
I think it's reasonable to assume a fixed hash size. |
Contributor
Author
There was a problem hiding this comment.
Tarsum has its own parse method so isn't affected(not better or worse). I can try to fix tarsum as well if needed, of course.
Contributor
Author
|
@stevvooe If there is ever a case where fix length doesn't work, |
Collaborator
|
@tonistiigi I think we're okay. SHA3, blake etc. all have fixed length output. As a matter of security, this is probably more than okay to enforce. LGTM |
Collaborator
|
LGTM |
stevvooe
added a commit
that referenced
this pull request
Dec 3, 2015
Validate digest length on parsing
wking
added a commit
to wking/go-digest
that referenced
this pull request
Jun 9, 2017
This error was added in 9721254 (Validate digest length on parsing, 2015-12-02) and motivated with [1]: > The new error is mostly so that digest.Set doesn't need a special > way to figure out if invalid digest had a algorithm prefix or not. That doesn't make sense to me, because Digest.Validate returns ErrDigestUnsupported when there is no algorithm prefix (I've added a test to confirm this as well), and that seems orthogonal to encoding validation (which is the only place ErrDigestInvalidLength could matter). From the other changes in [1], I think the issue was just that invalid lengths weren't raising errors at all. I see no reason to distinguish between invalid lengths, invalid characters, and other invalid-encoded-portion errors, so this commit collapses the error into ErrDigestInvalidFormat. [1]: distribution/distribution#1231 Signed-off-by: W. Trevor King <wking@tremily.us>
wking
added a commit
to wking/go-digest
that referenced
this pull request
Jun 9, 2017
This error was added in 9721254 (Validate digest length on parsing, 2015-12-02) and motivated with [1]: > The new error is mostly so that digest.Set doesn't need a special > way to figure out if invalid digest had a algorithm prefix or not. That doesn't make sense to me, because Digest.Validate returns ErrDigestUnsupported when there is no algorithm prefix (I've added a test to confirm this as well), and that seems orthogonal to encoding validation (which is the only place ErrDigestInvalidLength could matter). From the other changes in [1], I think the issue was just that invalid lengths weren't raising errors at all. I see no reason to distinguish between invalid lengths, invalid characters, and other invalid-encoded-portion errors, so this commit collapses the error into ErrDigestInvalidFormat. The code I'm removing assumed hex (with '.Size() * 2'), so this commit removes that assumption. The sha256 and other hex-based algorithms are still getting their lengths validated via anchoredEncodedRegexps. [1]: distribution/distribution#1231 Signed-off-by: W. Trevor King <wking@tremily.us>
thaJeztah
pushed a commit
to thaJeztah/distribution
that referenced
this pull request
Apr 22, 2021
Validate digest length on parsing
thaJeztah
pushed a commit
to thaJeztah/distribution
that referenced
this pull request
Jan 19, 2022
Validate digest length on parsing
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@aaronlehmann @dmcgowan
The new error is mostly so that
digest.Setdoesn't need a special way to figure out if invalid digest had a algorithm prefix or not.Signed-off-by: Tonis Tiigi tonistiigi@gmail.com