Makefile: Build Git with non-collision-detecting SHA1

By default, Git uses SHA1DC for all SHA1 hashing operations. This implementation includes collision detection to protect against collision attacks, but comes with a performance penalty. Not all hashing operations performed by Git are vulnerable to collision attacks though.

In Git version 2.48.0, Git can be built with OPENSSL_SHA1_UNSAFE set to use a non-collision-detecting SHA1 implementation in non-cryptographic scenarios. This improves performance without compromising security.

Update the Gitaly Makefile to build the bundled Git v2.48 with this option set.

changed milestone to %17.9

added devopssystems groupgit maintenancerefactor sectioncore platform typemaintenance labels

assigned to @justintobler

mentioned in issue #6579

This change relates to #6579. To keep things simple, we are just modifying the current bundled Git v2.48 build which is currently behind a feature flag. Consequently, the feature flag has been turned off again in preparation for this change and will be rolled out again once this change lands.

@knayakgl @echui-gitlab Would you mind reviewing?

requested review from @knayakgl and @echui-gitlab

mentioned in issue #6595 (closed)

approved this merge request

approved this merge request

@echui-gitlab could you set to merge please?

approved this merge request

started a merge train

removed this merge request from the merge train because the pipeline did not succeed. Learn more.

Looks like there is a problem with macos builds. Investigating

marked this merge request as draft

added 1 commit

• 438e36a9 - Makefile: Build Git with non-collision-detecting SHA1

Compare with previous version

reset approvals from @echui-gitlab by pushing to the branch

marked this merge request as ready

added 1 commit

• 4e8ab4b4 - Makefile: Build Git with non-collision-detecting SHA1

Compare with previous version

added 1 commit

• ddf4696d - test

Compare with previous version

@knayakgl @echui-gitlab I had to make a small change to make sure this builds properly on MacOS. I took the approach of only building bundled Git with unsafe SHA1 on the Linux platform as MacOS does not have the require openSSL library. Let me know what you think.

requested review from @echui-gitlab

requested review from @knayakgl and removed approval

unapproved this merge request

requested review from @toon and removed review request for @knayakgl

approved this merge request

mentioned in epic gitlab-org/data-access/git#2

resolved all threads

approved this merge request

started a merge train

mentioned in epic gitlab-org/data-access/git&2

mentioned in epic gitlab-org/data-access/git&1

removed this merge request from the merge train because the pipeline did not succeed. Learn more.

added 27 commits

• 4e8ab4b4...70dee0e4 - 25 commits from branch master
• 8e9d4424 - Makefile: Build Git with non-collision-detecting SHA1
• e41f47d5 - Revert "Makefile: Build Git with non-collision-detecting SHA1"

Compare with previous version

Reviewer roulette

Changes that require review have been detected! A merge request is normally reviewed by both a reviewer and a maintainer in its primary category and by a maintainer in all other categories.

To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.

To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.

Once you've decided who will review this merge request, mention them as you normally would! Danger does not automatically notify them for you.

Category	Reviewer	Maintainer
None	@divya_gitlab (UTC+5.5, 11.5 hours ahead of author)	@echui-gitlab (UTC+11, 17 hours ahead of author)

If needed, you can retry the danger-review job that generated this comment.

Generated by Danger

started a merge train

removed this merge request from the merge train because the pipeline did not succeed. Learn more.

enabled automatic add to merge train when checks pass

started a merge train

removed this merge request from the merge train because the pipeline did not succeed. Learn more.

enabled automatic add to merge train when checks pass

started a merge train

merged

mentioned in commit 18529fc1

Thanks @pks-gitlab ! I'm still unsure why this was so difficult, but I'm happy it's merged now.

added workflowstaging-canary label

added workflowcanary label and removed workflowstaging-canary label

added workflowstaging label and removed workflowcanary label

added workflowproduction label and removed workflowstaging label

added releasedcandidate label

added releasedpublished label and removed releasedcandidate label

added devopsdata_access label and removed devopssystems label