Quit testssl.sh on all command line errors

[dcooper16]

As suggested in #1844, this PR changes testssl.sh so that the parent process quits immediately if there is an error in the command line for one of the child processes.

Currently, a signal is sent to the parent process to quit if the child process encounters an error and calls help(). But sometimes parse_cmd_line() just prints an error message and calls fatal() rather than help(), in which case the parent process does not stop. This PR addresses the issue by creating a new function, fatal_cmd_line(), which is almost the same as fatal(), but additionally sends a signal to the parent indicating that the parent should stop. This PR also changes calls to fatal() to calls to fatal_cmd_line() if json_header(), csv_header(), html_header(), or prepare_logging() encounter a problem. The same is done if prettyprint_local() with the command-line option provided for it.

There may be other places in which it would be appropriate to call fatal_cmd_line() rather than fatal() (e.g., in parse_hn_port() or check_proxy()), but those changes are not made in this PR.

Another issue that this PR addresses is that sometimes when parse_cmd_line() prints an error messsage and then calls help() the error message is send to stderr and other times it is sent to stdout. This PR sends all of these error messages to stderr.

[dcooper16]

When working on this PR I noticed that the parse_cmd_line() parses the -V (or --local) option within the while loop rather than at the beginning where the --help and --banner options are handled, even though -V is also a standalone option. Is there a reason for this?

[drwetter]

even though -V is also a standalone option. Is there a reason for this?

It's not a standalone option as one can supply a pattern. However we need to question the usefulness of this legacy implementation as it only lists the ciphers from OpenSSL:

[drwetter]

Argh. Sorry, David, to let this hang for a longer time. It seems my comment I thought to have posted wasn't. My point was that (besides honoring your work) it is not clear to me whether this PR does always send a signal to the parent when one host has a problem -- independent on whether the output is in one file or multiple, see my comment #1844 (comment) .

If I don't miss anything we maybe should not default to this behavior. Maybe a cmdline flag would do good here like ~ --term-on-error of a logic which does this automatically if the output is JSON and if the output is supposed to be in one file.

[dcooper16]

If you don't think this PR makes sense, then it's fine to just close it. However, at the moment testssl.sh's behavior is inconsistent with respect to command line errors. For example, if the file provided to the --file option contains:

--protocols 127.0.0.1
--nodns=minimum 127.0.0.1
--protocols 127.0.0.1:631

then an error message will be printed for the second line, but the tests for the other two lines will run. On the other hand, if the file provided to the --file option contains:

--protocols 127.0.0.1
--color 4 127.0.0.1
--protocols 127.0.0.1:631

then testssl.sh will stop as soon as the second line is parsed. The third line is never run, and in parallel mode the test from the first line is stopped before it completes.

The difference is that parse_cmd_line() calls fatal() when an invalid option is provided for --nodns, but calls help() when an invalid option is provided for --color. fatal() just stops the child process. help() sends a USR1 signal to the parent, which causes the parent to terminate all child processes and then stop itself.

[drwetter]

ok, thanks for your heads up.

[polarathene]

Is the rebasing automated? Is there a blocker to merging /resolving this?

Just curious as I regularly get notifications from the activity, but it's unclear what the status is?

Looks like the decision was to opt-in to this added logic via an option? But there doesn't seem to be any activity towards that.

[drwetter]

Yeah, that one is on me, sorry. I had some concerns in the beginning but as time passed by I forgot what it was and wasn't able to put this fwd in my queue. That will be solved for release,

[drwetter]

Thanks, David. Sorry to keep this pending for a long time.