Re: [PATCH] Limit the size of the data block passed to SHA1_Update()

Atousa Duprat <atousa.p@gmail.com> writes:

Yes, you are correct that these two cannot be used at the same time. In general (not limited to BLK_SHA1 and APPLE_COMMON_CRYPTO) you can pick only _one_ underlying SHA-1 implementation to use with the system.

If you use APPLE_COMMON_CRYPTO, you may have to use the Chunked thing, because of APPLE_COMMON_CRYPTO implementation's limitation.

But the above two facts taken together does not have to imply that you are forbidden from choosing to use Chunked thing if you are using BLK_SHA1 or OpenSSL's SHA-1 implementation. If only for making sure that the Chunked wrapper passes compilation test, for trying it out to see how well it works, or just for satisfying curiosity, it would be nice if we allowed such a combination.

The original arrangement of macro was:

 * The user code uses git_SHA1_Update()

 * cache.h renames git_SHA1_Update() to refer to the underlying
   SHA1_Update() function, either from OpenSSL or AppleCommonCrypto,
   or block-sha1/sha1.h renames git_SHA1_Update() to refer to our
   implementation blk_SHA1_Update().

What we want with Chunked is:

 * The user code uses git_SHA1_Update(); we must not change this, as
   there are many existing calls.

 * We want git_SHA1_Update() to call the Chunked thing when
   SHA1_MAX_BLOCK_SIZE is set.

 * The Chunked thing must delegate the actual hashing to underlying
   SHA1_Update(), either from OpenSSL or AppleCommonCrypto.  If we
   are using BLK_SHA1, we want the Chunked thing to instead call
   blk_SHA1_Update().

I do not seem to be able to find a way to do this with the current two-level indirection. If we added another level, we can.

* In cache.h, define platform_SHA1_Update() to refer to
  SHA1_Update() from the platform (unless block-sha1/ is used).
  git_SHA1_Update() in the user code may directly call it, or it may
  go to the Chunked thing.

  #ifndef git_SHA1_CTX
  #define platform_SHA1_Update  SHA1_Update
  #endif

  #ifdef SHA1_MAX_BLOCK_SIZE
  #define git_SHA1_Update       git_SHA1_Update_Chunked
  #else
  #define git_SHA1_Update       platform_SHA1_Update
  #endif

* In block-sha1/sha1.h, redirect platform_SHA1_Update() to
  blk_SHA1_Update().

  #define platform_SHA1_Update  blk_SHA1_Update

* In compat/sha1_chunked.c, implement the Chunked thing in terms of
  the platform_SHA1_Update():

  git_SHA1_Update_Chunked(...)
  {
        ...
        while (...) {
                platform_SHA1_Update(...);
        }
  }

I am not sure if the above is worth it, but I suspect the damage is localized enough that this may be OK.