From 853ac02b9c8ddf6045581befc953397d967e7f45 Mon Sep 17 00:00:00 2001 From: Leo Sandoval Date: Mon, 12 May 2025 12:24:56 -0600 Subject: [PATCH] Better return codes Introduce --is-sb-enabled parameter and better return codes. Also show help on unsupported systems. Resolves: #RHEL-90836 Resolves: #RHEL-90839 Signed-off-by: Leo Sandoval --- ...age-instead-of-aborting-on-bad-flags.patch | 31 ---- ...elp-if-no-args-or-help-even-on-unsup.patch | 45 ++++++ 0002-mokutil-bugfix-del-unused-opt-s.patch | 26 ---- 0002-mokutil-fix-a-typo-mock.patch | 27 ++++ ...-of-list-in-delete_data_from_req_var.patch | 28 ---- 0003-mokutil-remove-unused-int_to_b64.patch | 56 +++++++ 0004-Fix-leak-of-fd-in-mok_get_variable.patch | 70 --------- ...t-key-return-non-zero-if-test-key-is.patch | 27 ++++ ...il-introduce-is-sb-enabled-parameter.patch | 138 ++++++++++++++++++ mokutil.patches | 5 + mokutil.spec | 10 +- 11 files changed, 307 insertions(+), 156 deletions(-) delete mode 100644 0001-Show-usage-instead-of-aborting-on-bad-flags.patch create mode 100644 0001-mokutil.c-show-help-if-no-args-or-help-even-on-unsup.patch delete mode 100644 0002-mokutil-bugfix-del-unused-opt-s.patch create mode 100644 0002-mokutil-fix-a-typo-mock.patch delete mode 100644 0003-Fix-leak-of-list-in-delete_data_from_req_var.patch create mode 100644 0003-mokutil-remove-unused-int_to_b64.patch delete mode 100644 0004-Fix-leak-of-fd-in-mok_get_variable.patch create mode 100644 0004-mokutil.c-on-test-key-return-non-zero-if-test-key-is.patch create mode 100644 0005-mokutil-introduce-is-sb-enabled-parameter.patch diff --git a/0001-Show-usage-instead-of-aborting-on-bad-flags.patch b/0001-Show-usage-instead-of-aborting-on-bad-flags.patch deleted file mode 100644 index 0f7fda1..0000000 --- a/0001-Show-usage-instead-of-aborting-on-bad-flags.patch +++ /dev/null @@ -1,31 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: Robbie Harwood -Date: Tue, 17 May 2022 11:23:28 -0400 -Subject: [PATCH] Show usage instead of aborting on bad flags - -Aborting here just confuses users and is sufficiently unexpected to -cause the filing of bugs. - -Related: https://bugzilla.redhat.com/show_bug.cgi?id=2087066 -Signed-off-by: Robbie Harwood -(cherry picked from commit 82694cb1ce3b29c3705c25ae4cea3d07fe57b558) ---- - src/mokutil.c | 3 +-- - 1 file changed, 1 insertion(+), 2 deletions(-) - -diff --git a/src/mokutil.c b/src/mokutil.c -index 5d725c9..e8228af 100644 ---- a/src/mokutil.c -+++ b/src/mokutil.c -@@ -2087,10 +2087,9 @@ main (int argc, char *argv[]) - goto out; - case 'h': - case '?': -+ default: - command |= HELP; - break; -- default: -- abort (); - } - } - diff --git a/0001-mokutil.c-show-help-if-no-args-or-help-even-on-unsup.patch b/0001-mokutil.c-show-help-if-no-args-or-help-even-on-unsup.patch new file mode 100644 index 0000000..b103ee7 --- /dev/null +++ b/0001-mokutil.c-show-help-if-no-args-or-help-even-on-unsup.patch @@ -0,0 +1,45 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Leonardo Sandoval +Date: Fri, 25 Oct 2024 13:53:05 -0600 +Subject: [PATCH] mokutil.c: show help if no args or --help even on unsupported + system + +Signed-off-by: Leo Sandoval +--- + src/mokutil.c | 19 +++++++++++++++---- + 1 file changed, 15 insertions(+), 4 deletions(-) + +diff --git a/src/mokutil.c b/src/mokutil.c +index cb06be2..2da211d 100644 +--- a/src/mokutil.c ++++ b/src/mokutil.c +@@ -2156,14 +2156,25 @@ main (int argc, char *argv[]) + if (pw_hash_file && use_root_pw) + command |= HELP; + +- if (!(command & HELP) && !efi_variables_supported ()) { +- fprintf (stderr, "EFI variables are not supported on this system\n"); +- exit (1); +- } + + if (db_name != MOK_LIST_RT && !(command & ~MOKX)) + command |= LIST_ENROLLED; + ++ /* no matter if mokutil is supported (EFI) or not (BIOS) in the system, print ++ the help menu if no command line arguments provided or explicit help ++ requested */ ++ if (!command || (command & HELP)) { ++ print_help (); ++ ret = 0; ++ goto out; ++ } ++ ++ /* check if mock is supported on the system */ ++ if (!efi_variables_supported ()) { ++ fprintf (stderr, "EFI variables are not supported on this system\n"); ++ exit (1); ++ } ++ + sb_check = !(command & HELP || command & TEST_KEY || + command & VERBOSITY || command & TIMEOUT || + command & FB_VERBOSITY || command & FB_NOREBOOT); diff --git a/0002-mokutil-bugfix-del-unused-opt-s.patch b/0002-mokutil-bugfix-del-unused-opt-s.patch deleted file mode 100644 index a5ad40b..0000000 --- a/0002-mokutil-bugfix-del-unused-opt-s.patch +++ /dev/null @@ -1,26 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: gaoyusong -Date: Mon, 30 May 2022 17:54:47 +0800 -Subject: [PATCH] mokutil bugfix: del unused opt "-s" - -The -s option can cause unexcepted result. - -Signed-off-by: gaoyusong -(cherry picked from commit 04791c29e198b18808bca519267e31c8d3786a08) ---- - src/mokutil.c | 2 +- - 1 file changed, 1 insertion(+), 1 deletion(-) - -diff --git a/src/mokutil.c b/src/mokutil.c -index e8228af..6982ade 100644 ---- a/src/mokutil.c -+++ b/src/mokutil.c -@@ -1851,7 +1851,7 @@ main (int argc, char *argv[]) - }; - - int option_index = 0; -- c = getopt_long (argc, argv, "cd:f:g::hi:lmpst:xDNPXv", -+ c = getopt_long (argc, argv, "cd:f:g::hi:lmpt:xDNPXv", - long_options, &option_index); - - if (c == -1) diff --git a/0002-mokutil-fix-a-typo-mock.patch b/0002-mokutil-fix-a-typo-mock.patch new file mode 100644 index 0000000..40edb89 --- /dev/null +++ b/0002-mokutil-fix-a-typo-mock.patch @@ -0,0 +1,27 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Gary Lin +Date: Mon, 28 Apr 2025 18:18:00 +0800 +Subject: [PATCH] mokutil: fix a typo 'mock' + +'mock' is the typo of 'MOK'. Since efi_variables_supported() checks the +EFI variable support of the system, correct the comment to match the +code. + +Signed-off-by: Gary Lin +--- + src/mokutil.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/mokutil.c b/src/mokutil.c +index 2da211d..988b060 100644 +--- a/src/mokutil.c ++++ b/src/mokutil.c +@@ -2169,7 +2169,7 @@ main (int argc, char *argv[]) + goto out; + } + +- /* check if mock is supported on the system */ ++ /* check if EFI variable is supported on the system */ + if (!efi_variables_supported ()) { + fprintf (stderr, "EFI variables are not supported on this system\n"); + exit (1); diff --git a/0003-Fix-leak-of-list-in-delete_data_from_req_var.patch b/0003-Fix-leak-of-list-in-delete_data_from_req_var.patch deleted file mode 100644 index 23633a8..0000000 --- a/0003-Fix-leak-of-list-in-delete_data_from_req_var.patch +++ /dev/null @@ -1,28 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: Robbie Harwood -Date: Thu, 2 Jun 2022 12:56:31 -0400 -Subject: [PATCH] Fix leak of list in delete_data_from_req_var() - -Signed-off-by: Robbie Harwood -(cherry picked from commit d978c18f61b877afaab45a82d260b525423b8248) ---- - src/util.c | 6 ++++-- - 1 file changed, 4 insertions(+), 2 deletions(-) - -diff --git a/src/util.c b/src/util.c -index 621869f..6cd0302 100644 ---- a/src/util.c -+++ b/src/util.c -@@ -295,8 +295,10 @@ delete_data_from_req_var (const MokRequest req, const efi_guid_t *type, - } - - /* the key or hash is not in this list */ -- if (start == NULL) -- return 0; -+ if (start == NULL) { -+ ret = 0; -+ goto done; -+ } - - /* all keys are removed */ - if (total == 0) { diff --git a/0003-mokutil-remove-unused-int_to_b64.patch b/0003-mokutil-remove-unused-int_to_b64.patch new file mode 100644 index 0000000..86bc3e2 --- /dev/null +++ b/0003-mokutil-remove-unused-int_to_b64.patch @@ -0,0 +1,56 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Nicolas Frayer +Date: Wed, 29 Jan 2025 17:37:36 +0100 +Subject: [PATCH] mokutil: remove unused int_to_b64() + +static const char b64t[64] triggers compiler warning +which in turn makes the build fail with +-Werror=unterminated-string-initialization, so removing +this string with the the unused int_to_b64() function +which is the only function using this array. + +Signed-off-by: Nicolas Frayer +--- + src/password-crypt.c | 9 --------- + src/password-crypt.h | 1 - + 2 files changed, 10 deletions(-) + +diff --git a/src/password-crypt.c b/src/password-crypt.c +index db69b88..bdbf784 100644 +--- a/src/password-crypt.c ++++ b/src/password-crypt.c +@@ -46,9 +46,6 @@ + #define SHA256_DEFAULT_ROUNDS 5000 + #define SHA512_DEFAULT_ROUNDS 5000 + +-static const char b64t[64] = +-"./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; +- + static const char md5_prefix[] = "$1$"; + + static const char sha256_prefix[] = "$5$"; +@@ -357,12 +354,6 @@ decode_pass (const char *crypt_pass, pw_crypt_t *pw_crypt) + return -1; + } + +-char +-int_to_b64 (const int i) +-{ +- return b64t[i & 0x3f]; +-} +- + int + b64_to_int (const char c) + { +diff --git a/src/password-crypt.h b/src/password-crypt.h +index 214a65b..999e036 100644 +--- a/src/password-crypt.h ++++ b/src/password-crypt.h +@@ -68,7 +68,6 @@ uint16_t get_pw_salt_size (const HashMethod method); + int get_pw_hash_size (const HashMethod method); + const char *get_crypt_prefix (const HashMethod method); + int decode_pass (const char *crypt_pass, pw_crypt_t *pw_crypt); +-char int_to_b64 (const int i); + int b64_to_int (const char c); + + #endif /* __PASSWORD_CRYPT_H__ */ diff --git a/0004-Fix-leak-of-fd-in-mok_get_variable.patch b/0004-Fix-leak-of-fd-in-mok_get_variable.patch deleted file mode 100644 index f1a48f6..0000000 --- a/0004-Fix-leak-of-fd-in-mok_get_variable.patch +++ /dev/null @@ -1,70 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: Robbie Harwood -Date: Thu, 2 Jun 2022 13:00:22 -0400 -Subject: [PATCH] Fix leak of fd in mok_get_variable() - -On success, it was never closed. Refactor the code to use a single -egress path so its closure is clear. - -Signed-off-by: Robbie Harwood -(cherry picked from commit e498f6460ff5aea6a7cd61a33087d03e88a2f52a) ---- - src/util.c | 24 +++++++++++++----------- - 1 file changed, 13 insertions(+), 11 deletions(-) - -diff --git a/src/util.c b/src/util.c -index 6cd0302..f7fc033 100644 ---- a/src/util.c -+++ b/src/util.c -@@ -57,22 +57,21 @@ mok_get_variable(const char *name, uint8_t **datap, size_t *data_sizep) - return fd; - - rc = fstat(fd, &sb); -- if (rc < 0) { --err_close: -- close(fd); -- return rc; -- } -+ if (rc < 0) -+ goto done; - - if (sb.st_size == 0) { - errno = ENOENT; - rc = -1; -- goto err_close; -+ goto done; - } - - bufsz = sb.st_size; - buf = calloc(1, bufsz); -- if (!buf) -- goto err_close; -+ if (!buf) { -+ rc = -1; -+ goto done; -+ } - - while (pos < bufsz) { - ssz = read(fd, &buf[pos], bufsz - pos); -@@ -82,15 +81,18 @@ err_close: - errno == EINTR) - continue; - free(buf); -- goto err_close; -+ rc = -1; -+ goto done; - } - - pos += ssz; - } - *datap = buf; - *data_sizep = pos; -- -- return 0; -+ rc = 0; -+done: -+ close(fd); -+ return rc; - } - - MokListNode* diff --git a/0004-mokutil.c-on-test-key-return-non-zero-if-test-key-is.patch b/0004-mokutil.c-on-test-key-return-non-zero-if-test-key-is.patch new file mode 100644 index 0000000..5f3bd6b --- /dev/null +++ b/0004-mokutil.c-on-test-key-return-non-zero-if-test-key-is.patch @@ -0,0 +1,27 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Leo Sandoval +Date: Thu, 24 Apr 2025 12:49:29 -0600 +Subject: [PATCH] mokutil.c: on --test-key, return non-zero if test key is not + enrolled + +Aligns with the same behaviour as other common UNIX tools, +e.g. 'command -v program' returns 0 if program exists, non-zero if not + +Signed-off-by: Leo Sandoval +--- + src/mokutil.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/mokutil.c b/src/mokutil.c +index 988b060..e40579b 100644 +--- a/src/mokutil.c ++++ b/src/mokutil.c +@@ -1567,7 +1567,7 @@ test_key (const MokRequest req, const char *key_file) + + if (is_valid_request (&efi_guid_x509_cert, key, read_size, req)) { + printf ("%s is not enrolled\n", key_file); +- ret = 0; ++ ret = 1; + } else { + print_skip_message (key_file, key, read_size, req); + ret = 1; diff --git a/0005-mokutil-introduce-is-sb-enabled-parameter.patch b/0005-mokutil-introduce-is-sb-enabled-parameter.patch new file mode 100644 index 0000000..7453e2a --- /dev/null +++ b/0005-mokutil-introduce-is-sb-enabled-parameter.patch @@ -0,0 +1,138 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Leo Sandoval +Date: Fri, 25 Apr 2025 13:07:10 -0600 +Subject: [PATCH] mokutil: introduce --is-sb-enabled parameter + +The result would be 0 if SB is enable, 1 otherwise (or -1 in case of +error). This is a different outcome when using --sb-state, where it +returns 0 no matter the SB status (or -1 in case of error). + +Signed-off-by: Leo Sandoval +--- + man/mokutil.1 | 5 +++++ + src/mokutil.c | 30 ++++++++++++++++++++++++++++-- + 2 files changed, 33 insertions(+), 2 deletions(-) + +diff --git a/man/mokutil.1 b/man/mokutil.1 +index ccb285c..6b30610 100644 +--- a/man/mokutil.1 ++++ b/man/mokutil.1 +@@ -40,6 +40,8 @@ mokutil \- utility to manipulate machine owner keys + .br + \fBmokutil\fR [--sb-state] + .br ++\fBmokutil\fR [--is-sb-enabled] ++.br + \fBmokutil\fR [--test-key \fIkeyfile\fR | -t \fIkeyfile\fR] + ([--mokx | -X] | [--ca-check] | [--ignore-keyring]) + .br +@@ -133,6 +135,9 @@ Enable the validation process in shim + \fB--sb-state\fR + Show SecureBoot State + .TP ++\fB--is-sb-enabled\fR ++Indicates if SecureBoot is enabled ++.TP + \fB-t, --test-key\fR + Test if the key is enrolled or not + .TP +diff --git a/src/mokutil.c b/src/mokutil.c +index e40579b..918ca06 100644 +--- a/src/mokutil.c ++++ b/src/mokutil.c +@@ -89,6 +89,7 @@ + #define UNTRUST_MOK (1 << 28) + #define SET_SBAT (1 << 29) + #define SET_SSP (1 << 30) ++#define IS_SB_ENABLED (1 << 31) + + #define DEFAULT_CRYPT_METHOD SHA512_BASED + #define DEFAULT_SALT_SIZE SHA512_SALT_MAX +@@ -129,6 +130,7 @@ print_help () + printf (" --disable-validation\t\t\tDisable signature validation\n"); + printf (" --enable-validation\t\t\tEnable signature validation\n"); + printf (" --sb-state\t\t\t\tShow SecureBoot State\n"); ++ printf (" --is-sb-enabled\t\t\tIndicates if SecureBoot is enabled or not\n"); + printf (" --test-key, -t \t\tTest if the key is enrolled or not\n"); + printf (" --reset\t\t\t\tReset MOK list\n"); + printf (" --generate-hash[=password], -g\tGenerate the password hash\n"); +@@ -1400,7 +1402,7 @@ enable_validation(void) + } + + static int +-sb_state () ++sb_state_internal () + { + uint8_t *data = NULL; + size_t data_size; +@@ -1408,6 +1410,7 @@ sb_state () + int32_t secureboot = -1; + int32_t setupmode = -1; + int32_t moksbstate = -1; ++ int ret = 0; + + if (efi_get_variable (efi_guid_global, "SecureBoot", &data, &data_size, + &attributes) < 0) { +@@ -1453,17 +1456,34 @@ sb_state () + + if (secureboot == 1 && setupmode == 0) { + printf ("SecureBoot enabled\n"); ++ ret = 0; + if (moksbstate == 1) + printf ("SecureBoot validation is disabled in shim\n"); + } else if (secureboot == 0 || setupmode == 1) { + printf ("SecureBoot disabled\n"); ++ ret = 1; + if (setupmode == 1) + printf ("Platform is in Setup Mode\n"); + } else { + printf ("Cannot determine secure boot state.\n"); + } + +- return 0; ++ return ret; ++} ++ ++static int ++sb_state () ++{ ++ int ret = sb_state_internal (); ++ ++ /* in this case, ignore the ret value except on failure */ ++ return (ret < 0)? ret: 0; ++} ++ ++static int ++is_sb_enabled () ++{ ++ return sb_state_internal (); + } + + static inline int +@@ -1855,6 +1875,7 @@ main (int argc, char *argv[]) + {"disable-validation", no_argument, 0, 0 }, + {"enable-validation", no_argument, 0, 0 }, + {"sb-state", no_argument, 0, 0 }, ++ {"is-sb-enabled", no_argument, 0, 0 }, + {"test-key", required_argument, 0, 't'}, + {"reset", no_argument, 0, 0 }, + {"hash-file", required_argument, 0, 'f'}, +@@ -1908,6 +1929,8 @@ main (int argc, char *argv[]) + command |= ENABLE_VALIDATION; + } else if (strcmp (option, "sb-state") == 0) { + command |= SB_STATE; ++ } else if (strcmp (option, "is-sb-enabled") == 0) { ++ command |= IS_SB_ENABLED; + } else if (strcmp (option, "reset") == 0) { + command |= RESET; + } else if (strcmp (option, "ignore-db") == 0) { +@@ -2258,6 +2281,9 @@ main (int argc, char *argv[]) + case SB_STATE: + ret = sb_state (); + break; ++ case IS_SB_ENABLED: ++ ret = is_sb_enabled (); ++ break; + case TEST_KEY: + ret = test_key (ENROLL_MOK, key_file); + break; diff --git a/mokutil.patches b/mokutil.patches index e69de29..11c7a76 100644 --- a/mokutil.patches +++ b/mokutil.patches @@ -0,0 +1,5 @@ +Patch0001: 0001-mokutil.c-show-help-if-no-args-or-help-even-on-unsup.patch +Patch0002: 0002-mokutil-fix-a-typo-mock.patch +Patch0003: 0003-mokutil-remove-unused-int_to_b64.patch +Patch0004: 0004-mokutil.c-on-test-key-return-non-zero-if-test-key-is.patch +Patch0005: 0005-mokutil-introduce-is-sb-enabled-parameter.patch \ No newline at end of file diff --git a/mokutil.spec b/mokutil.spec index 268a170..c602908 100644 --- a/mokutil.spec +++ b/mokutil.spec @@ -1,6 +1,6 @@ Name: mokutil Version: 0.7.2 -Release: 1%{?dist} +Release: 2%{?dist} Epoch: 2 Summary: Tool to manage UEFI Secure Boot MoK Keys License: GPL-3.0-or-later @@ -47,6 +47,14 @@ mokutil provides a tool to manage keys for Secure Boot through the MoK %{_datadir}/bash-completion/completions/mokutil %changelog +* Mon May 12 2025 Leo Sandoval - 0.7.2-2 +- Rebase several patches from upstream which: + - Introduce --is-sb-enabled parameter and better return codes + - Remove unused int_to_b64() + - Show help on unsupported systems +- Resolves: #RHEL-90836 +- Resolves: #RHEL-90839 + * Mon Apr 28 2025 Leo Sandoval - 0.7.2-1 - Bump version to 0.7.2 upstream tag - Resolves: #RHEL-88740