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 10:54:11 -0700 [thread overview]
Message-ID: <xmqqa95nbn7g.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <xmqqk34rbqu0.fsf@gitster.dls.corp.google.com> (Junio C. Hamano's message of "Thu, 25 Sep 2014 09:35:51 -0700")
Junio C Hamano <gitster@pobox•com> writes:
> 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.
... and if we want to fix that, we would end up with a set of
changes, somewhat ugly like this.
Which might be an improvement, but let's start with your "sizeof(arg)
is the size of a pointer, even when the definition of arg[] is
spelled with bra-ket, a dummy maintainer!" fix.
I'd like to have your sign-off. I'd also prefer to retitle it as
something like "hmac_sha1: copy the entire SHA-1 hash out", as it is
deliberate that we do not include the entire SHA-1 in nonce.
Thanks.
---
builtin/receive-pack.c | 13 ++++++++-----
cache.h | 1 +
hex.c | 24 ++++++++++++++----------
3 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index efb13b1..e0e7c75 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 /* in 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,21 +324,23 @@ 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);
}
static char *prepare_push_cert_nonce(const char *path, unsigned long stamp)
{
struct strbuf buf = STRBUF_INIT;
- unsigned char sha1[20];
+ unsigned char hmac[HMAC_TRUNCATE];
+ char hmac_trunc[HMAC_TRUNCATE * 2 + 1];
strbuf_addf(&buf, "%s:%lu", path, stamp);
- hmac_sha1(sha1, buf.buf, buf.len, cert_nonce_seed, strlen(cert_nonce_seed));;
+ hmac_sha1(hmac, buf.buf, buf.len, cert_nonce_seed, strlen(cert_nonce_seed));;
strbuf_release(&buf);
/* RFC 2104 5. HMAC-SHA1-80 */
- strbuf_addf(&buf, "%lu-%.*s", stamp, 20, sha1_to_hex(sha1));
+ bin_to_hex(hmac, HMAC_TRUNCATE, hmac_trunc);
+ strbuf_addf(&buf, "%lu-%s", stamp, hmac_trunc);
return strbuf_detach(&buf, NULL);
}
diff --git a/cache.h b/cache.h
index fcb511d..bf508a2 100644
--- a/cache.h
+++ b/cache.h
@@ -965,6 +965,7 @@ extern int for_each_abbrev(const char *prefix, each_abbrev_fn, void *);
*/
extern int get_sha1_hex(const char *hex, unsigned char *sha1);
+extern void bin_to_hex(const unsigned char *bin, int, char *hexout);
extern char *sha1_to_hex(const unsigned char *sha1); /* static buffer result! */
extern int read_ref_full(const char *refname, unsigned char *sha1,
int reading, int *flags);
diff --git a/hex.c b/hex.c
index 9ebc050..1b30e6e 100644
--- a/hex.c
+++ b/hex.c
@@ -56,20 +56,24 @@ int get_sha1_hex(const char *hex, unsigned char *sha1)
return 0;
}
-char *sha1_to_hex(const unsigned char *sha1)
+void bin_to_hex(const unsigned char *bin, int len, char *hexout)
{
- static int bufno;
- static char hexbuffer[4][50];
static const char hex[] = "0123456789abcdef";
- char *buffer = hexbuffer[3 & ++bufno], *buf = buffer;
- int i;
- for (i = 0; i < 20; i++) {
- unsigned int val = *sha1++;
- *buf++ = hex[val >> 4];
- *buf++ = hex[val & 0xf];
+ while (0 < len--) {
+ unsigned int val = *bin++;
+ *hexout++ = hex[val >> 4];
+ *hexout++ = hex[val & 0xf];
}
- *buf = '\0';
+ *hexout = '\0';
+}
+
+char *sha1_to_hex(const unsigned char *sha1)
+{
+ static int bufno;
+ static char hexbuffer[4][50];
+ char *buffer = hexbuffer[3 & ++bufno];
+ bin_to_hex(sha1, 20, buffer);
return buffer;
}
next prev parent reply other threads:[~2014-09-25 17:54 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
2014-09-25 17:54 ` Junio C Hamano [this message]
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=xmqqa95nbn7g.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