bin/58876: mtree: Produces a different checksum when verifying

[jlduran%gmail.com@localhost]

>Number:         58876
>Category:       bin
>Synopsis:       mtree: Produces a different checksum when verifying
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Dec 05 16:40:02 +0000 2024
>Originator:     Jose Luis Duran
>Release:        Not applicable to NetBSD
>Organization:
FreeBSD
>Environment:
NOTE: This bug is not present on NetBSD, only on FreeBSD. FreeBSD uses mtree from NetBSD.
>Description:
A bug report was opened[^1] stating that a user following the procedure described in the FreeBSD handbook[^2] to generate and verify an mtree-based specification file produced non-matching checksums. Everything is working fine, except that the output produces two different checksums.

Prior to FreeBSD using NetBSD's mtree, a commit fixed this issue[^3]. This fix is not present on NetBSD, as the issue is not manifested on NetBSD.

[^1]: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=231072
[^2]: https://docs.freebsd.org/en/books/handbook/security/#security-ids-generate-spec-file
[^3]: https://cgit.freebsd.org/src/commit/?id=afe54b5818317b574a1b4b0f88c1643efc2c8da2
>How-To-Repeat:
On FreeBSD:

# mtree -s 123456789 -c -K cksum,sha512 -p /bin > /home/user/.bin_chksum_mtree
mtree: /bin checksum: 1461382254
# mtree -s 123456789 -p /bin < /home/user/.bin_chksum_mtree >> /home/user/.bin_chksum_output
mtree: /bin checksum: 3158826036

The resulting file is empty (there is no mismatch), however the checksum is different.

>Fix:
Use a fix similar to the one proposed in the pull request, rather than the original FreeBSD fix, as I *think* it blends better with the current code, but feel free to re-arrange it as you see fit.

The idea is to traverse the existing directory tree in lexical order, to allow a direct comparison between different runs regardless of the order of what readdir(3) returns.

Move dcmp() next to nodecmp(), as these two functions go hand-in-hand, and pass it as the compar() argument to fts_open(3) in vwalk(), just like in cwalk().

The proposed patch is attached:

Subject: [PATCH] mtree: Traverse in lexical order

When applying a spec, traverse the existing directory tree in lexical
order.  This allows a direct comparison between the output of two
different runs, regardless of the order in which readdir(3) returns
directory entries.

FreeBSD PR:	231072
---
 usr.sbin/mtree/create.c | 28 ----------------------------
 usr.sbin/mtree/extern.h |  7 +++++++
 usr.sbin/mtree/spec.c   | 21 +++++++++++++++++++++
 usr.sbin/mtree/verify.c |  2 +-
 4 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/usr.sbin/mtree/create.c b/usr.sbin/mtree/create.c
index bb5d2f21ad80..4962b51050ec 100644
--- a/usr.sbin/mtree/create.c
+++ b/usr.sbin/mtree/create.c
@@ -84,13 +84,6 @@ static uid_t uid;
 static mode_t mode;
 static u_long flags;
 
-#if defined(__FreeBSD__) && !defined(HAVE_NBTOOL_CONFIG_H)
-#define	FTS_CONST const
-#else
-#define	FTS_CONST
-#endif
-
-static int	dcmp(const FTSENT *FTS_CONST *, const FTSENT *FTS_CONST *);
 static void	output(FILE *, int, int *, const char *, ...)
     __printflike(4, 5);
 static int	statd(FILE *, FTS *, FTSENT *, uid_t *, gid_t *, mode_t *,
@@ -444,27 +437,6 @@ statd(FILE *fp, FTS *t, FTSENT *parent, uid_t *puid, gid_t *pgid, mode_t *pmode,
 	return (0);
 }
 
-/*
- * dcmp --
- *	used as a comparison function passed to fts_open() to control
- *	the order in which fts_read() returns results.	We make
- *	directories sort after non-directories, but otherwise sort in
- *	strcmp() order.
- *
- * Keep this in sync with nodecmp() in spec.c.
- */
-static int
-dcmp(const FTSENT *FTS_CONST *a, const FTSENT *FTS_CONST *b)
-{
-
-	if (S_ISDIR((*a)->fts_statp->st_mode)) {
-		if (!S_ISDIR((*b)->fts_statp->st_mode))
-			return (1);
-	} else if (S_ISDIR((*b)->fts_statp->st_mode))
-		return (-1);
-	return (strcmp((*a)->fts_name, (*b)->fts_name));
-}
-
 void
 output(FILE *fp, int indent, int *offset, const char *fmt, ...)
 {
diff --git a/usr.sbin/mtree/extern.h b/usr.sbin/mtree/extern.h
index da34e99f1a9a..bb82bd19f8bd 100644
--- a/usr.sbin/mtree/extern.h
+++ b/usr.sbin/mtree/extern.h
@@ -49,6 +49,12 @@
 #include <netdb.h>
 #endif
 
+#if defined(__FreeBSD__) && !defined(HAVE_NBTOOL_CONFIG_H)
+#define	FTS_CONST const
+#else
+#define	FTS_CONST
+#endif
+
 #ifndef MAXHOSTNAMELEN
 #define MAXHOSTNAMELEN 256
 #endif
@@ -64,6 +70,7 @@ int	 check_excludes(const char *, const char *);
 int	 compare(NODE *, FTSENT *);
 int	 crc(int, uint32_t *, uint32_t *);
 void	 cwalk(FILE *);
+int	 dcmp(const FTSENT *FTS_CONST *, const FTSENT *FTS_CONST *);
 void	 dump_nodes(FILE *, const char *, NODE *, int);
 void	 init_excludes(void);
 int	 matchtags(NODE *);
diff --git a/usr.sbin/mtree/spec.c b/usr.sbin/mtree/spec.c
index 25eac4b55ade..045fcea55ccb 100644
--- a/usr.sbin/mtree/spec.c
+++ b/usr.sbin/mtree/spec.c
@@ -855,3 +855,24 @@ nodecmp(const NODE *a, const NODE *b)
 		return -1;
 	return strcmp(a->name, b->name);
 }
+
+/*
+ * dcmp --
+ *	used as a comparison function passed to fts_open() to control
+ *	the order in which fts_read() returns results.	We make
+ *	directories sort after non-directories, but otherwise sort in
+ *	strcmp() order.
+ *
+ * Keep this in sync with nodecmp() above.
+ */
+int
+dcmp(const FTSENT *FTS_CONST *a, const FTSENT *FTS_CONST *b)
+{
+
+	if (S_ISDIR((*a)->fts_statp->st_mode)) {
+		if (!S_ISDIR((*b)->fts_statp->st_mode))
+			return (1);
+	} else if (S_ISDIR((*b)->fts_statp->st_mode))
+		return (-1);
+	return (strcmp((*a)->fts_name, (*b)->fts_name));
+}
diff --git a/usr.sbin/mtree/verify.c b/usr.sbin/mtree/verify.c
index 2430fb1b8066..520a329006ba 100644
--- a/usr.sbin/mtree/verify.c
+++ b/usr.sbin/mtree/verify.c
@@ -86,7 +86,7 @@ vwalk(void)
 	argv[0] = dot;
 	argv[1] = NULL;
 
-	if ((t = fts_open(argv, ftsoptions, NULL)) == NULL)
+	if ((t = fts_open(argv, ftsoptions, dcmp)) == NULL)
 		mtree_err("fts_open: %s", strerror(errno));
 	level = root;
 	specdepth = rval = 0;
-- 
Jose Luis Duran