Merge pull request #1321 from jluebbe/manifest-checks

refactor manifest checks
rauc · Jan 15, 2024 · 7e7ac98 · 7e7ac98
2 parents 689d76b + b63f14b
commit 7e7ac98
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 60 deletions.
diff --git a/include/manifest.h b/include/manifest.h
@@ -109,8 +109,6 @@ G_GNUC_WARN_UNUSED_RESULT;
  * version. As an internal manifest, this must only include some generated
  * values (such as hashes/sizes for images, but not for the verity format).
  *
- * Use free_manifest() to free the returned manifest.
- *
  * @param manifest Pointer to the manifest to check
  * @param error return location for a GError, or NULL
  *
@@ -125,8 +123,6 @@ G_GNUC_WARN_UNUSED_RESULT;
  * version. As an external manifest this must contain all generated values (such
  * as hashes/sizes for images and for the verity format).
  *
- * Use free_manifest() to free the returned manifest.
- *
  * @param manifest Pointer to the manifest to check
  * @param error return location for a GError, or NULL
  *

diff --git a/src/manifest.c b/src/manifest.c
@@ -393,6 +393,39 @@ static gboolean check_manifest_common(const RaucManifest *mf, GError **error)
 		}
 	}
 
+	/* Check for hook file set if hooks are enabled */
+
+	if (mf->hooks.install_check == TRUE)
+		have_hooks = TRUE;
+
+	for (GList *l = mf->images; l != NULL; l = l->next) {
+		RaucImage *image = l->data;
+		if (image->hooks.pre_install == TRUE) {
+			have_hooks = TRUE;
+			break;
+		}
+		if (image->hooks.install == TRUE) {
+			have_hooks = TRUE;
+			break;
+		}
+		if (image->hooks.post_install == TRUE) {
+			have_hooks = TRUE;
+			break;
+		}
+	}
+
+	if (have_hooks && !mf->hook_name) {
+		g_set_error(error, R_MANIFEST_ERROR, R_MANIFEST_CHECK_ERROR, "Hooks used, but no hook 'filename' defined in [hooks] section");
+		goto out;
+	}
+
+	res = TRUE;
+out:
+	return res;
+}
+
+static gboolean check_manifest_bundled(const RaucManifest *mf, GError **error)
+{
 	for (GList *l = mf->images; l != NULL; l = l->next) {
 		RaucImage *image = l->data;
 
@@ -406,11 +439,11 @@ static gboolean check_manifest_common(const RaucManifest *mf, GError **error)
 
 		if (image->checksum.type != G_CHECKSUM_SHA256) {
 			g_set_error(error, R_MANIFEST_ERROR, R_MANIFEST_CHECK_ERROR, "Unsupported checksum algorithm for image %s", image->filename);
-			goto out;
+			return FALSE;
 		}
 		if (!image->checksum.digest) {
 			g_set_error(error, R_MANIFEST_ERROR, R_MANIFEST_CHECK_ERROR, "Missing digest for image %s", image->filename);
-			goto out;
+			return FALSE;
 		}
 		if (image->checksum.size < 0) {
 			/* RAUC versions before v1.5 allowed zero-size images but did not handle this explicitly.
@@ -423,40 +456,12 @@ static gboolean check_manifest_common(const RaucManifest *mf, GError **error)
 				image->checksum.size = 0;
 			} else {
 				g_set_error(error, R_MANIFEST_ERROR, R_MANIFEST_CHECK_ERROR, "Missing size for image %s", image->filename);
-				goto out;
+				return FALSE;
 			}
 		}
 	}
 
-	/* Check for hook file set if hooks are enabled */
-
-	if (mf->hooks.install_check == TRUE)
-		have_hooks = TRUE;
-
-	for (GList *l = mf->images; l != NULL; l = l->next) {
-		RaucImage *image = l->data;
-		if (image->hooks.pre_install == TRUE) {
-			have_hooks = TRUE;
-			break;
-		}
-		if (image->hooks.install == TRUE) {
-			have_hooks = TRUE;
-			break;
-		}
-		if (image->hooks.post_install == TRUE) {
-			have_hooks = TRUE;
-			break;
-		}
-	}
-
-	if (have_hooks && !mf->hook_name) {
-		g_set_error(error, R_MANIFEST_ERROR, R_MANIFEST_CHECK_ERROR, "Hooks used, but no hook 'filename' defined in [hooks] section");
-		goto out;
-	}
-
-	res = TRUE;
-out:
-	return res;
+	return TRUE;
 }
 
 gboolean check_manifest_internal(const RaucManifest *mf, GError **error)
@@ -474,35 +479,36 @@ gboolean check_manifest_internal(const RaucManifest *mf, GError **error)
 	switch (mf->bundle_format) {
 		case R_MANIFEST_FORMAT_PLAIN:
 			break; /* no additional data needed */
-		case R_MANIFEST_FORMAT_CRYPT: {
-			if (mf->bundle_crypt_key) {
-				g_set_error_literal(error, R_MANIFEST_ERROR, R_MANIFEST_CHECK_ERROR, "Unexpected key for crypt bundle in internal manifest");
-				goto out;
-			}
-		};
-		/* Fallthrough */
-		case R_MANIFEST_FORMAT_VERITY: {
-			if (mf->bundle_verity_hash) {
-				g_set_error(error, R_MANIFEST_ERROR, R_MANIFEST_CHECK_ERROR, "Unexpected hash for %s bundle in internal manifest", r_manifest_bundle_format_to_str(mf->bundle_format));
-				goto out;
-			}
-			if (mf->bundle_verity_salt) {
-				g_set_error(error, R_MANIFEST_ERROR, R_MANIFEST_CHECK_ERROR, "Unexpected hash for %s bundle in internal manifest", r_manifest_bundle_format_to_str(mf->bundle_format));
-				goto out;
-			}
-			if (mf->bundle_verity_size) {
-				g_set_error(error, R_MANIFEST_ERROR, R_MANIFEST_CHECK_ERROR, "Unexpected hash for %s bundle in internal manifest", r_manifest_bundle_format_to_str(mf->bundle_format));
-				goto out;
-			}
-
-			break;
-		};
+		case R_MANIFEST_FORMAT_CRYPT:
+		case R_MANIFEST_FORMAT_VERITY:
 		default: {
-			g_set_error(error, R_MANIFEST_ERROR, R_MANIFEST_CHECK_ERROR, "Unsupported bundle format");
+			g_set_error(error, R_MANIFEST_ERROR, R_MANIFEST_CHECK_ERROR, "Bundle format '%s' not allowed for internal manifest", r_manifest_bundle_format_to_str(mf->bundle_format));
 			goto out;
 		}
 	}
 
+	if (!check_manifest_bundled(mf, &ierror)) {
+		g_propagate_error(error, ierror);
+		goto out;
+	}
+
+	if (mf->bundle_crypt_key) {
+		g_set_error_literal(error, R_MANIFEST_ERROR, R_MANIFEST_CHECK_ERROR, "Unexpected key for crypt bundle in internal manifest");
+		goto out;
+	}
+	if (mf->bundle_verity_hash) {
+		g_set_error(error, R_MANIFEST_ERROR, R_MANIFEST_CHECK_ERROR, "Unexpected verity hash for %s bundle in internal manifest", r_manifest_bundle_format_to_str(mf->bundle_format));
+		goto out;
+	}
+	if (mf->bundle_verity_salt) {
+		g_set_error(error, R_MANIFEST_ERROR, R_MANIFEST_CHECK_ERROR, "Unexpected verity salt for %s bundle in internal manifest", r_manifest_bundle_format_to_str(mf->bundle_format));
+		goto out;
+	}
+	if (mf->bundle_verity_size) {
+		g_set_error(error, R_MANIFEST_ERROR, R_MANIFEST_CHECK_ERROR, "Unexpected verity size for %s bundle in internal manifest", r_manifest_bundle_format_to_str(mf->bundle_format));
+		goto out;
+	}
+
 	res = TRUE;
 out:
 	r_context_end_step("check_manifest", res);
@@ -592,6 +598,11 @@ gboolean check_manifest_external(const RaucManifest *mf, GError **error)
 		}
 	}
 
+	if (!check_manifest_bundled(mf, &ierror)) {
+		g_propagate_error(error, ierror);
+		goto out;
+	}
+
 	res = TRUE;
 out:
 	r_context_end_step("check_manifest", res);