Fix random uuid generation, no need to convert between byte order.

Also add regression test.

MANIFEST

@@ -285,6 +285,7 @@ lib/util/regress/sudo_parseln/test5.out.ok
 lib/util/regress/sudo_parseln/test6.in
 lib/util/regress/sudo_parseln/test6.out.ok
 lib/util/regress/tailq/hltq_test.c
+lib/util/regress/uuid/uuid_test.c
 lib/util/roundup.c
 lib/util/secure_path.c
 lib/util/setgroups.c

lib/util/Makefile.in

@@ -109,9 +109,10 @@ PVS_IGNORE = 'V707,V011,V002,V536'
 PVS_LOG_OPTS = -a 'GA:1,2' -e -t errorfile -d $(PVS_IGNORE)
 
 # Regression tests
-TEST_PROGS = conf_test hltq_test parseln_test progname_test strsplit_test \
+TEST_PROGS = conf_test hltq_test parseln_test progname_test \
-	     strtobool_test strtoid_test strtomode_test strtonum_test \
+	     strsplit_test strtobool_test strtoid_test strtomode_test \
-	     parse_gids_test getgids getgrouplist_test @COMPAT_TEST_PROGS@
+	     strtonum_test parse_gids_test getgids getgrouplist_test \
+	     uuid_test @COMPAT_TEST_PROGS@
 TEST_LIBS = @LIBS@
 TEST_LDFLAGS = @LDFLAGS@
 
@@ -182,6 +183,8 @@ GETGROUPLIST_TEST_OBJS = getgrouplist_test.lo getgrouplist.lo
 
 STRSIG_TEST_OBJS = strsig_test.lo sig2str.lo str2sig.lo @SIGNAME@
 
+UUID_TEST_OBJS = uuid_test.lo uuid.lo
+
 FUZZ_SUDO_CONF_OBJS = fuzz_sudo_conf.lo
 
 FUZZ_SUDO_CONF_CORPUS = $(srcdir)/regress/corpus/seed/sudo_conf/sudo.conf.*
@@ -305,6 +308,9 @@ strtonum_test: $(STRTONUM_TEST_OBJS) libsudo_util.la
 strtoid_test: $(STRTOID_TEST_OBJS) libsudo_util.la
 	$(LIBTOOL) $(LTFLAGS) --mode=link $(CC) -o $@ $(STRTOID_TEST_OBJS) libsudo_util.la $(ASAN_LDFLAGS) $(PIE_LDFLAGS) $(SSP_LDFLAGS) $(TEST_LDFLAGS) $(TEST_LIBS)
 
+uuid_test: $(UUID_TEST_OBJS) libsudo_util.la
+	$(LIBTOOL) $(LTFLAGS) --mode=link $(CC) -o $@ $(UUID_TEST_OBJS) libsudo_util.la $(ASAN_LDFLAGS) $(PIE_LDFLAGS) $(SSP_LDFLAGS) $(TEST_LDFLAGS) $(TEST_LIBS)
+
 fuzz_sudo_conf: $(FUZZ_SUDO_CONF_OBJS) $(LIBFUZZSTUB) libsudo_util.la
 	$(LIBTOOL) $(LTFLAGS) --mode=link $(CC) -o $@ $(FUZZ_SUDO_CONF_OBJS) $(ASAN_LDFLAGS) $(PIE_LDFLAGS) $(SSP_LDFLAGS) $(FUZZ_LDFLAGS) $(FUZZ_LIBS) libsudo_util.la
 
@@ -436,6 +442,7 @@ check: $(TEST_PROGS) check-fuzzer
 	    ./strtoid_test || rval=`expr $$rval + $$?`; \
 	    ./strtomode_test || rval=`expr $$rval + $$?`; \
 	    ./strtonum_test || rval=`expr $$rval + $$?`; \
+	    ./uuid_test || rval=`expr $$rval + $$?`; \
 	    ./hltq_test || rval=`expr $$rval + $$?`; \
 	    ./progname_test || rval=`expr $$rval + $$?`; \
 	    rm -f ./progname_test2; ln -s ./progname_test ./progname_test2; \
@@ -1514,3 +1521,15 @@ uuid.i: $(srcdir)/uuid.c $(incdir)/compat/stdbool.h $(incdir)/sudo_compat.h \
 	$(CC) -E -o $@ $(CPPFLAGS) $<
 uuid.plog: uuid.i
 	rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/uuid.c --i-file $< --output-file $@
+uuid_test.lo: $(srcdir)/regress/uuid/uuid_test.c $(incdir)/compat/stdbool.h \
+              $(incdir)/sudo_compat.h $(incdir)/sudo_fatal.h \
+              $(incdir)/sudo_plugin.h $(incdir)/sudo_util.h \
+              $(top_builddir)/config.h
+	$(LIBTOOL) $(LTFLAGS) --mode=compile $(CC) -c -o $@ $(CPPFLAGS) $(CFLAGS) $(ASAN_CFLAGS) $(PIE_CFLAGS) $(SSP_CFLAGS) $(srcdir)/regress/uuid/uuid_test.c
+uuid_test.i: $(srcdir)/regress/uuid/uuid_test.c $(incdir)/compat/stdbool.h \
+              $(incdir)/sudo_compat.h $(incdir)/sudo_fatal.h \
+              $(incdir)/sudo_plugin.h $(incdir)/sudo_util.h \
+              $(top_builddir)/config.h
+	$(CC) -E -o $@ $(CPPFLAGS) $<
+uuid_test.plog: uuid_test.i
+	rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/regress/uuid/uuid_test.c --i-file $< --output-file $@

lib/util/regress/uuid/uuid_test.c

@@ -0,0 +1,92 @@
+/*
+ * SPDX-License-Identifier: ISC
+ *
+ * Copyright (c) 2021 Todd C. Miller <Todd.Miller@sudo.ws>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <config.h>
+
+#include <stdlib.h>
+#if defined(HAVE_STDINT_H)
+# include <stdint.h>
+#elif defined(HAVE_INTTYPES_H)
+# include <inttypes.h>
+#endif
+#include <string.h>
+
+#define SUDO_ERROR_WRAP 0
+
+#include "sudo_compat.h"
+#include "sudo_fatal.h"
+#include "sudo_util.h"
+
+sudo_dso_public int main(int argc, char *argv[]);
+
+/*
+ * Test that sudo_uuid_create() generates a variant 1, version 4 uuid.
+ */
+
+/* From RFC 4122. */
+struct uuid {
+    uint32_t time_low;
+    uint16_t time_mid;
+    uint16_t time_hi_and_version;
+    uint8_t clock_seq_hi_and_reserved;
+    uint8_t clock_seq_low;
+    uint8_t node[6];
+};
+
+int
+main(int argc, char *argv[])
+{
+    union {
+        struct uuid id;
+        unsigned char u8[16];
+    } uuid;
+    int errors = 0;
+    int ntests = 0;
+
+    initprogname(argc > 0 ? argv[0] : "uuid_test");
+
+    /* Do 16 passes. */
+    for (ntests = 0; ntests < 16; ntests++) {
+	sudo_uuid_create(uuid.u8);
+
+	/* Variant: two most significant bits (6 and 7) are 0 and 1. */
+	if (ISSET(uuid.id.clock_seq_hi_and_reserved, (1 << 6))) {
+	    sudo_warnx("uuid bit 6 set, should be clear");
+	    errors++;
+	    continue;
+	}
+	if (!ISSET(uuid.id.clock_seq_hi_and_reserved, (1 << 7))) {
+	    sudo_warnx("uuid bit 7 clear, should be set");
+	    errors++;
+	    continue;
+	}
+
+	/* Version: bits 12-15 are 0010. */
+	if ((uuid.id.time_hi_and_version & 0xf000) != 0x4000) {
+	    sudo_warnx("bad version: 0x%x", uuid.id.time_hi_and_version & 0xf000);
+	    errors++;
+	    continue;
+	}
+    }
+
+    if (ntests != 0) {
+	printf("%s: %d tests run, %d errors, %d%% success rate\n",
+	    getprogname(), ntests, errors, (ntests - errors) * 100 / ntests);
+    }
+    return 0;
+}

lib/util/uuid.c

@@ -39,13 +39,15 @@
 struct uuid {
     uint32_t time_low;
     uint16_t time_mid;
-    uint16_t time_high_and_version;
+    uint16_t time_hi_and_version;
-    uint16_t clock_seq_and_variant;
+    uint8_t clock_seq_hi_and_reserved;
-    unsigned char node[6];
+    uint8_t clock_seq_low;
+    uint8_t node[6];
 };
 
 /*
  * Create a type 4 (random), variant 1 universally unique identifier (UUID).
+ * As per RFC 4122 section 4.4.
  */
 void
 sudo_uuid_create_v1(unsigned char uuid_out[16])
@@ -57,25 +59,14 @@ sudo_uuid_create_v1(unsigned char uuid_out[16])
 
     arc4random_buf(&uuid, sizeof(uuid));
 
-    /* Convert fields to host by order. */
+    /* Set version to 4 (random), 4 most significant bits (12-15) are 0010. */
-    uuid.id.time_low = ntohl(uuid.id.time_low);
+    uuid.id.time_hi_and_version &= 0x0fff;
-    uuid.id.time_mid = ntohs(uuid.id.time_mid);
+    uuid.id.time_hi_and_version |= 0x4000;
-    uuid.id.time_high_and_version = ntohs(uuid.id.time_high_and_version);
-    uuid.id.clock_seq_and_variant = ntohs(uuid.id.clock_seq_and_variant);
 
-    /* Set version to 4 (random) in the high nibble. */
+    /* Set variant to 1: two most significant bits (6 and 7) are 01. */
-    uuid.id.time_high_and_version &= 0x0fff;
+    uuid.id.clock_seq_hi_and_reserved &= 0x3f;
-    uuid.id.time_high_and_version |= 0x4000;
+    uuid.id.clock_seq_hi_and_reserved |= 0x80;
 
-    /* Set variant to 1 (first two bits are 10) */
-    uuid.id.clock_seq_and_variant &= 0x3fff;
-    uuid.id.clock_seq_and_variant |= 0x8000;
-
-    /* Store fields in network byte order (big endian). */
-    uuid.id.time_low = htonl(uuid.id.time_low);
-    uuid.id.time_mid = htons(uuid.id.time_mid);
-    uuid.id.time_high_and_version = htons(uuid.id.time_high_and_version);
-    uuid.id.clock_seq_and_variant = htons(uuid.id.clock_seq_and_variant);
     memcpy(uuid_out, &uuid, 16);
 }