From 4bff82cab42af89b680b344c3d5fea2d87b33153 Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Tue, 31 Aug 2021 19:53:28 -0600 Subject: [PATCH] Fix random uuid generation, no need to convert between byte order. Also add regression test. --- MANIFEST | 1 + lib/util/Makefile.in | 25 ++++++++- lib/util/regress/uuid/uuid_test.c | 92 +++++++++++++++++++++++++++++++ lib/util/uuid.c | 31 ++++------- 4 files changed, 126 insertions(+), 23 deletions(-) create mode 100644 lib/util/regress/uuid/uuid_test.c diff --git a/MANIFEST b/MANIFEST index f67077051..d958e2241 100644 --- a/MANIFEST +++ b/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 diff --git a/lib/util/Makefile.in b/lib/util/Makefile.in index 6966043e7..740af7f78 100644 --- a/lib/util/Makefile.in +++ b/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 \ - strtobool_test strtoid_test strtomode_test strtonum_test \ - parse_gids_test getgids getgrouplist_test @COMPAT_TEST_PROGS@ +TEST_PROGS = conf_test hltq_test parseln_test progname_test \ + strsplit_test strtobool_test strtoid_test strtomode_test \ + 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 $@ diff --git a/lib/util/regress/uuid/uuid_test.c b/lib/util/regress/uuid/uuid_test.c new file mode 100644 index 000000000..9c21bc28a --- /dev/null +++ b/lib/util/regress/uuid/uuid_test.c @@ -0,0 +1,92 @@ +/* + * SPDX-License-Identifier: ISC + * + * Copyright (c) 2021 Todd C. Miller + * + * 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 + +#include +#if defined(HAVE_STDINT_H) +# include +#elif defined(HAVE_INTTYPES_H) +# include +#endif +#include + +#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; +} diff --git a/lib/util/uuid.c b/lib/util/uuid.c index cb6726e2e..6417851d0 100644 --- a/lib/util/uuid.c +++ b/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 clock_seq_and_variant; - unsigned char node[6]; + uint16_t time_hi_and_version; + uint8_t clock_seq_hi_and_reserved; + 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. */ - uuid.id.time_low = ntohl(uuid.id.time_low); - uuid.id.time_mid = ntohs(uuid.id.time_mid); - 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), 4 most significant bits (12-15) are 0010. */ + uuid.id.time_hi_and_version &= 0x0fff; + uuid.id.time_hi_and_version |= 0x4000; - /* Set version to 4 (random) in the high nibble. */ - uuid.id.time_high_and_version &= 0x0fff; - uuid.id.time_high_and_version |= 0x4000; + /* Set variant to 1: two most significant bits (6 and 7) are 01. */ + uuid.id.clock_seq_hi_and_reserved &= 0x3f; + 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); }