GNOME Bugzilla – Bug 724741
hmac: Fix support for SHA-512 in GHmac
Last modified: 2014-04-16 15:22:12 UTC
Patches attached to fix GHmac so it doesn’t explode if you call g_hmac_new(G_CHECKSUM_SHA512).
Created attachment 269713 [details] [review] hmac: Fix support for SHA-512 in GHmac The block size wasn’t configured before, so calling g_hmac_new() with G_CHECKSUM_SHA512 would hit a g_assert_not_reached() and explode. Implement G_CHECKSUM_SHA512 and add unit tests for HMACs with SHA-256 and SHA-512 using the test vectors from RFC 4868.
Created attachment 269714 [details] [review] hmac: Make unit test const-correct
Comment on attachment 269713 [details] [review] hmac: Fix support for SHA-512 in GHmac >Subject: [PATCH] hmac: Fix support for SHA-512 in GHmac This isn't really "fixing" support for SHA-512, it's *adding* support for it. >Implement G_CHECKSUM_SHA512 and add unit tests for HMACs with SHA-256 >and SHA-512 using the test vectors from RFC 4868. Yay! Tests! >-/* HMAC-SHA1 test vectors as per RFC 2202 */ >+/* HMAC-SHA1 and HMAC-SHA256 test vectors as per RFCs 2202 and 4868. and HMAC-SHA512 > guint8 result_sha1_test1[] = { > 0xb6, 0x17, 0x31, 0x86, 0x55, 0x05, 0x72, 0x64, 0xe2, 0x8b, > 0xc0, 0xb6, 0xfb, 0x37, 0x8c, 0x8e, 0xf1, 0x46, 0xbe, 0x00 }; >+guint8 result_sha256_test1[] = { >+ 0xb0, 0x34, 0x4c, 0x61, 0xd8, 0xdb, 0x38, 0x53, 0x5c, 0xa8, >+ 0xaf, 0xce, 0xaf, 0x0b, 0xf1, 0x2b, 0x88, 0x1d, 0xc2, 0x00, >+ 0xc9, 0x83, 0x3d, 0xa7, 0x26, 0xe9, 0x37, 0x6c, 0x2e, 0x32, >+ 0xcf, 0xf7, >+}; lose the trailing comma (which isn't valid C90), and put the "}" at the end of the preceding line (to match the sha1_test result vectors). (likewise elsewhere) >-/* Test 5 */ >+/* Test 5 (note: doesnât exist for SHA-256 and SHA-512) */ I think we're still mostly avoiding UTF-8 in source files, so fix that to be a dumb apostrophe (likewise elsewhere). Although "doesn't exist" doesn't sound quite right anyway. It's just that the SHA-1 and -256/-512 tests are different. >-/* Test 6 & 7*/ >-guint8 key_sha1_test6_7[] = { >+/* Test 6 & 7 (note: different for SHA-1 and SHA-256/SHA-512) */ >+guint8 key_sha_test6_7[] = { so since this is SHA-1 specific, shouldn't the variable keep the "1" in its name? >+ { G_CHECKSUM_SHA256, key_sha256_test5_6, 131, >+ "This is a test using a larger than block-size key" \ >+ " and a larger than block-size data. " \ >+ "The key needs to be hashed before being " \ >+ "used by the HMAC algorithm.", 152, result_sha256_test6, }, The 2nd-4th lines of the string should be indented to the same column as the first line. Also, the backslashes are not syntactically necessary. Also, the space at the start of the second line of the string should be moved to the end of the first line. (likewise in the -512 case)
Comment on attachment 269714 [details] [review] hmac: Make unit test const-correct Attachment 269714 [details] pushed as fdf14e9 - hmac: Make unit test const-correct
Created attachment 270157 [details] [review] hmac: Add support for SHA-512 in GHmac The block size wasn’t configured before, so calling g_hmac_new() with G_CHECKSUM_SHA512 would hit a g_assert_not_reached() and explode. Implement G_CHECKSUM_SHA512 and add unit tests for HMACs with SHA-256 and SHA-512 using the test vectors from RFC 4868.
Ping?
Comment on attachment 270157 [details] [review] hmac: Add support for SHA-512 in GHmac sorry, looks good. You might want to add a note to the toplevel docs and/or g_hmac_new() mentioning that G_CHECKSUM_SHA512 support is first available in 2.42.
Comment on attachment 270157 [details] [review] hmac: Add support for SHA-512 in GHmac Committed with changes to the GHmac and g_hmac_new() documentation as suggested. Thanks! commit 7a86a6690a767b48379c9233049a6a9d2a21a86e Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Wed Feb 19 18:02:37 2014 +0000 hmac: Add support for SHA-512 in GHmac The block size wasn’t configured before, so calling g_hmac_new() with G_CHECKSUM_SHA512 would hit a g_assert_not_reached() and explode. Implement G_CHECKSUM_SHA512 and add unit tests for HMACs with SHA-256 and SHA-512 using the test vectors from RFC 4868. https://bugzilla.gnome.org/show_bug.cgi?id=724741 glib/ghmac.c | 10 ++++- glib/tests/hmac.c | 177 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 173 insertions(+), 14 deletions(-)