Git v2.13.1 SHA1 very broken

Git v2.13.1 SHA1 very broken

[Adam Dinwoodie]

I'm trying to compile Git v2.13.1 to release for Cygwin, but it appears
a010391 ("sha1dc: update from upstream", 2017-05-20) is breaking a very
significant number of test cases in both 32-bit and 64-bit Cygwin
builds.

The first failure is t0000.46 "validate object ID of a known tree"; output with
-x and -v is below, although it's not very interesting:

    expecting success:
	    test "$tree" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a

    ++ test ceb282701536fe61bea01075664405caa7d6343f = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a
    + test_eval_ret_=1
    + want_trace
    + test t = t
    + test t = t
    + set +x
    error: last command exited with $?=1
    not ok 46 - validate object ID of a known tree
    #
    #               test "$tree" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a
    #

I have no idea where to even begin debugging this, but I'm happy to take
pointers / try things out on my box.

Cheers,

Adam

Re: Git v2.13.1 SHA1 very broken

[Ævar Arnfjörð Bjarmason]

On Mon, Jun 5, 2017 at 10:34 PM, Adam Dinwoodie <adam@dinwoodie.org> wrote:
> I'm trying to compile Git v2.13.1 to release for Cygwin, but it appears
> a010391 ("sha1dc: update from upstream", 2017-05-20) is breaking a very
> significant number of test cases in both 32-bit and 64-bit Cygwin
> builds.
>
> The first failure is t0000.46 "validate object ID of a known tree"; output with
> -x and -v is below, although it's not very interesting:
>
>     expecting success:
>             test "$tree" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a
>
>     ++ test ceb282701536fe61bea01075664405caa7d6343f = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a
>     + test_eval_ret_=1
>     + want_trace
>     + test t = t
>     + test t = t
>     + set +x
>     error: last command exited with $?=1
>     not ok 46 - validate object ID of a known tree
>     #
>     #               test "$tree" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a
>     #
>
> I have no idea where to even begin debugging this, but I'm happy to take
> pointers / try things out on my box.

That looks scary, can you please comment out this:

    #define SHA1DC_ALLOW_UNALIGNED_ACCESS

In sha1dc/sha1.c and see if that helps, alternatively comment out the
ifdefs guarded by "#ifdef _MSC_VER" calls in sha1dc/sha1.c

The functional differences between 2.13.0 and 2.13.1 on that platform
should be none aside from possibly those changes, unless I've missed
something.

Re: Git v2.13.1 SHA1 very broken

[Ramsay Jones]

On 05/06/17 22:05, Ævar Arnfjörð Bjarmason wrote:
> On Mon, Jun 5, 2017 at 10:34 PM, Adam Dinwoodie <adam@dinwoodie.org> wrote:
>> I'm trying to compile Git v2.13.1 to release for Cygwin, but it appears
>> a010391 ("sha1dc: update from upstream", 2017-05-20) is breaking a very
>> significant number of test cases in both 32-bit and 64-bit Cygwin
>> builds.
>>
>> The first failure is t0000.46 "validate object ID of a known tree"; output with
>> -x and -v is below, although it's not very interesting:
>>
>>     expecting success:
>>             test "$tree" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a
>>
>>     ++ test ceb282701536fe61bea01075664405caa7d6343f = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a
>>     + test_eval_ret_=1
>>     + want_trace
>>     + test t = t
>>     + test t = t
>>     + set +x
>>     error: last command exited with $?=1
>>     not ok 46 - validate object ID of a known tree
>>     #
>>     #               test "$tree" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a
>>     #
>>
>> I have no idea where to even begin debugging this, but I'm happy to take
>> pointers / try things out on my box.
> 
> That looks scary, can you please comment out this:
> 
>     #define SHA1DC_ALLOW_UNALIGNED_ACCESS

No, that doesn't fix it.

> 
> In sha1dc/sha1.c and see if that helps, alternatively comment out the
> ifdefs guarded by "#ifdef _MSC_VER" calls in sha1dc/sha1.c

This can't possibly make a difference! ;-)

However, rebuilding with:

    $ make OPENSSL_SHA1=YesPlease >out2 2>&1

... make t0000-basic.sh pass just fine, so ...

ATB,
Ramsay Jones

Re: Git v2.13.1 SHA1 very broken

[Ramsay Jones]

On 06/06/17 00:20, Ramsay Jones wrote:
> 
> 
> On 05/06/17 22:05, Ævar Arnfjörð Bjarmason wrote:
>> On Mon, Jun 5, 2017 at 10:34 PM, Adam Dinwoodie <adam@dinwoodie.org> wrote:
>>> I'm trying to compile Git v2.13.1 to release for Cygwin, but it appears
>>> a010391 ("sha1dc: update from upstream", 2017-05-20) is breaking a very
>>> significant number of test cases in both 32-bit and 64-bit Cygwin
>>> builds.
>>>
>>> The first failure is t0000.46 "validate object ID of a known tree"; output with
>>> -x and -v is below, although it's not very interesting:
>>>
>>>     expecting success:
>>>             test "$tree" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a
>>>
>>>     ++ test ceb282701536fe61bea01075664405caa7d6343f = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a
>>>     + test_eval_ret_=1
>>>     + want_trace
>>>     + test t = t
>>>     + test t = t
>>>     + set +x
>>>     error: last command exited with $?=1
>>>     not ok 46 - validate object ID of a known tree
>>>     #
>>>     #               test "$tree" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a
>>>     #
>>>
>>> I have no idea where to even begin debugging this, but I'm happy to take
>>> pointers / try things out on my box.
>>
>> That looks scary, can you please comment out this:
>>
>>     #define SHA1DC_ALLOW_UNALIGNED_ACCESS
> 
> No, that doesn't fix it.
> 
>>
>> In sha1dc/sha1.c and see if that helps, alternatively comment out the
>> ifdefs guarded by "#ifdef _MSC_VER" calls in sha1dc/sha1.c
> 
> This can't possibly make a difference! ;-)
> 
> However, rebuilding with:
> 
>     $ make OPENSSL_SHA1=YesPlease >out2 2>&1
> 
> ... make t0000-basic.sh pass just fine, so ...

commit 7e71542e8b ("sha1dc: avoid CPP macro collisions", 25-03-2017)
runs t0000-basic.sh just fine.

commit a0103914c2 ("sha1dc: update from upstream", 20-05-2017) fails
when running t0000-basic.sh.

I will look into this more tomorrow (it is late, I need sleep), unless
someone finds the solution overnight, of course.

Adam, you can build using the OPENSSL_SHA1 build variable for now
(if you want to release v2.13.1), or wait for another maint release
I suppose, ... I'll leave that to you! ;-)

ATB,
Ramsay Jones

Re: Git v2.13.1 SHA1 very broken

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> That looks scary, can you please comment out this:
>
>     #define SHA1DC_ALLOW_UNALIGNED_ACCESS
>
> In sha1dc/sha1.c and see if that helps, alternatively comment out the
> ifdefs guarded by "#ifdef _MSC_VER" calls in sha1dc/sha1.c

That is merely a performance (and theoretical correctness) thing,
no?

> The functional differences between 2.13.0 and 2.13.1 on that platform
> should be none aside from possibly those changes, unless I've missed
> something.

If it does not hash correctly, the cause is more likely that the
endianness detection is going haywire.

    make CFLAGS="-DSHA1DC_FORCE_LITTLEENDIAN -g -O2 -Wall"

or something like that, perhaps?

Re: Git v2.13.1 SHA1 very broken

[Adam Dinwoodie]

On Tue, Jun 06, 2017 at 10:20:45AM +0900, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> 
> > That looks scary, can you please comment out this:
> >
> >     #define SHA1DC_ALLOW_UNALIGNED_ACCESS
> >
> > In sha1dc/sha1.c and see if that helps, alternatively comment out the
> > ifdefs guarded by "#ifdef _MSC_VER" calls in sha1dc/sha1.c
> 
> That is merely a performance (and theoretical correctness) thing,
> no?

Confirmed rebuilding with either of these suggested changes has t0000.46
still failing.

> > The functional differences between 2.13.0 and 2.13.1 on that platform
> > should be none aside from possibly those changes, unless I've missed
> > something.
> 
> If it does not hash correctly, the cause is more likely that the
> endianness detection is going haywire.
> 
>     make CFLAGS="-DSHA1DC_FORCE_LITTLEENDIAN -g -O2 -Wall"

Confirmed rebuilding with this option has t0000 passing.  I can also get
the test to pass with Ramsay Jones' suggestion of using OpenSSL's SHA1.

Digging briefly into the endianness detection, it appears Cygwin has
both _LITTLE_ENDIAN and _BIG_ENDIAN defined.  Git's detection works by
assuming it's in a little endian environment and switching to big endian
if it detects any of the defines that indicate such, and a010391 adds
_BIG_ENDIAN to the set of defines that indicate big endianness.

The obvious quick fix would be one of the two below patches.  I'll also
take the discussion to the Cygwin mailing list in the hope that someone
can explain why Cygwin defines both _LITTLE_ENDIAN and _BIG_ENDIAN (and
indeed _PDP_ENDIAN).

Patch 1 (probably safer?):

diff --git a/sha1dc/sha1.c b/sha1dc/sha1.c
index 3dff80ac7..e47d004b1 100644
--- a/sha1dc/sha1.c
+++ b/sha1dc/sha1.c
@@ -36,6 +36,7 @@
 #undef SHA1DC_BIGENDIAN
 #endif
 #if (!defined SHA1DC_FORCE_LITTLEENDIAN) && \
+    (!defined _LITTLE_ENDIAN) && \
     ((defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) || \
     (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __BIG_ENDIAN__)) || \
     defined(_BIG_ENDIAN) || defined(__BIG_ENDIAN__) || defined(__ARMEB__) || defined(__THUMBEB__) ||  defined(__AARCH64EB__) || \

Patch 2:

diff --git a/sha1dc/sha1.c b/sha1dc/sha1.c
index 3dff80ac7..8d7b1ee7d 100644
--- a/sha1dc/sha1.c
+++ b/sha1dc/sha1.c
@@ -38,7 +38,7 @@
 #if (!defined SHA1DC_FORCE_LITTLEENDIAN) && \
     ((defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) || \
     (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __BIG_ENDIAN__)) || \
-    defined(_BIG_ENDIAN) || defined(__BIG_ENDIAN__) || defined(__ARMEB__) || defined(__THUMBEB__) ||  defined(__AARCH64EB__) || \
+    defined(__BIG_ENDIAN__) || defined(__ARMEB__) || defined(__THUMBEB__) ||  defined(__AARCH64EB__) || \
     defined(_MIPSEB) || defined(__MIPSEB) || defined(__MIPSEB__) || defined(SHA1DC_FORCE_BIGENDIAN))

 #define SHA1DC_BIGENDIAN

Re: Git v2.13.1 SHA1 very broken

[Junio C Hamano]

Adam Dinwoodie <adam@dinwoodie.org> writes:

> Digging briefly into the endianness detection, it appears Cygwin has
> both _LITTLE_ENDIAN and _BIG_ENDIAN defined.  Git's detection works by
> assuming it's in a little endian environment and switching to big endian
> if it detects any of the defines that indicate such, and a010391 adds
> _BIG_ENDIAN to the set of defines that indicate big endianness.

I suspect that the upstream has already fixed this one to cope with
FreeBSD.  My preference is that we do another import on top of the
ab/sha1dc-maint topic, below the commit on ab/sha1dc that adds the
upstream as a submodule.

Re: Git v2.13.1 SHA1 very broken

[Adam Dinwoodie]

On Tue, Jun 06, 2017 at 08:55:04PM +0900, Junio C Hamano wrote:
> Adam Dinwoodie <adam@dinwoodie.org> writes:
> 
> > Digging briefly into the endianness detection, it appears Cygwin has
> > both _LITTLE_ENDIAN and _BIG_ENDIAN defined.  Git's detection works by
> > assuming it's in a little endian environment and switching to big endian
> > if it detects any of the defines that indicate such, and a010391 adds
> > _BIG_ENDIAN to the set of defines that indicate big endianness.
> 
> I suspect that the upstream has already fixed this one to cope with
> FreeBSD.  My preference is that we do another import on top of the
> ab/sha1dc-maint topic, below the commit on ab/sha1dc that adds the
> upstream as a submodule.

Apparently so!  a010391 brings Git up to the upstream's cc46554 ("Skip
temporary variable for SHA1DC_ALLOW_UNSIGNED_ACCESS", 2017-05-18); the
problem has been fixed in upstream's a24eef5 ("rewrote Endianness
selection", 2017-05-29).

In the interim, I'll use the CFLAGS route to try to get a v2.13.1 build
ready to release for Cygwin.

Continous Integration (was: RE: Git v2.13.1 SHA1 very broken)

[Jason Pyeron]

Do we have Jenkins (or something else) setup for Git?

We would be happy to donate (slave) VMs for cygwin builds og Git.  

-Jason Pyeron

Re: Continous Integration (was: RE: Git v2.13.1 SHA1 very broken)

[Lars Schneider]

> On 06 Jun 2017, at 16:47, Jason Pyeron <jpyeron@pdinc.us> wrote:
> 
> Do we have Jenkins (or something else) setup for Git?
> 
> We would be happy to donate (slave) VMs for cygwin builds og Git.  
> 
> -Jason Pyeron
> 

We use TravisCI for Linux, Mac, and (in a special way) Windows: 
https://travis-ci.org/git/git

Windows is not supported by TravisCI. We just use TravisCI to
trigger a build on MS Azure and read the results:
https://github.com/git/git/commit/029aeeed55f6bfe8014e8ffe5fc7a6f2e5b110fc

Maybe we could trigger a Cgywin build on your slaves in the same way?

- Lars

Re: Continous Integration (was: RE: Git v2.13.1 SHA1 very broken)

[Adam Dinwoodie]

On Tue, Jun 06, 2017 at 05:04:32PM +0200, Lars Schneider wrote:
> > On 06 Jun 2017, at 16:47, Jason Pyeron <jpyeron@pdinc.us> wrote:
> > 
> > Do we have Jenkins (or something else) setup for Git?
> > 
> > We would be happy to donate (slave) VMs for cygwin builds og Git.  
> > 
> > -Jason Pyeron
> > 
> 
> We use TravisCI for Linux, Mac, and (in a special way) Windows: 
> https://travis-ci.org/git/git
> 
> Windows is not supported by TravisCI. We just use TravisCI to
> trigger a build on MS Azure and read the results:
> https://github.com/git/git/commit/029aeeed55f6bfe8014e8ffe5fc7a6f2e5b110fc
> 
> Maybe we could trigger a Cgywin build on your slaves in the same way?

I tried setting up a Cygwin Git build on AppVeyor a little while ago,
but the problem I found is that Cygwin builds are slow, particularly if
you want to also run the test suite.  I do the builds for the Cygwin
distribution on my normal PC (so reasonably powerful but definitely not
devoted to the purpose), and doing the build and running the default
tests takes in the region of 8 hours for the 64-bit build and 12 hours
for the 32-bit build.

At the moment, I'm trying to set up automated regular builds on my PC
using BuildBot.  I think that, short of someone donating some fairly
significant resources for Cygwin builds, that's going to be the closest
I'll be able to find for spotting problems early.  It's a project in my
currently limited spare time, though, and not something I've done
before, so it's taking a little while to get going.

Adam

Re: Continous Integration (was: RE: Git v2.13.1 SHA1 very broken)

[Johannes Schindelin]

Hi Adam,

On Sun, 2 Jul 2017, Adam Dinwoodie wrote:

> I do the builds for the Cygwin distribution on my normal PC (so
> reasonably powerful but definitely not devoted to the purpose), and
> doing the build and running the default tests takes in the region of 8
> hours for the 64-bit build and 12 hours for the 32-bit build.

Wow. 8 hours. I take it that you are bitten by the same issue as Git for
Windows, where Git's test suite uses Unix shell scripting rather heavily,
and Unix shell simply requiring tons of expensive emulation to run
decently on Windows.

> At the moment, I'm trying to set up automated regular builds on my PC
> using BuildBot.  I think that, short of someone donating some fairly
> significant resources for Cygwin builds, that's going to be the closest
> I'll be able to find for spotting problems early.  It's a project in my
> currently limited spare time, though, and not something I've done
> before, so it's taking a little while to get going.

How automated is your process? (Including, most importantly, updating
the Cygwin build setup...)

I ask because I could possibly set up something using the existing
infrastructure for Git for Windows' testing, as long as

1) everything is scripted, and

2) you do not need interactive access to any box (I mainly use Docker
   containers to run the builds)

If you are interested, feel free to ping me via private mail.

Ciao,
Johannes

[PATCH 0/3] update sha1dc

[Ævar Arnfjörð Bjarmason]

This updates sha1dc fixing the issue on Cygwin introduced in 2.13.1,
and hopefully not regressing elsewhere. Liam, it would be much
appreciated if you could test this on SPARC.

As before the "sha1dc: update from upstream" patch is what should
fast-track to master/maint and be in 2.13.2, the other two are the
cooking submodule use, that's all unchanged aside from of course the
submodule pointing to the same upstream commit as the code import
itself does.

Junio: There's a whitespace change to sha1.h that am warns about, but
which it applies anyway that you didn't apply from my previous
patch. I think it probably makes sense to just take upstream's
whitespace shenanigans as-is instead of seeing that diff every time we
update. I guess we could also send them a pull request...

Junio C Hamano (1):
  sha1collisiondetection: automatically enable when submodule is
    populated

Ævar Arnfjörð Bjarmason (2):
  sha1dc: update from upstream
  sha1dc: optionally use sha1collisiondetection as a submodule

 .gitmodules            |  4 ++++
 Makefile               | 16 ++++++++++++++++
 hash.h                 |  4 ++++
 sha1collisiondetection |  1 +
 sha1dc/sha1.c          | 30 ++++++++++++++++++++++++------
 sha1dc/sha1.h          |  6 +++---
 6 files changed, 52 insertions(+), 9 deletions(-)
 create mode 100644 .gitmodules
 create mode 160000 sha1collisiondetection

-- 
2.13.0.506.g27d5fe0cd

[PATCH 1/3] sha1dc: update from upstream

[Ævar Arnfjörð Bjarmason]

Update sha1dc from the latest version by the upstream
maintainer[1].

See commit a0103914c2 ("sha1dc: update from upstream", 2017-05-20) for
the latest update. That update was done sans some whitespace changes
by upstream, which is why the diff here isn't the same as the upstream
cc46554..e139984.

It also brings in a change[2] upstream made which should hopefully
address the breakage in 2.13.1 on Cygwin, see [3]. Cygwin defines both
_BIG_ENDIAN and _LITTLE_ENDIAN.

Adam Dinwoodie reports on the mailing list that that upstream commit
fixes the issue on Cygwin[4].

1. https://github.com/cr-marcstevens/sha1collisiondetection/commit/e1399840b501a68ac6c8d7ed9a5cb1455480200e
2. https://github.com/cr-marcstevens/sha1collisiondetection/commit/a24eef58c0684078405f8c7a89f9b78271432005
3. <20170606100355.GC25777@dinwoodie.org> (https://public-inbox.org/git/20170606100355.GC25777@dinwoodie.org/)
4. <20170606124323.GD25777@dinwoodie.org> (https://public-inbox.org/git/20170606124323.GD25777@dinwoodie.org/)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 sha1dc/sha1.c | 30 ++++++++++++++++++++++++------
 sha1dc/sha1.h |  6 +++---
 2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/sha1dc/sha1.c b/sha1dc/sha1.c
index 3dff80ac72..facea1bb56 100644
--- a/sha1dc/sha1.c
+++ b/sha1dc/sha1.c
@@ -35,15 +35,33 @@
 #ifdef SHA1DC_BIGENDIAN
 #undef SHA1DC_BIGENDIAN
 #endif
-#if (!defined SHA1DC_FORCE_LITTLEENDIAN) && \
-    ((defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) || \
-    (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __BIG_ENDIAN__)) || \
-    defined(_BIG_ENDIAN) || defined(__BIG_ENDIAN__) || defined(__ARMEB__) || defined(__THUMBEB__) ||  defined(__AARCH64EB__) || \
-    defined(_MIPSEB) || defined(__MIPSEB) || defined(__MIPSEB__) || defined(SHA1DC_FORCE_BIGENDIAN))
 
+#if (defined(_BYTE_ORDER) || defined(__BYTE_ORDER) || defined(__BYTE_ORDER__))
+
+#if ((defined(_BYTE_ORDER) && (_BYTE_ORDER == _BIG_ENDIAN)) || \
+     (defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) || \
+     (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __BIG_ENDIAN__)) )
 #define SHA1DC_BIGENDIAN
+#endif
+
+#else
+
+#if (defined(_BIG_ENDIAN) || defined(__BIG_ENDIAN) || defined(__BIG_ENDIAN__) || \
+     defined(__ARMEB__) || defined(__THUMBEB__) || defined(__AARCH64EB__) || \
+     defined(__MIPSEB__) || defined(__MIPSEB) || defined(_MIPSEB) || \
+     defined(__sparc))
+#define SHA1DC_BIGENDIAN
+#endif
 
-#endif /*ENDIANNESS SELECTION*/
+#endif
+
+#if (defined(SHA1DC_FORCE_LITTLEENDIAN) && defined(SHA1DC_BIGENDIAN))
+#undef SHA1DC_BIGENDIAN
+#endif
+#if (defined(SHA1DC_FORCE_BIGENDIAN) && !defined(SHA1DC_BIGENDIAN))
+#define SHA1DC_BIGENDIAN
+#endif
+/*ENDIANNESS SELECTION*/
 
 #if (defined SHA1DC_FORCE_UNALIGNED_ACCESS || \
      defined(__amd64__) || defined(__amd64) || defined(__x86_64__) || defined(__x86_64) || \
diff --git a/sha1dc/sha1.h b/sha1dc/sha1.h
index a0ff5d1305..1e4e94be54 100644
--- a/sha1dc/sha1.h
+++ b/sha1dc/sha1.h
@@ -61,9 +61,9 @@ void SHA1DCInit(SHA1_CTX*);
     Function to enable safe SHA-1 hashing:
     Collision attacks are thwarted by hashing a detected near-collision block 3 times.
     Think of it as extending SHA-1 from 80-steps to 240-steps for such blocks:
-	The best collision attacks against SHA-1 have complexity about 2^60,
-	thus for 240-steps an immediate lower-bound for the best cryptanalytic attacks would be 2^180.
-	An attacker would be better off using a generic birthday search of complexity 2^80.
+        The best collision attacks against SHA-1 have complexity about 2^60,
+        thus for 240-steps an immediate lower-bound for the best cryptanalytic attacks would be 2^180.
+        An attacker would be better off using a generic birthday search of complexity 2^80.
 
    Enabling safe SHA-1 hashing will result in the correct SHA-1 hash for messages where no collision attack was detected,
    but it will result in a different SHA-1 hash for messages where a collision attack was detected.
-- 
2.13.0.506.g27d5fe0cd

[PATCH 2/3] sha1dc: optionally use sha1collisiondetection as a submodule

[Ævar Arnfjörð Bjarmason]

Add an option to use the sha1collisiondetection library from the
submodule in sha1collisiondetection/ instead of in the copy in the
sha1dc/ directory.

This allows us to try out the submodule in sha1collisiondetection
without breaking the build for anyone who's not expecting them as we
work out any kinks.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .gitmodules            |  4 ++++
 Makefile               | 12 ++++++++++++
 hash.h                 |  4 ++++
 sha1collisiondetection |  1 +
 4 files changed, 21 insertions(+)
 create mode 100644 .gitmodules
 create mode 160000 sha1collisiondetection

diff --git a/.gitmodules b/.gitmodules
new file mode 100644
index 0000000000..cbeebdab7a
--- /dev/null
+++ b/.gitmodules
@@ -0,0 +1,4 @@
+[submodule "sha1collisiondetection"]
+	path = sha1collisiondetection
+	url = https://github.com/cr-marcstevens/sha1collisiondetection.git
+	branch = master
diff --git a/Makefile b/Makefile
index 7c621f7f76..adeff26d57 100644
--- a/Makefile
+++ b/Makefile
@@ -146,6 +146,12 @@ all::
 # algorithm. This is slower, but may detect attempted collision attacks.
 # Takes priority over other *_SHA1 knobs.
 #
+# Define DC_SHA1_SUBMODULE in addition to DC_SHA1 to use the
+# sha1collisiondetection shipped as a submodule instead of the
+# non-submodule copy in sha1dc/. This is an experimental option used
+# by the git project to migrate to using sha1collisiondetection as a
+# submodule.
+#
 # Define OPENSSL_SHA1 environment variable when running make to link
 # with the SHA1 routine from openssl library.
 #
@@ -1416,8 +1422,14 @@ ifdef APPLE_COMMON_CRYPTO
 	BASIC_CFLAGS += -DSHA1_APPLE
 else
 	DC_SHA1 := YesPlease
+ifdef DC_SHA1_SUBMODULE
+	LIB_OBJS += sha1collisiondetection/lib/sha1.o
+	LIB_OBJS += sha1collisiondetection/lib/ubc_check.o
+	BASIC_CFLAGS += -DDC_SHA1_SUBMODULE
+else
 	LIB_OBJS += sha1dc/sha1.o
 	LIB_OBJS += sha1dc/ubc_check.o
+endif
 	BASIC_CFLAGS += \
 		-DSHA1_DC \
 		-DSHA1DC_NO_STANDARD_INCLUDES \
diff --git a/hash.h b/hash.h
index a11fc9233f..bef3e630a0 100644
--- a/hash.h
+++ b/hash.h
@@ -8,7 +8,11 @@
 #elif defined(SHA1_OPENSSL)
 #include <openssl/sha.h>
 #elif defined(SHA1_DC)
+#ifdef DC_SHA1_SUBMODULE
+#include "sha1collisiondetection/lib/sha1.h"
+#else
 #include "sha1dc/sha1.h"
+#endif
 #else /* SHA1_BLK */
 #include "block-sha1/sha1.h"
 #endif
diff --git a/sha1collisiondetection b/sha1collisiondetection
new file mode 160000
index 0000000000..e1399840b5
--- /dev/null
+++ b/sha1collisiondetection
@@ -0,0 +1 @@
+Subproject commit e1399840b501a68ac6c8d7ed9a5cb1455480200e
-- 
2.13.0.506.g27d5fe0cd

Re: [PATCH 2/3] sha1dc: optionally use sha1collisiondetection as a submodule

[Stefan Beller]

On Tue, Jun 6, 2017 at 8:12 AM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> Add an option to use the sha1collisiondetection library from the
> submodule in sha1collisiondetection/ instead of in the copy in the
> sha1dc/ directory.
>
> This allows us to try out the submodule in sha1collisiondetection
> without breaking the build for anyone who's not expecting them as we
> work out any kinks.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

Other projects using submodules sometimes have
a .gitattributes entry to have .gitmodules not exported
via git-archive. Do we want a similar thing?

Speaking of attributes, I wonder if we want to specify
the .gitmodules file to be text with unixy file endings:
Having an entry
    .gitattributes eol=crlf
to simulate a Windows environment doesn't harm
submodule operation, which is good. I'll check if we
have a test for that.

Re: [PATCH 2/3] sha1dc: optionally use sha1collisiondetection as a submodule

[Ævar Arnfjörð Bjarmason]

On Tue, Jun 6, 2017 at 8:48 PM, Stefan Beller <sbeller@google.com> wrote:
> On Tue, Jun 6, 2017 at 8:12 AM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> Add an option to use the sha1collisiondetection library from the
>> submodule in sha1collisiondetection/ instead of in the copy in the
>> sha1dc/ directory.
>>
>> This allows us to try out the submodule in sha1collisiondetection
>> without breaking the build for anyone who's not expecting them as we
>> work out any kinks.
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>
> Other projects using submodules sometimes have
> a .gitattributes entry to have .gitmodules not exported
> via git-archive. Do we want a similar thing?

Right now we end up with an empty directory due to the issue you noted
in https://public-inbox.org/git/CAGZ79kZC98CxA69QjmX2s_SU6z1CSgKgwZeqvwiMRAQc6+S3xg@mail.gmail.com/

It's probably best to have the .gitmodules file as some hint that
something should be there. We also ship the other .git* files.

> Speaking of attributes, I wonder if we want to specify
> the .gitmodules file to be text with unixy file endings:
> Having an entry
>     .gitattributes eol=crlf
> to simulate a Windows environment doesn't harm
> submodule operation, which is good. I'll check if we
> have a test for that.

I have no idea what that would do or why we'd have it, but I'm going
to understand this as you looking into it :)

Re: [PATCH 2/3] sha1dc: optionally use sha1collisiondetection as a submodule

[Stefan Beller]

On Tue, Jun 6, 2017 at 12:03 PM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Tue, Jun 6, 2017 at 8:48 PM, Stefan Beller <sbeller@google.com> wrote:
>> On Tue, Jun 6, 2017 at 8:12 AM, Ævar Arnfjörð Bjarmason
>> <avarab@gmail.com> wrote:
>>> Add an option to use the sha1collisiondetection library from the
>>> submodule in sha1collisiondetection/ instead of in the copy in the
>>> sha1dc/ directory.
>>>
>>> This allows us to try out the submodule in sha1collisiondetection
>>> without breaking the build for anyone who's not expecting them as we
>>> work out any kinks.
>>>
>>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>>
>> Other projects using submodules sometimes have
>> a .gitattributes entry to have .gitmodules not exported
>> via git-archive. Do we want a similar thing?
>
> Right now we end up with an empty directory due to the issue you noted
> in https://public-inbox.org/git/CAGZ79kZC98CxA69QjmX2s_SU6z1CSgKgwZeqvwiMRAQc6+S3xg@mail.gmail.com/
>
> It's probably best to have the .gitmodules file as some hint that
> something should be there. We also ship the other .git* files.

Ok, but then let's talk about the other .git* files, would we want to
distribute these via tarballs? (I guess it is a minor thing if at all and
nobody downloading a git tarball would be surprised by these metadata
files or annoyed by them, so all is good?)

>
>> Speaking of attributes, I wonder if we want to specify
>> the .gitmodules file to be text with unixy file endings:
>> Having an entry
>>     .gitattributes eol=crlf
>> to simulate a Windows environment doesn't harm
>> submodule operation, which is good. I'll check if we
>> have a test for that.
>
> I have no idea what that would do or why we'd have it, but I'm going
> to understand this as you looking into it :)

I looked briefly into it and it seems to be no problem just as config files
on Windows are no problem. I just spoke up too quickly.

[PATCH 3/3] sha1collisiondetection: automatically enable when submodule is populated

[Ævar Arnfjörð Bjarmason]

From: Junio C Hamano <gitster@pobox.com>

If a user wants to experiment with the version of collision
detecting sha1 from the submodule, the user needed to not just
populate the submodule but also needed to turn the knob.

A Makefile trick is easy enough to do so, so let's do this.  When
somebody with a copy of the submodule populated wants not to use it,
that can be done by overriding it in config.mak or from the command
line.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Makefile b/Makefile
index adeff26d57..eeccbff1cd 100644
--- a/Makefile
+++ b/Makefile
@@ -993,6 +993,10 @@ EXTLIBS =
 
 GIT_USER_AGENT = git/$(GIT_VERSION)
 
+ifeq ($(wildcard sha1collisiondetection/lib/sha1.h),sha1collisiondetection/lib/sha1.h)
+DC_SHA1_SUBMODULE = auto
+endif
+
 include config.mak.uname
 -include config.mak.autogen
 -include config.mak
-- 
2.13.0.506.g27d5fe0cd

Re: [PATCH 0/3] update sha1dc

[Stefan Beller]

On Tue, Jun 6, 2017 at 8:12 AM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> This updates sha1dc fixing the issue on Cygwin introduced in 2.13.1,
> and hopefully not regressing elsewhere. Liam, it would be much
> appreciated if you could test this on SPARC.
>
> As before the "sha1dc: update from upstream" patch is what should
> fast-track to master/maint and be in 2.13.2, the other two are the
> cooking submodule use, that's all unchanged aside from of course the
> submodule pointing to the same upstream commit as the code import
> itself does.
>
> Junio: There's a whitespace change to sha1.h that am warns about, but
> which it applies anyway that you didn't apply from my previous
> patch. I think it probably makes sense to just take upstream's
> whitespace shenanigans as-is instead of seeing that diff every time we
> update. I guess we could also send them a pull request...

I would suggest the pull request.

Also as to not make the mistake from before that I jump on the
submodule bandwagon here:
Patch 1 ought to go in its on series/patch, so with that out the way
we have more time to consider the pros and cons of the rest of
the series?

Thanks,
Stefan

Re: [PATCH 0/3] update sha1dc

[Ævar Arnfjörð Bjarmason]

On Tue, Jun 6, 2017 at 8:23 PM, Stefan Beller <sbeller@google.com> wrote:
> On Tue, Jun 6, 2017 at 8:12 AM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> This updates sha1dc fixing the issue on Cygwin introduced in 2.13.1,
>> and hopefully not regressing elsewhere. Liam, it would be much
>> appreciated if you could test this on SPARC.
>>
>> As before the "sha1dc: update from upstream" patch is what should
>> fast-track to master/maint and be in 2.13.2, the other two are the
>> cooking submodule use, that's all unchanged aside from of course the
>> submodule pointing to the same upstream commit as the code import
>> itself does.
>>
>> Junio: There's a whitespace change to sha1.h that am warns about, but
>> which it applies anyway that you didn't apply from my previous
>> patch. I think it probably makes sense to just take upstream's
>> whitespace shenanigans as-is instead of seeing that diff every time we
>> update. I guess we could also send them a pull request...
>
> I would suggest the pull request.

Looking at this again it's not a bug, just upstream choosing to indent
a comment with spaces, not a bug.

So it makes sense to just apply as-is so we don't have that diff with
them / different sha1s on the files etc.

> Also as to not make the mistake from before that I jump on the
> submodule bandwagon here:
> Patch 1 ought to go in its on series/patch, so with that out the way
> we have more time to consider the pros and cons of the rest of
> the series?

Yes it makes perfect sense to just take the 1st patch here and make
the submodule changes cook. This is just how I submitted it the last
time and Junio took the 1st patch into a maint topic, so I figured I'd
send it like this again.

[PATCH] sha1dc: ignore indent-with-non-tab whitespace violations

[Jeff King]

On Tue, Jun 06, 2017 at 08:51:35PM +0200, Ævar Arnfjörð Bjarmason wrote:

> On Tue, Jun 6, 2017 at 8:23 PM, Stefan Beller <sbeller@google.com> wrote:
> > On Tue, Jun 6, 2017 at 8:12 AM, Ævar Arnfjörð Bjarmason
> > <avarab@gmail.com> wrote:
> >> This updates sha1dc fixing the issue on Cygwin introduced in 2.13.1,
> >> and hopefully not regressing elsewhere. Liam, it would be much
> >> appreciated if you could test this on SPARC.
> >>
> >> As before the "sha1dc: update from upstream" patch is what should
> >> fast-track to master/maint and be in 2.13.2, the other two are the
> >> cooking submodule use, that's all unchanged aside from of course the
> >> submodule pointing to the same upstream commit as the code import
> >> itself does.
> >>
> >> Junio: There's a whitespace change to sha1.h that am warns about, but
> >> which it applies anyway that you didn't apply from my previous
> >> patch. I think it probably makes sense to just take upstream's
> >> whitespace shenanigans as-is instead of seeing that diff every time we
> >> update. I guess we could also send them a pull request...
> >
> > I would suggest the pull request.
> 
> Looking at this again it's not a bug, just upstream choosing to indent
> a comment with spaces, not a bug.
> 
> So it makes sense to just apply as-is so we don't have that diff with
> them / different sha1s on the files etc.

Agreed. Maybe we'd also want this patch:

-- >8 --
Subject: sha1dc: ignore indent-with-non-tab whitespace violations

The upstream sha1dc code indents some lines with spaces.
While this doesn't match Git's coding guidelines, it's better
to leave this imported code untouched than to try to make it
match our style. However, we can use .gitattributes to tell
"diff --check" and "git am" not to bother us about it.

Signed-off-by: Jeff King <peff@peff.net>
---
 sha1dc/.gitattributes | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 sha1dc/.gitattributes

diff --git a/sha1dc/.gitattributes b/sha1dc/.gitattributes
new file mode 100644
index 000000000..da53f4054
--- /dev/null
+++ b/sha1dc/.gitattributes
@@ -0,0 +1 @@
+* whitespace=-indent-with-non-tab
-- 
2.13.1.664.g1b5a21ec3

Re: [PATCH] sha1dc: ignore indent-with-non-tab whitespace violations

[Ævar Arnfjörð Bjarmason]

On Tue, Jun 6, 2017 at 9:01 PM, Jeff King <peff@peff.net> wrote:
> On Tue, Jun 06, 2017 at 08:51:35PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> On Tue, Jun 6, 2017 at 8:23 PM, Stefan Beller <sbeller@google.com> wrote:
>> > On Tue, Jun 6, 2017 at 8:12 AM, Ævar Arnfjörð Bjarmason
>> > <avarab@gmail.com> wrote:
>> >> This updates sha1dc fixing the issue on Cygwin introduced in 2.13.1,
>> >> and hopefully not regressing elsewhere. Liam, it would be much
>> >> appreciated if you could test this on SPARC.
>> >>
>> >> As before the "sha1dc: update from upstream" patch is what should
>> >> fast-track to master/maint and be in 2.13.2, the other two are the
>> >> cooking submodule use, that's all unchanged aside from of course the
>> >> submodule pointing to the same upstream commit as the code import
>> >> itself does.
>> >>
>> >> Junio: There's a whitespace change to sha1.h that am warns about, but
>> >> which it applies anyway that you didn't apply from my previous
>> >> patch. I think it probably makes sense to just take upstream's
>> >> whitespace shenanigans as-is instead of seeing that diff every time we
>> >> update. I guess we could also send them a pull request...
>> >
>> > I would suggest the pull request.
>>
>> Looking at this again it's not a bug, just upstream choosing to indent
>> a comment with spaces, not a bug.
>>
>> So it makes sense to just apply as-is so we don't have that diff with
>> them / different sha1s on the files etc.
>
> Agreed. Maybe we'd also want this patch:

Great, that makes perfect sense for prepending to the series.

> -- >8 --
> Subject: sha1dc: ignore indent-with-non-tab whitespace violations
>
> The upstream sha1dc code indents some lines with spaces.
> While this doesn't match Git's coding guidelines, it's better
> to leave this imported code untouched than to try to make it
> match our style. However, we can use .gitattributes to tell
> "diff --check" and "git am" not to bother us about it.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  sha1dc/.gitattributes | 1 +
>  1 file changed, 1 insertion(+)
>  create mode 100644 sha1dc/.gitattributes
>
> diff --git a/sha1dc/.gitattributes b/sha1dc/.gitattributes
> new file mode 100644
> index 000000000..da53f4054
> --- /dev/null
> +++ b/sha1dc/.gitattributes
> @@ -0,0 +1 @@
> +* whitespace=-indent-with-non-tab
> --
> 2.13.1.664.g1b5a21ec3
>

Re: [PATCH] sha1dc: ignore indent-with-non-tab whitespace violations

[Stefan Beller]

> Subject: sha1dc: ignore indent-with-non-tab whitespace violations
>
> The upstream sha1dc code indents some lines with spaces.
> While this doesn't match Git's coding guidelines, it's better
> to leave this imported code untouched than to try to make it
> match our style. However, we can use .gitattributes to tell
> "diff --check" and "git am" not to bother us about it.
>
> Signed-off-by: Jeff King <peff@peff.net>

Reviewed-by: Stefan Beller <sbeller@google.com>

> ---
>  sha1dc/.gitattributes | 1 +
>  1 file changed, 1 insertion(+)
>  create mode 100644 sha1dc/.gitattributes
>
> diff --git a/sha1dc/.gitattributes b/sha1dc/.gitattributes
> new file mode 100644
> index 000000000..da53f4054
> --- /dev/null
> +++ b/sha1dc/.gitattributes
> @@ -0,0 +1 @@
> +* whitespace=-indent-with-non-tab
> --
> 2.13.1.664.g1b5a21ec3
>

Re: [PATCH 0/3] update sha1dc

[Liam R. Howlett]

* ?var Arnfj?r? Bjarmason <avarab@gmail.com> [170606 11:12]:
> This updates sha1dc fixing the issue on Cygwin introduced in 2.13.1,
> and hopefully not regressing elsewhere. Liam, it would be much
> appreciated if you could test this on SPARC.

Tested and confirmed this works for SPARC with no build flags.

Sorry for the delay, I was away last week.

Thanks,
Liam

Re: Git v2.13.1 SHA1 very broken

[Morten Welinder]

One could have configure ask some existing dependency that has already
determined the byte order.  For example:

# perl -e 'use Config; $o=$Config{byteorder}; print(($o=~/^1234/ ?
"little" : ($o=~/4321$/ ? "big" : "weird")), "\n");'
little

Good: less #ifdef soup; bad: not so great for cross-compiling.

(That's the integer byte order.  The floating-point byte order can be different;
hopefully git doesn't care.)

Morten





On Tue, Jun 6, 2017 at 7:55 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Adam Dinwoodie <adam@dinwoodie.org> writes:
>
>> Digging briefly into the endianness detection, it appears Cygwin has
>> both _LITTLE_ENDIAN and _BIG_ENDIAN defined.  Git's detection works by
>> assuming it's in a little endian environment and switching to big endian
>> if it detects any of the defines that indicate such, and a010391 adds
>> _BIG_ENDIAN to the set of defines that indicate big endianness.
>
> I suspect that the upstream has already fixed this one to cope with
> FreeBSD.  My preference is that we do another import on top of the
> ab/sha1dc-maint topic, below the commit on ab/sha1dc that adds the
> upstream as a submodule.
>

[PATCH 0/3] Update sha1dc from upstream & optionally make it a submodule

[Ævar Arnfjörð Bjarmason]

This series:

Ævar Arnfjörð Bjarmason (3):
  sha1dc: update from upstream
   
   * Fixes the Big-Endian detection on Solaris SPARC (and probably
     others) which broke the build as of 2.13.0 due to sha1dc being the
     dauflt.
   
   * Includes a patch from upstream fixing unaligned access, which broke
     SPARC even more. This replaces Junio's "[PATCH] sha1dc: fix issues
     with a big endian platform"
     (<xmqq37c4xcr6.fsf_-_@gitster.mtv.corp.google.com>) with something
     which brings in upstream as-is.
   
   * Most importantly: Uses upstream code as-is with no modifications,
     which is possible due to a pull request I sent them.

   * This patch can be picked stand-alone without [23]/3.

  sha1dc: use sha1collisiondetection as a submodule

   * Since we can now use upstream code as-is let's use it as a
     submodule.
   
     Yes there are still (solvable) UX issues with submodules, but
     there's no project better equipped to deal with them than
     git.git.

  sha1dc: remove the unused sha1dc/ directory

   * Sent as a separate patch for readability. Can be squashed into
     2/3.

 .gitmodules            |   4 +
 Makefile               |  13 +-
 hash.h                 |   2 +-
 sha1collisiondetection |   1 +
 sha1dc/LICENSE.txt     |  30 ----
 sha1dc/sha1.c          |  99 +++++++++-----
 sha1dc/sha1.h          | 122 -----------------
 sha1dc/ubc_check.c     | 363 -------------------------------------------------
 sha1dc/ubc_check.h     |  44 ------
 sha1dc_git.c           |  24 ++++
 sha1dc_git.h           |  19 +++
 11 files changed, 124 insertions(+), 597 deletions(-)
 create mode 100644 .gitmodules
 create mode 160000 sha1collisiondetection
 delete mode 100644 sha1dc/LICENSE.txt
 delete mode 100644 sha1dc/sha1.h
 delete mode 100644 sha1dc/ubc_check.c
 delete mode 100644 sha1dc/ubc_check.h
 create mode 100644 sha1dc_git.c
 create mode 100644 sha1dc_git.h

-- 
2.13.0.303.g4ebf302169

[PATCH 1/3] sha1dc: update from upstream

[Ævar Arnfjörð Bjarmason]

Update sha1dc from the latest version by the upstream
maintainer[1].

This version includes a commit of mine which allows for replacing the
local modifications done to the upstream files in git.git with macro
definitions to monkeypatch it in place.

It also brings in a change[2] upstream made for the breakage 2.13.0
introduced on SPARC and other platforms that forbid unaligned
access[3].

This means that the code customizations done since the initial import
in commit 28dc98e343 ("sha1dc: add collision-detecting sha1
implementation", 2017-03-16) can be done purely via Makefile
definitions and by including the content of our own sha1dc_git.[ch] in
sha1dc/sha1.c via a macro.

1. https://github.com/cr-marcstevens/sha1collisiondetection/commit/b45fcefc71270d9a159028c22e6d36c3817da188
2. https://github.com/cr-marcstevens/sha1collisiondetection/commit/33a694a9ee1b79c24be45f9eab5ac0e1aeeaf271
3. "Git 2.13.0 segfaults on Solaris SPARC due to DC_SHA1=YesPlease
   being on by default"
   (https://public-inbox.org/git/CACBZZX6nmKK8af0-UpjCKWV4R+hV-uk2xWXVA5U+_UQ3VXU03g@mail.gmail.com/)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile           |  9 ++++-
 sha1dc/sha1.c      | 99 +++++++++++++++++++++++++++++++++++-------------------
 sha1dc/sha1.h      | 92 ++++++++++++++++++++++----------------------------
 sha1dc/ubc_check.c | 13 +++++--
 sha1dc/ubc_check.h | 14 ++++++--
 sha1dc_git.c       | 24 +++++++++++++
 sha1dc_git.h       | 19 +++++++++++
 7 files changed, 178 insertions(+), 92 deletions(-)
 create mode 100644 sha1dc_git.c
 create mode 100644 sha1dc_git.h

diff --git a/Makefile b/Makefile
index e35542e631..ffa6da71b7 100644
--- a/Makefile
+++ b/Makefile
@@ -1414,7 +1414,14 @@ else
 	DC_SHA1 := YesPlease
 	LIB_OBJS += sha1dc/sha1.o
 	LIB_OBJS += sha1dc/ubc_check.o
-	BASIC_CFLAGS += -DSHA1_DC
+	BASIC_CFLAGS += \
+		-DSHA1_DC \
+		-DSHA1DC_NO_STANDARD_INCLUDES \
+		-DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 \
+		-DSHA1DC_CUSTOM_INCLUDE_SHA1_C="\"cache.h\"" \
+		-DSHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_C="\"sha1dc_git.c\"" \
+		-DSHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_H="\"sha1dc_git.h\"" \
+		-DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="\"git-compat-util.h\""
 endif
 endif
 endif
diff --git a/sha1dc/sha1.c b/sha1dc/sha1.c
index 35e9dd5bf4..26516b102f 100644
--- a/sha1dc/sha1.c
+++ b/sha1dc/sha1.c
@@ -5,12 +5,26 @@
 * https://opensource.org/licenses/MIT
 ***/
 
-#include "cache.h"
-#include "sha1dc/sha1.h"
-#include "sha1dc/ubc_check.h"
+#ifndef SHA1DC_NO_STANDARD_INCLUDES
+#include <string.h>
+#include <memory.h>
+#include <stdio.h>
+#include <stdlib.h>
+#endif
+
+#ifdef SHA1DC_CUSTOM_INCLUDE_SHA1_C
+#include SHA1DC_CUSTOM_INCLUDE_SHA1_C
+#endif
+
+#ifndef SHA1DC_INIT_SAFE_HASH_DEFAULT
+#define SHA1DC_INIT_SAFE_HASH_DEFAULT 1
+#endif
 
+#include "sha1.h"
+#include "ubc_check.h"
 
-/*
+
+/* 
    Because Little-Endian architectures are most common,
    we only set SHA1DC_BIGENDIAN if one of these conditions is met.
    Note that all MSFT platforms are little endian,
@@ -18,16 +32,30 @@
    If you are compiling on a big endian platform and your compiler does not define one of these,
    you will have to add whatever macros your tool chain defines to indicate Big-Endianness.
  */
-#if (defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) || \
+#ifdef SHA1DC_BIGENDIAN
+#undef SHA1DC_BIGENDIAN
+#endif
+#if (!defined SHA1DC_FORCE_LITTLEENDIAN) && \
+    ((defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) || \
     (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __BIG_ENDIAN__)) || \
-    defined(__BIG_ENDIAN__) || defined(__ARMEB__) || defined(__THUMBEB__) ||  defined(__AARCH64EB__) || \
-    defined(_MIPSEB) || defined(__MIPSEB) || defined(__MIPSEB__)
+    defined(_BIG_ENDIAN) || defined(__BIG_ENDIAN__) || defined(__ARMEB__) || defined(__THUMBEB__) ||  defined(__AARCH64EB__) || \
+    defined(_MIPSEB) || defined(__MIPSEB) || defined(__MIPSEB__) || defined(SHA1DC_FORCE_BIGENDIAN))
+
+#define SHA1DC_BIGENDIAN
 
-#define SHA1DC_BIGENDIAN	1
-#else
-#undef SHA1DC_BIGENDIAN
 #endif /*ENDIANNESS SELECTION*/
 
+#if (defined(__amd64__) || defined(__amd64) || defined(__x86_64__) || defined(__x86_64) || \
+     defined(i386) || defined(__i386) || defined(__i386__) || defined(__i486__)  || \
+     defined(__i586__) || defined(__i686__) || defined(_M_IX86) || defined(__X86__) || \
+     defined(_X86_) || defined(__THW_INTEL__) || defined(__I86__) || defined(__INTEL__) || \
+     defined(__386) || defined(_M_X64) || defined(_M_AMD64))
+
+#define SHA1DC_ALLOW_UNALIGNED_ACCESS
+
+#endif /*UNALIGNMENT DETECTION*/
+
+
 #define rotate_right(x,n) (((x)>>(n))|((x)<<(32-(n))))
 #define rotate_left(x,n)  (((x)<<(n))|((x)>>(32-(n))))
 
@@ -36,11 +64,11 @@
 
 #define sha1_mix(W, t)  (rotate_left(W[t - 3] ^ W[t - 8] ^ W[t - 14] ^ W[t - 16], 1))
 
-#if defined(SHA1DC_BIGENDIAN)
+#ifdef SHA1DC_BIGENDIAN
 	#define sha1_load(m, t, temp)  { temp = m[t]; }
 #else
 	#define sha1_load(m, t, temp)  { temp = m[t]; sha1_bswap32(temp); }
-#endif /* !defined(SHA1DC_BIGENDIAN) */
+#endif
 
 #define sha1_store(W, t, x)	*(volatile uint32_t *)&W[t] = x
 
@@ -869,6 +897,11 @@ static void sha1recompress_fast_ ## t (uint32_t ihvin[5], uint32_t ihvout[5], co
 	ihvout[0] = ihvin[0] + a; ihvout[1] = ihvin[1] + b; ihvout[2] = ihvin[2] + c; ihvout[3] = ihvin[3] + d; ihvout[4] = ihvin[4] + e; \
 }
 
+#ifdef _MSC_VER
+#pragma warning(push)
+#pragma warning(disable: 4127)  /* Complier complains about the checks in the above macro being constant. */
+#endif
+
 #ifdef DOSTORESTATE0
 SHA1_RECOMPRESS(0)
 #endif
@@ -1189,6 +1222,10 @@ SHA1_RECOMPRESS(78)
 SHA1_RECOMPRESS(79)
 #endif
 
+#ifdef _MSC_VER
+#pragma warning(pop)
+#endif
+
 static void sha1_recompression_step(uint32_t step, uint32_t ihvin[5], uint32_t ihvout[5], const uint32_t me2[80], const uint32_t state[5])
 {
 	switch (step)
@@ -1606,7 +1643,7 @@ static void sha1_process(SHA1_CTX* ctx, const uint32_t block[16])
 	unsigned i, j;
 	uint32_t ubc_dv_mask[DVMASKSIZE] = { 0xFFFFFFFF };
 	uint32_t ihvtmp[5];
-
+	
 	ctx->ihv1[0] = ctx->ihv[0];
 	ctx->ihv1[1] = ctx->ihv[1];
 	ctx->ihv1[2] = ctx->ihv[2];
@@ -1662,7 +1699,7 @@ void SHA1DCInit(SHA1_CTX* ctx)
 	ctx->ihv[3] = 0x10325476;
 	ctx->ihv[4] = 0xC3D2E1F0;
 	ctx->found_collision = 0;
-	ctx->safe_hash = 0;
+	ctx->safe_hash = SHA1DC_INIT_SAFE_HASH_DEFAULT;
 	ctx->ubc_check = 1;
 	ctx->detect_coll = 1;
 	ctx->reduced_round_coll = 0;
@@ -1710,6 +1747,9 @@ void SHA1DCSetCallback(SHA1_CTX* ctx, collision_block_callback callback)
 void SHA1DCUpdate(SHA1_CTX* ctx, const char* buf, size_t len)
 {
 	unsigned left, fill;
+
+    const uint32_t* buffer_to_hash = NULL;
+
 	if (len == 0)
 		return;
 
@@ -1728,7 +1768,14 @@ void SHA1DCUpdate(SHA1_CTX* ctx, const char* buf, size_t len)
 	while (len >= 64)
 	{
 		ctx->total += 64;
-		sha1_process(ctx, (uint32_t*)(buf));
+
+#if defined(SHA1DC_ALLOW_UNALIGNED_ACCESS)
+        buffer_to_hash = (const uint32_t*)buf;
+#else
+        buffer_to_hash = (const uint32_t*)ctx->buffer;
+        memcpy(ctx->buffer, buf, 64);
+#endif /* defined(SHA1DC_ALLOW_UNALIGNED_ACCESS) */
+		sha1_process(ctx, buffer_to_hash);
 		buf += 64;
 		len -= 64;
 	}
@@ -1788,22 +1835,6 @@ int SHA1DCFinal(unsigned char output[20], SHA1_CTX *ctx)
 	return ctx->found_collision;
 }
 
-void git_SHA1DCFinal(unsigned char hash[20], SHA1_CTX *ctx)
-{
-	if (!SHA1DCFinal(hash, ctx))
-		return;
-	die("SHA-1 appears to be part of a collision attack: %s",
-	    sha1_to_hex(hash));
-}
-
-void git_SHA1DCUpdate(SHA1_CTX *ctx, const void *vdata, unsigned long len)
-{
-	const char *data = vdata;
-	/* We expect an unsigned long, but sha1dc only takes an int */
-	while (len > INT_MAX) {
-		SHA1DCUpdate(ctx, data, INT_MAX);
-		data += INT_MAX;
-		len -= INT_MAX;
-	}
-	SHA1DCUpdate(ctx, data, len);
-}
+#ifdef SHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_C
+#include SHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_C
+#endif
diff --git a/sha1dc/sha1.h b/sha1dc/sha1.h
index bd8bd928fb..dd41b0a5a6 100644
--- a/sha1dc/sha1.h
+++ b/sha1dc/sha1.h
@@ -4,6 +4,7 @@
 * See accompanying file LICENSE.txt or copy at
 * https://opensource.org/licenses/MIT
 ***/
+
 #ifndef SHA1DC_SHA1_H
 #define SHA1DC_SHA1_H
 
@@ -11,36 +12,30 @@
 extern "C" {
 #endif
 
-/* uses SHA-1 message expansion to expand the first 16 words of W[] to 80 words */
-/* void sha1_message_expansion(uint32_t W[80]); */
-
-/* sha-1 compression function; first version takes a message block pre-parsed as 16 32-bit integers, second version takes an already expanded message) */
-/* void sha1_compression(uint32_t ihv[5], const uint32_t m[16]);
-void sha1_compression_W(uint32_t ihv[5], const uint32_t W[80]); */
+#ifndef SHA1DC_NO_STANDARD_INCLUDES
+#include <stdint.h>
+#endif
 
-/* same as sha1_compression_W, but additionally store intermediate states */
+/* sha-1 compression function that takes an already expanded message, and additionally store intermediate states */
 /* only stores states ii (the state between step ii-1 and step ii) when DOSTORESTATEii is defined in ubc_check.h */
 void sha1_compression_states(uint32_t[5], const uint32_t[16], uint32_t[80], uint32_t[80][5]);
 
 /*
-// function type for sha1_recompression_step_T (uint32_t ihvin[5], uint32_t ihvout[5], const uint32_t me2[80], const uint32_t state[5])
-// where 0 <= T < 80
-//       me2 is an expanded message (the expansion of an original message block XOR'ed with a disturbance vector's message block difference)
-//       state is the internal state (a,b,c,d,e) before step T of the SHA-1 compression function while processing the original message block
-// the function will return:
-//       ihvin: the reconstructed input chaining value
-//       ihvout: the reconstructed output chaining value
+// Function type for sha1_recompression_step_T (uint32_t ihvin[5], uint32_t ihvout[5], const uint32_t me2[80], const uint32_t state[5]).
+// Where 0 <= T < 80
+//       me2 is an expanded message (the expansion of an original message block XOR'ed with a disturbance vector's message block difference.)
+//       state is the internal state (a,b,c,d,e) before step T of the SHA-1 compression function while processing the original message block.
+// The function will return:
+//       ihvin: The reconstructed input chaining value.
+//       ihvout: The reconstructed output chaining value.
 */
 typedef void(*sha1_recompression_type)(uint32_t*, uint32_t*, const uint32_t*, const uint32_t*);
 
-/* table of sha1_recompression_step_0, ... , sha1_recompression_step_79 */
-/* extern sha1_recompression_type sha1_recompression_step[80];*/
-
-/* a callback function type that can be set to be called when a collision block has been found: */
+/* A callback function type that can be set to be called when a collision block has been found: */
 /* void collision_block_callback(uint64_t byteoffset, const uint32_t ihvin1[5], const uint32_t ihvin2[5], const uint32_t m1[80], const uint32_t m2[80]) */
 typedef void(*collision_block_callback)(uint64_t, const uint32_t*, const uint32_t*, const uint32_t*, const uint32_t*);
 
-/* the SHA-1 context */
+/* The SHA-1 context. */
 typedef struct {
 	uint64_t total;
 	uint32_t ihv[5];
@@ -59,30 +54,34 @@ typedef struct {
 	uint32_t states[80][5];
 } SHA1_CTX;
 
-/* initialize SHA-1 context */
+/* Initialize SHA-1 context. */
 void SHA1DCInit(SHA1_CTX*);
 
 /*
-// function to enable safe SHA-1 hashing:
-// collision attacks are thwarted by hashing a detected near-collision block 3 times
-// think of it as extending SHA-1 from 80-steps to 240-steps for such blocks:
-//   the best collision attacks against SHA-1 have complexity about 2^60,
-//   thus for 240-steps an immediate lower-bound for the best cryptanalytic attacks would 2^180
-//   an attacker would be better off using a generic birthday search of complexity 2^80
-//
-// enabling safe SHA-1 hashing will result in the correct SHA-1 hash for messages where no collision attack was detected
-// but it will result in a different SHA-1 hash for messages where a collision attack was detected
-// this will automatically invalidate SHA-1 based digital signature forgeries
-// enabled by default
+    Function to enable safe SHA-1 hashing:
+    Collision attacks are thwarted by hashing a detected near-collision block 3 times.
+    Think of it as extending SHA-1 from 80-steps to 240-steps for such blocks:
+        The best collision attacks against SHA-1 have complexity about 2^60,
+        thus for 240-steps an immediate lower-bound for the best cryptanalytic attacks would be 2^180.
+        An attacker would be better off using a generic birthday search of complexity 2^80.
+  
+   Enabling safe SHA-1 hashing will result in the correct SHA-1 hash for messages where no collision attack was detected,
+   but it will result in a different SHA-1 hash for messages where a collision attack was detected.
+   This will automatically invalidate SHA-1 based digital signature forgeries.
+   Enabled by default.
 */
 void SHA1DCSetSafeHash(SHA1_CTX*, int);
 
-/* function to disable or enable the use of Unavoidable Bitconditions (provides a significant speed up) */
-/* enabled by default */
+/*
+    Function to disable or enable the use of Unavoidable Bitconditions (provides a significant speed up).
+    Enabled by default
+ */
 void SHA1DCSetUseUBC(SHA1_CTX*, int);
 
-/* function to disable or enable the use of Collision Detection */
-/* enabled by default */
+/*
+    Function to disable or enable the use of Collision Detection.
+    Enabled by default.
+ */
 void SHA1DCSetUseDetectColl(SHA1_CTX*, int);
 
 /* function to disable or enable the detection of reduced-round SHA-1 collisions */
@@ -98,25 +97,14 @@ void SHA1DCUpdate(SHA1_CTX*, const char*, size_t);
 
 /* obtain SHA-1 hash from SHA-1 context */
 /* returns: 0 = no collision detected, otherwise = collision found => warn user for active attack */
-int  SHA1DCFinal(unsigned char[20], SHA1_CTX*);
-
-/*
- * Same as SHA1DCFinal, but convert collision attack case into a verbose die().
- */
-void git_SHA1DCFinal(unsigned char [20], SHA1_CTX *);
-
-/*
- * Same as SHA1DCUpdate, but adjust types to match git's usual interface.
- */
-void git_SHA1DCUpdate(SHA1_CTX *ctx, const void *data, unsigned long len);
-
-#define platform_SHA_CTX SHA1_CTX
-#define platform_SHA1_Init SHA1DCInit
-#define platform_SHA1_Update git_SHA1DCUpdate
-#define platform_SHA1_Final git_SHA1DCFinal
+int  SHA1DCFinal(unsigned char[20], SHA1_CTX*); 
 
 #if defined(__cplusplus)
 }
 #endif
 
-#endif /* SHA1DC_SHA1_H */
+#ifdef SHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_H
+#include SHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_H
+#endif
+
+#endif
diff --git a/sha1dc/ubc_check.c b/sha1dc/ubc_check.c
index 089dd4743d..b3beff2afb 100644
--- a/sha1dc/ubc_check.c
+++ b/sha1dc/ubc_check.c
@@ -24,8 +24,13 @@
 // ubc_check has been verified against ubc_check_verify using the 'ubc_check_test' program in the tools section
 */
 
-#include "git-compat-util.h"
-#include "sha1dc/ubc_check.h"
+#ifndef SHA1DC_NO_STANDARD_INCLUDES
+#include <stdint.h>
+#endif
+#ifdef SHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C
+#include SHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C
+#endif
+#include "ubc_check.h"
 
 static const uint32_t DV_I_43_0_bit 	= (uint32_t)(1) << 0;
 static const uint32_t DV_I_44_0_bit 	= (uint32_t)(1) << 1;
@@ -361,3 +366,7 @@ if (mask) {
 
 	dvmask[0]=mask;
 }
+
+#ifdef SHA1DC_CUSTOM_TRAILING_INCLUDE_UBC_CHECK_C
+#include SHA1DC_CUSTOM_TRAILING_INCLUDE_UBC_CHECK_C
+#endif
diff --git a/sha1dc/ubc_check.h b/sha1dc/ubc_check.h
index b64c306d77..d7e17dc734 100644
--- a/sha1dc/ubc_check.h
+++ b/sha1dc/ubc_check.h
@@ -20,13 +20,17 @@
 // thus one needs to do the recompression check for each DV that has its bit set
 */
 
-#ifndef UBC_CHECK_H
-#define UBC_CHECK_H
+#ifndef SHA1DC_UBC_CHECK_H
+#define SHA1DC_UBC_CHECK_H
 
 #if defined(__cplusplus)
 extern "C" {
 #endif
 
+#ifndef SHA1DC_NO_STANDARD_INCLUDES
+#include <stdint.h>
+#endif
+
 #define DVMASKSIZE 1
 typedef struct { int dvType; int dvK; int dvB; int testt; int maski; int maskb; uint32_t dm[80]; } dv_info_t;
 extern dv_info_t sha1_dvs[];
@@ -41,4 +45,8 @@ void ubc_check(const uint32_t W[80], uint32_t dvmask[DVMASKSIZE]);
 }
 #endif
 
-#endif /* UBC_CHECK_H */
+#ifdef SHA1DC_CUSTOM_TRAILING_INCLUDE_UBC_CHECK_H
+#include SHA1DC_CUSTOM_TRAILING_INCLUDE_UBC_CHECK_H
+#endif
+
+#endif
diff --git a/sha1dc_git.c b/sha1dc_git.c
new file mode 100644
index 0000000000..4d32b4f77e
--- /dev/null
+++ b/sha1dc_git.c
@@ -0,0 +1,24 @@
+/*
+ * This code is included at the end of sha1dc/sha1.c with the
+ * SHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_C macro.
+ */
+
+void git_SHA1DCFinal(unsigned char hash[20], SHA1_CTX *ctx)
+{
+	if (!SHA1DCFinal(hash, ctx))
+		return;
+	die("SHA-1 appears to be part of a collision attack: %s",
+	    sha1_to_hex(hash));
+}
+
+void git_SHA1DCUpdate(SHA1_CTX *ctx, const void *vdata, unsigned long len)
+{
+	const char *data = vdata;
+	/* We expect an unsigned long, but sha1dc only takes an int */
+	while (len > INT_MAX) {
+		SHA1DCUpdate(ctx, data, INT_MAX);
+		data += INT_MAX;
+		len -= INT_MAX;
+	}
+	SHA1DCUpdate(ctx, data, len);
+}
diff --git a/sha1dc_git.h b/sha1dc_git.h
new file mode 100644
index 0000000000..a8a5c1da16
--- /dev/null
+++ b/sha1dc_git.h
@@ -0,0 +1,19 @@
+/*
+ * This code is included at the end of sha1dc/sha1.h with the
+ * SHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_H macro.
+ */
+
+/*
+ * Same as SHA1DCFinal, but convert collision attack case into a verbose die().
+ */
+void git_SHA1DCFinal(unsigned char [20], SHA1_CTX *);
+
+/*
+ * Same as SHA1DCUpdate, but adjust types to match git's usual interface.
+ */
+void git_SHA1DCUpdate(SHA1_CTX *ctx, const void *data, unsigned long len);
+
+#define platform_SHA_CTX SHA1_CTX
+#define platform_SHA1_Init SHA1DCInit
+#define platform_SHA1_Update git_SHA1DCUpdate
+#define platform_SHA1_Final git_SHA1DCFinal
-- 
2.13.0.303.g4ebf302169