From: Junio C Hamano <gitster@pobox•com>
To: Brian Gernhardt <brian@gernhardtsoftware•com>
Cc: Git List <git@vger•kernel.org>
Subject: Re: [PATCH] Receive-pack: include entire SHA1 in nonce
Date: Thu, 25 Sep 2014 09:35:51 -0700 [thread overview]
Message-ID: <xmqqk34rbqu0.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1411657340-62950-1-git-send-email-brian@gernhardtsoftware.com> (Brian Gernhardt's message of "Thu, 25 Sep 2014 11:02:20 -0400")
Brian Gernhardt <brian@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);
}
next prev parent reply other threads:[~2014-09-25 16:35 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-25 15:02 [PATCH] Receive-pack: include entire SHA1 in nonce Brian Gernhardt
2014-09-25 16:23 ` Junio C Hamano
2014-09-25 16:35 ` Junio C Hamano [this message]
2014-09-25 17:54 ` Junio C Hamano
2014-09-25 18:03 ` Brian Gernhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqk34rbqu0.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=brian@gernhardtsoftware$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox