testssl/testssl.sh

[BUG / possible BUG] Comma seperated certs in ADDTL_CA_FILES do not work in batch mode

Closed this issue · 2 comments

Maroko commented

Before you open an issue please check which version you are running and whether it is the latest in stable / dev branch

I am running version 3.2.2 (Docker)

Before you open an issue please whether this is a known problem by searching the issues

Is caused by #2653

Command line / docker command to reproduce

When the ADDTL_CA_FILES environment variable is set to multiple certificates and testssl.sh runs in batch mode, the script fails with the error:
Fatal error: The CA file "/config/cert1.crt /config/cert2.crt /config/cert3.crt" must not contain spaces

./config/testssl.conf

https://duckduckgo.com
https://google.com

Docker command
docker run --rm -it -v ./config:/config -e "ADDTL_CA_FILES=/config/cert1.crt,/config/cert2.crt,/config/cert3.crt" drwetter/testssl.sh:3.2 --file /config/testssl.conf --parallel

In #2653 a check was added, if the ADDTL_CA_FILES variable contains spaces. When using multiple certificates in batch mode, this check passes, then the variable gets splitted in https://github.com/testssl/testssl.sh/blob/c4856bef7255fec85affb50ca112d68bfa9c28d5/testssl.sh#L24751 and then testssl.sh gets called again (batch mode), but this time the check fails because the variable already got splitted (and now contains spaces).

Expected behavior

The script should allow multiple certificates in ADDTL_CA_FILES when running in batch mode.
The space check should not fail after the variable has already been split internally.

Your system (please complete the following information):

  • OS: Ubuntu 22.04.5 LTS
  • Platform: Linux 6.6.87.2-microsoft-standard-WSL2 x86_64
  • OpenSSL + bash: testssl.sh -b 2>/dev/null | grep Using '

Additional context

Add any other context about the problem goes here.

dcooper16 commented

Hi @Maroko,

Thanks for reporting the issue. This was an edge case, as it only happened when running in batch mode if ADDTL_CA_FILES was set as an environment variable rather than via the --add-ca command line option.

As you noted, the problem is caused by adding a check for spaces and then replacing the commas in the variable with spaces. This substitution is useful, since other uses of ADDTL_CA_FILES require a space-separated list, but it causes the check to fail when running in batch mode.

In #2897, I've proposed to fix the problem by deleting the line that splits ADDTL_CA_FILES, and then modifying every use of $ADDTL_CA_FILES to make the substitution as the variable is being used. If this proposed fix is accepted, then I can create a similar PR for the 3.2 branch.

  • 👍1
Maroko commented

Thank you @dcooper16 and @drwetter for the quick fix! I appreciate it!