On 24 April 2018 at 01:39, brian m. carlson
<[email protected]> wrote:
> Use the GIT_SHA1_RAWSZ and GIT_SHA1_HEXSZ macros instead of hard-coding
> the constants 20 and 40. Switch one use of 20 with a format specifier
> for a hex value to use the hex constant instead, as the original appears
> to have been a typo.
>
> At this point, avoid converting the hard-coded use of SHA-1 to use
> the_hash_algo. SHA-1, even if not collision resistant, is secure in the
> context in which it is used here, and the hash algorithm of the repo
> need not match what is used here. When we adopt a new hash algorithm,
> we can simply adopt the new algorithm wholesale here, as the nonce is
> opaque and its length and validity are entirely controlled by the
> server. Consequently, defer updating this code until that point.
>
> Signed-off-by: brian m. carlson <[email protected]>
> ---
> builtin/receive-pack.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index c4272fbc96..5f35596c14 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -454,21 +454,21 @@ static void hmac_sha1(unsigned char *out,
> /* RFC 2104 2. (6) & (7) */
> git_SHA1_Init(&ctx);
> git_SHA1_Update(&ctx, k_opad, sizeof(k_opad));
> - git_SHA1_Update(&ctx, out, 20);
> + git_SHA1_Update(&ctx, out, GIT_SHA1_RAWSZ);
> git_SHA1_Final(out, &ctx);
> }
Since we do HMAC with SHA-1, we use the functions `git_SHA1_foo()`. Ok.
But then why not just use "20"? Isn't GIT_SHA1_RAWSZ coupled to the
whole hash transition thing? This use of "20" is not, IMHO, the "length
in bytes [...] of an object name" (quoting cache.h).
> static char *prepare_push_cert_nonce(const char *path, timestamp_t stamp)
> {
> struct strbuf buf = STRBUF_INIT;
> - unsigned char sha1[20];
> + unsigned char sha1[GIT_SHA1_RAWSZ];
>
> strbuf_addf(&buf, "%s:%"PRItime, path, stamp);
> hmac_sha1(sha1, buf.buf, buf.len, cert_nonce_seed,
> strlen(cert_nonce_seed));;
> strbuf_release(&buf);
>
> /* RFC 2104 5. HMAC-SHA1-80 */
> - strbuf_addf(&buf, "%"PRItime"-%.*s", stamp, 20, sha1_to_hex(sha1));
> + strbuf_addf(&buf, "%"PRItime"-%.*s", stamp, GIT_SHA1_HEXSZ,
> sha1_to_hex(sha1));
> return strbuf_detach(&buf, NULL);
> }
Same comment here.
Martin
