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.

Sign up for GitHub

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

Jump to bottom

add compatibility note on sha3 as alias for sha3-512 #22

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

add compatibility note on sha3 as alias for sha3-512 #22

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ code name
# 0x14 formerly had the name "sha3", now deprecated
```

## Compatibility note

Implementations should make sure keep the string code `sha3` as an alias for `sha3-512` so that `encode('sha3', digest) == encode('sha3-512', digest)`.
Copy link
Member

Choose a reason for hiding this comment

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

I think this is right. can someone else comment on this? cc @lgierth @whyrusleeping

Copy link
Member

Choose a reason for hiding this comment

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

Should be: "...should make sure to keep the..."

Content sounds right, if 512 is the default bit-size of the digest. But I'm not sure where that assumption is coming from, as there are 3 other SHA-3 hashing functions, and two more if you count the XOFs.

Copy link
Member

Choose a reason for hiding this comment

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

@RichardLitt the comment here is that there are multiple variants of the hash function, not the output size. similar to how sha2-256 and sha2-512 are different functions. (the default output size matches the classification)

Copy link
Member

Choose a reason for hiding this comment

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

Unless I misread the spec correctly, they differ mainly in the size of the digest. What I'm not sure about is whether or not sha3 should by default mean sha3-512, as opposed to one of the other five (like sha3-256).

Copy link
Member

Choose a reason for hiding this comment

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

This looks correct to me.

Copy link
Member

Choose a reason for hiding this comment

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

See https://en.m.wikipedia.org/wiki/SHA-3 sections: instances (show
different postfixes) and examples (show totally different hash values for
same input, not just a truncation of same value)

On Wed, Aug 3, 2016 at 16:57 Richard Littauer [email protected]
wrote:

In README.md
#22 (comment):

@@ -83,6 +83,9 @@ code name

0x14 formerly had the name "sha3", now deprecated


+## Compatibility note
+
+Implementations should make sure keep the string code `sha3` as an alias for `sha3-512` so that `encode('sha3', digest) == encode('sha3-512', digest)`.

Unless I misread the spec correctly, they differ mainly in the size of the
digest. What I'm not sure about is whether or not sha3 should by default
mean sha3-512, as opposed to one of the other five (like sha3-256).

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/multiformats/multihash/pull/22/files/0638385dd25a1f5b186819b50c9999a4621117b2#r73419387,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAIcoc4EDpDrw_azWOSUeeTT6sg51rL3ks5qcQC4gaJpZM4HLQD1
.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, the digest is different; you're right. It is not simply a matter of length.

However, I still am not sure - why is this PR suggesting we use sha3 as an alias for sha3-512, and not some other one of the sha3 functions?

Copy link
Member

Choose a reason for hiding this comment

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

I believe because that's what was used by an implementation.

Copy link
Member

Choose a reason for hiding this comment

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

@candeira Can you back up this decision?

Copy link

Choose a reason for hiding this comment

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

This multihash repo defines a spec, not an API, so the proposed wording that references a "string code" feels weird to me (many implementations will not use a string to specify the hash type). What about something like this instead:

Implementations should be aware that a previous version of this spec used the label SHA3 for the SHA3-512 hash (code 0x14). If your implementation allows hash types to be specified as a string, you should consider making SHA3 and SHA3-512 aliases, or disallow the label SHA3 entirely.


### other tables

Expand Down