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

proposal: x/crypto/sha3: add KMAC #24988

Open
conradoplg opened this issue Apr 21, 2018 · 12 comments
Open

proposal: x/crypto/sha3: add KMAC #24988

conradoplg opened this issue Apr 21, 2018 · 12 comments

Comments

@conradoplg
Copy link
Contributor

@conradoplg conradoplg commented Apr 21, 2018

The KMAC message authentication code specified in NIST SP 800-185 may be a useful MAC implementation.

In particular, it can be used as a KDF as specified by NIST SP 800-56C.

I'm submitting a CL implementing it, in case there is interest. I can also add support for TupleHash and ParallelHash.

@gopherbot gopherbot added this to the Unreleased milestone Apr 21, 2018
@gopherbot
Copy link

@gopherbot gopherbot commented Apr 21, 2018

Change https://golang.org/cl/108715 mentions this issue: x/crypto/sha3: add NewKMAC128 and NewKMAC256

@agnivade
Copy link
Contributor

@agnivade agnivade commented Apr 22, 2018

This adds new API. Any new public API needs to go through a proposal and accepted first. Perhaps you can create a proposal citing proper reasons why this should be inside x/crypto/sha3 and not in an external package ?

/cc @FiloSottile / @agl

@conradoplg
Copy link
Contributor Author

@conradoplg conradoplg commented Apr 22, 2018

This adds new API. Any new public API needs to go through a proposal and accepted first. Perhaps you can create a proposal citing proper reasons why this should be inside x/crypto/sha3 and not in an external package ?

Sure. What's the proper procedure for that? Should I just add "proposal:" to the title of the issue?

Basically, the main reason is that KMAC is based on the underlying Keccak sponge implementation. If I were to create an external package, I'd have to duplicate the entire Keccak implementation from x/crypto/sha3.

@agnivade
Copy link
Contributor

@agnivade agnivade commented Apr 22, 2018

Perhaps, it's better to create a new issue altogether to preserve context, since this one is already polluted a bit. You can update the CL with the new "Fixes #xxxx" text once it gets accepted.

@FiloSottile FiloSottile changed the title x/crypto/sha3: add KMAC proposal: x/crypto/sha3: add KMAC Apr 23, 2018
@gopherbot gopherbot added the Proposal label Apr 23, 2018
@gopherbot gopherbot added the Proposal label Apr 23, 2018
@FiloSottile
Copy link
Member

@FiloSottile FiloSottile commented May 3, 2018

Relatedly, https://golang.org/cl/111281 implements cSHAKE.

@FiloSottile
Copy link
Member

@FiloSottile FiloSottile commented May 3, 2018

I'm inclined to accept both cSHAKE and KMAC, unless @agl has objections. SHA-3 has defined way too many functions, but making people reimplement them or roll their own MAC doesn't sound like the right answer.

@agl
Copy link
Contributor

@agl agl commented May 3, 2018

I thought it was a requirement of SHA-3 candidates that they not permit length-extension, so I'm unsure what the point of KMAC is. But it looks to be tiny wrapper around cSHAKE, so whatever, throw it in!

@conradoplg
Copy link
Contributor Author

@conradoplg conradoplg commented May 3, 2018

That's a good question, apparently KMAC was proposed in a SHA-3 workshop but I did not found a rationale for it. I could only deduce that it's for providing domain separation and to avoid some weird pitfalls when using SHAKE directly such as a 16-byte MAC being the prefix of a 32-byte MAC over the same message/key, or a MAC collision of MAC(K,M) and MAC(K[0:-1], K[-1]||M).

When https://golang.org/cl/111281 is accepted I'll change this CL to use its implementation of cSHAKE.

@FiloSottile
Copy link
Member

@FiloSottile FiloSottile commented May 4, 2018

Ok on adding KMAC and cSHAKE. Docs should point to hmac.Equal.

@Scratch-net
Copy link

@Scratch-net Scratch-net commented Aug 5, 2018

Please consider adding TupleHash too. It's the only way to hash multiple slices without collisions

@conradoplg
Copy link
Contributor Author

@conradoplg conradoplg commented Oct 15, 2019

I updated my CL with the new cSHAKE some time ago, but forgot to comment here...

I can add TupleHash if desired.

@Scratch-net
Copy link

@Scratch-net Scratch-net commented Oct 15, 2019

That would be awesome

@FiloSottile FiloSottile added NeedsFix and removed Proposal labels Mar 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.