git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: "brian m. carlson" <sandals@crustytoothpaste.net>
Cc: git@vger.kernel.org, "Derrick Stolee" <stolee@gmail.com>,
	"Duy Nguyen" <pclouds@gmail.com>,
	"SZEDER Gábor" <szeder.dev@gmail.com>,
	"Jakub Narebski" <jnareb@gmail.com>,
	"Christian Couder" <christian.couder@gmail.com>
Subject: Re: [PATCH v5 10/12] Add a base implementation of SHA-256 support
Date: Mon, 05 Nov 2018 12:39:14 +0100	[thread overview]
Message-ID: <87h8gv7nz1.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <20181104234458.139223-11-sandals@crustytoothpaste.net>


On Sun, Nov 04 2018, brian m. carlson wrote:

> SHA-1 is weak and we need to transition to a new hash function.  For
> some time, we have referred to this new function as NewHash.  Recently,
> we decided to pick SHA-256 as NewHash.  The reasons behind the choice of
> SHA-256 are outlined in the thread starting at [1] and in the commit
> history for the hash function transition document.

Nit: In some contradiction now to what's said in
hash-function-transition.txt, see 5988eb631a ("doc
hash-function-transition: clarify what SHAttered means", 2018-03-26).

> +	{
> +		"sha256",
> +		/* "s256", big-endian */

The existing entry/comment for sha1 is:

		"sha1",
		/* "sha1", big-endian */

So why the sha256/s256 difference in the code/comment? Wondering if I'm
missing something and we're using "s256" for something.

>  const char *empty_tree_oid_hex(void)
> diff --git a/sha256/block/sha256.c b/sha256/block/sha256.c
> [...]

I had a question before about whether we see ourselves perma-forking
this implementation based off libtomcrypt, as I recall you said yes.

Still, I think it would be better to introduce this in at least two-four
commits where the upstream code is added as-is, then trimmed down to
size, then adapted to our coding style, and finally we add our own
utility functions.

It'll make it easier to forward-port any future upstream changes.

> +	perl -E "for (1..100000) { print q{aaaaaaaaaa}; }" | \
> +		test-tool sha256 >actual &&
> +	grep cdc76e5c9914fb9281a1c7e284d73e67f1809a48a497200e046d39ccc7112cd0 actual &&
> +	perl -E "for (1..100000) { print q{abcdefghijklmnopqrstuvwxyz}; }" | \
> +		test-tool sha256 >actual &&

I've been wanting to make use depend on perl >= 5.10 (previous noises
about that on-list), but for now we claim to support >=5.8, which
doesn't have the -E switch.

But most importantly you aren't even using -E features here, and this
isn't very idoimatic Perl. Instead do, respectively:

    perl -e 'print q{aaaaaaaaaa} x 100000'
    perl -e "print q{abcdefghijklmnopqrstuvwxyz} x 100000"

  reply	other threads:[~2018-11-05 11:39 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-25  2:39 [PATCH v4 00/12] Base SHA-256 implementation brian m. carlson
2018-10-25  2:39 ` [PATCH v4 01/12] sha1-file: rename algorithm to "sha1" brian m. carlson
2018-10-25  2:39 ` [PATCH v4 02/12] sha1-file: provide functions to look up hash algorithms brian m. carlson
2018-10-25  2:39 ` [PATCH v4 03/12] hex: introduce functions to print arbitrary hashes brian m. carlson
2018-10-25  2:39 ` [PATCH v4 04/12] cache: make hashcmp and hasheq work with larger hashes brian m. carlson
2018-10-25  2:39 ` [PATCH v4 05/12] t: add basic tests for our SHA-1 implementation brian m. carlson
2018-10-25  2:39 ` [PATCH v4 06/12] t: make the sha1 test-tool helper generic brian m. carlson
2018-10-25  2:40 ` [PATCH v4 07/12] sha1-file: add a constant for hash block size brian m. carlson
2018-10-25  2:40 ` [PATCH v4 08/12] t/helper: add a test helper to compute hash speed brian m. carlson
2018-10-25  2:40 ` [PATCH v4 09/12] commit-graph: convert to using the_hash_algo brian m. carlson
2018-10-25  2:40 ` [PATCH v4 10/12] Add a base implementation of SHA-256 support brian m. carlson
2018-10-25  3:02   ` Carlo Arenas
2018-10-28 15:52     ` brian m. carlson
2018-10-29  0:39       ` Junio C Hamano
2018-10-31 22:55         ` brian m. carlson
2018-11-01  5:29           ` Junio C Hamano
2018-10-27  9:03   ` Christian Couder
2018-10-25  2:40 ` [PATCH v4 11/12] sha256: add an SHA-256 implementation using libgcrypt brian m. carlson
2018-10-25  2:40 ` [PATCH v4 12/12] hash: add an SHA-256 implementation using OpenSSL brian m. carlson
2018-11-04 23:44 ` [PATCH v5 00/12] Base SHA-256 implementation brian m. carlson
2018-11-04 23:44   ` [PATCH v5 01/12] sha1-file: rename algorithm to "sha1" brian m. carlson
2018-11-05  7:21     ` Ævar Arnfjörð Bjarmason
2018-11-04 23:44   ` [PATCH v5 02/12] sha1-file: provide functions to look up hash algorithms brian m. carlson
2018-11-13 18:42     ` Derrick Stolee
2018-11-13 18:45       ` Duy Nguyen
2018-11-14  1:01         ` brian m. carlson
2018-11-14  0:11       ` Ramsay Jones
2018-11-14  0:42         ` Ramsay Jones
2018-11-14  0:51           ` Jeff King
2018-11-14  2:11         ` brian m. carlson
2018-11-14  3:53           ` Ramsay Jones
2018-11-04 23:44   ` [PATCH v5 03/12] hex: introduce functions to print arbitrary hashes brian m. carlson
2018-11-04 23:44   ` [PATCH v5 04/12] cache: make hashcmp and hasheq work with larger hashes brian m. carlson
2018-11-04 23:44   ` [PATCH v5 05/12] t: add basic tests for our SHA-1 implementation brian m. carlson
2018-11-04 23:44   ` [PATCH v5 06/12] t: make the sha1 test-tool helper generic brian m. carlson
2018-11-04 23:44   ` [PATCH v5 07/12] sha1-file: add a constant for hash block size brian m. carlson
2018-11-04 23:44   ` [PATCH v5 08/12] t/helper: add a test helper to compute hash speed brian m. carlson
2018-11-04 23:44   ` [PATCH v5 09/12] commit-graph: convert to using the_hash_algo brian m. carlson
2018-11-04 23:44   ` [PATCH v5 10/12] Add a base implementation of SHA-256 support brian m. carlson
2018-11-05 11:39     ` Ævar Arnfjörð Bjarmason [this message]
2018-11-07  1:30       ` brian m. carlson
2018-11-10 15:52         ` Ævar Arnfjörð Bjarmason
2018-11-04 23:44   ` [PATCH v5 11/12] sha256: add an SHA-256 implementation using libgcrypt brian m. carlson
2018-11-04 23:44   ` [PATCH v5 12/12] hash: add an SHA-256 implementation using OpenSSL brian m. carlson
2018-11-05  2:45   ` [PATCH v5 00/12] Base SHA-256 implementation Junio C Hamano
2018-11-14  4:09   ` [PATCH v6 " brian m. carlson
2018-11-14  4:09     ` [PATCH v6 01/12] sha1-file: rename algorithm to "sha1" brian m. carlson
2018-11-14  4:09     ` [PATCH v6 02/12] sha1-file: provide functions to look up hash algorithms brian m. carlson
2018-11-14  4:09     ` [PATCH v6 03/12] hex: introduce functions to print arbitrary hashes brian m. carlson
2018-11-14  4:09     ` [PATCH v6 04/12] cache: make hashcmp and hasheq work with larger hashes brian m. carlson
2018-11-14  4:09     ` [PATCH v6 05/12] t: add basic tests for our SHA-1 implementation brian m. carlson
2018-11-14  4:09     ` [PATCH v6 06/12] t: make the sha1 test-tool helper generic brian m. carlson
2018-11-14  4:09     ` [PATCH v6 07/12] sha1-file: add a constant for hash block size brian m. carlson
2018-11-14  4:09     ` [PATCH v6 08/12] t/helper: add a test helper to compute hash speed brian m. carlson
2018-11-14  4:09     ` [PATCH v6 09/12] commit-graph: convert to using the_hash_algo brian m. carlson
2018-11-14  4:09     ` [PATCH v6 10/12] Add a base implementation of SHA-256 support brian m. carlson
2018-11-14  4:09     ` [PATCH v6 11/12] sha256: add an SHA-256 implementation using libgcrypt brian m. carlson
2018-11-14  4:09     ` [PATCH v6 12/12] hash: add an SHA-256 implementation using OpenSSL brian m. carlson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h8gv7nz1.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jnareb@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=sandals@crustytoothpaste.net \
    --cc=stolee@gmail.com \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).