go.crypto/sha3: new package

go.crypto/sha3: new package
Added a pure Go implementation of SHA3 (Keccak) which implements the hash.Hash interface.
A test file is included with performance benchmarks and standard test vectors.

[nigeltao, 2013-03-20 00:56:29]

agl leads the go.crypto sub-repository, and
https://groups.google.com/d/msg/golang-dev/clWZwaQz9cs/rJ5zORSKk20J suggests
that he's not in favor of including SHA3 in go.crypto.

You are of course free to publish your SHA3 implementation somewhere "go
get"table: bitbucket, github, code.google.com, etc.

[agl1, 2013-03-20 01:11:32]

On Tue, Mar 19, 2013 at 8:56 PM,  <nigeltao@golang.org> wrote:
> suggests that he's not in favor of including SHA3 in go.crypto.

I do rather wish that SHA-3 would disappear, but this is nicely done.
I'll try and take a better look tomorrow.

One thing that does give me pause is that NIST are still consulting on
the parameters for the different `official' hashes (the capacity etc).
So I don't think that SHA-3 is final yet.


Cheers

AGL

[nigeltao, 2013-03-20 02:17:43]

In general, your top-level // comment lines are uncomfortably long. I'd break it
at somewhere between 80-100 bytes.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go
File sha3/sha3.go (right):

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode1
sha3/sha3.go:1: // Copyright 2012 The Go Authors. All rights reserved.
2013, and similarly in the other files.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode13
sha3/sha3.go:13: 
Delete the blank line, so that the comment above is recognized by godoc as a
package doc comment.

Run "godoc code.google.com/p/go.crypto/sha3" to check that your godoc output
looks good.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode74
sha3/sha3.go:74: func (d *digest) Capacity() int {
Rename Capacity to capacity unless there's a reason to export this method.
Similarly for Rate.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode78
sha3/sha3.go:78: // Rate returns number of bytes of the internal state which can
be absorbed or squeezed in between calls to the permutation function.
"returns the number"

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode80
sha3/sha3.go:80: return (stateSize - d.Capacity())
Outer parens are unnecessary.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode87
sha3/sha3.go:87: for x := 0; x < numLanes; x++ {
for i := range d.a {
  d.a[i] = 0
}

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode92
sha3/sha3.go:92: // BlockSize, required by the Digest interface, does not have a
standard intepretation for a sponge-based construction like SHA3.
What is the Digest interface?

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode206
sha3/sha3.go:206: func (d *digest) dumpState() {
If this is just for debugging then move it to sha3_test.go or delete it
entirely.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode222
sha3/sha3.go:222: return &digest{outputSize: outputSize}
I don't think this function is doing much (and "new" shadows a built-in
function). Just inline it below:

func New224() hash.Hash { &digest{outputSize: 224 / 8} }

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode225
sha3/sha3.go:225: // The NewX() constructors enable initializing a hash in any
of the four standard sizes of output, specified in bits.
Drop the parens.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode232
sha3/sha3.go:232: //Given no parameters, a new instance with the highest
security level is provided.
Go doc comments should begin with the name of the thing being commented upon.
Thus:
// New returns a new instance etc.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go
File sha3/sha3_test.go (right):

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode5
sha3/sha3_test.go:5: // This file implements test code for the SHA3 package.
I'd drop this sentence. It's obvious from the file name.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode6
sha3/sha3_test.go:6: // These tests are a subset of those provided by the Keccak
web site: http://keccak.noekeon.org/
Place a file-level comment below the package clause, so that it's less likely to
be confused for a package doc comment:
--------
// Copyright 2013 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package sha3

// These tests are a subset of those provided by the Keccak web site:
http://keccak.noekeon.org/

import (
--------

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode29
sha3/sha3_test.go:29: repeatCount     int  // If repeatCount is greater than 1,
the input input is concatenated this many times.
"input input"

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode36
sha3/sha3_test.go:36: func (t *hashTest) Test(testDigests map[int]*digest)
(bool, string) {
This should return error, not (bool, string), and the caller can test err !=
nil.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode46
sha3/sha3_test.go:46: switch t.inputHex {
if t.inputHex {

but instead of an inputHex field, I'd make a top-level

func decodeHex(s string) string {
  b, err := hex.DecodeString(s)
  if err != nil {
    panic(err)
  }
  return string(b)
}

and make your test cases call that, instead of having this function do the
hex-decoding.

You could then do the string to []byte conversion for t.input outside the loop,
which for TestLongVectors would avoid 16777216-1 unnecessary pieces of garbage.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode58
sha3/sha3_test.go:58: for size, expectedOutput := range t.expectedOutputs {
The usual Go language is got and want, not computedOutput and expectedOutput.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode70
sha3/sha3_test.go:70: return false, fmt.Sprintf("SHA3[%d](%s) = %s want %s",
testDigests[size].Size()*8, errorInput, computedOutput,
strings.ToLower(expectedOutput))
Use fmt.Errorf instead of fmt.Sprintf to return an error.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode70
sha3/sha3_test.go:70: return false, fmt.Sprintf("SHA3[%d](%s) = %s want %s",
testDigests[size].Size()*8, errorInput, computedOutput,
strings.ToLower(expectedOutput))
You probably want %q instead of %s.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode80
sha3/sha3_test.go:80: for i := 0; i < len(result); i++ {
for i := range result {
  result[i] = byte(i)
}

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode142
sha3/sha3_test.go:142: // benchmarkBulkHashHelper tests the speed to hash a 16
kiB buffer.
Drop the "Helper". The difference in "Benchmark" and "benchmark" is enough to
distinguish these functions.

Similarly for testVectorHelper.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode158
sha3/sha3_test.go:158: // benchmarkBulkHashSHA3_X test the speed to hash a 16
kiB buffer by calling benchmarkBulkHashHelper.
kiB should be KiB.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode169
sha3/sha3_test.go:169: repeatCount: 16777216,
"repeatCount: 16 * 1024 * 1024," might be clearer.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3internal.go
File sha3/sha3internal.go (right):

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3internal.go#newcode1
sha3/sha3internal.go:1: // Copyright 2012 The Go Authors. All rights reserved.
I'd rename the file from sha3internal.go to internal.go. We're already in the
sha3 directory. There's no need to stutter.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3internal.go#newcode5
sha3/sha3internal.go:5: // Package sha3internal implements the core Keccak
permutation function necessary for computing SHA3.
It's not package sha3internal.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3internal.go#newcode14
sha3/sha3internal.go:14: var RC = [...]uint64{
Rename RC to rc unless there's a reason to export these values. Similarly for
R0.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3internal.go#newcode48
sha3/sha3internal.go:48: return w<<i ^ w>>(64-i)
The compiler will optimize "w<<i ^ w>>(64-i)" as a rotate if i is constant, but
not if i is a variable (or in this case, a function parameter).

Instead of a rotateLeft function, it may be better to define r0_xx constants
like so:

const (
  r0_00 = 0
  r0_01 = 36
  r0_02 = 3
  // etc
)

and below:

d.b[0] = d.a[0]<<r0_00 ^ d.a[0]>>(64-r0_00)
d.b[1] = d.a[6]<<r0_06 ^ d.a[6]>>(64-r0_06)
// etc

I filed https://code.google.com/p/go/issues/detail?id=5084 for the compiler bug.

[nigeltao, 2013-03-20 02:22:15]

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3internal.go
File sha3/sha3internal.go (right):

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3internal.go#newcode1
sha3/sha3internal.go:1: // Copyright 2012 The Go Authors. All rights reserved.
On 2013/03/20 02:17:43, nigeltao wrote:
> I'd rename the file from sha3internal.go to internal.go. 

Or, if the purpose of this file is to implement the Keccak-f function, I'd call
it keccakf.go.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3internal.go#newcode124
sha3/sha3internal.go:124: d.a[0] = d.b[0] ^ (^(d.b[1]) & d.b[2])
The inner-most parens are unnecessary.

[agl1, 2013-03-20 14:48:50]

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go
File sha3/sha3.go (right):

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode39
sha3/sha3.go:39: var padOpen = [laneSize]uint64{
Was this the result of benchmarking? I'd assume that a shift would be faster
than a memory access for this.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode57
sha3/sha3.go:57: a             [numLanes]uint64  //main state of the hash
space after "//"

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode73
sha3/sha3.go:73: // Capacity returns the number of bytes not touched by directly
during squeezing or absorbing, but which are only being updated by the
permutation function.
"by directly" might be a typo.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode93
sha3/sha3.go:93: // Currently we return the data rate, the number of bytes which
can be absorbed per invocation of the permutation function.
consider s/,/:/, but up to you.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode96
sha3/sha3.go:96: func (d *digest) BlockSize() int { return d.Rate() }
Yes, BlockSize is an unfortunate wart in the Hash interface that was added in
order to support HMAC. I think your choice here is reasonable.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode107
sha3/sha3.go:107: offset := (d.totalAbsorbed + absorbed) % d.Rate()
I suspect this function could be simpler and faster with something like:

var t uint64
for i := len(p) - 1; i >= 0; i-- {
  t |= p[i]
  t <<= 8
}
t <<= 8*(offset%laneSize)
d.a[lane] ^= t

(There's probably a bug in there, just coding out loud.)

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode111
sha3/sha3.go:111: copy(tempBuf[offset%laneSize:(offset%laneSize)+len(p)], p)
I think the text between ':' and ']' can be omitted. copy will only copy the
minimum of the length of the source and destination, so it will already only
copy len(p) bytes.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode118
sha3/sha3.go:118: // Since lanes are stored internall as type uin64, this
requires converting the incoming bytes into uint64 using a little endian
interpretation.
'y' on the end of internally.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode118
sha3/sha3.go:118: // Since lanes are stored internall as type uin64, this
requires converting the incoming bytes into uint64 using a little endian
interpretation.
s/type uin64/uint64s/ and s/uint64/uint64s/ later on.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode122
sha3/sha3.go:122: 
No need for the blank line at the start of the function.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode136
sha3/sha3.go:136: for absorbed < len(p)-7 {
This isn't really a problem, but this code doesn't look very canonical. Go code
typically reslices when processing a []byte. i.e. lots of p = p[8:] and then
testing len(p) > 0 etc.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode142
sha3/sha3.go:142: //fmt.Println(lane, absorbed, absorbed+lane*laneSize)
looks like debugging left in.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode165
sha3/sha3.go:165: // The padding is 10*1 with enough 0s to fill out an entire
block.
"The padding starts and ends with a one bit, with one or more zero bits in
between in order to fill out of entire block"

(I read "10*1" as "ten times one" for a while before figuring it out.)

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode175
sha3/sha3.go:175: // Sum takes a (possibly nil) set of remaining bytes and
"squeezes" out bytes from the state into an output buffer.
Sum takes a slice to which the hash is appended. The slice is not input data!

(This allows one to avoid allocating for every hash computation.)

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode178
sha3/sha3.go:178: 
Blank line not needed at start of function.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode191
sha3/sha3.go:191: 
rm blank line.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode232
sha3/sha3.go:232: //Given no parameters, a new instance with the highest
security level is provided.
I don't think we need this function. In other packages we have done something
like this because the package is called, say, "sha256", therefore "sha256.New"
is obvious. However, in this case, "sha3.New512" will be clear but "sha3.New"
much less so.

[jcb, 2013-03-20 23:36:15]

Here are fixes for the issues you brought up. With some of the optimizations
suggested it's now about twice as fast, and about 75% as fast as SHA512.

Note that I added a new test function to find bugs in the suggested changes to
the absorption code, and slightly re-factored the sum including fixing the fact
that I badly misinterpreted the interface to Sum()

Thanks!

Joe

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go
File sha3/sha3.go (right):

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode1
sha3/sha3.go:1: // Copyright 2012 The Go Authors. All rights reserved.
On 2013/03/20 02:17:43, nigeltao wrote:
> 2013, and similarly in the other files.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode13
sha3/sha3.go:13: 
Done. When I run godoc it picks up the Package doc okay but not most of the
functions. Am I missing something?

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode39
sha3/sha3.go:39: var padOpen = [laneSize]uint64{
Not the result of benchmarking, but vestigal. I originally was going to
pre-compute all padding blocks, before realizing that doesn't work since they
may end up different lanes. Changed to a shift for padOpen.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode57
sha3/sha3.go:57: a             [numLanes]uint64  //main state of the hash
On 2013/03/20 14:48:50, agl1 wrote:
> space after "//"

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode73
sha3/sha3.go:73: // Capacity returns the number of bytes not touched by directly
during squeezing or absorbing, but which are only being updated by the
permutation function.
On 2013/03/20 14:48:50, agl1 wrote:
> "by directly" might be a typo.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode74
sha3/sha3.go:74: func (d *digest) Capacity() int {
On 2013/03/20 02:17:43, nigeltao wrote:
> Rename Capacity to capacity unless there's a reason to export this method.
> Similarly for Rate.

Done for now. It may be useful to some clients to expose direct access to the
sponge function (including absorbing and squeezing data).

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode78
sha3/sha3.go:78: // Rate returns number of bytes of the internal state which can
be absorbed or squeezed in between calls to the permutation function.
On 2013/03/20 02:17:43, nigeltao wrote:
> "returns the number"

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode80
sha3/sha3.go:80: return (stateSize - d.Capacity())
On 2013/03/20 02:17:43, nigeltao wrote:
> Outer parens are unnecessary.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode87
sha3/sha3.go:87: for x := 0; x < numLanes; x++ {
On 2013/03/20 02:17:43, nigeltao wrote:
> for i := range d.a {
>   d.a[i] = 0
> }

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode92
sha3/sha3.go:92: // BlockSize, required by the Digest interface, does not have a
standard intepretation for a sponge-based construction like SHA3.
On 2013/03/20 02:17:43, nigeltao wrote:
> What is the Digest interface?

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode93
sha3/sha3.go:93: // Currently we return the data rate, the number of bytes which
can be absorbed per invocation of the permutation function.
On 2013/03/20 14:48:50, agl1 wrote:
> consider s/,/:/, but up to you.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode96
sha3/sha3.go:96: func (d *digest) BlockSize() int { return d.Rate() }
On 2013/03/20 14:48:50, agl1 wrote:
> Yes, BlockSize is an unfortunate wart in the Hash interface that was added in
> order to support HMAC. I think your choice here is reasonable. 

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode107
sha3/sha3.go:107: offset := (d.totalAbsorbed + absorbed) % d.Rate()
Yes, your approach is a little bit faster (~10% or so). There was a bug in your
attempt, the two lines of the loop need to be swapped or else you shift the
buffer an extra time.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode111
sha3/sha3.go:111: copy(tempBuf[offset%laneSize:(offset%laneSize)+len(p)], p)
On 2013/03/20 14:48:50, agl1 wrote:
> I think the text between ':' and ']' can be omitted. copy will only copy the
> minimum of the length of the source and destination, so it will already only
> copy len(p) bytes.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode118
sha3/sha3.go:118: // Since lanes are stored internall as type uin64, this
requires converting the incoming bytes into uint64 using a little endian
interpretation.
On 2013/03/20 14:48:50, agl1 wrote:
> 'y' on the end of internally.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode118
sha3/sha3.go:118: // Since lanes are stored internall as type uin64, this
requires converting the incoming bytes into uint64 using a little endian
interpretation.
On 2013/03/20 14:48:50, agl1 wrote:
> s/type uin64/uint64s/ and s/uint64/uint64s/ later on.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode122
sha3/sha3.go:122: 
On 2013/03/20 14:48:50, agl1 wrote:
> No need for the blank line at the start of the function.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode136
sha3/sha3.go:136: for absorbed < len(p)-7 {
I re-wrote it in this idiom, it ended up cleaner.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode142
sha3/sha3.go:142: //fmt.Println(lane, absorbed, absorbed+lane*laneSize)
On 2013/03/20 14:48:50, agl1 wrote:
> looks like debugging left in.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode165
sha3/sha3.go:165: // The padding is 10*1 with enough 0s to fill out an entire
block.
On 2013/03/20 14:48:50, agl1 wrote:
> "The padding starts and ends with a one bit, with one or more zero bits in
> between in order to fill out of entire block"
> 
> (I read "10*1" as "ten times one" for a while before figuring it out.)

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode175
sha3/sha3.go:175: // Sum takes a (possibly nil) set of remaining bytes and
"squeezes" out bytes from the state into an output buffer.
Yes, I interpreted the Interface very incorrectly. I reworked this and split the
squeezing into a separate function-if necessary this could be exported to use
SHA3 directly as a PRNG.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode178
sha3/sha3.go:178: 
On 2013/03/20 14:48:50, agl1 wrote:
> Blank line not needed at start of function.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode191
sha3/sha3.go:191: 
On 2013/03/20 14:48:50, agl1 wrote:
> rm blank line.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode206
sha3/sha3.go:206: func (d *digest) dumpState() {
On 2013/03/20 02:17:43, nigeltao wrote:
> If this is just for debugging then move it to sha3_test.go or delete it
> entirely.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode222
sha3/sha3.go:222: return &digest{outputSize: outputSize}
On 2013/03/20 02:17:43, nigeltao wrote:
> I don't think this function is doing much (and "new" shadows a built-in
> function). Just inline it below:
> 
> func New224() hash.Hash { &digest{outputSize: 224 / 8} }

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode225
sha3/sha3.go:225: // The NewX() constructors enable initializing a hash in any
of the four standard sizes of output, specified in bits.
On 2013/03/20 02:17:43, nigeltao wrote:
> Drop the parens.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode232
sha3/sha3.go:232: //Given no parameters, a new instance with the highest
security level is provided.
On 2013/03/20 02:17:43, nigeltao wrote:
> Go doc comments should begin with the name of the thing being commented upon.
> Thus:
> // New returns a new instance etc.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3.go#newcode232
sha3/sha3.go:232: //Given no parameters, a new instance with the highest
security level is provided.
On 2013/03/20 14:48:50, agl1 wrote:
> I don't think we need this function. In other packages we have done something
> like this because the package is called, say, "sha256", therefore "sha256.New"
> is obvious. However, in this case, "sha3.New512" will be clear but "sha3.New"
> much less so.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go
File sha3/sha3_test.go (right):

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode5
sha3/sha3_test.go:5: // This file implements test code for the SHA3 package.
On 2013/03/20 02:17:43, nigeltao wrote:
> I'd drop this sentence. It's obvious from the file name.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode6
sha3/sha3_test.go:6: // These tests are a subset of those provided by the Keccak
web site: http://keccak.noekeon.org/
On 2013/03/20 02:17:43, nigeltao wrote:
> Place a file-level comment below the package clause, so that it's less likely
to
> be confused for a package doc comment:
> --------
> // Copyright 2013 The Go Authors. All rights reserved.
> // Use of this source code is governed by a BSD-style
> // license that can be found in the LICENSE file.
> 
> package sha3
> 
> // These tests are a subset of those provided by the Keccak web site:
> http://keccak.noekeon.org/
> 
> import (
> --------

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode29
sha3/sha3_test.go:29: repeatCount     int  // If repeatCount is greater than 1,
the input input is concatenated this many times.
On 2013/03/20 02:17:43, nigeltao wrote:
> "input input"

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode36
sha3/sha3_test.go:36: func (t *hashTest) Test(testDigests map[int]*digest)
(bool, string) {
On 2013/03/20 02:17:43, nigeltao wrote:
> This should return error, not (bool, string), and the caller can test err !=
> nil.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode46
sha3/sha3_test.go:46: switch t.inputHex {
On 2013/03/20 02:17:43, nigeltao wrote:
> if t.inputHex {
> 
> but instead of an inputHex field, I'd make a top-level
> 
> func decodeHex(s string) string {
>   b, err := hex.DecodeString(s)
>   if err != nil {
>     panic(err)
>   }
>   return string(b)
> }
> 
> and make your test cases call that, instead of having this function do the
> hex-decoding.
> 
> You could then do the string to []byte conversion for t.input outside the
loop,
> which for TestLongVectors would avoid 16777216-1 unnecessary pieces of
garbage.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode58
sha3/sha3_test.go:58: for size, expectedOutput := range t.expectedOutputs {
On 2013/03/20 02:17:43, nigeltao wrote:
> The usual Go language is got and want, not computedOutput and expectedOutput.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode70
sha3/sha3_test.go:70: return false, fmt.Sprintf("SHA3[%d](%s) = %s want %s",
testDigests[size].Size()*8, errorInput, computedOutput,
strings.ToLower(expectedOutput))
On 2013/03/20 02:17:43, nigeltao wrote:
> You probably want %q instead of %s.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode70
sha3/sha3_test.go:70: return false, fmt.Sprintf("SHA3[%d](%s) = %s want %s",
testDigests[size].Size()*8, errorInput, computedOutput,
strings.ToLower(expectedOutput))
On 2013/03/20 02:17:43, nigeltao wrote:
> Use fmt.Errorf instead of fmt.Sprintf to return an error.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode80
sha3/sha3_test.go:80: for i := 0; i < len(result); i++ {
On 2013/03/20 02:17:43, nigeltao wrote:
> for i := range result {
>   result[i] = byte(i)
> }

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode142
sha3/sha3_test.go:142: // benchmarkBulkHashHelper tests the speed to hash a 16
kiB buffer.
On 2013/03/20 02:17:43, nigeltao wrote:
> Drop the "Helper". The difference in "Benchmark" and "benchmark" is enough to
> distinguish these functions.
> 
> Similarly for testVectorHelper.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode142
sha3/sha3_test.go:142: // benchmarkBulkHashHelper tests the speed to hash a 16
kiB buffer.
On 2013/03/20 02:17:43, nigeltao wrote:
> Drop the "Helper". The difference in "Benchmark" and "benchmark" is enough to
> distinguish these functions.
> 
> Similarly for testVectorHelper.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode158
sha3/sha3_test.go:158: // benchmarkBulkHashSHA3_X test the speed to hash a 16
kiB buffer by calling benchmarkBulkHashHelper.
On 2013/03/20 02:17:43, nigeltao wrote:
> kiB should be KiB.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode169
sha3/sha3_test.go:169: repeatCount: 16777216,
On 2013/03/20 02:17:43, nigeltao wrote:
> "repeatCount: 16 * 1024 * 1024," might be clearer.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3internal.go
File sha3/sha3internal.go (right):

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3internal.go#newcode1
sha3/sha3internal.go:1: // Copyright 2012 The Go Authors. All rights reserved.
On 2013/03/20 02:22:15, nigeltao wrote:
> On 2013/03/20 02:17:43, nigeltao wrote:
> > I'd rename the file from sha3internal.go to internal.go. 
> 
> Or, if the purpose of this file is to implement the Keccak-f function, I'd
call
> it keccakf.go.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3internal.go#newcode1
sha3/sha3internal.go:1: // Copyright 2012 The Go Authors. All rights reserved.
On 2013/03/20 02:17:43, nigeltao wrote:
> I'd rename the file from sha3internal.go to internal.go. We're already in the
> sha3 directory. There's no need to stutter.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3internal.go#newcode5
sha3/sha3internal.go:5: // Package sha3internal implements the core Keccak
permutation function necessary for computing SHA3.
On 2013/03/20 02:17:43, nigeltao wrote:
> It's not package sha3internal.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3internal.go#newcode14
sha3/sha3internal.go:14: var RC = [...]uint64{
On 2013/03/20 02:17:43, nigeltao wrote:
> Rename RC to rc unless there's a reason to export these values. Similarly for
> R0.

Done.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3internal.go#newcode48
sha3/sha3internal.go:48: return w<<i ^ w>>(64-i)
Great catch! This roughly doubled the speed of the benchmarks.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3internal.go#newcode124
sha3/sha3internal.go:124: d.a[0] = d.b[0] ^ (^(d.b[1]) & d.b[2])
On 2013/03/20 02:22:15, nigeltao wrote:
> The inner-most parens are unnecessary.

Done.

[nigeltao, 2013-03-21 02:36:49]

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go
File sha3/sha3_test.go (right):

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode46
sha3/sha3_test.go:46: switch t.inputHex {
On 2013/03/20 23:36:16, jcb wrote:
> On 2013/03/20 02:17:43, nigeltao wrote:
> > if t.inputHex {
> > 
> > but instead of an inputHex field, I'd make a top-level
> > 
> > func decodeHex(s string) string {
> >   b, err := hex.DecodeString(s)
> >   if err != nil {
> >     panic(err)
> >   }
> >   return string(b)
> > }
> > 
> > and make your test cases call that, instead of having this function do the
> > hex-decoding.
> > 
> > You could then do the string to []byte conversion for t.input outside the
> loop,
> > which for TestLongVectors would avoid 16777216-1 unnecessary pieces of
> garbage.
> 
> Done.

Not (entirely) done? I would eliminate the t.inputHex field entirely.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3.go
File sha3/sha3.go (right):

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3.go#newcode39
sha3/sha3.go:39: const padOpen = 0x0000000000000001
const (
    padOpen  = 0x0000000000000001
    padClose = 0x8000000000000000
)

will let gofmt align the RHSs.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3.go#newcode136
sha3/sha3.go:136: lastLane := minInt(d.rate()/laneSize,
firstLane+(len(p))/laneSize)
Unnecessary parens around len(p).

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3.go#newcode205
sha3/sha3.go:205: func (d_original *digest) Sum(in []byte) []byte {
Call the method receiver d, and the new variable a few lines below dup.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3.go#newcode206
sha3/sha3.go:206: 
Delete the blank line.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go
File sha3/sha3_test.go (right):

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode29
sha3/sha3_test.go:29: repeatCount     int  // input will be concatenated the
input this many times.
I would s/repeatCount/repeat/ and s/expectedOutputs/want/.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode44
sha3/sha3_test.go:44: // It returns true or false based on the test passing,
plus an error string for printing.
The comment needs updating.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode45
sha3/sha3_test.go:45: func (t *hashTest) Test(testDigests map[int]*digest) error
{
Remove the argument, since it's always the global testDigests.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode46
sha3/sha3_test.go:46: 
Delete the blank line.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode60
sha3/sha3_test.go:60: for i := 0; i < t.repeatCount || i == 0; i++ {
When is t.repeatCount zero?

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode68
sha3/sha3_test.go:68: got :=
strings.ToLower(hex.EncodeToString(testDigests[size].Sum(nil)))
Call ToUpper instead of ToLower if want is always going to be upper case.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode70
sha3/sha3_test.go:70: errorInput := t.input
Instead of constructing an errorInput string, I'd add a desc field to hashTest,
containing a short human-readable description of that test (including the ×%d
for the long test). For example, see
http://golang.org/src/pkg/compress/zlib/reader_test.go

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode100
sha3/sha3_test.go:100: for size := range testDigests {
for _, d := range testDigests {
  d.Reset()
  d.Write(buf)
  etc
}

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode106
sha3/sha3_test.go:106: for i < len(buf) {
for i := 0; i < len(buf); {

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode117
sha3/sha3_test.go:117: if !bytes.Equal(want, got) {
The usual Go order is got and want, not want and got. Similarly on the next
line.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode118
sha3/sha3_test.go:118: t.Fatalf(fmt.Sprintf("Unalighned SHA3[%d] = wanted %x,
got %x", testDigests[size].Size()*8, want, got))
Drop the inner fmt.Sprintf. t.Fatalf is already an 'f' function.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode229
sha3/sha3_test.go:229: func TestLongVectors(t *testing.T) {
I'd just roll this together with TestShortVectors, and then inline the
testVector helper function:

func TestVectors(t *testing.T) {
  testCases := append([]hashTest{}, shortTestVectors...)
  if !testing.Short {
    testCases = append(testCases, longTestVectors...)
  }
loop:
  for _, tc := range testCases {
    if err := tc.Test(); err != nil {
      t.Errorf("%s: %v", tc.desc, err)
    }
  }
}

Or remove the hashTest.Test method and just put that code inside the for loop
above, replacing "return fmt.Errorf(etc)" with "t.Errorf(etc); continue loop".

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode286
sha3/sha3_test.go:286: error := test.Test(testDigests)
s/error/err/

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode288
sha3/sha3_test.go:288: t.Fatalf(error.Error())
t.Fatal(err), otherwise you'll print garbage if e.g. the error string contains
"%s".

Also, change Fatal to Error. It's worth distinguishing whether all the vectors
fail, or only the first one.

[nigeltao, 2013-03-21 02:37:48]

Also change the CL description opening line from:
sha3/: new package
to
go.crypto/sha3: new package.

[jcb, 2013-03-21 19:06:25]

I think we're down to two issues of how the test vectors are stored-I think the
current format is simplest but could be convinced otherwise.

Thanks again!

Joe

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go
File sha3/sha3_test.go (right):

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode46
sha3/sha3_test.go:46: switch t.inputHex {
I don't follow what you're suggesting here. To support string and hex input (and
the standard tests include both) we have to store somewhere how to interpret a
string. Are you suggesting hard-coding it by having the test function call it
for some test vectors and not others?

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3.go
File sha3/sha3.go (right):

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3.go#newcode39
sha3/sha3.go:39: const padOpen = 0x0000000000000001
Thinking about it further, abstracting these as named constants makes the code
less readable so I inlined them. Their structure is straightforward and any
change to the padding scheme would require changing the code anyways, there's no
agility gained by abstracting them.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3.go#newcode136
sha3/sha3.go:136: lastLane := minInt(d.rate()/laneSize,
firstLane+(len(p))/laneSize)
On 2013/03/21 02:36:50, nigeltao wrote:
> Unnecessary parens around len(p).

Done.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3.go#newcode205
sha3/sha3.go:205: func (d_original *digest) Sum(in []byte) []byte {
On 2013/03/21 02:36:50, nigeltao wrote:
> Call the method receiver d, and the new variable a few lines below dup.

Done.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3.go#newcode206
sha3/sha3.go:206: 
On 2013/03/21 02:36:50, nigeltao wrote:
> Delete the blank line.

Done.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go
File sha3/sha3_test.go (right):

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode29
sha3/sha3_test.go:29: repeatCount     int  // input will be concatenated the
input this many times.
On 2013/03/21 02:36:50, nigeltao wrote:
> I would s/repeatCount/repeat/ and s/expectedOutputs/want/.

Done.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode44
sha3/sha3_test.go:44: // It returns true or false based on the test passing,
plus an error string for printing.
On 2013/03/21 02:36:50, nigeltao wrote:
> The comment needs updating.

Done.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode45
sha3/sha3_test.go:45: func (t *hashTest) Test(testDigests map[int]*digest) error
{
On 2013/03/21 02:36:50, nigeltao wrote:
> Remove the argument, since it's always the global testDigests.

Done.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode46
sha3/sha3_test.go:46: 
On 2013/03/21 02:36:50, nigeltao wrote:
> Delete the blank line.

Done.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode60
sha3/sha3_test.go:60: for i := 0; i < t.repeatCount || i == 0; i++ {
On 2013/03/21 02:36:50, nigeltao wrote:
> When is t.repeatCount zero?

Done.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode68
sha3/sha3_test.go:68: got :=
strings.ToLower(hex.EncodeToString(testDigests[size].Sum(nil)))
On 2013/03/21 02:36:50, nigeltao wrote:
> Call ToUpper instead of ToLower if want is always going to be upper case.

Done.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode70
sha3/sha3_test.go:70: errorInput := t.input
I don't think this is helpful for test vectors like these which don't admit any
short human-readable summary. They're just random, they don't exercise any
specific functionality. Adding a fixed error string would just mean hard-coding
what's produced here, which would mean specifying the data in two places.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode100
sha3/sha3_test.go:100: for size := range testDigests {
On 2013/03/21 02:36:50, nigeltao wrote:
> for _, d := range testDigests {
>   d.Reset()
>   d.Write(buf)
>   etc
> }

Done.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode106
sha3/sha3_test.go:106: for i < len(buf) {
On 2013/03/21 02:36:50, nigeltao wrote:
> for i := 0; i < len(buf); {

Done.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode117
sha3/sha3_test.go:117: if !bytes.Equal(want, got) {
On 2013/03/21 02:36:50, nigeltao wrote:
> The usual Go order is got and want, not want and got. Similarly on the next
> line.

Done.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode118
sha3/sha3_test.go:118: t.Fatalf(fmt.Sprintf("Unalighned SHA3[%d] = wanted %x,
got %x", testDigests[size].Size()*8, want, got))
On 2013/03/21 02:36:50, nigeltao wrote:
> Drop the inner fmt.Sprintf. t.Fatalf is already an 'f' function.

Done.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode229
sha3/sha3_test.go:229: func TestLongVectors(t *testing.T) {
On 2013/03/21 02:36:50, nigeltao wrote:
> I'd just roll this together with TestShortVectors, and then inline the
> testVector helper function:
> 
> func TestVectors(t *testing.T) {
>   testCases := append([]hashTest{}, shortTestVectors...)
>   if !testing.Short {
>     testCases = append(testCases, longTestVectors...)
>   }
> loop:
>   for _, tc := range testCases {
>     if err := tc.Test(); err != nil {
>       t.Errorf("%s: %v", tc.desc, err)
>     }
>   }
> }
> 
> Or remove the hashTest.Test method and just put that code inside the for loop
> above, replacing "return fmt.Errorf(etc)" with "t.Errorf(etc); continue loop".

Done.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode286
sha3/sha3_test.go:286: error := test.Test(testDigests)
On 2013/03/21 02:36:50, nigeltao wrote:
> s/error/err/

Done.

https://codereview.appspot.com/7760044/diff/20001/sha3/sha3_test.go#newcode288
sha3/sha3_test.go:288: t.Fatalf(error.Error())
On 2013/03/21 02:36:50, nigeltao wrote:
> t.Fatal(err), otherwise you'll print garbage if e.g. the error string contains
> "%s".
> 
> Also, change Fatal to Error. It's worth distinguishing whether all the vectors
> fail, or only the first one.

Done.

[agl1, 2013-03-21 20:27:23]

LGTM with nits.

If Nigel says LGTM then I'll land it.

https://codereview.appspot.com/7760044/diff/27001/sha3/sha3.go
File sha3/sha3.go (right):

https://codereview.appspot.com/7760044/diff/27001/sha3/sha3.go#newcode20
sha3/sha3.go:20: // Note that changing this to a size besides 8 requires using
other than uint64 to store each lane.
"using a type other than a uint64" or maybe "using other than a uint64".

https://codereview.appspot.com/7760044/diff/27001/sha3/sha3.go#newcode23
sha3/sha3.go:23: // sliceSize represent the dimensions of the internal state, a
square matrix of
s on the end of "represent"

https://codereview.appspot.com/7760044/diff/27001/sha3/sha3.go#newcode24
sha3/sha3.go:24: // sliceSize ** 2 lanes. This is both the size of "rows" and
"columns" dimension in the
missing "the": "size of the "rows" and ...

https://codereview.appspot.com/7760044/diff/27001/sha3/sha3.go#newcode106
sha3/sha3.go:106: // Track the total number of bytes absorbed and update the
hash state at the end.
I don't think you want this variable, although it's your call.

If you eliminate it, and just update d.totalAbsorbed, then you can remove an
argument to unalignedAbsorb. You do need to change the += in the unaligned case
to something like:

n := d.unalignedAbsorb(...
p = p[n:]
d.totalAbsorbed += n

Although this does remove the += in the original code which makes the
p[absorbed:] look wrong, until one notes that absorbed must be zero beforehand.

It also saves you lots of additions to d.totalAbsorbed.

Since this function always processes the whole input, it can just have `total :=
len(p)` at the top to keep the value for the return.

https://codereview.appspot.com/7760044/diff/27001/sha3/sha3.go#newcode183
sha3/sha3.go:183: var buf [8]byte
I'd remove buf because the multiple appends make Sum(nil) a worst-case, as
append probably allocates and moves multiple times.

if cap(in) >= toSqueeze {
  in = in[:toSqueeze]
} else {
  in = make([]byte, toSqueeze)
}

then:

binary.LittleEndian.PutUint64(in, d.a[i/laneSize])
in = in[laneSize:]

https://codereview.appspot.com/7760044/diff/27001/sha3/sha3.go#newcode206
sha3/sha3.go:206: // The NewX constructors enable initializing a hash in any of
the four standard sizes.
These will probably need to be updated, right? NIST hasn't settled on the final
capacities for the various sizes as I understand.

https://codereview.appspot.com/7760044/diff/27001/sha3/sha3_test.go
File sha3/sha3_test.go (right):

https://codereview.appspot.com/7760044/diff/27001/sha3/sha3_test.go#newcode26
sha3/sha3_test.go:26: // hashTest represents a test vector inputs and expected
outputs at multiple sizes.
s/inputs/input/

https://codereview.appspot.com/7760044/diff/27001/sha3/sha3_test.go#newcode109
sha3/sha3_test.go:109: for size := range testDigests {
for _, digest := range testDigests {
  digest.Reset()
}

https://codereview.appspot.com/7760044/diff/27001/sha3/sha3_test.go#newcode112
sha3/sha3_test.go:112: // Convert input data to a byte array, decoding Hex if
necessary
s/decoding Hex/hex decoding/

[jcb, 2013-03-22 01:00:38]

Fixed nits plus two substantial items:
*Changed the squeeze() function to only allocate one or fewer times. Please
confirm that this is a reasonable implementation.
*To hedge against future standardization changes, changed the names from New512
to NewKeccak512, etc. Whatever they call the final variants (there's still
debate about this, they might just be named like SHA3-512 by output size or they
might be named something ghastly like SHA3-512_1024 to indicate the underlying
capacity parameter. If we put Keccak in the name it should be clear this may not
be the eventual standard.

https://codereview.appspot.com/7760044/diff/27001/sha3/sha3.go
File sha3/sha3.go (right):

https://codereview.appspot.com/7760044/diff/27001/sha3/sha3.go#newcode20
sha3/sha3.go:20: // Note that changing this to a size besides 8 requires using
other than uint64 to store each lane.
On 2013/03/21 20:27:23, agl1 wrote:
> "using a type other than a uint64" or maybe "using other than a uint64".

Done.

https://codereview.appspot.com/7760044/diff/27001/sha3/sha3.go#newcode23
sha3/sha3.go:23: // sliceSize represent the dimensions of the internal state, a
square matrix of
On 2013/03/21 20:27:23, agl1 wrote:
> s on the end of "represent"

Done.

https://codereview.appspot.com/7760044/diff/27001/sha3/sha3.go#newcode24
sha3/sha3.go:24: // sliceSize ** 2 lanes. This is both the size of "rows" and
"columns" dimension in the
On 2013/03/21 20:27:23, agl1 wrote:
> missing "the": "size of the "rows" and ...

Done.

https://codereview.appspot.com/7760044/diff/27001/sha3/sha3.go#newcode106
sha3/sha3.go:106: // Track the total number of bytes absorbed and update the
hash state at the end.
Done. This made the code cleaner.

https://codereview.appspot.com/7760044/diff/27001/sha3/sha3.go#newcode183
sha3/sha3.go:183: var buf [8]byte
I don't think your suggested code complies with the Hash interface, since the
output needs to be appended to any input data (though I've mistaken the
intention of this interface once already).

Point taken about not calling multiple appends though-please review my updated
implementation which performs only a single memory allocation (if needed).

https://codereview.appspot.com/7760044/diff/27001/sha3/sha3.go#newcode206
sha3/sha3.go:206: // The NewX constructors enable initializing a hash in any of
the four standard sizes.
Indeed. To avoid issues later on, I changed the names from NewX to NewKeccakX,
so that whatever the eventual standard SHA3 algorithm names won't be confused
with these (which reflect the final Keccak proposal). They haven't settled on
the final capacity/output combinations so I changed capacity to a free
parameter.

https://codereview.appspot.com/7760044/diff/27001/sha3/sha3_test.go
File sha3/sha3_test.go (right):

https://codereview.appspot.com/7760044/diff/27001/sha3/sha3_test.go#newcode26
sha3/sha3_test.go:26: // hashTest represents a test vector inputs and expected
outputs at multiple sizes.
On 2013/03/21 20:27:23, agl1 wrote:
> s/inputs/input/

Done.

https://codereview.appspot.com/7760044/diff/27001/sha3/sha3_test.go#newcode109
sha3/sha3_test.go:109: for size := range testDigests {
This might call extraneous Resets (not that they're very expensive) if a
testCase doesn't use all available hash versions.

https://codereview.appspot.com/7760044/diff/27001/sha3/sha3_test.go#newcode112
sha3/sha3_test.go:112: // Convert input data to a byte array, decoding Hex if
necessary
On 2013/03/21 20:27:23, agl1 wrote:
> s/decoding Hex/hex decoding/

Done.

[nigeltao, 2013-03-22 04:21:21]

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go
File sha3/sha3_test.go (right):

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode46
sha3/sha3_test.go:46: switch t.inputHex {
On 2013/03/21 19:06:25, jcb wrote:
> I don't follow what you're suggesting here.

I'd write it like this:

// testVector represents a test input and expected outputs from multiple
algorithm variants.
type testVector struct {
	desc   string
	input  []byte
	repeat int // The input will be written this many times.
	want   map[string]string
}

// decodeHex converts an hex-encoded string into a raw byte string.
func decodeHex(s string) []byte {
	b, err := hex.DecodeString(s)
	if err != nil {
		panic(err)
	}
	return b
}

// shortTestVectors stores a series of short testVectors.
// Inputs of 8, 248, and 264 bits from http://keccak.noekeon.org/ are included
below.
// The standard defines additional test inputs of all sizes between 0 and 2047
bits.
// Because the current implementation can only handle an integral number of
bytes,
// most of the standard test inputs can't be used.
var shortKeccakTestVectors = []testVector{
	{
		desc:   "short8",
		input:  decodeHex("CC"),
		repeat: 1,
		want: map[string]string{
			"Keccak224": "A9CAB59EB40A10B246290F2D6086E32E3689FAF1D26B470C899F2802",
			"Keccak256":
"EEAD6DBFC7340A56CAEDC044696A168870549A6A7F6F56961E84A54BD9970B8A",
			"Keccak384":
"1B84E62A46E5A201861754AF5DC95C4A1A69CAF4A796AE405680161E29572641F5FA1E8641D7958336EE7B11C58F73E9",
			"Keccak512":
"8630C13CBD066EA74BBE7FE468FEC1DEE10EDC1254FB4C1B7C5FD69B646E44160B8CE01D05A0908CA790DFB080F4B513BC3B6225ECE7A810371441A5AC666EB9",
		},
	},
	{
		desc:   "short248",
		input: 
decodeHex("84FB51B517DF6C5ACCB5D022F8F28DA09B10232D42320FFC32DBECC3835B29"),
		repeat: 1,
		want: map[string]string{
			"Keccak224": "81AF3A7A5BD4C1F948D6AF4B96F93C3B0CF9C0E7A6DA6FCD71EEC7F6",
			"Keccak256":
"D477FB02CAAA95B3280EC8EE882C29D9E8A654B21EF178E0F97571BF9D4D3C1C",
			"Keccak384":
"503DCAA4ADDA5A9420B2E436DD62D9AB2E0254295C2982EF67FCE40F117A2400AB492F7BD5D133C6EC2232268BC27B42",
			"Keccak512":
"9D8098D8D6EDBBAA2BCFC6FB2F89C3EAC67FEC25CDFE75AA7BD570A648E8C8945FF2EC280F6DCF73386109155C5BBC444C707BB42EAB873F5F7476657B1BC1A8",
		},
	},
	{
		desc:   "short264",
		input: 
decodeHex("DE8F1B3FAA4B7040ED4563C3B8E598253178E87E4D0DF75E4FF2F2DEDD5A0BE046"),
		repeat: 1,
		want: map[string]string{
			"Keccak224": "F217812E362EC64D4DC5EACFABC165184BFA456E5C32C2C7900253D0",
			"Keccak256":
"E78C421E6213AFF8DE1F025759A4F2C943DB62BBDE359C8737E19B3776ED2DD2",
			"Keccak384":
"CF38764973F1EC1C34B5433AE75A3AAD1AAEF6AB197850C56C8617BCD6A882F6666883AC17B2DCCDBAA647075D0972B5",
			"Keccak512":
"9A7688E31AAF40C15575FC58C6B39267AAD3722E696E518A9945CF7F7C0FEA84CB3CB2E9F0384A6B5DC671ADE7FB4D2B27011173F3EEEAF17CB451CF26542031",
		},
	},
}

// longTestVectors stores longer testVector (currently only one).
// The computed test vector is about 413 MiB long, taken from
http://keccak.noekeon.org/.
var longKeccakTestVectors = []testVector{
	{
		desc:   "long-abc",
		input: 
[]byte("abcdefghbcdefghicdefghijdefghijkefghijklfghijklmghijklmnhijklmno"),
		repeat: 16 * 1024 * 1024,
		want: map[string]string{
			"Keccak224": "C42E4AEE858E1A8AD2976896B9D23DD187F64436EE15969AFDBC68C5",
			"Keccak256":
"5F313C39963DCF792B5470D4ADE9F3A356A3E4021748690A958372E2B06F82A4",
			"Keccak384":
"9B7168B4494A80A86408E6B9DC4E5A1837C85DD8FF452ED410F2832959C08C8C0D040A892EB9A755776372D4A8732315",
			"Keccak512":
"3E122EDAF37398231CFACA4C7C216C9D66D5B899EC1D7AC617C40C7261906A45FC01617A021E5DA3BD8D4182695B5CB785A28237CBB167590E34718E56D8AAB8",
		},
	},
}

// TestKeccakVectors checks that correct output is produced for a set of known
testVectors.
func TestKeccakVectors(t *testing.T) {
	testCases := append([]testVector{}, shortKeccakTestVectors...)
	if !testing.Short() {
		testCases = append(testCases, longKeccakTestVectors...)
	}
	for _, tc := range testCases {
		for alg, want := range tc.want {
			d := testDigests[alg]
			d.Reset()
			for i := 0; i < tc.repeat; i++ {
				d.Write(tc.input)
			}
			got := strings.ToUpper(hex.EncodeToString(d.Sum(nil)))
			if got != want {
				t.Errorf("%s, alg=%s\ngot  %q\nwant %q", tc.desc, alg, got, want)
				d.dumpState()
			}
		}
	}
}

https://codereview.appspot.com/7760044/diff/39001/sha3/sha3_test.go
File sha3/sha3_test.go (right):

https://codereview.appspot.com/7760044/diff/39001/sha3/sha3_test.go#newcode103
sha3/sha3_test.go:103: // In short testing mode, the longer tests are skipped.
I don't think this comment is adding much.

https://codereview.appspot.com/7760044/diff/39001/sha3/sha3_test.go#newcode121
sha3/sha3_test.go:121: for alg := range tc.want {
I'd reverse the order of the loops, and avoid 64 million map lookups. After
doing that, you're ranging over tc.want three times; you can collapse that.

On my workstation, that cuts the "go test" running time by 10%.

https://codereview.appspot.com/7760044/diff/39001/sha3/sha3_test.go#newcode139
sha3/sha3_test.go:139: t.Errorf("Test vector failed for %s(%s): got %q, wanted
%q", alg, errorInput, got, want)
s/wanted/want/

https://codereview.appspot.com/7760044/diff/39001/sha3/sha3_test.go#newcode179
sha3/sha3_test.go:179: t.Errorf("Inconsistent output after unaligned writes to
%s: got %x, wanted %x", alg, got, want)
s/wanted/want/

[jcb, 2013-03-22 16:15:51]

Test code cleaned up.

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go
File sha3/sha3_test.go (right):

https://codereview.appspot.com/7760044/diff/7001/sha3/sha3_test.go#newcode46
sha3/sha3_test.go:46: switch t.inputHex {
On 2013/03/22 04:21:21, nigeltao wrote:
> On 2013/03/21 19:06:25, jcb wrote:
> > I don't follow what you're suggesting here.
> 
> I'd write it like this:
> 
> // testVector represents a test input and expected outputs from multiple
> algorithm variants.
> type testVector struct {
> 	desc   string
> 	input  []byte
> 	repeat int // The input will be written this many times.
> 	want   map[string]string
> }
> 
> // decodeHex converts an hex-encoded string into a raw byte string.
> func decodeHex(s string) []byte {
> 	b, err := hex.DecodeString(s)
> 	if err != nil {
> 		panic(err)
> 	}
> 	return b
> }
> 
> // shortTestVectors stores a series of short testVectors.
> // Inputs of 8, 248, and 264 bits from http://keccak.noekeon.org/ are included
> below.
> // The standard defines additional test inputs of all sizes between 0 and 2047
> bits.
> // Because the current implementation can only handle an integral number of
> bytes,
> // most of the standard test inputs can't be used.
> var shortKeccakTestVectors = []testVector{
> 	{
> 		desc:   "short8",
> 		input:  decodeHex("CC"),
> 		repeat: 1,
> 		want: map[string]string{
> 			"Keccak224": "A9CAB59EB40A10B246290F2D6086E32E3689FAF1D26B470C899F2802",
> 			"Keccak256":
> "EEAD6DBFC7340A56CAEDC044696A168870549A6A7F6F56961E84A54BD9970B8A",
> 			"Keccak384":
>
"1B84E62A46E5A201861754AF5DC95C4A1A69CAF4A796AE405680161E29572641F5FA1E8641D7958336EE7B11C58F73E9",
> 			"Keccak512":
>
"8630C13CBD066EA74BBE7FE468FEC1DEE10EDC1254FB4C1B7C5FD69B646E44160B8CE01D05A0908CA790DFB080F4B513BC3B6225ECE7A810371441A5AC666EB9",
> 		},
> 	},
> 	{
> 		desc:   "short248",
> 		input: 
> decodeHex("84FB51B517DF6C5ACCB5D022F8F28DA09B10232D42320FFC32DBECC3835B29"),
> 		repeat: 1,
> 		want: map[string]string{
> 			"Keccak224": "81AF3A7A5BD4C1F948D6AF4B96F93C3B0CF9C0E7A6DA6FCD71EEC7F6",
> 			"Keccak256":
> "D477FB02CAAA95B3280EC8EE882C29D9E8A654B21EF178E0F97571BF9D4D3C1C",
> 			"Keccak384":
>
"503DCAA4ADDA5A9420B2E436DD62D9AB2E0254295C2982EF67FCE40F117A2400AB492F7BD5D133C6EC2232268BC27B42",
> 			"Keccak512":
>
"9D8098D8D6EDBBAA2BCFC6FB2F89C3EAC67FEC25CDFE75AA7BD570A648E8C8945FF2EC280F6DCF73386109155C5BBC444C707BB42EAB873F5F7476657B1BC1A8",
> 		},
> 	},
> 	{
> 		desc:   "short264",
> 		input: 
>
decodeHex("DE8F1B3FAA4B7040ED4563C3B8E598253178E87E4D0DF75E4FF2F2DEDD5A0BE046"),
> 		repeat: 1,
> 		want: map[string]string{
> 			"Keccak224": "F217812E362EC64D4DC5EACFABC165184BFA456E5C32C2C7900253D0",
> 			"Keccak256":
> "E78C421E6213AFF8DE1F025759A4F2C943DB62BBDE359C8737E19B3776ED2DD2",
> 			"Keccak384":
>
"CF38764973F1EC1C34B5433AE75A3AAD1AAEF6AB197850C56C8617BCD6A882F6666883AC17B2DCCDBAA647075D0972B5",
> 			"Keccak512":
>
"9A7688E31AAF40C15575FC58C6B39267AAD3722E696E518A9945CF7F7C0FEA84CB3CB2E9F0384A6B5DC671ADE7FB4D2B27011173F3EEEAF17CB451CF26542031",
> 		},
> 	},
> }
> 
> // longTestVectors stores longer testVector (currently only one).
> // The computed test vector is about 413 MiB long, taken from
> http://keccak.noekeon.org/.
> var longKeccakTestVectors = []testVector{
> 	{
> 		desc:   "long-abc",
> 		input: 
> []byte("abcdefghbcdefghicdefghijdefghijkefghijklfghijklmghijklmnhijklmno"),
> 		repeat: 16 * 1024 * 1024,
> 		want: map[string]string{
> 			"Keccak224": "C42E4AEE858E1A8AD2976896B9D23DD187F64436EE15969AFDBC68C5",
> 			"Keccak256":
> "5F313C39963DCF792B5470D4ADE9F3A356A3E4021748690A958372E2B06F82A4",
> 			"Keccak384":
>
"9B7168B4494A80A86408E6B9DC4E5A1837C85DD8FF452ED410F2832959C08C8C0D040A892EB9A755776372D4A8732315",
> 			"Keccak512":
>
"3E122EDAF37398231CFACA4C7C216C9D66D5B899EC1D7AC617C40C7261906A45FC01617A021E5DA3BD8D4182695B5CB785A28237CBB167590E34718E56D8AAB8",
> 		},
> 	},
> }
> 
> // TestKeccakVectors checks that correct output is produced for a set of known
> testVectors.
> func TestKeccakVectors(t *testing.T) {
> 	testCases := append([]testVector{}, shortKeccakTestVectors...)
> 	if !testing.Short() {
> 		testCases = append(testCases, longKeccakTestVectors...)
> 	}
> 	for _, tc := range testCases {
> 		for alg, want := range tc.want {
> 			d := testDigests[alg]
> 			d.Reset()
> 			for i := 0; i < tc.repeat; i++ {
> 				d.Write(tc.input)
> 			}
> 			got := strings.ToUpper(hex.EncodeToString(d.Sum(nil)))
> 			if got != want {
> 				t.Errorf("%s, alg=%s\ngot  %q\nwant %q", tc.desc, alg, got, want)
> 				d.dumpState()
> 			}
> 		}
> 	}
> }

Done.

https://codereview.appspot.com/7760044/diff/39001/sha3/sha3_test.go
File sha3/sha3_test.go (right):

https://codereview.appspot.com/7760044/diff/39001/sha3/sha3_test.go#newcode103
sha3/sha3_test.go:103: // In short testing mode, the longer tests are skipped.
On 2013/03/22 04:21:21, nigeltao wrote:
> I don't think this comment is adding much.

Done.

https://codereview.appspot.com/7760044/diff/39001/sha3/sha3_test.go#newcode121
sha3/sha3_test.go:121: for alg := range tc.want {
On 2013/03/22 04:21:21, nigeltao wrote:
> I'd reverse the order of the loops, and avoid 64 million map lookups. After
> doing that, you're ranging over tc.want three times; you can collapse that.
> 
> On my workstation, that cuts the "go test" running time by 10%.

Done.

https://codereview.appspot.com/7760044/diff/39001/sha3/sha3_test.go#newcode139
sha3/sha3_test.go:139: t.Errorf("Test vector failed for %s(%s): got %q, wanted
%q", alg, errorInput, got, want)
On 2013/03/22 04:21:21, nigeltao wrote:
> s/wanted/want/

Done.

https://codereview.appspot.com/7760044/diff/39001/sha3/sha3_test.go#newcode179
sha3/sha3_test.go:179: t.Errorf("Inconsistent output after unaligned writes to
%s: got %x, wanted %x", alg, got, want)
On 2013/03/22 04:21:21, nigeltao wrote:
> s/wanted/want/

Done.

[agl1, 2013-03-22 16:37:02]

On Fri, Mar 22, 2013 at 12:15 PM,  <jcb@google.com> wrote:
> Test code cleaned up.

I was just about to submit but:

a) I need to put you in CONTRIBUTORS first. I can take care of that.
b) the test is taking 70s to complete and I don't believe that it's
worth the time. By reducing the long test from 1GB to 64MB, the test
takes ~5 seconds to run, which is much more reasonable.

Do you object to the latter? Obviously we're not taking values
directly from the upstream reference tables any longer, which is a
shame, but I think it's either that or delete the long test.


Cheers

AGL

[jcb, 2013-03-22 16:47:38]

Hmmm, alternative is that there are upstream test vectors of up to 4KB, but
they are random. I'd say shipping those around is not worth the trouble
either.

I can cut the long vectors down to size and run them through the Python
version to cross-check.


On Fri, Mar 22, 2013 at 12:37 PM, Adam Langley <agl@golang.org> wrote:

> On Fri, Mar 22, 2013 at 12:15 PM,  <jcb@google.com> wrote:
> > Test code cleaned up.
>
> I was just about to submit but:
>
> a) I need to put you in CONTRIBUTORS first. I can take care of that.
> b) the test is taking 70s to complete and I don't believe that it's
> worth the time. By reducing the long test from 1GB to 64MB, the test
> takes ~5 seconds to run, which is much more reasonable.
>
> Do you object to the latter? Obviously we're not taking values
> directly from the upstream reference tables any longer, which is a
> shame, but I think it's either that or delete the long test.
>
>
> Cheers
>
> AGL
>

[agl1, 2013-03-22 16:52:20]

On Fri, Mar 22, 2013 at 12:47 PM, Joseph Bonneau <jcb@google.com> wrote:
> Hmmm, alternative is that there are upstream test vectors of up to 4KB, but
> they are random. I'd say shipping those around is not worth the trouble
> either.
>
> I can cut the long vectors down to size and run them through the Python
> version to cross-check.

If you can confirm this with the Python code then I'll land it:

var longKeccakTestVectors = []testVector{
»·······{
»·······»·······desc:   "long-1GiB",
»·······»·······input:
[]byte("abcdefghbcdefghicdefghijdefghijkefghijklfghijklmghijklmnhijklmno"),
»·······»·······repeat: 1024 * 1024,
»·······»·······want: map[string]string{
»·······»·······»·······"Keccak224":
"50E35E40980FEEFF1EA490957B0E970257F75EA0D410EE0F0B8A7A58",
»·······»·······»·······"Keccak256":
"5015A4935F0B51E091C6550A94DCD262C08998232CCAA22E7F0756DEAC0DC0D0",
»·······»·······»·······"Keccak384":
"7907A8D0FAA7BC6A90FE14C6C958C956A0877E751455D8F13ACDB96F144B5896E716C06EC0CB56557A94EF5C3355F6F3",
»·······»·······»·······"Keccak512":
"3EC327D6759F769DEB74E80CA70C831BC29CAB048A4BF4190E4A1DD5C6507CF2B4B58937FDE81D36014E7DFE1B1DD8B0F27CB7614F9A645FEC114F1DAAEFC056",
»·······»·······},
»·······},
}



Cheers

AGL

[jcb, 2013-03-22 18:22:19]

Took 30 min, but the Python code confirmed these.


On Fri, Mar 22, 2013 at 12:52 PM, Adam Langley <agl@golang.org> wrote:

> On Fri, Mar 22, 2013 at 12:47 PM, Joseph Bonneau <jcb@google.com> wrote:
> > Hmmm, alternative is that there are upstream test vectors of up to 4KB,
> but
> > they are random. I'd say shipping those around is not worth the trouble
> > either.
> >
> > I can cut the long vectors down to size and run them through the Python
> > version to cross-check.
>
> If you can confirm this with the Python code then I'll land it:
>
> var longKeccakTestVectors = []testVector{
> »·······{
> »·······»·······desc:   "long-1GiB",
> »·······»·······input:
> []byte("abcdefghbcdefghicdefghijdefghijkefghijklfghijklmghijklmnhijklmno"),
> »·······»·······repeat: 1024 * 1024,
> »·······»·······want: map[string]string{
> »·······»·······»·······"Keccak224":
> "50E35E40980FEEFF1EA490957B0E970257F75EA0D410EE0F0B8A7A58",
> »·······»·······»·······"Keccak256":
> "5015A4935F0B51E091C6550A94DCD262C08998232CCAA22E7F0756DEAC0DC0D0",
> »·······»·······»·······"Keccak384":
>
>
"7907A8D0FAA7BC6A90FE14C6C958C956A0877E751455D8F13ACDB96F144B5896E716C06EC0CB56557A94EF5C3355F6F3",
> »·······»·······»·······"Keccak512":
>
>
"3EC327D6759F769DEB74E80CA70C831BC29CAB048A4BF4190E4A1DD5C6507CF2B4B58937FDE81D36014E7DFE1B1DD8B0F27CB7614F9A645FEC114F1DAAEFC056",
> »·······»·······},
> »·······},
> }
>
>
>
> Cheers
>
> AGL
>

[jcb, 2013-03-22 18:22:55]

Uploaded a final CL with these test vectors


On Fri, Mar 22, 2013 at 2:22 PM, Joseph Bonneau <jcb@google.com> wrote:

> Took 30 min, but the Python code confirmed these.
>
>
> On Fri, Mar 22, 2013 at 12:52 PM, Adam Langley <agl@golang.org> wrote:
>
>> On Fri, Mar 22, 2013 at 12:47 PM, Joseph Bonneau <jcb@google.com> wrote:
>> > Hmmm, alternative is that there are upstream test vectors of up to 4KB,
>> but
>> > they are random. I'd say shipping those around is not worth the trouble
>> > either.
>> >
>> > I can cut the long vectors down to size and run them through the Python
>> > version to cross-check.
>>
>> If you can confirm this with the Python code then I'll land it:
>>
>> var longKeccakTestVectors = []testVector{
>> »·······{
>> »·······»·······desc:   "long-1GiB",
>> »·······»·······input:
>>
>> []byte("abcdefghbcdefghicdefghijdefghijkefghijklfghijklmghijklmnhijklmno"),
>> »·······»·······repeat: 1024 * 1024,
>> »·······»·······want: map[string]string{
>> »·······»·······»·······"Keccak224":
>> "50E35E40980FEEFF1EA490957B0E970257F75EA0D410EE0F0B8A7A58",
>> »·······»·······»·······"Keccak256":
>> "5015A4935F0B51E091C6550A94DCD262C08998232CCAA22E7F0756DEAC0DC0D0",
>> »·······»·······»·······"Keccak384":
>>
>>
"7907A8D0FAA7BC6A90FE14C6C958C956A0877E751455D8F13ACDB96F144B5896E716C06EC0CB56557A94EF5C3355F6F3",
>> »·······»·······»·······"Keccak512":
>>
>>
"3EC327D6759F769DEB74E80CA70C831BC29CAB048A4BF4190E4A1DD5C6507CF2B4B58937FDE81D36014E7DFE1B1DD8B0F27CB7614F9A645FEC114F1DAAEFC056",
>> »·······»·······},
>> »·······},
>> }
>>
>>
>>
>> Cheers
>>
>> AGL
>>
>
>

[agl1, 2013-03-22 19:00:11]

*** Submitted as
https://code.google.com/p/go/source/detail?r=87c30004e36a&repo=crypto ***

go.crypto/sha3: new package
Added a pure Go implementation of SHA3 (Keccak) which implements the hash.Hash
interface.
A test file is included with performance benchmarks and standard test vectors.

R=agl, nigeltao
CC=golang-dev
https://codereview.appspot.com/7760044

Committer: Adam Langley <agl@golang.org>