mkcomposefs: Add a hidden CFS_PARSE_STRICT

Right now in some cases we have "loose" semantics; a
simple case I noticed earlier is we accepted whatever
input size for a symlink but ignored it.

I also noticed we accepted (and ignored) a nonzero rdev
for a non-device.

We have prior cases around missing trailing newline, etc.

In preparation for a more official "mkcomposefs --strict"
mode, add a hidden environment variable and use it to validate
some of the above.

There's other bits missing though - for example in strict
mode I'd like to hard require that the xattrs are already
in canonical form (sorted, no duplicates).

This touches on a general topic that there's "strict" and
then something even stronger around "normal form" where
we disallow any ambiguous input such as duplicate `//` in
filenames etc.

That'd be a good bit more work. For now let's just
lay the groundwork for stricter validation.

Signed-off-by: Colin Walters <[email protected]>

tests/test-checksums.sh

@@ -14,6 +14,12 @@ tmpfile=$(mktemp --tmpdir lcfs-test.XXXXXX)
 tmpfile2=$(mktemp --tmpdir lcfs-test.XXXXXX)
 trap 'rm -rf -- "$tmpfile" "$tmpfile2"' EXIT
 
+# Ensure our builtin dumpfiles are strict, minus some exceptions
+declare -A nonstrict
+nonstrict=(
+    ["no-newline.dump"]="1"
+)
+
 for format in erofs ; do
     for file in ${TEST_ASSETS} ; do
         if [ ! -f $ASSET_DIR/$file ] ; then
@@ -35,6 +41,12 @@ for format in erofs ; do
             CAT=cat
         fi
 
+        if test -v "nonstrict[$file]"; then
+            unset CFS_PARSE_STRICT
+        else
+            export CFS_PARSE_STRICT=1
+        fi
+
         $CAT $ASSET_DIR/$file | ${VALGRIND_PREFIX} ${BINDIR}/mkcomposefs $VERSION_ARG --from-file - $tmpfile
         SHA=$(sha256sum $tmpfile | awk "{print \$1}")
 
@@ -60,6 +72,6 @@ for format in erofs ; do
             echo Invalid $format checksum of file generated from $file: $SHA, expected $EXPECTED_SHA
             exit 1
         fi
-        echo "ok $file"
+        echo "ok $file (CFS_PARSE_STRICT=${CFS_PARSE_STRICT:-0})"
     done
 done

tools/mkcomposefs.c

@@ -401,7 +401,8 @@ static char *tree_resolve_hardlinks(dump_info *info)
 	return NULL;
 }
 
-static char *tree_from_dump_line(dump_info *info, const char *line, size_t line_len)
+static char *tree_from_dump_line(dump_info *info, const char *line,
+				 size_t line_len, bool strict_mode)
 {
 	int ret;
 
@@ -515,13 +516,28 @@ static char *tree_from_dump_line(dump_info *info, const char *line, size_t line_
 	lcfs_node_set_nlink(node, nlink);
 	lcfs_node_set_uid(node, uid);
 	lcfs_node_set_gid(node, gid);
-	lcfs_node_set_rdev64(node, rdev);
+	if (type == S_IFCHR || type == S_IFBLK) {
+		lcfs_node_set_rdev64(node, rdev);
+	} else if (strict_mode && rdev != 0) {
+		return make_error("Non-zero devnum on non-device");
+	}
 	lcfs_node_set_mtime(node, &mtime);
 	// Validate that symlinks are non-empty
 	if (type == S_IFLNK) {
 		if (lcfs_node_set_symlink_payload(node, payload) < 0) {
 			return make_error("Invalid symlink");
 		}
+		if (strict_mode) {
+			size_t payload_len = strlen(payload);
+			if (size != payload_len) {
+				return make_error(
+					"Invalid symlink size %lld, must match size %lld",
+					(long long)payload_len, (long long)size);
+			}
+			if (content && *content) {
+				return make_error("Symlink cannot have content");
+			}
+		}
 	} else {
 		if (lcfs_node_set_payload(node, payload) < 0)
 			return make_error("Invalid payload");
@@ -617,6 +633,10 @@ static struct lcfs_node_s *tree_from_dump(FILE *input, char **out_err)
 
 	struct buffer buf = { NULL };
 
+	// For now a hidden environment variable, may be promoted to a stable CLI
+	// option once we're happy with semantics.
+	bool strict_mode = getenv("CFS_PARSE_STRICT") != NULL;
+
 	while (!feof(input)) {
 		size_t bytes_read = buffer_fill(&buf, input);
 		bool short_read = bytes_read == 0;
@@ -632,7 +652,8 @@ static struct lcfs_node_s *tree_from_dump(FILE *input, char **out_err)
 				split_at(&data, &remaining_data, '\n', &partial);
 
 			if (!partial || short_read) {
-				char *err = tree_from_dump_line(&info, line, line_len);
+				char *err = tree_from_dump_line(
+					&info, line, line_len, strict_mode);
 				if (err != NULL) {
 					*out_err = err;
 					buffer_free(&buf);
@@ -648,13 +669,16 @@ static struct lcfs_node_s *tree_from_dump(FILE *input, char **out_err)
 		}
 	}
 	// Handle no trailing newline
-	if (buf.size > 0) {
-		char *err = tree_from_dump_line(&info, buf.buf, buf.size);
+	if (buf.size > 0 && !strict_mode) {
+		char *err = tree_from_dump_line(&info, buf.buf, buf.size, strict_mode);
 		if (err != NULL) {
 			*out_err = err;
 			buffer_free(&buf);
 			return NULL;
 		}
+	} else if (buf.size > 0) {
+		*out_err = make_error("Missing trailing newline");
+		return NULL;
 	}
 
 	buffer_free(&buf);