HMAC-SHA256/512 non 32 byte keys supporting #166

gdwrd · 2017-12-14T15:33:19Z

Hi,
Here is my PR with non 32 keys supporting.
HMAC RFC 2104: https://tools.ietf.org/html/rfc2104#section-3
Thanks!

gdwrd · 2017-12-14T15:34:25Z

gdwrd · 2017-12-19T10:45:44Z

Hi @tarcieri, what do you think about my PR?

tarcieri · 2017-12-19T16:53:35Z

I think if you're doing this much, you might as well expose the full IUF API... and ideally make it API-compatible with OpenSSL::HMAC

gdwrd · 2017-12-21T14:43:21Z

@tarcieri You mean, change hmac512256 libsodium calls to Init/Update/Final API? And what about OpenSSL::HMAC?

tarcieri · 2017-12-21T15:01:21Z

@nsheremet you've done most of the work here to expose the IUF API (i.e. initialize, update, finalize or as the OpenSSL::HMAC, or perhaps more generally the Ruby Digest API calls them #initialize, #update, and #digest), but it's not a particularly friendly or idiomatic API. I think if you're adding access to this API it probably should be, and would ideally implement something more akin to a Ruby Digest or OpenSSL::HMAC

gdwrd · 2017-12-21T15:17:31Z

@tarcieri hm. I will add IUF Api soon.

… hmacsha512256

gdwrd · 2018-01-11T16:05:50Z

@tarcieri Hi, I pushed some updates. Can you review, please?

tarcieri · 2018-01-12T07:00:18Z

I can review it soon

gdwrd · 2018-01-23T09:08:53Z

Hi @tarcieri, did you see the PR?

tarcieri · 2018-01-24T06:13:15Z

@nsheremet yes, sorry, I haven't had time to review it

tarcieri · 2018-02-12T00:14:43Z

lib/rbnacl/hmac/sha256.rb

+      # @params [#to_str] message message to construct an authenticator for
+      def update(message)
+        self.class.auth_hmacsha256_update(@state, message, message.bytesize)
+        self.class.auth_hmacsha256_final(@state.clone, @authenticator)


This should probably be moved to #digest below so as to allow #update to be called multiple times. I'm also curious how this handles #update being called after it's been finalized. That should probably raise an exception.

Hi @tarcieri. You can use this update multiple times.
But the point is, #update method without calling this line self.class.auth_hmacsha256_final(@state.clone, @authenticator) every time will be rewriting the @state. I can add spec test to compare with openssl multiple usage. But I know that it's work because I test it.

here is RbNaCl example:

pry(main)> key = ['07c0'].pack('H*') pry(main)> hmac = RbNaCl::HMAC::SHA256.new(key) pry(main)> hmac.update 'abcd' pry(main)> hmac.update '1' pry(main)> hmac.digest => "G\xCC[\xBDI\x1A\xA3\x95\xCD\xA2@\xC2\xB2L\xCD\xE9\x03kV\xDA\x7F\xAB@\xBE\xB5\xA7V\x1C\x11G\xBE\xD8"

OpenSSL example:

pry(main)> key = ['07c0'].pack('H*') pry(main)> hmac = OpenSSL::HMAC.new(key, d) pry(main)> hmac.update 'abcd' pry(main)> hmac.update '1' pry(main)> hmac.digest => "G\xCC[\xBDI\x1A\xA3\x95\xCD\xA2@\xC2\xB2L\xCD\xE9\x03kV\xDA\x7F\xAB@\xBE\xB5\xA7V\x1C\x11G\xBE\xD8"

tarcieri · 2018-02-12T00:15:53Z

lib/rbnacl/hmac/sha512.rb

+      # @params [#to_str] message message to construct an authenticator for
+      def update(message)
+        self.class.auth_hmacsha512_update(@state, message, message.bytesize)
+        self.class.auth_hmacsha512_final(@state.clone, @authenticator)


Ditto here re: https://github.com/crypto-rb/rbnacl/pull/166/files#r167456561

here is the answer #166 (comment)

tarcieri · 2018-02-12T00:16:14Z

lib/rbnacl/hmac/sha512256.rb

+      # @params [#to_str] message message to construct an authenticator for
+      def update(message)
+        self.class.auth_hmacsha512256_update(@state, message, message.bytesize)
+        self.class.auth_hmacsha512256_final(@state.clone, @authenticator)


And ditto here too.

here is the answer #166 (comment)

tarcieri · 2018-02-12T00:17:55Z

This looks mostly good but I think all three primitives you're implementing here should both handle multiple invocations of #update and raise if updated after they've been finalized.

gdwrd · 2018-02-12T16:11:26Z

@tarcieri I also can make that after you call #update method, it will return hexdigest. The same functionality as in openssl #update method.

tarcieri · 2018-02-12T16:36:28Z

@nsheremet sorry I misunderstood how this works. I think the change you suggest (to match the OpenSSL::HMAC behavior) sounds good

gdwrd · 2018-02-12T18:51:47Z

@tarcieri I updated #update method. Done.

gdwrd · 2018-02-13T15:53:18Z

@tarcieri so, will you merge this PR?

tarcieri · 2018-02-14T00:28:54Z

lib/rbnacl/hmac/sha256.rb

      end
    end
+
+    # The crupto_auth_hmacsha256_state struct representation


tarcieri · 2018-02-14T00:30:24Z

spec/shared/hmac.rb

+# encoding: binary
+# frozen_string_literal: true
+
+RSpec.shared_examples "hmac" do


tiny nit but I'd personally prefer "HMAC" here

tarcieri · 2018-02-14T00:30:57Z

One typo and one nit, otherwise it looks good, thanks for your contribution!

gdwrd · 2018-02-14T10:27:34Z

@tarcieri I fixed typos.

gdwrd · 2018-02-15T10:21:19Z

@tarcieri I think that's all 😄

tarcieri · 2018-02-15T17:02:20Z

Thanks!

gdwrd · 2018-02-15T17:10:43Z

@tarcieri I am glad to help!

@tarcieri

## [6.0.0] (2018-11-08) [6.0.0]: RubyCrypto/rbnacl#182 * [#180](RubyCrypto/rbnacl#180) Deprecate rbnacl-libsodium. ([@tarcieri]) * [#176](RubyCrypto/rbnacl#176) Add support for XChaCha20-Poly1305. ([@AnIrishDuck]) * [#174](RubyCrypto/rbnacl#174) Fix buffer size type in `randombytes_buf` binding. ([@elijh]) * [#172](RubyCrypto/rbnacl#172) Add support for argon2id digest. ([@trofi]) * [#166](RubyCrypto/rbnacl#166) Support for non-32-byte HMAC-SHA256/512 keys. ([@nsheremet])

gdwrd force-pushed the hmac_keysize_fix branch from d7afd19 to 0742fd9 Compare December 14, 2017 15:36

non 32 byte keys supporting

c8ccf4f

gdwrd force-pushed the hmac_keysize_fix branch from 0742fd9 to c8ccf4f Compare December 14, 2017 15:51

gdwrd added 2 commits December 14, 2017 17:53

add ExcludedMethods to Metrics/BlockLength in rubocop config

0414fb3

Merge branch 'master' into hmac_keysize_fix

5e03238

gdwrd added 3 commits January 9, 2018 15:34

Merge branch 'master' into hmac_keysize_fix

9f961e8

added #update, #digest, #hexdigest methods to hmac instance, refactor…

74a5f97

… hmacsha512256

fixed rubocop offenses

d0c5edc

tarcieri reviewed Feb 12, 2018

View reviewed changes

gdwrd force-pushed the hmac_keysize_fix branch from 18f5952 to 0b3a757 Compare February 12, 2018 18:31

hmac #update method returns a produced hexdigest

def63ed

gdwrd force-pushed the hmac_keysize_fix branch from 0b3a757 to def63ed Compare February 12, 2018 18:38

tarcieri reviewed Feb 14, 2018

View reviewed changes

lib/rbnacl/hmac/sha256.rb Outdated

end

end

# The crupto_auth_hmacsha256_state struct representation

Copy link

Contributor

tarcieri Feb 14, 2018

Choose a reason for hiding this comment

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

Typo

tarcieri reviewed Feb 14, 2018

View reviewed changes

fixed typos

c23f841

tarcieri merged commit 4cba02e into RubyCrypto:master Feb 15, 2018

gdwrd deleted the hmac_keysize_fix branch February 15, 2018 17:10

tarcieri mentioned this pull request Nov 8, 2018

v0.6.0 #182

Merged

anakinj mentioned this pull request Oct 9, 2022

Separated rbnacl hmac implementations jwt/ruby-jwt#521

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HMAC-SHA256/512 non 32 byte keys supporting #166

HMAC-SHA256/512 non 32 byte keys supporting #166

gdwrd commented Dec 14, 2017 •

edited

gdwrd commented Dec 14, 2017

gdwrd commented Dec 19, 2017

tarcieri commented Dec 19, 2017 •

edited

gdwrd commented Dec 21, 2017

tarcieri commented Dec 21, 2017

gdwrd commented Dec 21, 2017

gdwrd commented Jan 11, 2018

tarcieri commented Jan 12, 2018

gdwrd commented Jan 23, 2018

tarcieri commented Jan 24, 2018

tarcieri Feb 12, 2018

gdwrd Feb 12, 2018

gdwrd Feb 12, 2018

tarcieri Feb 12, 2018

gdwrd Feb 12, 2018

tarcieri Feb 12, 2018

gdwrd Feb 12, 2018

tarcieri commented Feb 12, 2018

gdwrd commented Feb 12, 2018

tarcieri commented Feb 12, 2018

gdwrd commented Feb 12, 2018

gdwrd commented Feb 13, 2018

tarcieri Feb 14, 2018

tarcieri Feb 14, 2018

tarcieri commented Feb 14, 2018

gdwrd commented Feb 14, 2018

gdwrd commented Feb 15, 2018

tarcieri commented Feb 15, 2018

gdwrd commented Feb 15, 2018

HMAC-SHA256/512 non 32 byte keys supporting #166

HMAC-SHA256/512 non 32 byte keys supporting #166

Conversation

gdwrd commented Dec 14, 2017 • edited

gdwrd commented Dec 14, 2017

gdwrd commented Dec 19, 2017

tarcieri commented Dec 19, 2017 • edited

gdwrd commented Dec 21, 2017

tarcieri commented Dec 21, 2017

gdwrd commented Dec 21, 2017

gdwrd commented Jan 11, 2018

tarcieri commented Jan 12, 2018

gdwrd commented Jan 23, 2018

tarcieri commented Jan 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tarcieri commented Feb 12, 2018

gdwrd commented Feb 12, 2018

tarcieri commented Feb 12, 2018

gdwrd commented Feb 12, 2018

gdwrd commented Feb 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tarcieri commented Feb 14, 2018

gdwrd commented Feb 14, 2018

gdwrd commented Feb 15, 2018

tarcieri commented Feb 15, 2018

gdwrd commented Feb 15, 2018

gdwrd commented Dec 14, 2017 •

edited

tarcieri commented Dec 19, 2017 •

edited