Re: [PATCH 25/41] builtin/receive-pack: avoid hard-coded constants for push certs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 24 April 2018 at 01:39, brian m. carlson
<sandals@xxxxxxxxxxxxxxxxxxxx> 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 <sandals@xxxxxxxxxxxxxxxxxxxx>
> ---
>  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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux