Conversation
|
Sorry, about the late reply. First: much appreciated, thanks! I read and tried that in December. I can't get to it and need a few days for a consolidate answer! |
| [[ -z "$key" ]] && return 0 | ||
|
|
||
| # Check if key exists in response. If not, the API may have changed or something. | ||
| ! grep -Fiaqw "$key" $tmpfile && debugme echo "HSTS preloadlist key unrecognized: $key" && return 21 |
There was a problem hiding this comment.
This grep is legacy code. Can't this be changed into something like [[ "$tmpfile" =~ \ ${key}\ ]] ? (please doublecheck)
| ! grep -Fiaqw "$key" $tmpfile && debugme echo "HSTS preloadlist key unrecognized: $key" && return 21 | ||
|
|
||
| # Check if there is a match, return 10 if there is, 20 if not | ||
| grep -Fiaqw "\"$key\": $value" $tmpfile && return 10 || return 20 |
| out "$indent"; pr_bold " HSTS preload list " | ||
|
|
||
| # Check if the domain is also the preloadedDomain. If so, it *may* be correct that the server response header does not have 'preloaded' included. | ||
| check_hsts_preloadlist_match "$NODE" "preloadedDomain" "\"$NODE\"" |
There was a problem hiding this comment.
Is there any reason you use here and other places string why you pass the double quotes here? If they are needed at all I would rather handle this in the function and not in the call.
| debugme echo "Temporary lookupvariable: $preloadcombined" | ||
|
|
||
| # Determine and show the outcome | ||
| case "$(check_hsts_preloadlist_value "$NODE" "status")" in |
There was a problem hiding this comment.
for a plain string there should be no need to enclose it in quotes
| case $key in | ||
| "status") values=("\"unknown\"" "\"pending\"" "\"rejected\"" "\"preloaded\"") ;; | ||
| "bulk") values=("true" "false") ;; | ||
| *) return 1 ;; # No supported key provided. |
There was a problem hiding this comment.
Please double check here the usage of quotes. I believe for status and bulk they can be removed
For the array values I am not sure whether they need contain quotes.
| check_hsts_preloadlist_value() { | ||
| local domain="$1" | ||
| local key="$2" | ||
| local values |
There was a problem hiding this comment.
local -a value=() would declare the type better.
|
Good work! You spend a lot of time for the lookup table and the decision table you provided is quite impressive. The information on the screen for both easy cases (HSTS and no preload anywhere, HSTS and preload eerywhere)
I miss use case to test every variant of this. If I sleep over it I probably can test 1-2 more than the basic variants. If you could address the minor points in the code; I'll do a few more tests and then it should be set. Thanks for your work! |
2 participants
First attempt at addressing #1248: adding support for HSTS preload list lookups.
Feedback is welcome. Especially regarding the way the results are displayed. The idea is that the check is done even when the HSTS preload header is missing. This would show unintended and/or undesired situations such as having a list addition rejected because the 'preload' header itself is missing. Based on my understanding of how the list should work, I have currently assigned the following:
Explanation of some columns:
As I was writing this check, it grew from just a simple 'status' to this entire 'lookup table'. If this is too much, it can also be changed back to just viewing the status without any severity and advice.