[PATCH v4 0/8] hash.h: support choosing a separate SHA-1 for non-cryptographic uses

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Here is a reroll of mine and Peff's series to add a build-time knob to
allow selecting an alternative SHA-1 implementation for
non-cryptographic hashing within Git, starting with the `hashwrite()`
family of functions.

This version is another fairly substantial reroll, with the main
differences being renaming the "fast" SHA-1 options to "unsafe", as well
as not running the collision checks via finalize_object_file() when
handling loose objects (see the relevant patches for details on why).

Note also there is an important bug fix in finalize_object_file() to
unlink() the temporary file when we do run the collision check, but no
collisions were found. This bug was causing a pile-up of tmp_obj_XXXXXX
files in GitHub's infrastructure.

Thanks in advance for your review!

Taylor Blau (8):
  finalize_object_file(): check for name collision before renaming
  finalize_object_file(): refactor unlink_or_warn() placement
  finalize_object_file(): implement collision check
  pack-objects: use finalize_object_file() to rename pack/idx/etc
  sha1: do not redefine `platform_SHA_CTX` and friends
  hash.h: scaffolding for _unsafe hashing variants
  Makefile: allow specifying a SHA-1 for non-cryptographic uses
  csum-file.c: use unsafe SHA-1 implementation when available

 Makefile                              |  25 ++++++
 block-sha1/sha1.h                     |   2 +
 csum-file.c                           |  18 ++--
 hash.h                                |  72 +++++++++++++++
 object-file.c                         | 124 ++++++++++++++++++++++++--
 object-file.h                         |   6 ++
 pack-write.c                          |   7 +-
 sha1/openssl.h                        |   2 +
 sha1dc_git.h                          |   3 +
 t/t5303-pack-corruption-resilience.sh |   7 +-
 tmp-objdir.c                          |  26 ++++--
 11 files changed, 266 insertions(+), 26 deletions(-)

Range-diff against v3:
 1:  738b1eb17b4 =  1:  6f1ee91fff3 finalize_object_file(): check for name collision before renaming
 2:  e1c2c39711f =  2:  133047ca8c9 finalize_object_file(): refactor unlink_or_warn() placement
 3:  0feee5d1d4f !  3:  ed9eeef8513 finalize_object_file(): implement collision check
    @@ Commit message
     
         The new check will cause the write of new differing content to be a
         failure, rather than a silent noop, and we'll retain the temporary file
    -    on disk.
    +    on disk. If there's no collision present, we'll clean up the temporary
    +    file as usual after either rename()-ing or link()-ing it into place.
     
         Note that this may cause some extra computation when the files are in
    -    fact identical, but this should happen rarely. For example, when writing
    -    a loose object, we compute the object id first, then check manually for
    -    an existing object (so that we can freshen its timestamp) before
    -    actually trying to write and link the new data.
    +    fact identical, but this should happen rarely.
    +
    +    Loose objects are exempt from this check, and the collision check may be
    +    skipped by calling the _flags variant of this function with the
    +    FOF_SKIP_COLLISION_CHECK bit set. This is done for a couple of reasons:
    +
    +      - We don't treat the hash of the loose object file's contents as a
    +        checksum, since the same loose object can be stored using different
    +        bytes on disk (e.g., when adjusting core.compression, using a
    +        different version of zlib, etc.).
    +
    +        This is fundamentally different from cases where
    +        finalize_object_file() is operating over a file which uses the hash
    +        value as a checksum of the contents. In other words, a pair of
    +        identical loose objects can be stored using different bytes on disk,
    +        and that should not be treated as a collision.
    +
    +      - We already use the path of the loose object as its hash value /
    +        object name, so checking for collisions at the content level doesn't
    +        add anything.
    +
    +        This is why we do not bother to check the inflated object contents
    +        for collisions either, since either (a) the object contents have the
    +        fingerprint of a SHA-1 collision, in which case the collision
    +        detecting SHA-1 implementation used to hash the contents to give us
    +        a path would have already rejected it, or (b) the contents are part
    +        of a colliding pair which does not bear the same fingerprints of
    +        known collision attacks, in which case we would not have caught it
    +        anyway.
    +
    +        So skipping the collision check here does not change for better or
    +        worse the hardness of loose object writes.
    +
    +    As a small note related to the latter bullet point above, we must teach
    +    the tmp-objdir routines to similarly skip the content-level collision
    +    checks when calling migrate_one() on a loose object file, which we do by
    +    setting the FOF_SKIP_COLLISION_CHECK bit when we are inside of a loose
    +    object shard.
     
         Co-authored-by: Jeff King <peff@xxxxxxxx>
         Signed-off-by: Jeff King <peff@xxxxxxxx>
    @@ object-file.c: static void write_object_file_prepare_literally(const struct git_
      /*
       * Move the just written object into its final resting place.
       */
    + int finalize_object_file(const char *tmpfile, const char *filename)
    ++{
    ++	return finalize_object_file_flags(tmpfile, filename, 0);
    ++}
    ++
    ++int finalize_object_file_flags(const char *tmpfile, const char *filename,
    ++			       enum finalize_object_file_flags flags)
    + {
    + 	struct stat st;
    + 	int ret = 0;
     @@ object-file.c: int finalize_object_file(const char *tmpfile, const char *filename)
      			errno = saved_errno;
      			return error_errno(_("unable to write file %s"), filename);
      		}
     -		/* FIXME!!! Collision check here ? */
    --		unlink_or_warn(tmpfile);
    -+		if (check_collision(tmpfile, filename))
    -+			return -1;
    ++		if (!(flags & FOF_SKIP_COLLISION_CHECK) &&
    ++		    check_collision(tmpfile, filename))
    ++				return -1;
    + 		unlink_or_warn(tmpfile);
      	}
      
    - out:
    +@@ object-file.c: static int write_loose_object(const struct object_id *oid, char *hdr,
    + 			warning_errno(_("failed utime() on %s"), tmp_file.buf);
    + 	}
    + 
    +-	return finalize_object_file(tmp_file.buf, filename.buf);
    ++	return finalize_object_file_flags(tmp_file.buf, filename.buf,
    ++					  FOF_SKIP_COLLISION_CHECK);
    + }
    + 
    + static int freshen_loose_object(const struct object_id *oid)
    +@@ object-file.c: int stream_loose_object(struct input_stream *in_stream, size_t len,
    + 		strbuf_release(&dir);
    + 	}
    + 
    +-	err = finalize_object_file(tmp_file.buf, filename.buf);
    ++	err = finalize_object_file_flags(tmp_file.buf, filename.buf,
    ++					 FOF_SKIP_COLLISION_CHECK);
    + 	if (!err && compat)
    + 		err = repo_add_loose_object_map(the_repository, oid, &compat_oid);
    + cleanup:
    +
    + ## object-file.h ##
    +@@ object-file.h: int check_object_signature(struct repository *r, const struct object_id *oid,
    +  */
    + int stream_object_signature(struct repository *r, const struct object_id *oid);
    + 
    ++enum finalize_object_file_flags {
    ++	FOF_SKIP_COLLISION_CHECK = 1,
    ++};
    ++
    + int finalize_object_file(const char *tmpfile, const char *filename);
    ++int finalize_object_file_flags(const char *tmpfile, const char *filename,
    ++			       enum finalize_object_file_flags flags);
    + 
    + /* Helper to check and "touch" a file */
    + int check_and_freshen_file(const char *fn, int freshen);
    +
    + ## tmp-objdir.c ##
    +@@ tmp-objdir.c: static int read_dir_paths(struct string_list *out, const char *path)
    + 	return 0;
    + }
    + 
    +-static int migrate_paths(struct strbuf *src, struct strbuf *dst);
    ++static int migrate_paths(struct strbuf *src, struct strbuf *dst,
    ++			 enum finalize_object_file_flags flags);
    + 
    +-static int migrate_one(struct strbuf *src, struct strbuf *dst)
    ++static int migrate_one(struct strbuf *src, struct strbuf *dst,
    ++		       enum finalize_object_file_flags flags)
    + {
    + 	struct stat st;
    + 
    +@@ tmp-objdir.c: static int migrate_one(struct strbuf *src, struct strbuf *dst)
    + 				return -1;
    + 		} else if (errno != EEXIST)
    + 			return -1;
    +-		return migrate_paths(src, dst);
    ++		return migrate_paths(src, dst, flags);
    + 	}
    +-	return finalize_object_file(src->buf, dst->buf);
    ++	return finalize_object_file_flags(src->buf, dst->buf, flags);
    + }
    + 
    +-static int migrate_paths(struct strbuf *src, struct strbuf *dst)
    ++static int is_loose_object_shard(const char *name)
    ++{
    ++	return strlen(name) == 2 && isxdigit(name[0]) && isxdigit(name[1]);
    ++}
    ++
    ++static int migrate_paths(struct strbuf *src, struct strbuf *dst,
    ++			 enum finalize_object_file_flags flags)
    + {
    + 	size_t src_len = src->len, dst_len = dst->len;
    + 	struct string_list paths = STRING_LIST_INIT_DUP;
    +@@ tmp-objdir.c: static int migrate_paths(struct strbuf *src, struct strbuf *dst)
    + 
    + 	for (i = 0; i < paths.nr; i++) {
    + 		const char *name = paths.items[i].string;
    ++		enum finalize_object_file_flags flags_copy = flags;
    + 
    + 		strbuf_addf(src, "/%s", name);
    + 		strbuf_addf(dst, "/%s", name);
    + 
    +-		ret |= migrate_one(src, dst);
    ++		if (is_loose_object_shard(name))
    ++			flags_copy |= FOF_SKIP_COLLISION_CHECK;
    ++
    ++		ret |= migrate_one(src, dst, flags_copy);
    + 
    + 		strbuf_setlen(src, src_len);
    + 		strbuf_setlen(dst, dst_len);
    +@@ tmp-objdir.c: int tmp_objdir_migrate(struct tmp_objdir *t)
    + 	strbuf_addbuf(&src, &t->path);
    + 	strbuf_addstr(&dst, repo_get_object_directory(the_repository));
    + 
    +-	ret = migrate_paths(&src, &dst);
    ++	ret = migrate_paths(&src, &dst, 0);
    + 
    + 	strbuf_release(&src);
    + 	strbuf_release(&dst);
 4:  620dde48a9d =  4:  3cc7f7b1f67 pack-objects: use finalize_object_file() to rename pack/idx/etc
 5:  bfe992765cd <  -:  ----------- i5500-git-daemon.sh: use compile-able version of Git without OpenSSL
 6:  22863d9f6df =  5:  8f8ac0f5b0e sha1: do not redefine `platform_SHA_CTX` and friends
 7:  119c318d812 !  6:  d300e9c6887 hash.h: scaffolding for _fast hashing variants
    @@ Metadata
     Author: Taylor Blau <me@xxxxxxxxxxxx>
     
      ## Commit message ##
    -    hash.h: scaffolding for _fast hashing variants
    +    hash.h: scaffolding for _unsafe hashing variants
     
         Git's default SHA-1 implementation is collision-detecting, which hardens
         us against known SHA-1 attacks against Git objects. This makes Git
    @@ Commit message
         the collision-detecting implementation, which is slower than
         non-collision detecting alternatives.
     
    -    Prepare for loading a separate "fast" SHA-1 implementation that can be
    +    Prepare for loading a separate "unsafe" SHA-1 implementation that can be
         used for non-cryptographic purposes, like computing the checksum of
         files that use the hashwrite() API.
     
         This commit does not actually introduce any new compile-time knobs to
    -    control which implementation is used as the fast SHA-1 variant, but does
    -    add scaffolding so that the "git_hash_algo" structure has five new
    -    function pointers which are "fast" variants of the five existing
    +    control which implementation is used as the unsafe SHA-1 variant, but
    +    does add scaffolding so that the "git_hash_algo" structure has five new
    +    function pointers which are "unsafe" variants of the five existing
         hashing-related function pointers:
     
    -      - git_hash_init_fn fast_init_fn
    -      - git_hash_clone_fn fast_clone_fn
    -      - git_hash_update_fn fast_update_fn
    -      - git_hash_final_fn fast_final_fn
    -      - git_hash_final_oid_fn fast_final_oid_fn
    +      - git_hash_init_fn unsafe_init_fn
    +      - git_hash_clone_fn unsafe_clone_fn
    +      - git_hash_update_fn unsafe_update_fn
    +      - git_hash_final_fn unsafe_final_fn
    +      - git_hash_final_oid_fn unsafe_final_oid_fn
     
         The following commit will introduce compile-time knobs to specify which
         SHA-1 implementation is used for non-cryptographic uses.
    @@ hash.h
      #define platform_SHA1_Final    	SHA1_Final
      #endif
      
    -+#ifndef platform_SHA_CTX_fast
    -+#  define platform_SHA_CTX_fast     platform_SHA_CTX
    -+#  define platform_SHA1_Init_fast   platform_SHA1_Init
    -+#  define platform_SHA1_Update_fast platform_SHA1_Update
    -+#  define platform_SHA1_Final_fast  platform_SHA1_Final
    ++#ifndef platform_SHA_CTX_unsafe
    ++#  define platform_SHA_CTX_unsafe      platform_SHA_CTX
    ++#  define platform_SHA1_Init_unsafe    platform_SHA1_Init
    ++#  define platform_SHA1_Update_unsafe  platform_SHA1_Update
    ++#  define platform_SHA1_Final_unsafe   platform_SHA1_Final
     +#  ifdef platform_SHA1_Clone
    -+#    define platform_SHA1_Clone_fast platform_SHA1_Clone
    ++#    define platform_SHA1_Clone_unsafe platform_SHA1_Clone
     +#  endif
     +#endif
     +
    @@ hash.h
      #define git_SHA1_Update		platform_SHA1_Update
      #define git_SHA1_Final		platform_SHA1_Final
      
    -+#define git_SHA_CTX_fast	platform_SHA_CTX_fast
    -+#define git_SHA1_Init_fast	platform_SHA1_Init_fast
    -+#define git_SHA1_Update_fast	platform_SHA1_Update_fast
    -+#define git_SHA1_Final_fast	platform_SHA1_Final_fast
    ++#define git_SHA_CTX_unsafe	platform_SHA_CTX_unsafe
    ++#define git_SHA1_Init_unsafe	platform_SHA1_Init_unsafe
    ++#define git_SHA1_Update_unsafe	platform_SHA1_Update_unsafe
    ++#define git_SHA1_Final_unsafe	platform_SHA1_Final_unsafe
     +
      #ifdef platform_SHA1_Clone
      #define git_SHA1_Clone	platform_SHA1_Clone
      #endif
    -+#ifdef platform_SHA1_Clone_fast
    -+#  define git_SHA1_Clone_fast platform_SHA1_Clone_fast
    ++#ifdef platform_SHA1_Clone_unsafe
    ++#  define git_SHA1_Clone_unsafe platform_SHA1_Clone_unsafe
     +#endif
      
      #ifndef platform_SHA256_CTX
    @@ hash.h: static inline void git_SHA1_Clone(git_SHA_CTX *dst, const git_SHA_CTX *s
      	memcpy(dst, src, sizeof(*dst));
      }
      #endif
    -+#ifndef SHA1_NEEDS_CLONE_HELPER_FAST
    -+static inline void git_SHA1_Clone_fast(git_SHA_CTX_fast *dst,
    -+				       const git_SHA_CTX_fast *src)
    ++#ifndef SHA1_NEEDS_CLONE_HELPER_UNSAFE
    ++static inline void git_SHA1_Clone_unsafe(git_SHA_CTX_unsafe *dst,
    ++				       const git_SHA_CTX_unsafe *src)
     +{
     +	memcpy(dst, src, sizeof(*dst));
     +}
    @@ hash.h: enum get_oid_result {
      /* A suitably aligned type for stack allocations of hash contexts. */
      union git_hash_ctx {
      	git_SHA_CTX sha1;
    -+	git_SHA_CTX_fast sha1_fast;
    ++	git_SHA_CTX_unsafe sha1_unsafe;
     +
      	git_SHA256_CTX sha256;
      };
    @@ hash.h: struct git_hash_algo {
      	/* The hash finalization function for object IDs. */
      	git_hash_final_oid_fn final_oid_fn;
      
    -+	/* The fast / non-cryptographic hash initialization function. */
    -+	git_hash_init_fn fast_init_fn;
    ++	/* The non-cryptographic hash initialization function. */
    ++	git_hash_init_fn unsafe_init_fn;
     +
    -+	/* The fast / non-cryptographic hash context cloning function. */
    -+	git_hash_clone_fn fast_clone_fn;
    ++	/* The non-cryptographic hash context cloning function. */
    ++	git_hash_clone_fn unsafe_clone_fn;
     +
    -+	/* The fast / non-cryptographic hash update function. */
    -+	git_hash_update_fn fast_update_fn;
    ++	/* The non-cryptographic hash update function. */
    ++	git_hash_update_fn unsafe_update_fn;
     +
    -+	/* The fast / non-cryptographic hash finalization function. */
    -+	git_hash_final_fn fast_final_fn;
    ++	/* The non-cryptographic hash finalization function. */
    ++	git_hash_final_fn unsafe_final_fn;
     +
    -+	/* The fast / non-cryptographic hash finalization function. */
    -+	git_hash_final_oid_fn fast_final_oid_fn;
    ++	/* The non-cryptographic hash finalization function. */
    ++	git_hash_final_oid_fn unsafe_final_oid_fn;
     +
      	/* The OID of the empty tree. */
      	const struct object_id *empty_tree;
    @@ object-file.c: static void git_hash_sha1_final_oid(struct object_id *oid, git_ha
      	oid->algo = GIT_HASH_SHA1;
      }
      
    -+static void git_hash_sha1_init_fast(git_hash_ctx *ctx)
    ++static void git_hash_sha1_init_unsafe(git_hash_ctx *ctx)
     +{
    -+	git_SHA1_Init_fast(&ctx->sha1_fast);
    ++	git_SHA1_Init_unsafe(&ctx->sha1_unsafe);
     +}
     +
    -+static void git_hash_sha1_clone_fast(git_hash_ctx *dst, const git_hash_ctx *src)
    ++static void git_hash_sha1_clone_unsafe(git_hash_ctx *dst, const git_hash_ctx *src)
     +{
    -+	git_SHA1_Clone_fast(&dst->sha1_fast, &src->sha1_fast);
    ++	git_SHA1_Clone_unsafe(&dst->sha1_unsafe, &src->sha1_unsafe);
     +}
     +
    -+static void git_hash_sha1_update_fast(git_hash_ctx *ctx, const void *data,
    ++static void git_hash_sha1_update_unsafe(git_hash_ctx *ctx, const void *data,
     +				      size_t len)
     +{
    -+	git_SHA1_Update_fast(&ctx->sha1_fast, data, len);
    ++	git_SHA1_Update_unsafe(&ctx->sha1_unsafe, data, len);
     +}
     +
    -+static void git_hash_sha1_final_fast(unsigned char *hash, git_hash_ctx *ctx)
    ++static void git_hash_sha1_final_unsafe(unsigned char *hash, git_hash_ctx *ctx)
     +{
    -+	git_SHA1_Final_fast(hash, &ctx->sha1_fast);
    ++	git_SHA1_Final_unsafe(hash, &ctx->sha1_unsafe);
     +}
     +
    -+static void git_hash_sha1_final_oid_fast(struct object_id *oid, git_hash_ctx *ctx)
    ++static void git_hash_sha1_final_oid_unsafe(struct object_id *oid, git_hash_ctx *ctx)
     +{
    -+	git_SHA1_Final_fast(oid->hash, &ctx->sha1_fast);
    ++	git_SHA1_Final_unsafe(oid->hash, &ctx->sha1_unsafe);
     +	memset(oid->hash + GIT_SHA1_RAWSZ, 0, GIT_MAX_RAWSZ - GIT_SHA1_RAWSZ);
     +	oid->algo = GIT_HASH_SHA1;
     +}
    @@ object-file.c: const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {
      		.update_fn = git_hash_unknown_update,
      		.final_fn = git_hash_unknown_final,
      		.final_oid_fn = git_hash_unknown_final_oid,
    -+		.fast_init_fn = git_hash_unknown_init,
    -+		.fast_clone_fn = git_hash_unknown_clone,
    -+		.fast_update_fn = git_hash_unknown_update,
    -+		.fast_final_fn = git_hash_unknown_final,
    -+		.fast_final_oid_fn = git_hash_unknown_final_oid,
    ++		.unsafe_init_fn = git_hash_unknown_init,
    ++		.unsafe_clone_fn = git_hash_unknown_clone,
    ++		.unsafe_update_fn = git_hash_unknown_update,
    ++		.unsafe_final_fn = git_hash_unknown_final,
    ++		.unsafe_final_oid_fn = git_hash_unknown_final_oid,
      		.empty_tree = NULL,
      		.empty_blob = NULL,
      		.null_oid = NULL,
    @@ object-file.c: const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {
      		.update_fn = git_hash_sha1_update,
      		.final_fn = git_hash_sha1_final,
      		.final_oid_fn = git_hash_sha1_final_oid,
    -+		.fast_init_fn = git_hash_sha1_init_fast,
    -+		.fast_clone_fn = git_hash_sha1_clone_fast,
    -+		.fast_update_fn = git_hash_sha1_update_fast,
    -+		.fast_final_fn = git_hash_sha1_final_fast,
    -+		.fast_final_oid_fn = git_hash_sha1_final_oid_fast,
    ++		.unsafe_init_fn = git_hash_sha1_init_unsafe,
    ++		.unsafe_clone_fn = git_hash_sha1_clone_unsafe,
    ++		.unsafe_update_fn = git_hash_sha1_update_unsafe,
    ++		.unsafe_final_fn = git_hash_sha1_final_unsafe,
    ++		.unsafe_final_oid_fn = git_hash_sha1_final_oid_unsafe,
      		.empty_tree = &empty_tree_oid,
      		.empty_blob = &empty_blob_oid,
      		.null_oid = &null_oid_sha1,
    @@ object-file.c: const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {
      		.update_fn = git_hash_sha256_update,
      		.final_fn = git_hash_sha256_final,
      		.final_oid_fn = git_hash_sha256_final_oid,
    -+		.fast_init_fn = git_hash_sha256_init,
    -+		.fast_clone_fn = git_hash_sha256_clone,
    -+		.fast_update_fn = git_hash_sha256_update,
    -+		.fast_final_fn = git_hash_sha256_final,
    -+		.fast_final_oid_fn = git_hash_sha256_final_oid,
    ++		.unsafe_init_fn = git_hash_sha256_init,
    ++		.unsafe_clone_fn = git_hash_sha256_clone,
    ++		.unsafe_update_fn = git_hash_sha256_update,
    ++		.unsafe_final_fn = git_hash_sha256_final,
    ++		.unsafe_final_oid_fn = git_hash_sha256_final_oid,
      		.empty_tree = &empty_tree_oid_sha256,
      		.empty_blob = &empty_blob_oid_sha256,
      		.null_oid = &null_oid_sha256,
 8:  137ec30d68a !  7:  af8fd9aa4ed Makefile: allow specifying a SHA-1 for non-cryptographic uses
    @@ Metadata
      ## Commit message ##
         Makefile: allow specifying a SHA-1 for non-cryptographic uses
     
    -    Introduce _FAST variants of the OPENSSL_SHA1, BLK_SHA1, and
    +    Introduce _UNSAFE variants of the OPENSSL_SHA1, BLK_SHA1, and
         APPLE_COMMON_CRYPTO_SHA1 compile-time knobs which indicate which SHA-1
         implementation is to be used for non-cryptographic uses.
     
    @@ Makefile: include shared.mak
      # Define APPLE_COMMON_CRYPTO_SHA1 to use Apple's CommonCrypto for
      # SHA-1.
      #
    -+# Define the same Makefile knobs as above, but suffixed with _FAST to
    -+# use the corresponding implementations for "fast" SHA-1 hashing for
    ++# Define the same Makefile knobs as above, but suffixed with _UNSAFE to
    ++# use the corresponding implementations for unsafe SHA-1 hashing for
     +# non-cryptographic purposes.
     +#
      # If don't enable any of the *_SHA1 settings in this section, Git will
    @@ Makefile: endif
      endif
      endif
      
    -+ifdef OPENSSL_SHA1_FAST
    ++ifdef OPENSSL_SHA1_UNSAFE
     +ifndef OPENSSL_SHA1
     +	EXTLIBS += $(LIB_4_CRYPTO)
    -+	BASIC_CFLAGS += -DSHA1_OPENSSL_FAST
    ++	BASIC_CFLAGS += -DSHA1_OPENSSL_UNSAFE
     +endif
     +else
    -+ifdef BLK_SHA1_FAST
    ++ifdef BLK_SHA1_UNSAFE
     +ifndef BLK_SHA1
     +	LIB_OBJS += block-sha1/sha1.o
    -+	BASIC_CFLAGS += -DSHA1_BLK_FAST
    ++	BASIC_CFLAGS += -DSHA1_BLK_UNSAFE
     +endif
     +else
    -+ifdef APPLE_COMMON_CRYPTO_SHA1_FAST
    ++ifdef APPLE_COMMON_CRYPTO_SHA1_UNSAFE
     +ifndef APPLE_COMMON_CRYPTO_SHA1
     +	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
    -+	BASIC_CFLAGS += -DSHA1_APPLE_FAST
    ++	BASIC_CFLAGS += -DSHA1_APPLE_UNSAFE
     +endif
     +endif
     +endif
    @@ hash.h
      #include "block-sha1/sha1.h"
      #endif
      
    -+#if defined(SHA1_APPLE_FAST)
    ++#if defined(SHA1_APPLE_UNSAFE)
     +#  include <CommonCrypto/CommonDigest.h>
    -+#  define platform_SHA_CTX_fast CC_SHA1_CTX
    -+#  define platform_SHA1_Init_fast CC_SHA1_Init
    -+#  define platform_SHA1_Update_fast CC_SHA1_Update
    -+#  define platform_SHA1_Final_fast CC_SHA1_Final
    -+#elif defined(SHA1_OPENSSL_FAST)
    ++#  define platform_SHA_CTX_unsafe CC_SHA1_CTX
    ++#  define platform_SHA1_Init_unsafe CC_SHA1_Init
    ++#  define platform_SHA1_Update_unsafe CC_SHA1_Update
    ++#  define platform_SHA1_Final_unsafe CC_SHA1_Final
    ++#elif defined(SHA1_OPENSSL_UNSAFE)
     +#  include <openssl/sha.h>
     +#  if defined(OPENSSL_API_LEVEL) && OPENSSL_API_LEVEL >= 3
    -+#    define SHA1_NEEDS_CLONE_HELPER_FAST
    ++#    define SHA1_NEEDS_CLONE_HELPER_UNSAFE
     +#    include "sha1/openssl.h"
    -+#    define platform_SHA_CTX_fast openssl_SHA1_CTX
    -+#    define platform_SHA1_Init_fast openssl_SHA1_Init
    -+#    define platform_SHA1_Clone_fast openssl_SHA1_Clone
    -+#    define platform_SHA1_Update_fast openssl_SHA1_Update
    -+#    define platform_SHA1_Final_fast openssl_SHA1_Final
    ++#    define platform_SHA_CTX_unsafe openssl_SHA1_CTX
    ++#    define platform_SHA1_Init_unsafe openssl_SHA1_Init
    ++#    define platform_SHA1_Clone_unsafe openssl_SHA1_Clone
    ++#    define platform_SHA1_Update_unsafe openssl_SHA1_Update
    ++#    define platform_SHA1_Final_unsafe openssl_SHA1_Final
     +#  else
    -+#    define platform_SHA_CTX_fast SHA_CTX
    -+#    define platform_SHA1_Init_fast SHA1_Init
    -+#    define platform_SHA1_Update_fast SHA1_Update
    -+#    define platform_SHA1_Final_fast SHA1_Final
    ++#    define platform_SHA_CTX_unsafe SHA_CTX
    ++#    define platform_SHA1_Init_unsafe SHA1_Init
    ++#    define platform_SHA1_Update_unsafe SHA1_Update
    ++#    define platform_SHA1_Final_unsafe SHA1_Final
     +#  endif
    -+#elif defined(SHA1_BLK_FAST)
    ++#elif defined(SHA1_BLK_UNSAFE)
     +#  include "block-sha1/sha1.h"
    -+#  define platform_SHA_CTX_fast blk_SHA_CTX
    -+#  define platform_SHA1_Init_fast blk_SHA1_Init
    -+#  define platform_SHA1_Update_fast blk_SHA1_Update
    -+#  define platform_SHA1_Final_fast blk_SHA1_Final
    ++#  define platform_SHA_CTX_unsafe blk_SHA_CTX
    ++#  define platform_SHA1_Init_unsafe blk_SHA1_Init
    ++#  define platform_SHA1_Update_unsafe blk_SHA1_Update
    ++#  define platform_SHA1_Final_unsafe blk_SHA1_Final
     +#endif
     +
      #if defined(SHA256_NETTLE)
 9:  4018261366f !  8:  4b83dd05e9f csum-file.c: use fast SHA-1 implementation when available
    @@ Metadata
     Author: Taylor Blau <me@xxxxxxxxxxxx>
     
      ## Commit message ##
    -    csum-file.c: use fast SHA-1 implementation when available
    +    csum-file.c: use unsafe SHA-1 implementation when available
     
    -    Update hashwrite() and friends to use the fast_-variants of hashing
    -    functions, calling for e.g., "the_hash_algo->fast_update_fn()" instead
    +    Update hashwrite() and friends to use the unsafe_-variants of hashing
    +    functions, calling for e.g., "the_hash_algo->unsafe_update_fn()" instead
         of "the_hash_algo->update_fn()".
     
         These callers only use the_hash_algo to produce a checksum, which we
         depend on for data integrity, but not for cryptographic purposes, so
    -    these callers are safe to use the fast (and potentially non-collision
    +    these callers are safe to use the unsafe (and potentially non-collision
         detecting) SHA-1 implementation.
     
         To time this, I took a freshly packed copy of linux.git, and ran the
    -    following with and without the OPENSSL_SHA1_FAST=1 build-knob. Both
    +    following with and without the OPENSSL_SHA1_UNSAFE=1 build-knob. Both
         versions were compiled with -O3:
     
             $ git for-each-ref --format='%(objectname)' refs/heads refs/tags >in
             $ valgrind --tool=callgrind ~/src/git/git-pack-objects \
                 --revs --stdout --all-progress --use-bitmap-index <in >/dev/null
     
    -    Without OPENSSL_SHA1_FAST=1 (that is, using the collision-detecting
    +    Without OPENSSL_SHA1_UNSAFE=1 (that is, using the collision-detecting
         SHA-1 implementation for both cryptographic and non-cryptographic
         purposes), we spend a significant amount of our instruction count in
         hashwrite():
    @@ Commit message
         , and the resulting "clone" takes 19.219 seconds of wall clock time,
         18.94 seconds of user time and 0.28 seconds of system time.
     
    -    Compiling with OPENSSL_SHA1_FAST=1, we spend ~60% fewer instructions in
    -    hashwrite():
    +    Compiling with OPENSSL_SHA1_UNSAFE=1, we spend ~60% fewer instructions
    +    in hashwrite():
     
             $ callgrind_annotate --inclusive=yes | grep hashwrite | head -n1
              59,164,001,176 (58.79%)  /home/ttaylorr/src/git/csum-file.c:hashwrite [/home/ttaylorr/src/git/git-pack-objects]
     
    -    , and generate the resulting "clone" much faster, in only 11.597 seconds
    +    , and generate the resulting "clone" much unsafeer, in only 11.597 seconds
         of wall time, 11.37 seconds of user time, and 0.23 seconds of system
         time, for a ~40% speed-up.
     
    @@ csum-file.c: void hashflush(struct hashfile *f)
      	if (offset) {
      		if (!f->skip_hash)
     -			the_hash_algo->update_fn(&f->ctx, f->buffer, offset);
    -+			the_hash_algo->fast_update_fn(&f->ctx, f->buffer, offset);
    ++			the_hash_algo->unsafe_update_fn(&f->ctx, f->buffer, offset);
      		flush(f, f->buffer, offset);
      		f->offset = 0;
      	}
    @@ csum-file.c: int finalize_hashfile(struct hashfile *f, unsigned char *result,
      		hashclr(f->buffer, the_repository->hash_algo);
      	else
     -		the_hash_algo->final_fn(f->buffer, &f->ctx);
    -+		the_hash_algo->fast_final_fn(f->buffer, &f->ctx);
    ++		the_hash_algo->unsafe_final_fn(f->buffer, &f->ctx);
      
      	if (result)
      		hashcpy(result, f->buffer, the_repository->hash_algo);
    @@ csum-file.c: void hashwrite(struct hashfile *f, const void *buf, unsigned int co
      			 */
      			if (!f->skip_hash)
     -				the_hash_algo->update_fn(&f->ctx, buf, nr);
    -+				the_hash_algo->fast_update_fn(&f->ctx, buf, nr);
    ++				the_hash_algo->unsafe_update_fn(&f->ctx, buf, nr);
      			flush(f, buf, nr);
      		} else {
      			/*
    @@ csum-file.c: static struct hashfile *hashfd_internal(int fd, const char *name,
      	f->do_crc = 0;
      	f->skip_hash = 0;
     -	the_hash_algo->init_fn(&f->ctx);
    -+	the_hash_algo->fast_init_fn(&f->ctx);
    ++	the_hash_algo->unsafe_init_fn(&f->ctx);
      
      	f->buffer_len = buffer_len;
      	f->buffer = xmalloc(buffer_len);
    @@ csum-file.c: void hashfile_checkpoint(struct hashfile *f, struct hashfile_checkp
      	hashflush(f);
      	checkpoint->offset = f->total;
     -	the_hash_algo->clone_fn(&checkpoint->ctx, &f->ctx);
    -+	the_hash_algo->fast_clone_fn(&checkpoint->ctx, &f->ctx);
    ++	the_hash_algo->unsafe_clone_fn(&checkpoint->ctx, &f->ctx);
      }
      
      int hashfile_truncate(struct hashfile *f, struct hashfile_checkpoint *checkpoint)
    @@ csum-file.c: int hashfile_truncate(struct hashfile *f, struct hashfile_checkpoin
      		return -1;
      	f->total = offset;
     -	the_hash_algo->clone_fn(&f->ctx, &checkpoint->ctx);
    -+	the_hash_algo->fast_clone_fn(&f->ctx, &checkpoint->ctx);
    ++	the_hash_algo->unsafe_clone_fn(&f->ctx, &checkpoint->ctx);
      	f->offset = 0; /* hashflush() was called in checkpoint */
      	return 0;
      }
    @@ csum-file.c: int hashfile_checksum_valid(const unsigned char *data, size_t total
     -	the_hash_algo->init_fn(&ctx);
     -	the_hash_algo->update_fn(&ctx, data, data_len);
     -	the_hash_algo->final_fn(got, &ctx);
    -+	the_hash_algo->fast_init_fn(&ctx);
    -+	the_hash_algo->fast_update_fn(&ctx, data, data_len);
    -+	the_hash_algo->fast_final_fn(got, &ctx);
    ++	the_hash_algo->unsafe_init_fn(&ctx);
    ++	the_hash_algo->unsafe_update_fn(&ctx, data, data_len);
    ++	the_hash_algo->unsafe_final_fn(got, &ctx);
      
      	return hasheq(got, data + data_len, the_repository->hash_algo);
      }

base-commit: 6258f68c3c1092c901337895c864073dcdea9213
-- 
2.46.2.636.g4b83dd05e9f




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux