<div dir="ltr"><div>Well damn. That's a lot of very good suggestions.</div><div><br></div><div>I will dispute one of them: you said (more than once I think) that I don't need to quote variables. You are technically correct ... right up until a variable value has a space in it. Then everything goes to hell in a handbasket. I use 'shellcheck' ( <a href="https://github.com/koalaman/shellcheck">https://github.com/koalaman/shellcheck</a> ) through NeoVim's Syntastic ( <a href="https://github.com/vim-syntastic/syntastic">https://github.com/vim-syntastic/syntastic</a> ) plugin, and it's a real nag about quoting variables. And I agree with it, having had unquoted variables explode in my face.</div><div><br></div><div>I hope to implement and test most of the rest of your suggestions (along with the previous ones) shortly. Thanks.<br></div><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, 11 Aug 2019 at 23:49, William Park via talk <<a href="mailto:talk@gtalug.org">talk@gtalug.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Sat, Aug 10, 2019 at 11:46:40AM -0400, Giles Orr via talk wrote:<br>
> If you're interested, you can find the details here:<br>
> <br>
> <a href="https://www.gilesorr.com/blog/tls-https-details.html" rel="noreferrer" target="_blank">https://www.gilesorr.com/blog/tls-https-details.html</a><br>
> <br>
> Any suggestions to improve the script would be most welcome.<br>
<br>
OK, here it goes...<br>
<br>
> # Put most desirable/least-likely last:<br>
> for opensslbinary in /usr/local/opt/openssl@1.1/bin/openssl /usr/local/opt/opens<br>
> sl/bin/openssl /usr/bin/openssl<br>
> do<br>
> # Every version of OpenSSL I've checked (several across multiple<br>
> # OpenSSL versions and Apple's LibreSSL) respond to 'openssl version'<br>
> # with '<brand> <version-no> ...' - there may or may not be stuff after<br>
> # those two items (usually a build date, but not with Libre).<br>
> #<br>
> if [ -f "${opensslbinary}" ]<br>
<br>
'-f' tests if it's a file. Maybe you should use '-x' which tests if it's<br>
executable.<br>
<br>
> then<br>
> output="$("${opensslbinary}" version)"<br>
> brand="$( echo "${output}" | awk '{ print $1 }')"<br>
> version="$(echo "${output}" | awk '{ print $2 }')"<br>
> if [[ "OpenSSL" == "${brand}" ]]<br>
> then<br>
> if [[ "${version}" == "1.1"* ]]<br>
> then<br>
> OPENSSL="${opensslbinary}"<br>
> fi<br>
> fi<br>
<br>
You got 7 subshells here. You can reduce it to 1 subshell:<br>
<br>
read a b rest <<< $($opensslbinary version)<br>
case $a:$b in<br>
OpenSSL:1.1*) OPENSSL=$opensslbinary ;;<br>
esac<br>
<br>
> fi<br>
> done<br>
<br>
Maybe you should break out of the for-loop as soon as you find something<br>
correct. As is, OPENSSL will be the last correct one.<br>
<br>
<br>
> function help() {<br>
> echo "Usage:"<br>
> echo " $(basename "${0}") [-h]|<domain-name>"<br>
> echo ""<br>
> echo "Show HTTP(s) and certificate details."<br>
> echo "Do not include the 'http(s)://' leader on the domain name."<br>
> echo ""<br>
> echo "-h show this help and exit"<br>
<br>
Instead of echo'ing each and every line, you can do<br>
<br>
cat << EOF<br>
Usage:<br>
$(basename "${0}") [-h]|<domain-name><br>
<br>
Show HTTP(s) and certificate details.<br>
Do not include the 'http(s)://' leader on the domain name.<br>
<br>
-h show this help and exit<br>
EOF<br>
<br>
<br>
> function expiry_date() {<br>
> echo "${1}" | ${OPENSSL} x509 -noout -dates | grep notAfter | awk 'BEGIN { F<br>
> S="=" } { print $2 }'<br>
> }<br>
<br>
You can get rid of grep, since you're already using awk.<br>
<br>
echo "$1" | ${OPENSSL} x509 -noout -dates | awk -F= '/notAfter/ {print $2}'<br>
<br>
> <br>
> function days_to_expiry() {<br>
> expiry_date="$(echo "${1}" | ${OPENSSL} x509 -noout -dates | grep notAfter |<br>
> awk 'BEGIN { FS="=" } { print $2 }')"<br>
<br>
You already defined expiry_date(), so why not use it here.<br>
<br>
expiry_date=$(expiry_date "$1")<br>
<br>
> if [[ "$(date --version 2>/dev/null)" == *"GNU"* ]]<br>
> then<br>
> # Linux (or at least GNU)<br>
> expiry_epoch_seconds=$(date --date="${expiry_date}" "+%s")<br>
> else<br>
> # Assuming the Mac version:<br>
> expiry_epoch_seconds=$(date -jf '%b %e %H:%M:%S %Y %Z' "${expiry_date}"<br>
> "+%s")<br>
> fi<br>
<br>
Testing for "GNU" string can be done more simply using grep.<br>
<br>
if date --version 2>/dev/null | grep -q GNU; then<br>
...<br>
else<br>
...<br>
fi<br>
<br>
> function tlsversions() {<br>
> successful=""<br>
> failed=""<br>
> for tlsversion in ssl2 ssl3 tls1 tls1_1 tls1_2 tls1_3<br>
> do<br>
> success=$(echo | ${OPENSSL} s_client -connect "${1}":443 -${tlsversion}<br>
> > /dev/null 2> /dev/null ; echo $?)<br>
> if [ ${success} -eq 0 ]<br>
> then<br>
> successful="${tlsversion} ${successful}"<br>
> else<br>
> failed="${tlsversion} ${failed}"<br>
> fi<br>
<br>
Testing if a command succeeded or failed can be done in-line.<br>
<br>
if echo | ${OPENSSL} s_client -connect "${1}":443 -${tlsversion} &>/dev/null; then<br>
successful=...<br>
else<br>
failed=...<br>
fi<br>
<br>
Also, do you really need to send empty line to openssl? Maybe you can redirect<br>
stdin from /dev/null as well, like<br>
<br>
openssl ... </dev/null ...<br>
<br>
That will reduce 3 subshells to 0.<br>
<br>
<br>
General observation:<br>
<br>
1. You don't need quotes in variable assignments like <br>
var="$(...)"<br>
var="$var1"<br>
<br>
2. The following tests are the same. They both tests if var is empty.<br>
[ "$var"x = x ]<br>
[ -z "$var" ]<br>
<br>
3. Testing if command succeeded or failed can be done in-line.<br>
if command1 | command2 | command2; then<br>
...<br>
else<br>
...<br>
fi<br>
<br>
4. Case statement is usually better than if statement. eg.<br>
if [[ "$var" == glob ]]; then<br>
...<br>
fi<br>
vs<br>
case $var in<br>
glob) ... ;;<br>
esac<br>
Note the missing quotes in case, too. You don't need it.<br>
-- <br>
William Park <<a href="mailto:opengeometry@yahoo.ca" target="_blank">opengeometry@yahoo.ca</a>><br>
---<br>
Post to this mailing list <a href="mailto:talk@gtalug.org" target="_blank">talk@gtalug.org</a><br>
Unsubscribe from this mailing list <a href="https://gtalug.org/mailman/listinfo/talk" rel="noreferrer" target="_blank">https://gtalug.org/mailman/listinfo/talk</a><br>
</blockquote></div><br clear="all"><br>-- <br><div dir="ltr" class="gmail_signature">Giles<br><a href="https://www.gilesorr.com/" target="_blank">https://www.gilesorr.com/</a><br><a href="mailto:gilesorr@gmail.com" target="_blank">gilesorr@gmail.com</a></div>