[PATCH 00/10] Fix segfaults when using the unsafe SHA1 backend

[PATCH 00/10] Fix segfaults when using the unsafe SHA1 backend

[Patrick Steinhardt]

Hi,

this patch series fixes two segfaults when using the unsafe SHA1
backend. The segfaults are caused by us mixing safe and unsafe variants
of the API. As such, it only surfaces when actually building Git with an
unsafe backend that is different from the safe one.

I noticed those errors while wiring up the build option in the Meson
build system. As such, this patch series doesn't only fix the segfaults,
but also includes patches for the Meson infrastructure and is structured
as follows:

  - Patches 1 and 2 fix the segfaults.

  - Patch 3 makes the segfaults visible via our CI.

  - Patches 4 to 10 wire up the build infrastructure in Meson.

Given that we're close to the release of Git v2.48 I'd be fine with
merging just the first three patches. I can then re-send the Meson stuff
early in the next development cycle.

Thanks!

Patrick

---
Patrick Steinhardt (10):
      bulk-checkin: fix segfault with unsafe SHA1 backend
      builtin/fast-import: fix segfault with unsafe SHA1 backend
      ci: exercise unsafe OpenSSL backend
      meson: consistenlty spell 'CommonCrypto'
      meson: deduplicate access to SHA1/SHA256 backend options
      meson: require SecurityFramework when it's used as SHA1 backend
      meson: simplify conditions for HTTPS and SHA1 dependencies
      meson: add missing dots for build options
      meson: wire up unsafe SHA1 backend
      meson: provide a summary of configured backends

 builtin/fast-import.c     |  2 +-
 bulk-checkin.c            |  2 +-
 ci/run-build-and-tests.sh |  1 +
 meson.build               | 51 ++++++++++++++++++++++++++++++++++++-----------
 meson_options.txt         |  8 +++++---
 5 files changed, 47 insertions(+), 17 deletions(-)


---
base-commit: 306ab352f4e98f6809ce52fc4e5d63fb947d0635
change-id: 20241230-pks-meson-sha1-unsafe-03c785824eee

[PATCH 01/10] bulk-checkin: fix segfault with unsafe SHA1 backend

[Patrick Steinhardt]

In 1b9e9be8b4 (csum-file.c: use unsafe SHA-1 implementation when
available, 2024-09-26) we have converted our `struct hashfile` to use
the unsafe SHA1 backend, which results in a significant speedup. One
needs to be careful with how to use that structure now though because
callers need to consistently use either the safe or unsafe variants of
SHA1, as otherwise one can easily trigger corruption.

As it turns out, we have one inconsistent usage in our tree because we
directly initialize `struct hashfile_checkpoint::ctx` with the safe
variant of SHA1, but end up writing to that context with the unsafe
ones. This went unnoticed so far because our CI systems do not exercise
different hash functions for these two backends, and consequently safe
and unsafe variants are equivalent. But when using SHA1DC as safe and
OpenSSL as unsafe backend this leads to a crash an t1050:

    ++ git -c core.compression=0 add large1
    AddressSanitizer:DEADLYSIGNAL
    =================================================================
    ==1367==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000040 (pc 0x7ffff7a01a99 bp 0x507000000db0 sp 0x7fffffff5690 T0)
    ==1367==The signal is caused by a READ memory access.
    ==1367==Hint: address points to the zero page.
        #0 0x7ffff7a01a99 in EVP_MD_CTX_copy_ex (/nix/store/h1ydpxkw9qhjdxjpic1pdc2nirggyy6f-openssl-3.3.2/lib/libcrypto.so.3+0x201a99) (BuildId: 41746a580d39075fc85e8c8065b6c07fb34e97d4)
        #1 0x555555ddde56 in openssl_SHA1_Clone ../sha1/openssl.h:40:2
        #2 0x555555dce2fc in git_hash_sha1_clone_unsafe ../object-file.c:123:2
        #3 0x555555c2d5f8 in hashfile_checkpoint ../csum-file.c:211:2
        #4 0x555555b9905d in deflate_blob_to_pack ../bulk-checkin.c:286:4
        #5 0x555555b98ae9 in index_blob_bulk_checkin ../bulk-checkin.c:362:15
        #6 0x555555ddab62 in index_blob_stream ../object-file.c:2756:9
        #7 0x555555dda420 in index_fd ../object-file.c:2778:9
        #8 0x555555ddad76 in index_path ../object-file.c:2796:7
        #9 0x555555e947f3 in add_to_index ../read-cache.c:771:7
        #10 0x555555e954a4 in add_file_to_index ../read-cache.c:804:9
        #11 0x5555558b5c39 in add_files ../builtin/add.c:355:7
        #12 0x5555558b412e in cmd_add ../builtin/add.c:578:18
        #13 0x555555b1f493 in run_builtin ../git.c:480:11
        #14 0x555555b1bfef in handle_builtin ../git.c:740:9
        #15 0x555555b1e6f4 in run_argv ../git.c:807:4
        #16 0x555555b1b87a in cmd_main ../git.c:947:19
        #17 0x5555561649e6 in main ../common-main.c:64:11
        #18 0x7ffff742a1fb in __libc_start_call_main (/nix/store/65h17wjrrlsj2rj540igylrx7fqcd6vq-glibc-2.40-36/lib/libc.so.6+0x2a1fb) (BuildId: bf320110569c8ec2425e9a0c5e4eb7e97f1fb6e4)
        #19 0x7ffff742a2b8 in __libc_start_main@GLIBC_2.2.5 (/nix/store/65h17wjrrlsj2rj540igylrx7fqcd6vq-glibc-2.40-36/lib/libc.so.6+0x2a2b8) (BuildId: bf320110569c8ec2425e9a0c5e4eb7e97f1fb6e4)
        #20 0x555555772c84 in _start (git+0x21ec84)

    ==1367==Register values:
    rax = 0x0000511000001080  rbx = 0x0000000000000000  rcx = 0x000000000000000c  rdx = 0x0000000000000000
    rdi = 0x0000000000000000  rsi = 0x0000507000000db0  rbp = 0x0000507000000db0  rsp = 0x00007fffffff5690
     r8 = 0x0000000000000000   r9 = 0x0000000000000000  r10 = 0x0000000000000000  r11 = 0x00007ffff7a01a30
    r12 = 0x0000000000000000  r13 = 0x00007fffffff6b38  r14 = 0x00007ffff7ffd000  r15 = 0x00005555563b9910
    AddressSanitizer can not provide additional info.
    SUMMARY: AddressSanitizer: SEGV (/nix/store/h1ydpxkw9qhjdxjpic1pdc2nirggyy6f-openssl-3.3.2/lib/libcrypto.so.3+0x201a99) (BuildId: 41746a580d39075fc85e8c8065b6c07fb34e97d4) in EVP_MD_CTX_copy_ex
    ==1367==ABORTING
    ./test-lib.sh: line 1023:  1367 Aborted                 git $config add large1
    error: last command exited with $?=134
    not ok 4 - add with -c core.compression=0

Fix the issue by using the unsafe variant instead.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 bulk-checkin.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bulk-checkin.c b/bulk-checkin.c
index 4a70a70a951cfd1a488339a33bf3a76b5152a344..433070a3bda461a2ad62da67cc5515f8822d2df3 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -272,7 +272,7 @@ static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
 					  OBJ_BLOB, size);
 	the_hash_algo->init_fn(&ctx);
 	the_hash_algo->update_fn(&ctx, obuf, header_len);
-	the_hash_algo->init_fn(&checkpoint.ctx);
+	the_hash_algo->unsafe_init_fn(&checkpoint.ctx);
 
 	/* Note: idx is non-NULL when we are writing */
 	if ((flags & HASH_WRITE_OBJECT) != 0)

-- 
2.48.0.rc0.311.gb6c66824c1.dirty

[PATCH 02/10] builtin/fast-import: fix segfault with unsafe SHA1 backend

[Patrick Steinhardt]

Same as with the preceding commit, git-fast-import(1) is using the safe
variant to initialize a hashfile checkpoint. This leads to a segfault
when passing the checkpoint into the hashfile subsystem because it would
use the unsafe variants instead:

    ++ git --git-dir=R/.git fast-import --big-file-threshold=1
    AddressSanitizer:DEADLYSIGNAL
    =================================================================
    ==577126==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000040 (pc 0x7ffff7a01a99 bp 0x5070000009c0 sp 0x7fffffff5b30 T0)
    ==577126==The signal is caused by a READ memory access.
    ==577126==Hint: address points to the zero page.
        #0 0x7ffff7a01a99 in EVP_MD_CTX_copy_ex (/nix/store/h1ydpxkw9qhjdxjpic1pdc2nirggyy6f-openssl-3.3.2/lib/libcrypto.so.3+0x201a99) (BuildId: 41746a580d39075fc85e8c8065b6c07fb34e97d4)
        #1 0x555555ddde56 in openssl_SHA1_Clone ../sha1/openssl.h:40:2
        #2 0x555555dce2fc in git_hash_sha1_clone_unsafe ../object-file.c:123:2
        #3 0x555555c2d5f8 in hashfile_checkpoint ../csum-file.c:211:2
        #4 0x5555559647d1 in stream_blob ../builtin/fast-import.c:1110:2
        #5 0x55555596247b in parse_and_store_blob ../builtin/fast-import.c:2031:3
        #6 0x555555967f91 in file_change_m ../builtin/fast-import.c:2408:5
        #7 0x55555595d8a2 in parse_new_commit ../builtin/fast-import.c:2768:4
        #8 0x55555595bb7a in cmd_fast_import ../builtin/fast-import.c:3614:4
        #9 0x555555b1f493 in run_builtin ../git.c:480:11
        #10 0x555555b1bfef in handle_builtin ../git.c:740:9
        #11 0x555555b1e6f4 in run_argv ../git.c:807:4
        #12 0x555555b1b87a in cmd_main ../git.c:947:19
        #13 0x5555561649e6 in main ../common-main.c:64:11
        #14 0x7ffff742a1fb in __libc_start_call_main (/nix/store/65h17wjrrlsj2rj540igylrx7fqcd6vq-glibc-2.40-36/lib/libc.so.6+0x2a1fb) (BuildId: bf320110569c8ec2425e9a0c5e4eb7e97f1fb6e4)
        #15 0x7ffff742a2b8 in __libc_start_main@GLIBC_2.2.5 (/nix/store/65h17wjrrlsj2rj540igylrx7fqcd6vq-glibc-2.40-36/lib/libc.so.6+0x2a2b8) (BuildId: bf320110569c8ec2425e9a0c5e4eb7e97f1fb6e4)
        #16 0x555555772c84 in _start (git+0x21ec84)

    ==577126==Register values:
    rax = 0x0000511000000cc0  rbx = 0x0000000000000000  rcx = 0x000000000000000c  rdx = 0x0000000000000000
    rdi = 0x0000000000000000  rsi = 0x00005070000009c0  rbp = 0x00005070000009c0  rsp = 0x00007fffffff5b30
     r8 = 0x0000000000000000   r9 = 0x0000000000000000  r10 = 0x0000000000000000  r11 = 0x00007ffff7a01a30
    r12 = 0x0000000000000000  r13 = 0x00007fffffff6b60  r14 = 0x00007ffff7ffd000  r15 = 0x00005555563b9910
    AddressSanitizer can not provide additional info.
    SUMMARY: AddressSanitizer: SEGV (/nix/store/h1ydpxkw9qhjdxjpic1pdc2nirggyy6f-openssl-3.3.2/lib/libcrypto.so.3+0x201a99) (BuildId: 41746a580d39075fc85e8c8065b6c07fb34e97d4) in EVP_MD_CTX_copy_ex
    ==577126==ABORTING
    ./test-lib.sh: line 1039: 577126 Aborted                 git --git-dir=R/.git fast-import --big-file-threshold=1 < input
    error: last command exited with $?=134
    not ok 167 - R: blob bigger than threshold

The segfault is only exposed in case the unsafe and safe backends are
different from one another.

Fix the issue by initializing the context with the unsafe SHA1 variant.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/fast-import.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 1fa2929a01b7dfee52b653248bba802884f6be6a..0f86392761abbe6acb217fef7f4fe7c3ff5ac1fa 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -1106,7 +1106,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);
+	the_hash_algo->unsafe_init_fn(&checkpoint.ctx);
 	hashfile_checkpoint(pack_file, &checkpoint);
 	offset = checkpoint.offset;
 

-- 
2.48.0.rc0.311.gb6c66824c1.dirty

Re: [PATCH 02/10] builtin/fast-import: fix segfault with unsafe SHA1

[Taylor Blau]

On Mon, Dec 30, 2024 at 03:24:02PM +0100, Patrick Steinhardt wrote:
> Same as with the preceding commit, git-fast-import(1) is using the safe
> variant to initialize a hashfile checkpoint. This leads to a segfault
> when passing the checkpoint into the hashfile subsystem because it would
> use the unsafe variants instead:
>
>     ++ git --git-dir=R/.git fast-import --big-file-threshold=1
>     AddressSanitizer:DEADLYSIGNAL
>     =================================================================
>     ==577126==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000040 (pc 0x7ffff7a01a99 bp 0x5070000009c0 sp 0x7fffffff5b30 T0)
>     ==577126==The signal is caused by a READ memory access.
>     ==577126==Hint: address points to the zero page.
>         #0 0x7ffff7a01a99 in EVP_MD_CTX_copy_ex (/nix/store/h1ydpxkw9qhjdxjpic1pdc2nirggyy6f-openssl-3.3.2/lib/libcrypto.so.3+0x201a99) (BuildId: 41746a580d39075fc85e8c8065b6c07fb34e97d4)
>         #1 0x555555ddde56 in openssl_SHA1_Clone ../sha1/openssl.h:40:2
>         #2 0x555555dce2fc in git_hash_sha1_clone_unsafe ../object-file.c:123:2
>         #3 0x555555c2d5f8 in hashfile_checkpoint ../csum-file.c:211:2
>         #4 0x5555559647d1 in stream_blob ../builtin/fast-import.c:1110:2
>         #5 0x55555596247b in parse_and_store_blob ../builtin/fast-import.c:2031:3
>         #6 0x555555967f91 in file_change_m ../builtin/fast-import.c:2408:5
>         #7 0x55555595d8a2 in parse_new_commit ../builtin/fast-import.c:2768:4
>         #8 0x55555595bb7a in cmd_fast_import ../builtin/fast-import.c:3614:4
>         #9 0x555555b1f493 in run_builtin ../git.c:480:11
>         #10 0x555555b1bfef in handle_builtin ../git.c:740:9
>         #11 0x555555b1e6f4 in run_argv ../git.c:807:4
>         #12 0x555555b1b87a in cmd_main ../git.c:947:19
>         #13 0x5555561649e6 in main ../common-main.c:64:11
>         #14 0x7ffff742a1fb in __libc_start_call_main (/nix/store/65h17wjrrlsj2rj540igylrx7fqcd6vq-glibc-2.40-36/lib/libc.so.6+0x2a1fb) (BuildId: bf320110569c8ec2425e9a0c5e4eb7e97f1fb6e4)
>         #15 0x7ffff742a2b8 in __libc_start_main@GLIBC_2.2.5 (/nix/store/65h17wjrrlsj2rj540igylrx7fqcd6vq-glibc-2.40-36/lib/libc.so.6+0x2a2b8) (BuildId: bf320110569c8ec2425e9a0c5e4eb7e97f1fb6e4)
>         #16 0x555555772c84 in _start (git+0x21ec84)

Wow. This is a really good catch, and I'm embarrassed to have introduced
the bug in the first place.

If I can state back to you what I think is going on to make sure that
we're on the same page... In fast-import, we initialize a
hashfile_checkpoint struct using regular the_hash_algo callbacks, like
the_hash_algo->init_fn(), the_hash_algo->update_fn(), etc. But we also
have a hashfile, which will use the unsafe variants of these functions.

If we're using a specialized unsafe variant, then calling
hashfile_checkpoint() will crash. In your example, the crash happens
when we try and call the clone_fn, but I think it could also happen
with hashflush() when it tries to call update_fn().

I thought that my series here:

    https://lore.kernel.org/git/cover.1732130001.git.me@ttaylorr.com/

would have prevented this, but I don't think that it would have in its
current state. A little more on that in a second...

> ---
>  builtin/fast-import.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> index 1fa2929a01b7dfee52b653248bba802884f6be6a..0f86392761abbe6acb217fef7f4fe7c3ff5ac1fa 100644
> --- a/builtin/fast-import.c
> +++ b/builtin/fast-import.c
> @@ -1106,7 +1106,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);
> +	the_hash_algo->unsafe_init_fn(&checkpoint.ctx);

This will obviously fix the issue at hand, but I don't think this is any
less brittle than before. The hash function implementation here needs to
agree with that used in the hashfile API. This change makes that
happen, but only using side information that the hashfile API uses the
unsafe variants.

The series I mentioned above made the algop a property of the hashfile
itself, and introduced a specialized "unsafe" variant that did not
require using separate callbacks.

So that on its own would not have saved us here (or in the other spot
from the previous patch). But something on top like this would have:

--- 8< ---
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 76d5c20f14..89fc20d436 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -1101,7 +1101,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);
+	pack_file->algop->init_fn(&checkpoint.ctx);
 	hashfile_checkpoint(pack_file, &checkpoint);
 	offset = checkpoint.offset;
--- >8 ---

(and an analogous change to the other spot).

I think we should perhaps combine forces here. My ideal end-state is to
have the unsafe_hash_algo() stuff land from my earlier series, then have
these two fixes (adjusted to the new world order as above), and finally
the Meson fixes after that.

Does that seem like a plan to you? If so, I can put everything together
and send it out (if you're OK with me forging your s-o-b).

Thanks,
Taylor

Re: [PATCH 02/10] builtin/fast-import: fix segfault with unsafe SHA1

[Patrick Steinhardt]

On Mon, Dec 30, 2024 at 12:22:34PM -0500, Taylor Blau wrote:
> On Mon, Dec 30, 2024 at 03:24:02PM +0100, Patrick Steinhardt wrote:
> > diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> > index 1fa2929a01b7dfee52b653248bba802884f6be6a..0f86392761abbe6acb217fef7f4fe7c3ff5ac1fa 100644
> > --- a/builtin/fast-import.c
> > +++ b/builtin/fast-import.c
> > @@ -1106,7 +1106,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);
> > +	the_hash_algo->unsafe_init_fn(&checkpoint.ctx);
> 
> This will obviously fix the issue at hand, but I don't think this is any
> less brittle than before. The hash function implementation here needs to
> agree with that used in the hashfile API. This change makes that
> happen, but only using side information that the hashfile API uses the
> unsafe variants.

Yup, I only cared about fixing the segfault because we're close to the
v2.48 release. I agree that the overall state is still extremely brittle
right now.

[snip]
> I think we should perhaps combine forces here. My ideal end-state is to
> have the unsafe_hash_algo() stuff land from my earlier series, then have
> these two fixes (adjusted to the new world order as above), and finally
> the Meson fixes after that.
> 
> Does that seem like a plan to you? If so, I can put everything together
> and send it out (if you're OK with me forging your s-o-b).

I think the ideal state would be if the hashing function used was stored
as part of `struct git_hash_ctx`. So the flow basically becomes for
example:

    ```
    struct git_hash_ctx ctx;
    struct object_id oid;

    git_hash_sha1_init(&ctx);
    git_hash_update(&ctx, data);
    git_hash_final_oid(&oid, &ctx);
    ```

Note how the intermediate calls don't need to know which hash function
you used to initialize the `struct git_hash_ctx` -- the structure itself
should remember what it has been initilized with and do the right thing.

Patrick

Re: [PATCH 02/10] builtin/fast-import: fix segfault with unsafe SHA1

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> I think the ideal state would be if the hashing function used was stored
> as part of `struct git_hash_ctx`. So the flow basically becomes for
> example:
>
>     ```
>     struct git_hash_ctx ctx;
>     struct object_id oid;
>
>     git_hash_sha1_init(&ctx);
>     git_hash_update(&ctx, data);
>     git_hash_final_oid(&oid, &ctx);
>     ```
>
> Note how the intermediate calls don't need to know which hash function
> you used to initialize the `struct git_hash_ctx` -- the structure itself
> should remember what it has been initilized with and do the right thing.

Yup, that sounds perfect.

Re: [PATCH 02/10] builtin/fast-import: fix segfault with unsafe SHA1

[Taylor Blau]

On Fri, Jan 03, 2025 at 02:08:01PM +0100, Patrick Steinhardt wrote:
> On Mon, Dec 30, 2024 at 12:22:34PM -0500, Taylor Blau wrote:
> > On Mon, Dec 30, 2024 at 03:24:02PM +0100, Patrick Steinhardt wrote:
> > > diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> > > index 1fa2929a01b7dfee52b653248bba802884f6be6a..0f86392761abbe6acb217fef7f4fe7c3ff5ac1fa 100644
> > > --- a/builtin/fast-import.c
> > > +++ b/builtin/fast-import.c
> > > @@ -1106,7 +1106,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);
> > > +	the_hash_algo->unsafe_init_fn(&checkpoint.ctx);
> >
> > This will obviously fix the issue at hand, but I don't think this is any
> > less brittle than before. The hash function implementation here needs to
> > agree with that used in the hashfile API. This change makes that
> > happen, but only using side information that the hashfile API uses the
> > unsafe variants.
>
> Yup, I only cared about fixing the segfault because we're close to the
> v2.48 release. I agree that the overall state is still extremely brittle
> right now.
>
> [snip]
> > I think we should perhaps combine forces here. My ideal end-state is to
> > have the unsafe_hash_algo() stuff land from my earlier series, then have
> > these two fixes (adjusted to the new world order as above), and finally
> > the Meson fixes after that.
> >
> > Does that seem like a plan to you? If so, I can put everything together
> > and send it out (if you're OK with me forging your s-o-b).
>
> I think the ideal state would be if the hashing function used was stored
> as part of `struct git_hash_ctx`. So the flow basically becomes for
> example:
>
>     ```
>     struct git_hash_ctx ctx;
>     struct object_id oid;
>
>     git_hash_sha1_init(&ctx);
>     git_hash_update(&ctx, data);
>     git_hash_final_oid(&oid, &ctx);
>     ```
>
> Note how the intermediate calls don't need to know which hash function
> you used to initialize the `struct git_hash_ctx` -- the structure itself
> should remember what it has been initilized with and do the right thing.

I'm not sure I'm following you here. In the stream_blob() function
within fast-import, the problem isn't that we're switching hash
functions mid-stream, but that we're initializing the hashfile_context
structure with the wrong hash function to begin with.

You snipped it out of your reply, but I think that my suggestion to do:

    pack_file->algop->init_fn(&checkpoint.ctx);

would harden us against the broken behavior we're seeing here.

As a separate defense-in-depth measure, we could teach functions from
the hashfile API which deal with hashfile_checkpoint structure to ensure
that the hashfile and its checkpoint both use the same algorithm (by
adding a hash_algo field to the hashfile_checkpoint structure).

Thanks,
Taylor

Re: [PATCH 02/10] builtin/fast-import: fix segfault with unsafe SHA1

[Patrick Steinhardt]

On Mon, Jan 06, 2025 at 02:17:58PM -0500, Taylor Blau wrote:
> On Fri, Jan 03, 2025 at 02:08:01PM +0100, Patrick Steinhardt wrote:
> > On Mon, Dec 30, 2024 at 12:22:34PM -0500, Taylor Blau wrote:
> > > On Mon, Dec 30, 2024 at 03:24:02PM +0100, Patrick Steinhardt wrote:
> > > > diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> > > > index 1fa2929a01b7dfee52b653248bba802884f6be6a..0f86392761abbe6acb217fef7f4fe7c3ff5ac1fa 100644
> > > > --- a/builtin/fast-import.c
> > > > +++ b/builtin/fast-import.c
> > > > @@ -1106,7 +1106,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);
> > > > +	the_hash_algo->unsafe_init_fn(&checkpoint.ctx);
> > >
> > > This will obviously fix the issue at hand, but I don't think this is any
> > > less brittle than before. The hash function implementation here needs to
> > > agree with that used in the hashfile API. This change makes that
> > > happen, but only using side information that the hashfile API uses the
> > > unsafe variants.
> >
> > Yup, I only cared about fixing the segfault because we're close to the
> > v2.48 release. I agree that the overall state is still extremely brittle
> > right now.
> >
> > [snip]
> > > I think we should perhaps combine forces here. My ideal end-state is to
> > > have the unsafe_hash_algo() stuff land from my earlier series, then have
> > > these two fixes (adjusted to the new world order as above), and finally
> > > the Meson fixes after that.
> > >
> > > Does that seem like a plan to you? If so, I can put everything together
> > > and send it out (if you're OK with me forging your s-o-b).
> >
> > I think the ideal state would be if the hashing function used was stored
> > as part of `struct git_hash_ctx`. So the flow basically becomes for
> > example:
> >
> >     ```
> >     struct git_hash_ctx ctx;
> >     struct object_id oid;
> >
> >     git_hash_sha1_init(&ctx);
> >     git_hash_update(&ctx, data);
> >     git_hash_final_oid(&oid, &ctx);
> >     ```
> >
> > Note how the intermediate calls don't need to know which hash function
> > you used to initialize the `struct git_hash_ctx` -- the structure itself
> > should remember what it has been initilized with and do the right thing.
> 
> I'm not sure I'm following you here. In the stream_blob() function
> within fast-import, the problem isn't that we're switching hash
> functions mid-stream, but that we're initializing the hashfile_context
> structure with the wrong hash function to begin with.

True, but it would have been a non-issue if the hash context itself knew
which hash function to use for updates. Sure, we would've used the slow
variant of SHA1 instead of the fast-but-unsafe one. But that feels like
the lesser evil compared to crashing.

> You snipped it out of your reply, but I think that my suggestion to do:
> 
>     pack_file->algop->init_fn(&checkpoint.ctx);
> 
> would harden us against the broken behavior we're seeing here.
> 
> As a separate defense-in-depth measure, we could teach functions from
> the hashfile API which deal with hashfile_checkpoint structure to ensure
> that the hashfile and its checkpoint both use the same algorithm (by
> adding a hash_algo field to the hashfile_checkpoint structure).

I would think that it were even harder to abuse if it wasn't the
hashfile API, but the hash API that remembered the algorithm.

Patrick

Re: [PATCH 02/10] builtin/fast-import: fix segfault with unsafe SHA1

[Taylor Blau]

On Tue, Jan 07, 2025 at 01:06:20PM +0100, Patrick Steinhardt wrote:
> > > > I think we should perhaps combine forces here. My ideal end-state is to
> > > > have the unsafe_hash_algo() stuff land from my earlier series, then have
> > > > these two fixes (adjusted to the new world order as above), and finally
> > > > the Meson fixes after that.
> > > >
> > > > Does that seem like a plan to you? If so, I can put everything together
> > > > and send it out (if you're OK with me forging your s-o-b).
> > >
> > > I think the ideal state would be if the hashing function used was stored
> > > as part of `struct git_hash_ctx`. So the flow basically becomes for
> > > example:
> > >
> > >     ```
> > >     struct git_hash_ctx ctx;
> > >     struct object_id oid;
> > >
> > >     git_hash_sha1_init(&ctx);
> > >     git_hash_update(&ctx, data);
> > >     git_hash_final_oid(&oid, &ctx);
> > >     ```
> > >
> > > Note how the intermediate calls don't need to know which hash function
> > > you used to initialize the `struct git_hash_ctx` -- the structure itself
> > > should remember what it has been initilized with and do the right thing.
> >
> > I'm not sure I'm following you here. In the stream_blob() function
> > within fast-import, the problem isn't that we're switching hash
> > functions mid-stream, but that we're initializing the hashfile_context
> > structure with the wrong hash function to begin with.
>
> True, but it would have been a non-issue if the hash context itself knew
> which hash function to use for updates. Sure, we would've used the slow
> variant of SHA1 instead of the fast-but-unsafe one. But that feels like
> the lesser evil compared to crashing.

For posterity, Patrick and I used some of our monthly meeting this morning to
spend some time together pairing on this idea.

It ended up being a dead-end, since this approach only protects you
against changing the hash function mid-stream, and not using the
incorrect context type from the union.

That was along the lines of what I was originally thinking, and so I
resurrected my series to introduce 'unsafe_hash_algo()' here:

    https://lore.kernel.org/git/cover.1736363652.git.me@ttaylorr.com/

I got the impression that Patrick and I are on the same page there as
that being a good path forward, but I'll let him chime in in case I
misunderstood anything.

Thanks,
Taylor

Re: [PATCH 02/10] builtin/fast-import: fix segfault with unsafe SHA1

[Patrick Steinhardt]

On Wed, Jan 08, 2025 at 02:21:47PM -0500, Taylor Blau wrote:
> On Tue, Jan 07, 2025 at 01:06:20PM +0100, Patrick Steinhardt wrote:
> > > > > I think we should perhaps combine forces here. My ideal end-state is to
> > > > > have the unsafe_hash_algo() stuff land from my earlier series, then have
> > > > > these two fixes (adjusted to the new world order as above), and finally
> > > > > the Meson fixes after that.
> > > > >
> > > > > Does that seem like a plan to you? If so, I can put everything together
> > > > > and send it out (if you're OK with me forging your s-o-b).
> > > >
> > > > I think the ideal state would be if the hashing function used was stored
> > > > as part of `struct git_hash_ctx`. So the flow basically becomes for
> > > > example:
> > > >
> > > >     ```
> > > >     struct git_hash_ctx ctx;
> > > >     struct object_id oid;
> > > >
> > > >     git_hash_sha1_init(&ctx);
> > > >     git_hash_update(&ctx, data);
> > > >     git_hash_final_oid(&oid, &ctx);
> > > >     ```
> > > >
> > > > Note how the intermediate calls don't need to know which hash function
> > > > you used to initialize the `struct git_hash_ctx` -- the structure itself
> > > > should remember what it has been initilized with and do the right thing.
> > >
> > > I'm not sure I'm following you here. In the stream_blob() function
> > > within fast-import, the problem isn't that we're switching hash
> > > functions mid-stream, but that we're initializing the hashfile_context
> > > structure with the wrong hash function to begin with.
> >
> > True, but it would have been a non-issue if the hash context itself knew
> > which hash function to use for updates. Sure, we would've used the slow
> > variant of SHA1 instead of the fast-but-unsafe one. But that feels like
> > the lesser evil compared to crashing.
> 
> For posterity, Patrick and I used some of our monthly meeting this morning to
> spend some time together pairing on this idea.
> 
> It ended up being a dead-end, since this approach only protects you
> against changing the hash function mid-stream, and not using the
> incorrect context type from the union.
> 
> That was along the lines of what I was originally thinking, and so I
> resurrected my series to introduce 'unsafe_hash_algo()' here:
> 
>     https://lore.kernel.org/git/cover.1736363652.git.me@ttaylorr.com/
> 
> I got the impression that Patrick and I are on the same page there as
> that being a good path forward, but I'll let him chime in in case I
> misunderstood anything.

No misunderstanding, we're both on the same page. Thanks!

Patrick

[PATCH 03/10] ci: exercise unsafe OpenSSL backend

[Patrick Steinhardt]

In the preceding commit we have fixed a segfault when using an unsafe
SHA1 backend that is different from the safe one. This segfault only
went by unnoticed because we never set up an unsafe backend in our CI
systems. Fix this ommission by setting `OPENSSL_SHA1_UNSAFE` in our
TEST-vars job.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 ci/run-build-and-tests.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
index c4a41bba0b84df57f6e60aeac2de29dbc0e27dc1..76667a1277720d74e09e8da227b5e0832003e0e2 100755
--- a/ci/run-build-and-tests.sh
+++ b/ci/run-build-and-tests.sh
@@ -17,6 +17,7 @@ linux-gcc)
 	export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 	;;
 linux-TEST-vars)
+	export OPENSSL_SHA1_UNSAFE=YesPlease
 	export GIT_TEST_SPLIT_INDEX=yes
 	export GIT_TEST_MERGE_ALGORITHM=recursive
 	export GIT_TEST_FULL_IN_PACK_ARRAY=true

-- 
2.48.0.rc0.311.gb6c66824c1.dirty

Re: [PATCH 03/10] ci: exercise unsafe OpenSSL backend

[Taylor Blau]

On Mon, Dec 30, 2024 at 03:24:03PM +0100, Patrick Steinhardt wrote:
> In the preceding commit we have fixed a segfault when using an unsafe
> SHA1 backend that is different from the safe one. This segfault only
> went by unnoticed because we never set up an unsafe backend in our CI
> systems. Fix this ommission by setting `OPENSSL_SHA1_UNSAFE` in our
> TEST-vars job.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  ci/run-build-and-tests.sh | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
> index c4a41bba0b84df57f6e60aeac2de29dbc0e27dc1..76667a1277720d74e09e8da227b5e0832003e0e2 100755
> --- a/ci/run-build-and-tests.sh
> +++ b/ci/run-build-and-tests.sh
> @@ -17,6 +17,7 @@ linux-gcc)
>  	export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
>  	;;
>  linux-TEST-vars)
> +	export OPENSSL_SHA1_UNSAFE=YesPlease
>  	export GIT_TEST_SPLIT_INDEX=yes
>  	export GIT_TEST_MERGE_ALGORITHM=recursive
>  	export GIT_TEST_FULL_IN_PACK_ARRAY=true

I think that this is a great idea, thanks for adding it!

Thanks,
Taylor

[PATCH 04/10] meson: consistenlty spell 'CommonCrypto'

[Patrick Steinhardt]

The 'CommonCrypto' backend can be specified as HTTPS and SHA1 backends,
but the value that one needs to use is inconsistent across those two
build options. Unify it to 'CommonCrypto'.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 meson.build       | 2 +-
 meson_options.txt | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index 0064eb64f546a6349a8694ce251bd352febda6fe..9da58dafe0f1023cc72f4ea3eff5515c9d479099 100644
--- a/meson.build
+++ b/meson.build
@@ -1367,7 +1367,7 @@ if sha1_backend == 'sha1dc'
     'sha1dc/sha1.c',
     'sha1dc/ubc_check.c',
   ]
-elif sha1_backend == 'common-crypto'
+elif sha1_backend == 'CommonCrypto'
   libgit_c_args += '-DCOMMON_DIGEST_FOR_OPENSSL'
   libgit_c_args += '-DSHA1_APPLE'
   # Apple CommonCrypto requires chunking
diff --git a/meson_options.txt b/meson_options.txt
index 4be7eab39939178ae2ffde1ff9e78f83a1b482b2..a7f308d217f29ef301848e63623a49207ef83125 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -49,7 +49,7 @@ option('regex', type: 'feature', value: 'auto',
 # Backends.
 option('https_backend', type: 'combo', value: 'auto', choices: ['auto', 'openssl', 'CommonCrypto', 'none'],
   description: 'The HTTPS backend to use when connecting to remotes.')
-option('sha1_backend', type: 'combo', choices: ['openssl', 'block', 'sha1dc', 'common-crypto'], value: 'sha1dc',
+option('sha1_backend', type: 'combo', choices: ['openssl', 'block', 'sha1dc', 'CommonCrypto'], value: 'sha1dc',
   description: 'The backend used for hashing objects with the SHA1 object format')
 option('sha256_backend', type: 'combo', choices: ['openssl', 'nettle', 'gcrypt', 'block'], value: 'block',
   description: 'The backend used for hashing objects with the SHA256 object format')

-- 
2.48.0.rc0.311.gb6c66824c1.dirty

[PATCH 05/10] meson: deduplicate access to SHA1/SHA256 backend options

[Patrick Steinhardt]

We've got a couple of repeated calls to `get_option()` for the SHA1 and
SHA256 backend options. While not an issue, it makes the code needlessly
verbose.

Fix this by consistently using a local variable.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 meson.build | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/meson.build b/meson.build
index 9da58dafe0f1023cc72f4ea3eff5515c9d479099..6fa4d900ee02f0b80bc3c36d58a07a118ec3fb20 100644
--- a/meson.build
+++ b/meson.build
@@ -1326,6 +1326,8 @@ if not meson.is_cross_build() and fs.exists('/dev/tty')
 endif
 
 https_backend = get_option('https_backend')
+sha1_backend = get_option('sha1_backend')
+sha256_backend = get_option('sha256_backend')
 
 security_framework = dependency('Security', required: https_backend == 'CommonCrypto')
 core_foundation_framework = dependency('CoreFoundation', required: security_framework.found())
@@ -1333,7 +1335,7 @@ if https_backend == 'auto' and security_framework.found()
   https_backend = 'CommonCrypto'
 endif
 
-openssl_required = https_backend == 'openssl' or get_option('sha1_backend') == 'openssl' or get_option('sha256_backend') == 'openssl'
+openssl_required = https_backend == 'openssl' or sha1_backend == 'openssl' or sha256_backend == 'openssl'
 openssl = dependency('openssl', required: openssl_required, default_options: ['default_library=static'])
 if https_backend == 'auto' and openssl.found()
   https_backend = 'openssl'
@@ -1354,7 +1356,6 @@ if https_backend != 'openssl'
   libgit_c_args += '-DNO_OPENSSL'
 endif
 
-sha1_backend = get_option('sha1_backend')
 if sha1_backend == 'sha1dc'
   libgit_c_args += '-DSHA1_DC'
   libgit_c_args += '-DSHA1DC_NO_STANDARD_INCLUDES=1'
@@ -1382,7 +1383,6 @@ else
   error('Unhandled SHA1 backend ' + sha1_backend)
 endif
 
-sha256_backend = get_option('sha256_backend')
 if sha256_backend == 'openssl'
   libgit_c_args += '-DSHA256_OPENSSL'
   libgit_dependencies += openssl

-- 
2.48.0.rc0.311.gb6c66824c1.dirty

[PATCH 06/10] meson: require SecurityFramework when it's used as SHA1 backend

[Patrick Steinhardt]

The Security framework is required when we use CommonCrypto either as
HTTPS or SHA1 backend, but we only require it in case it is set up as
HTTPS backend. Fix this.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 6fa4d900ee02f0b80bc3c36d58a07a118ec3fb20..bc75ad954a4949342125b769d3d8d8362ef4e8a3 100644
--- a/meson.build
+++ b/meson.build
@@ -1329,7 +1329,7 @@ https_backend = get_option('https_backend')
 sha1_backend = get_option('sha1_backend')
 sha256_backend = get_option('sha256_backend')
 
-security_framework = dependency('Security', required: https_backend == 'CommonCrypto')
+security_framework = dependency('Security', required: https_backend == 'CommonCrypto' or sha1_backend == 'CommonCrypto')
 core_foundation_framework = dependency('CoreFoundation', required: security_framework.found())
 if https_backend == 'auto' and security_framework.found()
   https_backend = 'CommonCrypto'

-- 
2.48.0.rc0.311.gb6c66824c1.dirty

[PATCH 07/10] meson: simplify conditions for HTTPS and SHA1 dependencies

[Patrick Steinhardt]

The conditions used to figure out whteher the Security framework or
OpenSSL library is required are a bit convoluted because they can be
pulled in via the HTTPS, SHA1 or SHA256 backends. Refactor them to be
easier to read.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 meson.build | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index bc75ad954a4949342125b769d3d8d8362ef4e8a3..46f807899b7bae33dd6aa7a94a54931d69ab0d62 100644
--- a/meson.build
+++ b/meson.build
@@ -1329,13 +1329,13 @@ https_backend = get_option('https_backend')
 sha1_backend = get_option('sha1_backend')
 sha256_backend = get_option('sha256_backend')
 
-security_framework = dependency('Security', required: https_backend == 'CommonCrypto' or sha1_backend == 'CommonCrypto')
+security_framework = dependency('Security', required: 'CommonCrypto' in [https_backend, sha1_backend])
 core_foundation_framework = dependency('CoreFoundation', required: security_framework.found())
 if https_backend == 'auto' and security_framework.found()
   https_backend = 'CommonCrypto'
 endif
 
-openssl_required = https_backend == 'openssl' or sha1_backend == 'openssl' or sha256_backend == 'openssl'
+openssl_required = 'openssl' in [https_backend, sha1_backend, sha256_backend]
 openssl = dependency('openssl', required: openssl_required, default_options: ['default_library=static'])
 if https_backend == 'auto' and openssl.found()
   https_backend = 'openssl'

-- 
2.48.0.rc0.311.gb6c66824c1.dirty

[PATCH 08/10] meson: add missing dots for build options

[Patrick Steinhardt]

Most of our Meson build options end with a trailing dot, but those for
our SHA1 and SHA256 backends don't. Add it.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 meson_options.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/meson_options.txt b/meson_options.txt
index a7f308d217f29ef301848e63623a49207ef83125..d8d283982bcdd0f688556e0102c0133061dfb304 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -50,9 +50,9 @@ option('regex', type: 'feature', value: 'auto',
 option('https_backend', type: 'combo', value: 'auto', choices: ['auto', 'openssl', 'CommonCrypto', 'none'],
   description: 'The HTTPS backend to use when connecting to remotes.')
 option('sha1_backend', type: 'combo', choices: ['openssl', 'block', 'sha1dc', 'CommonCrypto'], value: 'sha1dc',
-  description: 'The backend used for hashing objects with the SHA1 object format')
+  description: 'The backend used for hashing objects with the SHA1 object format.')
 option('sha256_backend', type: 'combo', choices: ['openssl', 'nettle', 'gcrypt', 'block'], value: 'block',
-  description: 'The backend used for hashing objects with the SHA256 object format')
+  description: 'The backend used for hashing objects with the SHA256 object format.')
 
 # Build tweaks.
 option('macos_use_homebrew_gettext', type: 'boolean', value: true,

-- 
2.48.0.rc0.311.gb6c66824c1.dirty

[PATCH 09/10] meson: wire up unsafe SHA1 backend

[Patrick Steinhardt]

In 06c92dafb8 (Makefile: allow specifying a SHA-1 for non-cryptographic
uses, 2024-09-26), we have introduced a cryptographically-insecure
backend for SHA1 that can optionally be used in some contexts where the
processed data is not security relevant. This effort was in-flight with
the effort to introduce Meson, so we don't have an equivalent here.

Wire up a new build option that lets users pick an unsafe SHA1 backend.

Note that for simplicity's sake we have to drop the error condition
around an unhandled SHA1 backend. This should be fine though given that
Meson verifies the value for combo-options for us.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 meson.build       | 40 ++++++++++++++++++++++++++++++----------
 meson_options.txt |  2 ++
 2 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/meson.build b/meson.build
index 46f807899b7bae33dd6aa7a94a54931d69ab0d62..dc82c23cb4f07646a9a7bb96fefcf832f9840975 100644
--- a/meson.build
+++ b/meson.build
@@ -1327,15 +1327,16 @@ endif
 
 https_backend = get_option('https_backend')
 sha1_backend = get_option('sha1_backend')
+sha1_unsafe_backend = get_option('sha1_unsafe_backend')
 sha256_backend = get_option('sha256_backend')
 
-security_framework = dependency('Security', required: 'CommonCrypto' in [https_backend, sha1_backend])
+security_framework = dependency('Security', required: 'CommonCrypto' in [https_backend, sha1_backend, sha1_unsafe_backend])
 core_foundation_framework = dependency('CoreFoundation', required: security_framework.found())
 if https_backend == 'auto' and security_framework.found()
   https_backend = 'CommonCrypto'
 endif
 
-openssl_required = 'openssl' in [https_backend, sha1_backend, sha256_backend]
+openssl_required = 'openssl' in [https_backend, sha1_backend, sha1_unsafe_backend, sha256_backend]
 openssl = dependency('openssl', required: openssl_required, default_options: ['default_library=static'])
 if https_backend == 'auto' and openssl.found()
   https_backend = 'openssl'
@@ -1368,19 +1369,38 @@ if sha1_backend == 'sha1dc'
     'sha1dc/sha1.c',
     'sha1dc/ubc_check.c',
   ]
-elif sha1_backend == 'CommonCrypto'
+endif
+if sha1_backend == 'CommonCrypto' or sha1_unsafe_backend == 'CommonCrypto'
+  if sha1_backend == 'CommonCrypto'
+    libgit_c_args += '-DSHA1_APPLE'
+  endif
+  if sha1_unsafe_backend == 'CommonCrypto'
+    libgit_c_args += '-DSHA1_APPLE_UNSAFE'
+  endif
+
   libgit_c_args += '-DCOMMON_DIGEST_FOR_OPENSSL'
-  libgit_c_args += '-DSHA1_APPLE'
   # Apple CommonCrypto requires chunking
   libgit_c_args += '-DSHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L'
-elif sha1_backend == 'openssl'
-  libgit_c_args += '-DSHA1_OPENSSL'
+endif
+if sha1_backend == 'openssl' or sha1_unsafe_backend == 'openssl'
+  if sha1_backend == 'openssl'
+    libgit_c_args += '-DSHA1_OPENSSL'
+  endif
+  if sha1_unsafe_backend == 'openssl'
+    libgit_c_args += '-DSHA1_OPENSSL_UNSAFE'
+  endif
+
   libgit_dependencies += openssl
-elif sha1_backend == 'block'
-  libgit_c_args += '-DSHA1_BLK'
+endif
+if sha1_backend == 'block' or sha1_unsafe_backend == 'block'
+  if sha1_backend == 'block'
+    libgit_c_args += '-DSHA1_BLK'
+  endif
+  if sha1_unsafe_backend == 'block'
+    libgit_c_args += '-DSHA1_BLK_UNSAFE'
+  endif
+
   libgit_sources += 'block-sha1/sha1.c'
-else
-  error('Unhandled SHA1 backend ' + sha1_backend)
 endif
 
 if sha256_backend == 'openssl'
diff --git a/meson_options.txt b/meson_options.txt
index d8d283982bcdd0f688556e0102c0133061dfb304..8282b1dea8e852fbd3a28309a96fdc83412f245d 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -51,6 +51,8 @@ option('https_backend', type: 'combo', value: 'auto', choices: ['auto', 'openssl
   description: 'The HTTPS backend to use when connecting to remotes.')
 option('sha1_backend', type: 'combo', choices: ['openssl', 'block', 'sha1dc', 'CommonCrypto'], value: 'sha1dc',
   description: 'The backend used for hashing objects with the SHA1 object format.')
+option('sha1_unsafe_backend', type: 'combo', choices: ['openssl', 'block', 'CommonCrypto', 'none'], value: 'none',
+  description: 'The backend used for hashing data with the SHA1 object format in case no cryptographic security is needed.')
 option('sha256_backend', type: 'combo', choices: ['openssl', 'nettle', 'gcrypt', 'block'], value: 'block',
   description: 'The backend used for hashing objects with the SHA256 object format.')
 

-- 
2.48.0.rc0.311.gb6c66824c1.dirty

[PATCH 10/10] meson: provide a summary of configured backends

[Patrick Steinhardt]

There are a couple of backends from which the user can choose for HTTPS,
SHA1, its unsafe variant as well as SHA256. Provide a summary of the
configured values to make these more discoverable.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 meson.build | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/meson.build b/meson.build
index dc82c23cb4f07646a9a7bb96fefcf832f9840975..7361eb2eaad422e7a6c6ed95d275615836c21cdb 100644
--- a/meson.build
+++ b/meson.build
@@ -1943,3 +1943,10 @@ summary({
   'perl': perl_features_enabled,
   'python': python.found(),
 }, section: 'Auto-detected features')
+
+summary({
+  'https': https_backend,
+  'sha1': sha1_backend,
+  'sha1_unsafe': sha1_unsafe_backend,
+  'sha256': sha256_backend,
+}, section: 'Backends')

-- 
2.48.0.rc0.311.gb6c66824c1.dirty