Re: [PATCH] Receive-pack: include entire SHA1 in nonce
Junio C Hamano <gitster@pobox.com> writes:
Show 5 quoted lines
> 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(-)
Show changes to 3 files +23 −15
builtin/receive-pack.c, cache.h, hex.c
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;
}
Brian Gernhardt