On Thu, Dec 26, 2024 at 10:46:10AM -0500, William Rusnack wrote:
> >Synopsis: The ordering of the iked flags -d and -n erroneously changes
> >the debug level.
> >Category: bin
> >Description:
> I've found an issue with iked's command line flag processing where the
> order of
> the -d and -n flags affects the resulting debug level. This appears to
> be a bug
> since flag ordering shouldn't change the program's behavior.
>
> Problem:
> - `iked -d -n` results in debug level 1
> - `iked -n -d` results in debug level 2
>
> The issue occurs because the -n flag unconditionally sets debug=1
> rather than
> preserving any existing debug level:
>
> ```c
> case 'd':
> debug++;
> break;
> case 'n':
> debug = 1; /* Overwrites any previous debug
> setting */
> opts |= IKED_OPT_NOACTION;
> break;
> ```
>
> Impact:
> This can lead to unexpected behavior where debug messages may or may
> not appear
> depending solely on flag order. Users expecting to see debug output
> when using
> both flags may not see it if they happen to put the flags in the wrong
> order.
>
> >How-To-Repeat:
> 1. Run: iked -d -n -v -v
> 2. Note debug level and visible output
> 3. Run: iked -n -d -v -v
> 4. Note debug level and visible output
> 5. Observe that debug levels differ between the two commands
>
> >Fix:
> Make -n preserve existing debug level:
> ```c
> case 'n':
> if (!debug) debug = 1; /* Only set if not already set */
> opts |= IKED_OPT_NOACTION;
> break;
> ```
A test configuration file showing the issue is welcome; I can't
reproduce it with my local config:
$ doas iked -d -n -v -v 2>&1 | sha256
ab2e93095df893c0b4bd266f19c37072ac3529a9f9e6a8b48d1792e53a5a5624
$ doas iked -n -d -v -v 2>&1 | sha256
ab2e93095df893c0b4bd266f19c37072ac3529a9f9e6a8b48d1792e53a5a5624
There is a very small window where it can have some effect tho, bc debug
is passed to log_init, which initialize log.c:verbose. Maybe something
like below is more correct: before the getopt switch, there is a
log_init(1, LOG_DAEMON). None of the things between the removed log_init
and the
log_init(debug, LOG_DAEMON);
log_setverbose(verbose);
call a log_debug, which is the only place that can affect the output if
debug > 1.
diff /usr/src
path + /usr/src
commit - 76b1f2ebe5106c591f5eafcb9d7b05643509a2c4
blob - b69a354438a309f907540f63cd973beede9d029b
file + sbin/iked/iked.c
--- sbin/iked/iked.c
+++ sbin/iked/iked.c
@@ -97,7 +97,7 @@ main(int argc, char *argv[])
optarg);
break;
case 'd':
- debug++;
+ debug = 1;
break;
case 'f':
conffile = optarg;
@@ -154,9 +154,6 @@ main(int argc, char *argv[])
}
}
- /* log to stderr until daemonized */
- log_init(debug ? debug : 1, LOG_DAEMON);
-
argc -= optind;
if (argc > 0)
usage();
