FAQ
At the high level, we've not exposed a KDF API before and it's unclear
whether a Reader interface is the right one. The alternative would be to
pass in a length, or a []byte to be filled, and have it be a one-shot
operation.

However, since one typically takes a number of outputs (a couple of keys
and a couple of IVs) from a KDF, the Reader interface may save people
having to manually split up a single output so I think I like it.

However, the implementation itself allocates far more than is needed. I
can fix this up before landing if you wish but I've pointed out a few
cases in case you wish to iterate yourself.

(p.s. have you signed the CLA?)


https://codereview.appspot.com/7474049/diff/17001/hkdf/hkdf.go
File hkdf/hkdf.go (right):

https://codereview.appspot.com/7474049/diff/17001/hkdf/hkdf.go#newcode44
hkdf/hkdf.go:44: input := append(f.prev, append(f.info, f.counter)...)
this is better written as two, non-nested appends. The nested append is
actually copying f.info into a new buffer just to append a single byte
and returning it. input can also be reused between iterations.

https://codereview.appspot.com/7474049/diff/17001/hkdf/hkdf.go#newcode46
hkdf/hkdf.go:46: expander := hmac.New(f.hash, f.prk)
the HMAC from New can be passed in and Reset() rather than creating
afresh each time.

https://codereview.appspot.com/7474049/diff/17001/hkdf/hkdf.go#newcode48
hkdf/hkdf.go:48: output := expander.Sum(nil)
in the case where the full hash result fits in p, it could be written
directly.

https://codereview.appspot.com/7474049/diff/17001/hkdf/hkdf.go#newcode63
hkdf/hkdf.go:63: func New(hash func() hash.Hash, master []byte, salt
[]byte, info []byte) io.Reader {
argument names should either be commonly used, or match the RFC. Thus I
would call "master" either "secret" or "ikm".

https://codereview.appspot.com/7474049/diff/17001/hkdf/hkdf.go#newcode64
hkdf/hkdf.go:64: extractor := hmac.New(hash, salt)
If a salt is not provided, hash.Size() zero bytes should be used.

https://codereview.appspot.com/7474049/diff/17001/hkdf/hkdf.go#newcode67
hkdf/hkdf.go:67: return &hkdf{hash, hash().Size(), extractor.Sum(nil),
info, 1, []byte{}, []byte{}}
s/[]byte{}/nil/

https://codereview.appspot.com/7474049/

--

---
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Search Discussions

  • Peterke at Mar 12, 2013 at 6:40 pm
    I think I'll manage the fixes, I'll commit in a few hours, just let me
    finish up something first.

    Btw, there's one slight design change I've added since the review
    request: instead of passing in "hash-maker" functions, I've changed to
    the crypto.Hash enums:

    hkdf.New(sha1.New, ...) -> hkdf.New(crypto.SHA1, ...)

    Which design do you prefer? The original or the crypto-enum one? (the
    latter is available at
    https://github.com/karalabe/iris/blob/master/crypto/hkdf/hkdf.go ).

    PS: Yes I've signed the CLA (at least the digital version)

    https://codereview.appspot.com/7474049/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Adam Langley at Mar 12, 2013 at 3:45 pm

    On Tue, Mar 12, 2013 at 11:38 AM, wrote:
    Btw, there's one slight design change I've added since the review
    request: instead of passing in "hash-maker" functions, I've changed to
    the crypto.Hash enums:
    Passing in the function is more canonical.


    Cheers

    AGL

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Peterke at Mar 12, 2013 at 6:40 pm
    Issues should be resolved now, please check it again.

    https://codereview.appspot.com/7474049/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedMar 12, '13 at 3:12p
activeMar 12, '13 at 6:40p
posts4
users3
websitegolang.org

3 users in discussion

Peterke: 2 posts Agl: 1 post Adam Langley: 1 post

People

Translate

site design / logo © 2022 Grokbase