[REGRESSION] Can't clone GitHub repos (fetch-pack error) due to avoiding deprecated OpenSSL SHA-1 routines

[REGRESSION] Can't clone GitHub repos (fetch-pack error) due to avoiding deprecated OpenSSL SHA-1 routines

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 2610 bytes --]

Hi,

I built Git v2.42.0 on Debian testing, linked with OpenSSL (v3.0.10 from
distribution) with Makefile knob `OPENSSL_SHA1=YesPlease 
OPENSSL_SHA256=YesPlease`. I tried to shallow clone git.git repository:

```
$ git clone https://github.com/git/git --depth=1 git-scm
```

All the necessary objects were fetched but the clone errored instead with:

```
fatal: fetch-pack: invalid index-pack output
```

This issue is a regression since v2.41.0 doesn't have it. Bisecting, the
culprit is commit bda9c12073e7 (avoid SHA-1 functions deprecated in OpenSSL 3+,
2023-08-01). AFAIK, the culprit doesn't touch `fetch-pack.c` as I hoped.

The full bisection log is:

```
git bisect start '--term-good=ok' '--term-bad=oops'
# status: waiting for both good and bad commits
# ok: [fe86abd7511a9a6862d5706c6fa1d9b57a63ba09] Git 2.41
git bisect ok fe86abd7511a9a6862d5706c6fa1d9b57a63ba09
# status: waiting for bad commit, 1 good commit known
# oops: [43c8a30d150ecede9709c1f2527c8fba92c65f40] Git 2.42
git bisect oops 43c8a30d150ecede9709c1f2527c8fba92c65f40
# ok: [1d76e69212102c3373b552186590b76d6ad8d84c] Merge branch 'jc/doc-hash-object-types'
git bisect ok 1d76e69212102c3373b552186590b76d6ad8d84c
# ok: [914a353a128d4d885e138f189e235ad6094d436e] Merge branch 'jc/am-parseopt-fix'
git bisect ok 914a353a128d4d885e138f189e235ad6094d436e
# ok: [e48d9c78cc00805660b83ac809188d0c413e4c46] Merge branch 'am/doc-sha256'
git bisect ok e48d9c78cc00805660b83ac809188d0c413e4c46
# oops: [cecd6a5ffce2c35f18e8ac537c9e2f71ac99932b] Merge branch 'jc/send-email-pre-process-fix'
git bisect oops cecd6a5ffce2c35f18e8ac537c9e2f71ac99932b
# oops: [8cdd5e713d7ba54b9d26ac997408bb745ab55088] Merge branch 'ma/locate-in-path-for-windows'
git bisect oops 8cdd5e713d7ba54b9d26ac997408bb745ab55088
# ok: [a82fb66fed250e16d3010c75404503bea3f0ab61] A few more topics before -rc1
git bisect ok a82fb66fed250e16d3010c75404503bea3f0ab61
# oops: [cf07e53bae8492fc6ee8a8d394e2fba858daa0a4] Merge branch 'bc/ident-dot-is-no-longer-crud-letter'
git bisect oops cf07e53bae8492fc6ee8a8d394e2fba858daa0a4
# oops: [bda9c12073e786e2ffa2c3ec479c7fe098d49999] avoid SHA-1 functions deprecated in OpenSSL 3+
git bisect oops bda9c12073e786e2ffa2c3ec479c7fe098d49999
# ok: [3e440ea0aba0660f356a3e5b9fc366d5d6960847] sha256: avoid functions deprecated in OpenSSL 3+
git bisect ok 3e440ea0aba0660f356a3e5b9fc366d5d6960847
# first oops commit: [bda9c12073e786e2ffa2c3ec479c7fe098d49999] avoid SHA-1 functions deprecated in OpenSSL 3+
```

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [REGRESSION] Can't clone GitHub repos (fetch-pack error) due to avoiding deprecated OpenSSL SHA-1 routines

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 2875 bytes --]

On 2023-08-31 at 12:47:19, Bagas Sanjaya wrote:
> Hi,
> 
> I built Git v2.42.0 on Debian testing, linked with OpenSSL (v3.0.10 from
> distribution) with Makefile knob `OPENSSL_SHA1=YesPlease 
> OPENSSL_SHA256=YesPlease`. I tried to shallow clone git.git repository:

I should point out that using OpenSSL's SHA-1 support is insecure
because it doesn't check for collisions.  As a practical matter, no
distro builds that way, and if you distributed that build, it would
probably qualify for a CVE.

However, OPENSSL_SHA256 being set is fine for a local build or a build
where you're not distributing OpenSSL itself.

> ```
> $ git clone https://github.com/git/git --depth=1 git-scm
> ```
> 
> All the necessary objects were fetched but the clone errored instead with:
> 
> ```
> fatal: fetch-pack: invalid index-pack output
> ```
> 
> This issue is a regression since v2.41.0 doesn't have it. Bisecting, the
> culprit is commit bda9c12073e7 (avoid SHA-1 functions deprecated in OpenSSL 3+,
> 2023-08-01). AFAIK, the culprit doesn't touch `fetch-pack.c` as I hoped.

I also see this with that configuration on Debian sid, and it appears to
affect SHA-256 as well.  The testsuite fails in a variety of spectacular
ways.  For example, t0410 is broken in exactly this way.

A simple git index-pack on Git's codebase shows a segfault in
`EVP_DigestUpdate` after calling `flush`.  It's not clear to me why this
would occur, but I did note that the context is a static variable.  I
wonder if there's something about this configuration that results in
breakage if a context is reused, although looking at the code, nothing
jumps out to me.

I did, however, apply this patch, which I think makes the problem really
clear:

----
diff --git a/sha1/openssl.h b/sha1/openssl.h
index 006c1f4ba5..0390ba9da6 100644
--- a/sha1/openssl.h
+++ b/sha1/openssl.h
@@ -32,6 +32,7 @@ static inline void openssl_SHA1_Final(unsigned char *digest,
 {
 	EVP_DigestFinal_ex(ctx->ectx, digest, NULL);
 	EVP_MD_CTX_free(ctx->ectx);
+	ctx->ectx = NULL;
 }
 
 static inline void openssl_SHA1_Clone(struct openssl_SHA1_CTX *dst,
----

Now we see that when the segfault happens, `input_ctx.sha1.ectx` is
NULL.  I'm not sure why that is, or what needs to be fixed, but I think
it's clear that _someone_ isn't calling the `init_fn` method before
re-using the context, and they definitely should be.

Hopefully this gives someone a good push in the right direction on
solving the problem.

If someone wants to pick up the above patch to help make this problem
more obvious in the future (don't forget to do the same for SHA-256),
please do so with my blessing.  I wouldn't say you need my sign-off
since it's so trivial, but feel free to forge it if it makes you feel
better.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [REGRESSION] Can't clone GitHub repos (fetch-pack error) due to avoiding deprecated OpenSSL SHA-1 routines

[Eric Wong]

"brian m. carlson" <sandals@crustytoothpaste.net> wrote:
> Hopefully this gives someone a good push in the right direction on
> solving the problem.

Thanks.  Here's my WIP which fixes clones on SHA-1 and SHA-256
 <https://80x24.org/sha256test.git> and also t1050-large.sh, but
t1514-rev-parse-push.sh is still broken...

That said, I don't much understand some of the code I'm modifying
and just poking at it until tests pass and valgrind is happy :x

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 006ffdc9c5..dda94a9f46 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1166,6 +1166,7 @@ static void parse_pack_objects(unsigned char *hash)
 	struct ofs_delta_entry *ofs_delta = ofs_deltas;
 	struct object_id ref_delta_oid;
 	struct stat st;
+	git_hash_ctx tmp_ctx;
 
 	if (verbose)
 		progress = start_progress(
@@ -1202,7 +1203,9 @@ static void parse_pack_objects(unsigned char *hash)
 
 	/* Check pack integrity */
 	flush();
-	the_hash_algo->final_fn(hash, &input_ctx);
+	the_hash_algo->init_fn(&tmp_ctx);
+	the_hash_algo->clone_fn(&tmp_ctx, &input_ctx);
+	the_hash_algo->final_fn(hash, &tmp_ctx);
 	if (!hasheq(fill(the_hash_algo->rawsz), hash))
 		die(_("pack is corrupted (SHA1 mismatch)"));
 	use(the_hash_algo->rawsz);
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 73bff3a23d..92b9c8598b 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -268,6 +268,7 @@ static int deflate_to_pack(struct bulk_checkin_packfile *state,
 					  type, size);
 	the_hash_algo->init_fn(&ctx);
 	the_hash_algo->update_fn(&ctx, obuf, header_len);
+	the_hash_algo->init_fn(&checkpoint.ctx);
 
 	/* Note: idx is non-NULL when we are writing */
 	if ((flags & HASH_WRITE_OBJECT) != 0)
diff --git a/csum-file.c b/csum-file.c
index cd01713244..870748e016 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -207,7 +207,7 @@ int hashfile_truncate(struct hashfile *f, struct hashfile_checkpoint *checkpoint
 	    lseek(f->fd, offset, SEEK_SET) != offset)
 		return -1;
 	f->total = offset;
-	f->ctx = checkpoint->ctx;
+	the_hash_algo->clone_fn(&f->ctx, &checkpoint->ctx);
 	f->offset = 0; /* hashflush() was called in checkpoint */
 	return 0;
 }

[PATCH] treewide: fix various bugs w/ OpenSSL 3+ EVP API

[Eric Wong]

The OpenSSL 3+ EVP API for SHA-* cannot support our prior use cases
supported by other SHA-* implementations.  It has the following
differences:

1. ->init_fn is required before all use
2. struct assignments don't work and requires ->clone_fn
3. can't support ->update_fn after ->final_*fn

While fixing cases 1 and 2 is merely the matter of calling ->init_fn and
->clone_fn as appropriate, fixing case 3 requires calling ->final_*fn on
a temporary context that's cloned from the primary context.

Reported-by: Bagas Sanjaya <bagasdotme@gmail.com>
Link: https://lore.kernel.org/ZPCL11k38PXTkFga@debian.me/
Helped-by: brian m. carlson <sandals@crustytoothpaste.net>
Fixes: 3e440ea0aba0 ("sha256: avoid functions deprecated in OpenSSL 3+")
Fixes: bda9c12073e7 ("avoid SHA-1 functions deprecated in OpenSSL 3+")
Signed-off-by: Eric Wong <e@80x24.org>
---
 Ugh, I wonder if I setup my config.mak incorrectly when testing
 3e440ea0aba0 and bda9c12073e7 :x

 There may be other misuses not exposed by the test suite.  Making
 git_hash_ctx opaque could flush out some of them (but I dislike
 APIs which force heap allocations in the first place).  In any case,
 I really wish git relied less on globals so object lifetimes could be
 more obvious and really wish all C projects could rely on
 gcc/tinycc/clang-supported __attribute__((__cleanup__)) to make
 lifetimes easier-to-manage...

 builtin/fast-import.c    | 1 +
 builtin/index-pack.c     | 5 ++++-
 builtin/unpack-objects.c | 5 ++++-
 bulk-checkin.c           | 1 +
 csum-file.c              | 2 +-
 5 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 4dbb10aff3..444f41cf8c 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -1102,6 +1102,7 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark)
 		|| (pack_size + PACK_SIZE_THRESHOLD + len) < pack_size)
 		cycle_packfile();
 
+	the_hash_algo->init_fn(&checkpoint.ctx);
 	hashfile_checkpoint(pack_file, &checkpoint);
 	offset = checkpoint.offset;
 
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 006ffdc9c5..dda94a9f46 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1166,6 +1166,7 @@ static void parse_pack_objects(unsigned char *hash)
 	struct ofs_delta_entry *ofs_delta = ofs_deltas;
 	struct object_id ref_delta_oid;
 	struct stat st;
+	git_hash_ctx tmp_ctx;
 
 	if (verbose)
 		progress = start_progress(
@@ -1202,7 +1203,9 @@ static void parse_pack_objects(unsigned char *hash)
 
 	/* Check pack integrity */
 	flush();
-	the_hash_algo->final_fn(hash, &input_ctx);
+	the_hash_algo->init_fn(&tmp_ctx);
+	the_hash_algo->clone_fn(&tmp_ctx, &input_ctx);
+	the_hash_algo->final_fn(hash, &tmp_ctx);
 	if (!hasheq(fill(the_hash_algo->rawsz), hash))
 		die(_("pack is corrupted (SHA1 mismatch)"));
 	use(the_hash_algo->rawsz);
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 32505255a0..fef7423448 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -609,6 +609,7 @@ int cmd_unpack_objects(int argc, const char **argv, const char *prefix UNUSED)
 {
 	int i;
 	struct object_id oid;
+	git_hash_ctx tmp_ctx;
 
 	disable_replace_refs();
 
@@ -669,7 +670,9 @@ int cmd_unpack_objects(int argc, const char **argv, const char *prefix UNUSED)
 	the_hash_algo->init_fn(&ctx);
 	unpack_all();
 	the_hash_algo->update_fn(&ctx, buffer, offset);
-	the_hash_algo->final_oid_fn(&oid, &ctx);
+	the_hash_algo->init_fn(&tmp_ctx);
+	the_hash_algo->clone_fn(&tmp_ctx, &ctx);
+	the_hash_algo->final_oid_fn(&oid, &tmp_ctx);
 	if (strict) {
 		write_rest();
 		if (fsck_finish(&fsck_options))
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 73bff3a23d..92b9c8598b 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -268,6 +268,7 @@ static int deflate_to_pack(struct bulk_checkin_packfile *state,
 					  type, size);
 	the_hash_algo->init_fn(&ctx);
 	the_hash_algo->update_fn(&ctx, obuf, header_len);
+	the_hash_algo->init_fn(&checkpoint.ctx);
 
 	/* Note: idx is non-NULL when we are writing */
 	if ((flags & HASH_WRITE_OBJECT) != 0)
diff --git a/csum-file.c b/csum-file.c
index cd01713244..870748e016 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -207,7 +207,7 @@ int hashfile_truncate(struct hashfile *f, struct hashfile_checkpoint *checkpoint
 	    lseek(f->fd, offset, SEEK_SET) != offset)
 		return -1;
 	f->total = offset;
-	f->ctx = checkpoint->ctx;
+	the_hash_algo->clone_fn(&f->ctx, &checkpoint->ctx);
 	f->offset = 0; /* hashflush() was called in checkpoint */
 	return 0;
 }

Re: [PATCH] treewide: fix various bugs w/ OpenSSL 3+ EVP API

[Junio C Hamano]

Eric Wong <e@80x24.org> writes:

> The OpenSSL 3+ EVP API for SHA-* cannot support our prior use cases
> supported by other SHA-* implementations.  It has the following
> differences:
>
> 1. ->init_fn is required before all use
> 2. struct assignments don't work and requires ->clone_fn
> 3. can't support ->update_fn after ->final_*fn
>
> While fixing cases 1 and 2 is merely the matter of calling ->init_fn and
> ->clone_fn as appropriate, fixing case 3 requires calling ->final_*fn on
> a temporary context that's cloned from the primary context.
>
> Reported-by: Bagas Sanjaya <bagasdotme@gmail.com>
> Link: https://lore.kernel.org/ZPCL11k38PXTkFga@debian.me/
> Helped-by: brian m. carlson <sandals@crustytoothpaste.net>
> Fixes: 3e440ea0aba0 ("sha256: avoid functions deprecated in OpenSSL 3+")
> Fixes: bda9c12073e7 ("avoid SHA-1 functions deprecated in OpenSSL 3+")
> Signed-off-by: Eric Wong <e@80x24.org>
> ---
>  Ugh, I wonder if I setup my config.mak incorrectly when testing
>  3e440ea0aba0 and bda9c12073e7 :x

The third kind looks like a fun one to diagnoise and fix.

Thanks.  Will queue.

>  There may be other misuses not exposed by the test suite.  Making
>  git_hash_ctx opaque could flush out some of them (but I dislike
>  APIs which force heap allocations in the first place).  In any case,
>  I really wish git relied less on globals so object lifetimes could be
>  more obvious and really wish all C projects could rely on
>  gcc/tinycc/clang-supported __attribute__((__cleanup__)) to make
>  lifetimes easier-to-manage...
>
>  builtin/fast-import.c    | 1 +
>  builtin/index-pack.c     | 5 ++++-
>  builtin/unpack-objects.c | 5 ++++-
>  bulk-checkin.c           | 1 +
>  csum-file.c              | 2 +-
>  5 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> index 4dbb10aff3..444f41cf8c 100644
> --- a/builtin/fast-import.c
> +++ b/builtin/fast-import.c
> @@ -1102,6 +1102,7 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark)
>  		|| (pack_size + PACK_SIZE_THRESHOLD + len) < pack_size)
>  		cycle_packfile();
>  
> +	the_hash_algo->init_fn(&checkpoint.ctx);
>  	hashfile_checkpoint(pack_file, &checkpoint);
>  	offset = checkpoint.offset;
>  
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index 006ffdc9c5..dda94a9f46 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -1166,6 +1166,7 @@ static void parse_pack_objects(unsigned char *hash)
>  	struct ofs_delta_entry *ofs_delta = ofs_deltas;
>  	struct object_id ref_delta_oid;
>  	struct stat st;
> +	git_hash_ctx tmp_ctx;
>  
>  	if (verbose)
>  		progress = start_progress(
> @@ -1202,7 +1203,9 @@ static void parse_pack_objects(unsigned char *hash)
>  
>  	/* Check pack integrity */
>  	flush();
> -	the_hash_algo->final_fn(hash, &input_ctx);
> +	the_hash_algo->init_fn(&tmp_ctx);
> +	the_hash_algo->clone_fn(&tmp_ctx, &input_ctx);
> +	the_hash_algo->final_fn(hash, &tmp_ctx);
>  	if (!hasheq(fill(the_hash_algo->rawsz), hash))
>  		die(_("pack is corrupted (SHA1 mismatch)"));
>  	use(the_hash_algo->rawsz);
> diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
> index 32505255a0..fef7423448 100644
> --- a/builtin/unpack-objects.c
> +++ b/builtin/unpack-objects.c
> @@ -609,6 +609,7 @@ int cmd_unpack_objects(int argc, const char **argv, const char *prefix UNUSED)
>  {
>  	int i;
>  	struct object_id oid;
> +	git_hash_ctx tmp_ctx;
>  
>  	disable_replace_refs();
>  
> @@ -669,7 +670,9 @@ int cmd_unpack_objects(int argc, const char **argv, const char *prefix UNUSED)
>  	the_hash_algo->init_fn(&ctx);
>  	unpack_all();
>  	the_hash_algo->update_fn(&ctx, buffer, offset);
> -	the_hash_algo->final_oid_fn(&oid, &ctx);
> +	the_hash_algo->init_fn(&tmp_ctx);
> +	the_hash_algo->clone_fn(&tmp_ctx, &ctx);
> +	the_hash_algo->final_oid_fn(&oid, &tmp_ctx);
>  	if (strict) {
>  		write_rest();
>  		if (fsck_finish(&fsck_options))
> diff --git a/bulk-checkin.c b/bulk-checkin.c
> index 73bff3a23d..92b9c8598b 100644
> --- a/bulk-checkin.c
> +++ b/bulk-checkin.c
> @@ -268,6 +268,7 @@ static int deflate_to_pack(struct bulk_checkin_packfile *state,
>  					  type, size);
>  	the_hash_algo->init_fn(&ctx);
>  	the_hash_algo->update_fn(&ctx, obuf, header_len);
> +	the_hash_algo->init_fn(&checkpoint.ctx);
>  
>  	/* Note: idx is non-NULL when we are writing */
>  	if ((flags & HASH_WRITE_OBJECT) != 0)
> diff --git a/csum-file.c b/csum-file.c
> index cd01713244..870748e016 100644
> --- a/csum-file.c
> +++ b/csum-file.c
> @@ -207,7 +207,7 @@ int hashfile_truncate(struct hashfile *f, struct hashfile_checkpoint *checkpoint
>  	    lseek(f->fd, offset, SEEK_SET) != offset)
>  		return -1;
>  	f->total = offset;
> -	f->ctx = checkpoint->ctx;
> +	the_hash_algo->clone_fn(&f->ctx, &checkpoint->ctx);
>  	f->offset = 0; /* hashflush() was called in checkpoint */
>  	return 0;
>  }

Re: [PATCH] treewide: fix various bugs w/ OpenSSL 3+ EVP API

[Oswald Buddenhagen]

On Fri, Sep 01, 2023 at 02:09:28AM +0000, Eric Wong wrote:
>@@ -1202,7 +1203,9 @@ static void parse_pack_objects(unsigned char *hash)
> 
> 	/* Check pack integrity */
> 	flush();
>+	the_hash_algo->init_fn(&tmp_ctx);
>
does it make sense (and doesn't it potentially even cause a leak) to 
init the target before cloning into it? at least the fallback simply 
memcpy()s over it.

>@@ -669,7 +670,9 @@ int cmd_unpack_objects(int argc, const char **argv, const char *prefix UNUSED)
> 	the_hash_algo->init_fn(&ctx);
> 	unpack_all();
> 	the_hash_algo->update_fn(&ctx, buffer, offset);
>+	the_hash_algo->init_fn(&tmp_ctx);
>
ditto

>+	the_hash_algo->clone_fn(&tmp_ctx, &ctx);
>+	the_hash_algo->final_oid_fn(&oid, &tmp_ctx);

regards

Re: [PATCH] treewide: fix various bugs w/ OpenSSL 3+ EVP API

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 3411 bytes --]

On Fri, Sep 01, 2023 at 02:09:28AM +0000, Eric Wong wrote:
> diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> index 4dbb10aff3..444f41cf8c 100644
> --- a/builtin/fast-import.c
> +++ b/builtin/fast-import.c
> @@ -1102,6 +1102,7 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark)
>  		|| (pack_size + PACK_SIZE_THRESHOLD + len) < pack_size)
>  		cycle_packfile();
>  
> +	the_hash_algo->init_fn(&checkpoint.ctx);
>  	hashfile_checkpoint(pack_file, &checkpoint);
>  	offset = checkpoint.offset;
>  
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index 006ffdc9c5..dda94a9f46 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -1166,6 +1166,7 @@ static void parse_pack_objects(unsigned char *hash)
>  	struct ofs_delta_entry *ofs_delta = ofs_deltas;
>  	struct object_id ref_delta_oid;
>  	struct stat st;
> +	git_hash_ctx tmp_ctx;
>  
>  	if (verbose)
>  		progress = start_progress(
> @@ -1202,7 +1203,9 @@ static void parse_pack_objects(unsigned char *hash)
>  
>  	/* Check pack integrity */
>  	flush();
> -	the_hash_algo->final_fn(hash, &input_ctx);
> +	the_hash_algo->init_fn(&tmp_ctx);
> +	the_hash_algo->clone_fn(&tmp_ctx, &input_ctx);
> +	the_hash_algo->final_fn(hash, &tmp_ctx);
>  	if (!hasheq(fill(the_hash_algo->rawsz), hash))
>  		die(_("pack is corrupted (SHA1 mismatch)"));
>  	use(the_hash_algo->rawsz);
> diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
> index 32505255a0..fef7423448 100644
> --- a/builtin/unpack-objects.c
> +++ b/builtin/unpack-objects.c
> @@ -609,6 +609,7 @@ int cmd_unpack_objects(int argc, const char **argv, const char *prefix UNUSED)
>  {
>  	int i;
>  	struct object_id oid;
> +	git_hash_ctx tmp_ctx;
>  
>  	disable_replace_refs();
>  
> @@ -669,7 +670,9 @@ int cmd_unpack_objects(int argc, const char **argv, const char *prefix UNUSED)
>  	the_hash_algo->init_fn(&ctx);
>  	unpack_all();
>  	the_hash_algo->update_fn(&ctx, buffer, offset);
> -	the_hash_algo->final_oid_fn(&oid, &ctx);
> +	the_hash_algo->init_fn(&tmp_ctx);
> +	the_hash_algo->clone_fn(&tmp_ctx, &ctx);
> +	the_hash_algo->final_oid_fn(&oid, &tmp_ctx);
>  	if (strict) {
>  		write_rest();
>  		if (fsck_finish(&fsck_options))
> diff --git a/bulk-checkin.c b/bulk-checkin.c
> index 73bff3a23d..92b9c8598b 100644
> --- a/bulk-checkin.c
> +++ b/bulk-checkin.c
> @@ -268,6 +268,7 @@ static int deflate_to_pack(struct bulk_checkin_packfile *state,
>  					  type, size);
>  	the_hash_algo->init_fn(&ctx);
>  	the_hash_algo->update_fn(&ctx, obuf, header_len);
> +	the_hash_algo->init_fn(&checkpoint.ctx);
>  
>  	/* Note: idx is non-NULL when we are writing */
>  	if ((flags & HASH_WRITE_OBJECT) != 0)
> diff --git a/csum-file.c b/csum-file.c
> index cd01713244..870748e016 100644
> --- a/csum-file.c
> +++ b/csum-file.c
> @@ -207,7 +207,7 @@ int hashfile_truncate(struct hashfile *f, struct hashfile_checkpoint *checkpoint
>  	    lseek(f->fd, offset, SEEK_SET) != offset)
>  		return -1;
>  	f->total = offset;
> -	f->ctx = checkpoint->ctx;
> +	the_hash_algo->clone_fn(&f->ctx, &checkpoint->ctx);
>  	f->offset = 0; /* hashflush() was called in checkpoint */
>  	return 0;
>  }

The regression gone away, thanks!

Tested-by: Bagas Sanjaya <bagasdotme@gmail.com>

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [REGRESSION] Can't clone GitHub repos (fetch-pack error) due to avoiding deprecated OpenSSL SHA-1 routines

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 886 bytes --]

On Thu, Aug 31, 2023 at 11:19:14PM +0000, brian m. carlson wrote:
> On 2023-08-31 at 12:47:19, Bagas Sanjaya wrote:
> > Hi,
> > 
> > I built Git v2.42.0 on Debian testing, linked with OpenSSL (v3.0.10 from
> > distribution) with Makefile knob `OPENSSL_SHA1=YesPlease 
> > OPENSSL_SHA256=YesPlease`. I tried to shallow clone git.git repository:
> 
> I should point out that using OpenSSL's SHA-1 support is insecure
> because it doesn't check for collisions.  As a practical matter, no
> distro builds that way, and if you distributed that build, it would
> probably qualify for a CVE.
> 
> However, OPENSSL_SHA256 being set is fine for a local build or a build
> where you're not distributing OpenSSL itself.

Thanks for the disclaimer. I did such build for myself since the distro
version always lagging.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]