Brian Gernhardt <br...@gernhardtsoftware.com> writes:
> clang gives the following warning:
>
> builtin/receive-pack.c:327:35: error: sizeof on array function
> parameter will return size of 'unsigned char *' instead of 'unsigned
> char [20]' [-Werror,-Wsizeof-array-argument]
> git_SHA1_Update(&ctx, out, sizeof(out));
> ^
> builtin/receive-pack.c:292:37: note: declared here
> static void hmac_sha1(unsigned char out[20],
> ^
> ---
>
> I dislike changing sizeof to a magic constant, but clang informs me that
> sizeof is doing the wrong thing. Perhaps there's an appropriate constant
> #defined in the code somewhere?
By the way, the title is very misleading, as truncating the HMAC
when creating nonce is done deliberately and it sounds as if the
patch is breaking that part of the system.
We could pass "how many bytes of output do we want" as another
parameter to hmac_sha1() or define that as a constant, and copy out
only that many from here.
And then use the same constant when deciding to truncate the result
of sha1_to_hex() in the caller.
I am not happy with this version, either, though, because now we
have an uninitialized piece of memory at the end of sha1[20] of the
caller, which is given to sha1_to_hex() to produce garbage. It is
discarded by %.*s format so there is no negative net effect, but I
suspect that the compiler would not see that through.
builtin/receive-pack.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index efb13b1..93fc39d 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -287,8 +287,9 @@ static int copy_to_sideband(int in, int out, void *arg)
}
#define HMAC_BLOCK_SIZE 64
+#define HMAC_TRUNCATE 10 /* bytes */
-static void hmac_sha1(unsigned char out[20],
+static void hmac_sha1(unsigned char *out,
const char *key_in, size_t key_len,
const char *text, size_t text_len)
{
@@ -323,7 +324,7 @@ static void hmac_sha1(unsigned char out[20],
/* RFC 2104 2. (6) & (7) */
git_SHA1_Init(&ctx);
git_SHA1_Update(&ctx, k_opad, sizeof(k_opad));
- git_SHA1_Update(&ctx, out, sizeof(out));
+ git_SHA1_Update(&ctx, out, HMAC_TRUNCATE);
git_SHA1_Final(out, &ctx);
}
@@ -337,7 +338,8 @@ static char *prepare_push_cert_nonce(const char *path,
unsigned long stamp)
strbuf_release(&buf);
/* RFC 2104 5. HMAC-SHA1-80 */
- strbuf_addf(&buf, "%lu-%.*s", stamp, 20, sha1_to_hex(sha1));
+ strbuf_addf(&buf, "%lu-%.*s", stamp,
+ 2 * HMAC_TRUNCATE, sha1_to_hex(sha1));
return strbuf_detach(&buf, NULL);
}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
