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

Verify object hashes

Author: ethomson

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

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;
 
 /**

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)
 {

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.
  */

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;

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);
+}