Re: [PATCH 1/9] docs: update pack index v3 format

[Junio C Hamano <gitster@pobox.com>]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> Our current pack index v3 format uses 4-byte integers to find the
> trailer of the file.  This effectively means that the file cannot be
> much larger than 2^32.  While this might at first seem to be okay, we
> expect that each object will have at least 64 bytes worth of data, which
> means that no more than about 67 million objects can be stored.
>
> Again, this might seem fine, but unfortunately, we know of many users
> who attempt to create repos with extremely large numbers of commits to
> get a "high score," and we've already seen repositories with at least 55
> million commits.  In the interests of gracefully handling repositories
> even for these well-intentioned but ultimately misguided users, let's
> change these lengths to 8 bytes.

Very sensible.

I do also agree that 32-byte is the natural size for the trailing
hash, but I found that the two paragraphs below was far more than
necessary.  As they argue, we use a truncated hash anywhere in our
file formats, so I would have understood if the explanation were

    "20" in "A copy of the 20-byte SHA-256 checksum" is an obvious
    typo, as SHA-256 is longer than that.  Fix it to "32".

instead of these two paragraphs.

Or did we mean to use a truncated hash back when this transition
design was proposed originally?

> For the checksums at the end of the file, we're producing 32-byte
> SHA-256 checksums because that's what we already do with pack index v2
> and SHA-256.  Truncating SHA-256 doesn't pose any actual security
> problems other than those related to the reduced size, but our pack
> checksum must already be 32 bytes (since SHA-256 packs have 32-byte
> checksums) and it simplifies the code to use the existing hashfile logic
> for these cases for the index checksum as well.
>
> In addition, even though we may not need cryptographic security for the
> index checksum, we'd like to avoid arguments from auditors and such for
> organizations that may have compliance or security requirements.  Using
> the simple, boring choice of the full SHA-256 hash avoids all possible
> discussion related to hash truncation and removes impediments for these
> organizations.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  Documentation/technical/hash-function-transition.adoc | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/technical/hash-function-transition.adoc b/Documentation/technical/hash-function-transition.adoc
> index f047fd80ca..f2df1d618d 100644
> --- a/Documentation/technical/hash-function-transition.adoc
> +++ b/Documentation/technical/hash-function-transition.adoc
> @@ -227,9 +227,9 @@ network byte order):
>      ** 4-byte length in bytes of shortened object names. This is the
>        shortest possible length needed to make names in the shortened
>        object name table unambiguous.
> -    ** 4-byte integer, recording where tables relating to this format
> +    ** 8-byte integer, recording where tables relating to this format
>        are stored in this index file, as an offset from the beginning.
> -  * 4-byte offset to the trailer from the beginning of this file.
> +  * 8-byte offset to the trailer from the beginning of this file.
>    * Zero or more additional key/value pairs (4-byte key, 4-byte
>      value). Only one key is supported: 'PSRC'. See the "Loose objects
>      and unreachable objects" section for supported values and how this
> @@ -276,10 +276,10 @@ network byte order):
>    up to and not including the table of CRC32 values.
>  - Zero or more NUL bytes.
>  - The trailer consists of the following:
> -  * A copy of the 20-byte SHA-256 checksum at the end of the
> +  * A copy of the 32-byte SHA-256 checksum at the end of the
>      corresponding packfile.
>  
> -  * 20-byte SHA-256 checksum of all of the above.
> +  * 32-byte SHA-256 checksum of all of the above.
>  
>  Loose object index
>  ~~~~~~~~~~~~~~~~~~