Merge pull request #4197 from pks-t/pks/verify-object-hashes

Verify object hashes

7 files changed, 162 insertions, 25 deletions

diff --git a/include/git2/common.h b/include/git2/common.h
index 6d2092028..d83e8c3a0 100644
--- a/include/git2/common.h
+++ b/include/git2/common.h
@@ -182,6 +182,7 @@ typedef enum {
 	GIT_OPT_ENABLE_SYNCHRONOUS_OBJECT_CREATION,
 	GIT_OPT_GET_WINDOWS_SHAREMODE,
 	GIT_OPT_SET_WINDOWS_SHAREMODE,
+	GIT_OPT_ENABLE_STRICT_HASH_VERIFICATION,
 } git_libgit2_opt_t;
 
 /**
@@ -337,6 +338,13 @@ typedef enum {
  *		> is written to permanent storage, not simply cached.  This
  *		> defaults to disabled.
  *
+ *	 opts(GIT_OPT_ENABLE_STRICT_HASH_VERIFICATION, int enabled)
+ *
+ *		> Enable strict verification of object hashsums when reading
+ *		> objects from disk. This may impact performance due to an
+ *		> additional checksum calculation on each object. This defaults
+ *		> to enabled.
+ *
  * @param option Option key
  * @param ... value to set the option
  * @return 0 on success, <0 on failure
diff --git a/include/git2/errors.h b/include/git2/errors.h
index 71bff0f9d..6f5580253 100644
--- a/include/git2/errors.h
+++ b/include/git2/errors.h
@@ -54,6 +54,7 @@ typedef enum {
 	GIT_PASSTHROUGH     = -30,	/**< Internal only */
 	GIT_ITEROVER        = -31,	/**< Signals end of iteration with iterator */
 	GIT_RETRY           = -32,	/**< Internal only */
+	GIT_EMISMATCH       = -33,	/**< Hashsum mismatch in object */
 } git_error_code;
 
 /**
diff --git a/src/odb.c b/src/odb.c
index cf321f51e..3090ccaac 100644
--- a/src/odb.c
+++ b/src/odb.c
@@ -31,6 +31,8 @@
 
 #define GIT_ALTERNATES_MAX_DEPTH 5
 
+bool git_odb__strict_hash_verification = true;
+
 typedef struct
 {
 	git_odb_backend *backend;
@@ -998,7 +1000,9 @@ static int odb_read_1(git_odb_object **out, git_odb *db, const git_oid *id,
 	size_t i;
 	git_rawobj raw;
 	git_odb_object *object;
+	git_oid hashed;
 	bool found = false;
+	int error;
 
 	if (!only_refreshed && odb_read_hardcoded(&raw, id) == 0)
 		found = true;
@@ -1011,7 +1015,7 @@ static int odb_read_1(git_odb_object **out, git_odb *db, const git_oid *id,
 			continue;
 
 		if (b->read != NULL) {
-			int error = b->read(&raw.data, &raw.len, &raw.type, b, id);
+			error = b->read(&raw.data, &raw.len, &raw.type, b, id);
 			if (error == GIT_PASSTHROUGH || error == GIT_ENOTFOUND)
 				continue;
 
@@ -1025,12 +1029,26 @@ static int odb_read_1(git_odb_object **out, git_odb *db, const git_oid *id,
 	if (!found)
 		return GIT_ENOTFOUND;
 
+	if (git_odb__strict_hash_verification) {
+		if ((error = git_odb_hash(&hashed, raw.data, raw.len, raw.type)) < 0)
+			goto out;
+
+		if (!git_oid_equal(id, &hashed)) {
+			error = git_odb__error_mismatch(id, &hashed);
+			goto out;
+		}
+	}
+
 	giterr_clear();
 	if ((object = odb_object__alloc(id, &raw)) == NULL)
-		return -1;
+		goto out;
 
 	*out = git_cache_store_raw(odb_cache(db), object);
-	return 0;
+
+out:
+	if (error)
+		git__free(raw.data);
+	return error;
 }
 
 int git_odb_read(git_odb_object **out, git_odb *db, const git_oid *id)
@@ -1081,9 +1099,9 @@ static int read_prefix_1(git_odb_object **out, git_odb *db,
 		const git_oid *key, size_t len, bool only_refreshed)
 {
 	size_t i;
-	int error = GIT_ENOTFOUND;
+	int error;
 	git_oid found_full_oid = {{0}};
-	git_rawobj raw;
+	git_rawobj raw = {0};
 	void *data = NULL;
 	bool found = false;
 	git_odb_object *object;
@@ -1102,14 +1120,22 @@ static int read_prefix_1(git_odb_object **out, git_odb *db,
 				continue;
 
 			if (error)
-				return error;
+				goto out;
 
 			git__free(data);
 			data = raw.data;
 
 			if (found && git_oid__cmp(&full_oid, &found_full_oid)) {
-				git__free(raw.data);
-				return git_odb__error_ambiguous("multiple matches for prefix");
+				git_buf buf = GIT_BUF_INIT;
+
+				git_buf_printf(&buf, "multiple matches for prefix: %s",
+					git_oid_tostr_s(&full_oid));
+				git_buf_printf(&buf, " %s",
+					git_oid_tostr_s(&found_full_oid));
+
+				error = git_odb__error_ambiguous(buf.ptr);
+				git_buf_free(&buf);
+				goto out;
 			}
 
 			found_full_oid = full_oid;
@@ -1120,11 +1146,28 @@ static int read_prefix_1(git_odb_object **out, git_odb *db,
 	if (!found)
 		return GIT_ENOTFOUND;
 
+	if (git_odb__strict_hash_verification) {
+		git_oid hash;
+
+		if ((error = git_odb_hash(&hash, raw.data, raw.len, raw.type)) < 0)
+			goto out;
+
+		if (!git_oid_equal(&found_full_oid, &hash)) {
+			error = git_odb__error_mismatch(&found_full_oid, &hash);
+			goto out;
+		}
+	}
+
 	if ((object = odb_object__alloc(&found_full_oid, &raw)) == NULL)
-		return -1;
+		goto out;
 
 	*out = git_cache_store_raw(odb_cache(db), object);
-	return 0;
+
+out:
+	if (error)
+		git__free(raw.data);
+
+	return error;
 }
 
 int git_odb_read_prefix(
@@ -1411,6 +1454,19 @@ int git_odb_refresh(struct git_odb *db)
 	return 0;
 }
 
+int git_odb__error_mismatch(const git_oid *expected, const git_oid *actual)
+{
+	char expected_oid[GIT_OID_HEXSZ + 1], actual_oid[GIT_OID_HEXSZ + 1];
+
+	git_oid_tostr(expected_oid, sizeof(expected_oid), expected);
+	git_oid_tostr(actual_oid, sizeof(actual_oid), actual);
+
+	giterr_set(GITERR_ODB, "object hash mismatch - expected %s but got %s",
+		expected_oid, actual_oid);
+
+	return GIT_EMISMATCH;
+}
+
 int git_odb__error_notfound(
 	const char *message, const git_oid *oid, size_t oid_len)
 {
diff --git a/src/odb.h b/src/odb.h
index 4f548bbba..61d687abf 100644
--- a/src/odb.h
+++ b/src/odb.h
@@ -20,6 +20,8 @@
 #define GIT_OBJECT_DIR_MODE 0777
 #define GIT_OBJECT_FILE_MODE 0444
 
+extern bool git_odb__strict_hash_verification;
+
 /* DO NOT EXPORT */
 typedef struct {
 	void *data;			/**< Raw, decompressed object data. */
@@ -96,6 +98,12 @@ int git_odb__hashfd_filtered(
  */
 int git_odb__hashlink(git_oid *out, const char *path);
 
+/**
+ * Generate a GIT_EMISMATCH error for the ODB.
+ */
+int git_odb__error_mismatch(
+	const git_oid *expected, const git_oid *actual);
+
 /*
  * Generate a GIT_ENOTFOUND error for the ODB.
  */
diff --git a/src/settings.c b/src/settings.c
index 42f247aae..25c5aae6d 100644
--- a/src/settings.c
+++ b/src/settings.c
@@ -15,6 +15,7 @@
 #include "cache.h"
 #include "global.h"
 #include "object.h"
+#include "odb.h"
 #include "refs.h"
 #include "transports/smart.h"
 
@@ -243,6 +244,10 @@ int git_libgit2_opts(int key, ...)
 #endif
 		break;
 
+	case GIT_OPT_ENABLE_STRICT_HASH_VERIFICATION:
+		git_odb__strict_hash_verification = (va_arg(ap, int) != 0);
+		break;
+
 	default:
 		giterr_set(GITERR_INVALID, "invalid option key");
 		error = -1;
diff --git a/tests/object/lookup.c b/tests/object/lookup.c
index cfa6d4678..277e2e0c0 100644
--- a/tests/object/lookup.c
+++ b/tests/object/lookup.c
@@ -6,13 +6,12 @@ static git_repository *g_repo;
 
 void test_object_lookup__initialize(void)
 {
-   cl_git_pass(git_repository_open(&g_repo, cl_fixture("testrepo.git")));
+	g_repo = cl_git_sandbox_init("testrepo.git");
 }
 
 void test_object_lookup__cleanup(void)
 {
-	git_repository_free(g_repo);
-	g_repo = NULL;
+	cl_git_sandbox_cleanup();
 }
 
 void test_object_lookup__lookup_wrong_type_returns_enotfound(void)
@@ -63,3 +62,60 @@ void test_object_lookup__lookup_wrong_type_eventually_returns_enotfound(void)
 		GIT_ENOTFOUND, git_object_lookup(&object, g_repo, &oid, GIT_OBJ_TAG));
 }
 
+void test_object_lookup__lookup_corrupt_object_returns_error(void)
+{
+	const char *commit = "8e73b769e97678d684b809b163bebdae2911720f",
+	      *file = "objects/8e/73b769e97678d684b809b163bebdae2911720f";
+	git_buf path = GIT_BUF_INIT, contents = GIT_BUF_INIT;
+	git_oid oid;
+	git_object *object;
+	size_t i;
+
+	cl_git_pass(git_oid_fromstr(&oid, commit));
+	cl_git_pass(git_buf_joinpath(&path, git_repository_path(g_repo), file));
+	cl_git_pass(git_futils_readbuffer(&contents, path.ptr));
+
+	/* Corrupt and try to read the object */
+	for (i = 0; i < contents.size; i++) {
+		contents.ptr[i] ^= 0x1;
+		cl_git_pass(git_futils_writebuffer(&contents, path.ptr, O_RDWR, 0644));
+		cl_git_fail(git_object_lookup(&object, g_repo, &oid, GIT_OBJ_COMMIT));
+		contents.ptr[i] ^= 0x1;
+	}
+
+	/* Restore original content and assert we can read the object */
+	cl_git_pass(git_futils_writebuffer(&contents, path.ptr, O_RDWR, 0644));
+	cl_git_pass(git_object_lookup(&object, g_repo, &oid, GIT_OBJ_COMMIT));
+
+	git_object_free(object);
+	git_buf_free(&path);
+	git_buf_free(&contents);
+}
+
+void test_object_lookup__lookup_object_with_wrong_hash_returns_error(void)
+{
+	const char *oldloose = "objects/8e/73b769e97678d684b809b163bebdae2911720f",
+	      *newloose = "objects/8e/73b769e97678d684b809b163bebdae2911720e",
+	      *commit = "8e73b769e97678d684b809b163bebdae2911720e";
+	git_buf oldpath = GIT_BUF_INIT, newpath = GIT_BUF_INIT;
+	git_object *object;
+	git_oid oid;
+
+	cl_git_pass(git_oid_fromstr(&oid, commit));
+
+	/* Copy object to another location with wrong hash */
+	cl_git_pass(git_buf_joinpath(&oldpath, git_repository_path(g_repo), oldloose));
+	cl_git_pass(git_buf_joinpath(&newpath, git_repository_path(g_repo), newloose));
+	cl_git_pass(git_futils_cp(oldpath.ptr, newpath.ptr, 0644));
+
+	/* Verify that lookup fails due to a hashsum mismatch */
+	cl_git_fail_with(GIT_EMISMATCH, git_object_lookup(&object, g_repo, &oid, GIT_OBJ_COMMIT));
+
+	/* Disable verification and try again */
+	cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_HASH_VERIFICATION, 0));
+	cl_git_pass(git_object_lookup(&object, g_repo, &oid, GIT_OBJ_COMMIT));
+	cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_HASH_VERIFICATION, 1));
+
+	git_buf_free(&oldpath);
+	git_buf_free(&newpath);
+}
diff --git a/tests/odb/backend/nonrefreshing.c b/tests/odb/backend/nonrefreshing.c
index b43529479..f12ac741c 100644
--- a/tests/odb/backend/nonrefreshing.c
+++ b/tests/odb/backend/nonrefreshing.c
@@ -17,6 +17,9 @@ static git_repository *_repo;
 static fake_backend *_fake;
 static git_oid _oid;
 
+#define HASH "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"
+#define EMPTY_HASH "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391"
+
 static int fake_backend__exists(git_odb_backend *backend, const git_oid *oid)
 {
 	fake_backend *fake;
@@ -78,7 +81,6 @@ static int fake_backend__read_prefix(
 {
 	fake_backend *fake;
 
-	GIT_UNUSED(out_oid);
 	GIT_UNUSED(buffer_p);
 	GIT_UNUSED(len_p);
 	GIT_UNUSED(type_p);
@@ -89,6 +91,7 @@ static int fake_backend__read_prefix(
 
 	fake->read_prefix_calls++;
 
+	git_oid_cpy(out_oid, &_oid);
 	*len_p = 0;
 	*buffer_p = NULL;
 	*type_p = GIT_OBJ_BLOB;
@@ -130,7 +133,7 @@ static int build_fake_backend(
 	return 0;
 }
 
-static void setup_repository_and_backend(git_error_code error_code)
+static void setup_repository_and_backend(git_error_code error_code, const char *hash)
 {
 	git_odb *odb = NULL;
 	git_odb_backend *backend = NULL;
@@ -144,7 +147,7 @@ static void setup_repository_and_backend(git_error_code error_code)
 
 	_fake = (fake_backend *)backend;
 
-	cl_git_pass(git_oid_fromstr(&_oid, "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"));
+	cl_git_pass(git_oid_fromstr(&_oid, hash));
 }
 
 void test_odb_backend_nonrefreshing__cleanup(void)
@@ -156,7 +159,7 @@ void test_odb_backend_nonrefreshing__exists_is_invoked_once_on_failure(void)
 {
 	git_odb *odb;
 
-	setup_repository_and_backend(GIT_ENOTFOUND);
+	setup_repository_and_backend(GIT_ENOTFOUND, HASH);
 
 	cl_git_pass(git_repository_odb__weakptr(&odb, _repo));
 	cl_assert_equal_b(false, git_odb_exists(odb, &_oid));
@@ -168,7 +171,7 @@ void test_odb_backend_nonrefreshing__read_is_invoked_once_on_failure(void)
 {
 	git_object *obj;
 
-	setup_repository_and_backend(GIT_ENOTFOUND);
+	setup_repository_and_backend(GIT_ENOTFOUND, HASH);
 
 	cl_git_fail_with(
 		git_object_lookup(&obj, _repo, &_oid, GIT_OBJ_ANY),
@@ -181,7 +184,7 @@ void test_odb_backend_nonrefreshing__readprefix_is_invoked_once_on_failure(void)
 {
 	git_object *obj;
 
-	setup_repository_and_backend(GIT_ENOTFOUND);
+	setup_repository_and_backend(GIT_ENOTFOUND, HASH);
 
 	cl_git_fail_with(
 		git_object_lookup_prefix(&obj, _repo, &_oid, 7, GIT_OBJ_ANY),
@@ -196,7 +199,7 @@ void test_odb_backend_nonrefreshing__readheader_is_invoked_once_on_failure(void)
 	size_t len;
 	git_otype type;
 
-	setup_repository_and_backend(GIT_ENOTFOUND);
+	setup_repository_and_backend(GIT_ENOTFOUND, HASH);
 
 	cl_git_pass(git_repository_odb__weakptr(&odb, _repo));
 
@@ -211,7 +214,7 @@ void test_odb_backend_nonrefreshing__exists_is_invoked_once_on_success(void)
 {
 	git_odb *odb;
 
-	setup_repository_and_backend(GIT_OK);
+	setup_repository_and_backend(GIT_OK, HASH);
 
 	cl_git_pass(git_repository_odb__weakptr(&odb, _repo));
 	cl_assert_equal_b(true, git_odb_exists(odb, &_oid));
@@ -223,7 +226,7 @@ void test_odb_backend_nonrefreshing__read_is_invoked_once_on_success(void)
 {
 	git_object *obj;
 
-	setup_repository_and_backend(GIT_OK);
+	setup_repository_and_backend(GIT_OK, EMPTY_HASH);
 
 	cl_git_pass(git_object_lookup(&obj, _repo, &_oid, GIT_OBJ_ANY));
 
@@ -236,7 +239,7 @@ void test_odb_backend_nonrefreshing__readprefix_is_invoked_once_on_success(void)
 {
 	git_object *obj;
 
-	setup_repository_and_backend(GIT_OK);
+	setup_repository_and_backend(GIT_OK, EMPTY_HASH);
 
 	cl_git_pass(git_object_lookup_prefix(&obj, _repo, &_oid, 7, GIT_OBJ_ANY));
 
@@ -251,7 +254,7 @@ void test_odb_backend_nonrefreshing__readheader_is_invoked_once_on_success(void)
 	size_t len;
 	git_otype type;
 
-	setup_repository_and_backend(GIT_OK);
+	setup_repository_and_backend(GIT_OK, HASH);
 
 	cl_git_pass(git_repository_odb__weakptr(&odb, _repo));
 
@@ -264,7 +267,7 @@ void test_odb_backend_nonrefreshing__read_is_invoked_once_when_revparsing_a_full
 {
 	git_object *obj;
 
-	setup_repository_and_backend(GIT_ENOTFOUND);
+	setup_repository_and_backend(GIT_ENOTFOUND, HASH);
 
 	cl_git_fail_with(
 		git_revparse_single(&obj, _repo, "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"),