Sorry for the bad reproduction instructions. Let me try again. After, the cli arg parser while loop add ```c printf(“%d\n”, debug); exit(0); ``` run `make`
then run ```ksh iked -d -n iced -n -d ``` They should print different values. > On Dec 26, 2024, at 1:31 PM, Lucas Gabriel Vuotto <lu...@sexy.is> wrote: > > 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(); >
