I did some testing with http://jsonlint.com/ and discovered a missing comma when massing testing is being performed and a single JSON file is being created.
In `run_server_defaults()` the variable `success` is defined twice, once an an ordinary variable and once as an array. The PR removes the incorrect definition. It also removes the definitions of some variables that are no longer used and reorganizes the definitions so that each line has only one variable type.
I also noticed a typo later in `run_server_defaults()` and corrected it.
This PR introduces the environment variable `CHILD_MASS_TESTING`, and uses it as an indicator that testssl.sh is running as a child within mass testing rather than using the `$APPEND` flag. It also makes a number of other changes to make the handling, of HTML, CSV, JSON, and log files consistent, and it fixes a number of bugs related to the generation of these files when mass testing is being performed.
Please let me know if you disagree with any of the changes in this PR, or if you would prefer that it be broken up into multiple smaller PRs.
Some of the changes are as follows:
- When the `$APPEND` flag is true, all of these files are appended to and headers and footers are omitted. (Perhaps this should be changed. Appending to a log file isn't an issue, but appending to a JSON or HTML file without including headers or footers seems to just create an improperly formatted file).
- Following the code in `prepare_logging()`, an error is printed and the program stops if the `$APPEND` flag is false and one of the files to be written to already exists.
Some of the bugs fixed:
Creating log files did not work with mass testing:
- If `--logfile <logfile>` is used, then the parent and each child try to write to "logfile".
- If `--logging` is used, then a log file is created for each child, but an oddly-named log file is also created for the parent. The one created by the parent contains the entire output.
Plain JSON files:
- When `--jsonfile <jsonfile>` is run, there is no comma separating the final finding for one child and the first finding for the next child.
Pretty JSON files:
- When `--jsonfile-pretty <jsonfile>` is called without mass testing, the "target host" line is empty, since `$NODE` has not yet been set.
- When `--jsonfile <jsonfile>` is run with mass testing, there is no comma separating the final finding for one child and the first finding for the next child. In addition, `fileout_pretty_json_banner()` is never called, and the entries for individual tests have insufficient information to determine what is being tested (it lists "service" and "ip", but not port number).
For the final issue, when mass testing is being performed and all output is being placed in a single file, I have the parent call `fileout_pretty_json_banner()`, but tell `fileout_pretty_json_banner()` to not include a "target host" or "port", but then have each child include a "target host" or "port" (when the "service" and "ip" are being printed).
This PR improves `out_row_aligned_max_width()` in a few ways:
* It makes better use of bash's string manipulation capabilities in order to simplify the function.
* It improves the function's performance. One of the most costly parts of `out_row_aligned_max_width()` was the while loop to print each entry in the text. Since there is only one place in the code where the the entries are not all printed the same ways (the list of supported curves printed by `run_pfs()`), the PR changes `out_row_aligned_max_width()` to just return a plain text string, which the calling function prints in the appropriate way. For the curves printed by `run_pfs()`, a new function, `out_row_aligned_max_width_by_entry()` takes care of getting the output from `out_row_aligned_max_width()` and then printing each entry appropriately.
* The PR also introduces a trick so that when the TLS extensions are printed, the text for an extension won't get split across two rows. It does this by replacing the space charters within the text for an extension with "}", formatting the result with `out_row_aligned_max_width()`, and then converting the "}" back to space characters.
`$OPENSSL_LOCATION` is used in `fileout_pretty_json_banner()`, `html_banner()`, `mybanner()`, and `prepare_logging()`, but the value of `$OPENSSL_LOCATION` is populated in `mybanner()`. This is usually okay, since `mybanner()` is always called before the other three functions are called. However, if `$QUIET` is `true`, then `mybanner()` returns immediately, without populating `$OPENSSL_LOCATION`, even though the value of `$OPENSSL_LOCATION` may be needed by one or more of the other functions.
This PR addresses this problem by populating `$OPENSSL_LOCATION` in `find_openssl_binary()` rather than `mybanner()`.
With the commit made on March 26, "partly (1/2) fixing #653," an HTML banner isn't added to the HTML files anymore. A banner should be added to the top of the HTML file if mass testing is being performed and a separate HTML file is being created for each test.
The `$APPEND` flag being `true` is an indicator that mass testing is being performed and that this is one of the individual tests being run. Given that `$APPEND` is `true`, `$HTMLHEADER` being `true` indicates that testssl.sh is creating the file name for the HTML output. So, it is when both flags are `true` that the HTML banner should be created.
`std_cipherlists()` does not include line breaks between tests in the output to the terminal when `$DEBUG` is 1, and it does not include line break between tests in the HTML output whenever `$DEBUG` is greater than 0.
In `create_client_simulation_tls_clienthello()` the variable `sni_extension_found` should be set if the ClientHello includes an SNI extension. Instead it was being set if and only if the ClientHello included some extension other than SNI.
This bug wasn't detected before for two reasons:
* It is rare to have a ClientHello that includes an SNI extension, but no other extensions.
* The code still works correctly if `sni_extension_found` is set even if there is no SNI in the ClientHello.
So, the bug only creates a problem if the browser's ClientHello include an SNI extension and no other extensions (see "BingPreview Jun 2014" in the client_simulation branch).
When HTML output is not being created, the print functions last step is to call `html_out()`, which responds to `return` rather than `return 0`. This causes problems for lines of code that rely on receiving a return value of 0. For example:
```
[[ $VULN_COUNT -le $VULN_THRESHLD ]] && outln && pr_headlineln " Testing for LUCKY13 vulnerability " && outln
```
This PR fixes two minor bugs:
* In `run_hpkp()`, the call to `$OPENSSL s_client` includes the option `-showcerts` twice. This PR removes one of them.
* In `get_server_certificate()`, the first call to `$OPENSSL s_client` includes `$addcmd`, but `$addcmd` has not yet been initialized. Instead, `$SNI` should be used.
Dirk
David Cooper
Christian Dresen