From f9d166e10f12edfcadb6bc1f373f62329f4db0d7 Mon Sep 17 00:00:00 2001 From: Tim Kimber Date: Thu, 30 Sep 2021 18:10:06 +0100 Subject: [PATCH 1/4] Add test for delete key file with key alg changes (#706) Force new certificate if key alg changes (fixes #276) --- getssl | 33 ++++++----- test/39-private-key-alg-changed.bats | 88 ++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 15 deletions(-) create mode 100644 test/39-private-key-alg-changed.bats diff --git a/getssl b/getssl index 473f3e6..a688ca9 100755 --- a/getssl +++ b/getssl @@ -270,6 +270,7 @@ # 2021-07-30 Run tests with -d to catch intermittent failures, Use fork's repo for upgrade tests. (tlhackque) (#692) (2.41) # 2021-08-26 Improve upgrade check & make upgrade do a full install when possible (tlhackque) (#694) (2.42) # 2021-09-02 Fix version compare - cURL v8 may have single digit minor numbers. (tlhackque) (2.43) +# 2021-09-26 Delete key file when key algorithm has changed (makuhama) # ---------------------------------------------------------------------------------------- case :$SHELLOPTS: in @@ -838,7 +839,7 @@ check_getssl_upgrade() { # check if a more recent release is available fi if [[ ${_MUTE} -eq 0 ]]; then - echo "Updated getssl from v${VERSION} to v${release_tag}" + echo "Updated getssl from v${VERSION} to ${release_tag}" echo "The old version remains as ${0}.v${VERSION} and should be removed" echo "These update notifications can be turned off using the -Q option" echo "" @@ -3146,6 +3147,22 @@ else fi debug "created SAN list = $SANLIST" +# check if private key alg has changed from RSA to EC (or vice versa) +if [[ "$DUAL_RSA_ECDSA" == "false" ]] && [[ -s "$DOMAIN_DIR/${DOMAIN}.key" ]]; then + case "${PRIVATE_KEY_ALG}" in + rsa) + if grep --silent -- "-----BEGIN EC PRIVATE KEY-----" "$DOMAIN_DIR/${DOMAIN}.key"; then + rm -f "$DOMAIN_DIR/${DOMAIN}.key" + _FORCE_RENEW=1 + fi ;; + prime256v1|secp384r1|secp521r1) + if grep --silent -- "-----BEGIN RSA PRIVATE KEY-----" "$DOMAIN_DIR/${DOMAIN}.key"; then + rm -f "$DOMAIN_DIR/${DOMAIN}.key" + _FORCE_RENEW=1 + fi ;; + esac +fi + # if there is an existing certificate file, check details. if [[ -s "$CERT_FILE" ]]; then debug "certificate $CERT_FILE exists" @@ -3199,20 +3216,6 @@ if [[ "$REUSE_PRIVATE_KEY" != "true" ]]; then fi fi -# check if private key alg has changed from RSA to EC (or vice versa) -if [[ "$DUAL_RSA_ECDSA" == "false" ]] && [[ -s "$DOMAIN_DIR/${DOMAIN}.key" ]]; then - case "${PRIVATE_KEY_ALG}" in - rsa) - if grep --silent -- "-----BEGIN EC PRIVATE KEY-----" "$DOMAIN_DIR/${DOMAIN}.key"; then - rm -f "$DOMAIN_DIR/${DOMAIN}.key" - fi ;; - prime256v1|secp384r1|secp521r1) - if grep --silent -- "-----BEGIN RSA PRIVATE KEY-----" "$DOMAIN_DIR/${DOMAIN}.key"; then - rm -f "$DOMAIN_DIR/${DOMAIN}.key" - fi ;; - esac -fi - # create new domain keys if they don't already exist if [[ "$DUAL_RSA_ECDSA" == "false" ]]; then create_key "${PRIVATE_KEY_ALG}" "$DOMAIN_DIR/${DOMAIN}.key" "$DOMAIN_KEY_LENGTH" diff --git a/test/39-private-key-alg-changed.bats b/test/39-private-key-alg-changed.bats new file mode 100644 index 0000000..a8b0e43 --- /dev/null +++ b/test/39-private-key-alg-changed.bats @@ -0,0 +1,88 @@ +#! /usr/bin/env bats + +load '/bats-support/load.bash' +load '/bats-assert/load.bash' +load '/getssl/test/test_helper.bash' + + +# This is run for every test +teardown() { + [ -n "$BATS_TEST_COMPLETED" ] || touch $BATS_RUN_TMPDIR/failed.skip +} + +setup() { + [ ! -f $BATS_RUN_TMPDIR/failed.skip ] || skip "skipping tests after first failure" + export CURL_CA_BUNDLE=/root/pebble-ca-bundle.crt +} + +teardown_file() { + cleanup_environment +} + +@test "Create new certificate to create a private key" { + if [ -n "$STAGING" ]; then + skip "Using staging server, skipping internal test" + fi + CONFIG_FILE="getssl-http01.cfg" + setup_environment + init_getssl + create_certificate + assert_success + check_output_for_errors + # save a coy of the private key + cp "${INSTALL_DIR}/.getssl/${GETSSL_CMD_HOST}/${GETSSL_CMD_HOST}.key" "${INSTALL_DIR}/.getssl/${GETSSL_CMD_HOST}/${GETSSL_CMD_HOST}.key.orig" +} + +@test "Renew certificate (not force) and check nothing happens and key doesn't change" { + if [ -n "$STAGING" ]; then + skip "Using staging server, skipping internal test" + fi + + ORIG_KEY_HASH="$(cat ${INSTALL_DIR}/.getssl/${GETSSL_CMD_HOST}/${GETSSL_CMD_HOST}.key | sha256sum)" + + run ${CODE_DIR}/getssl -U -d $GETSSL_HOST + assert_success + assert_line --partial "certificate is valid for more than 30 days" + check_output_for_errors + + NEW_KEY_HASH="$(cat ${INSTALL_DIR}/.getssl/${GETSSL_CMD_HOST}/${GETSSL_CMD_HOST}.key | sha256sum)" + + assert [ "$NEW_KEY_HASH" == "$ORIG_KEY_HASH" ] +} + +@test "Force renewal and check key hasn't changed" { + if [ -n "$STAGING" ]; then + skip "Using staging server, skipping internal test" + fi + ORIG_KEY_HASH="$(cat ${INSTALL_DIR}/.getssl/${GETSSL_CMD_HOST}/${GETSSL_CMD_HOST}.key | sha256sum)" + + run ${CODE_DIR}/getssl -U -d -f $GETSSL_HOST + assert_success + check_output_for_errors + + NEW_KEY_HASH="$(cat ${INSTALL_DIR}/.getssl/${GETSSL_CMD_HOST}/${GETSSL_CMD_HOST}.key | sha256sum)" + + assert [ "$NEW_KEY_HASH" == "$ORIG_KEY_HASH" ] +} + +@test "Change key algorithm, force renewal, and check key has changed" { + if [ -n "$STAGING" ]; then + skip "Using staging server, skipping internal test" + fi + + ORIG_KEY_HASH="$(cat ${INSTALL_DIR}/.getssl/${GETSSL_CMD_HOST}/${GETSSL_CMD_HOST}.key | sha256sum)" + + cat <<- 'EOF' > ${INSTALL_DIR}/.getssl/${GETSSL_CMD_HOST}/getssl_test_specific.cfg +PRIVATE_KEY_ALG="prime256v1" +EOF + + run ${CODE_DIR}/getssl -U -d $GETSSL_HOST + assert_success + refute_line --partial "certificate is valid for more than 30 days" + + check_output_for_errors + + NEW_KEY_HASH="$(cat ${INSTALL_DIR}/.getssl/${GETSSL_CMD_HOST}/${GETSSL_CMD_HOST}.key | sha256sum)" + + assert [ "$NEW_KEY_HASH" != "$ORIG_KEY_HASH" ] +} From 99c285491960c2a337b8160ee298b01b5bec6468 Mon Sep 17 00:00:00 2001 From: Tim Kimber Date: Thu, 30 Sep 2021 18:33:24 +0100 Subject: [PATCH 2/4] Show curl error if obtain_ca_resource_location fails --- getssl | 13 ++++++++++++- test/0-test-curl-error.bats | 29 +++++++++++++++++++++++++++++ test/32-test-upgrade.bats | 2 +- 3 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 test/0-test-curl-error.bats diff --git a/getssl b/getssl index a688ca9..0365c83 100755 --- a/getssl +++ b/getssl @@ -2118,11 +2118,22 @@ json_get() { # get values from json obtain_ca_resource_locations() { + CURL_RESPONSE_FILE="$(mktemp 2>/dev/null || mktemp -t getssl.XXXXXX)" + for suffix in "" "/directory" "/dir"; do # Obtain CA resource locations # shellcheck disable=SC2086 - ca_all_loc=$(curl ${_NOMETER} --user-agent "$CURL_USERAGENT" "${CA}${suffix}" 2>/dev/null) + ca_all_loc=$(curl ${_NOMETER} --user-agent "$CURL_USERAGENT" "${CA}${suffix}" 2> $CURL_RESPONSE_FILE) + errcode=$? + if [[ $errcode -ne 0 ]]; then + response=$(cat "$CURL_RESPONSE_FILE") + rm "$CURL_RESPONSE_FILE" + error_exit "ERROR curl \"$CA$suffix\" failed with $errcode and returned:\n$response" + else + rm "$CURL_RESPONSE_FILE" + fi + debug "ca_all_loc from ${CA}${suffix} gives $ca_all_loc" # APIv1 URL_new_reg=$(echo "$ca_all_loc" | grep "new-reg" | awk -F'"' '{print $4}') diff --git a/test/0-test-curl-error.bats b/test/0-test-curl-error.bats new file mode 100644 index 0000000..1e9e2b7 --- /dev/null +++ b/test/0-test-curl-error.bats @@ -0,0 +1,29 @@ +#! /usr/bin/env bats + +load '/bats-support/load.bash' +load '/bats-assert/load.bash' +load '/getssl/test/test_helper.bash' + + +# This is run for every test +teardown() { + [ -n "$BATS_TEST_COMPLETED" ] || touch $BATS_RUN_TMPDIR/failed.skip +} + +setup() { + [ ! -f $BATS_RUN_TMPDIR/failed.skip ] || skip "skipping tests after first failure" + #export CURL_CA_BUNDLE=/root/pebble-ca-bundle.crt +} + + +@test "Run getssl without pebble certificates to check the error message" { + if [ -n "$STAGING" ]; then + skip "Using staging server, skipping internal test" + fi + CONFIG_FILE="getssl-http01.cfg" + setup_environment + init_getssl + create_certificate + refute_line "getssl: unknown API version" + assert_failure +} diff --git a/test/32-test-upgrade.bats b/test/32-test-upgrade.bats index 2bce33c..e698fc5 100644 --- a/test/32-test-upgrade.bats +++ b/test/32-test-upgrade.bats @@ -161,7 +161,7 @@ teardown() { # Check for current tag or file version otherwise push to master fails on a new version (or if the tag hasn't been updated) assert_line --regexp "Installed v(${CURRENT_TAG}|${FILE_VERSION}), restarting" - assert_line "Configuration check successful" + assert_line --partial "Configuration check successful" } From 544240d1f3ae8d818195d6a72039a9fc01b91b09 Mon Sep 17 00:00:00 2001 From: Tim Kimber Date: Thu, 30 Sep 2021 20:17:34 +0100 Subject: [PATCH 3/4] Fix grep --silent on alpine Add debug statement to investigate failures when running in github actions --- getssl | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/getssl b/getssl index 0365c83..ab3afce 100755 --- a/getssl +++ b/getssl @@ -822,9 +822,11 @@ check_getssl_upgrade() { # check if a more recent release is available fi CODE_LOCATION=$(sed -e"s/master/${release_tag}/" <<<"$CODE_LOCATION") # shellcheck disable=SC2086 + debug curl ${_NOMETER:---silent} --user-agent "$CURL_USERAGENT" "$CODE_LOCATION" --output "$TEMP_UPGRADE_FILE" + # shellcheck disable=SC2086 curl ${_NOMETER:---silent} --user-agent "$CURL_USERAGENT" "$CODE_LOCATION" --output "$TEMP_UPGRADE_FILE" - errcode=$? + if [[ $errcode -eq 60 ]]; then error_exit "curl needs updating, your version does not support SNI (multiple SSL domains on a single IP)" elif [[ $errcode -gt 0 ]]; then @@ -843,7 +845,7 @@ check_getssl_upgrade() { # check if a more recent release is available echo "The old version remains as ${0}.v${VERSION} and should be removed" echo "These update notifications can be turned off using the -Q option" echo "" - echo "Updates are;" + echo "Updates are:" awk "/\(${VERSION}\)$/ {s=1} s; /\(${release_tag}\)$/ || /^# ----/ {s=0}" "$TEMP_UPGRADE_FILE" | awk '{if(NR>1)print}' echo "" fi @@ -3162,12 +3164,12 @@ debug "created SAN list = $SANLIST" if [[ "$DUAL_RSA_ECDSA" == "false" ]] && [[ -s "$DOMAIN_DIR/${DOMAIN}.key" ]]; then case "${PRIVATE_KEY_ALG}" in rsa) - if grep --silent -- "-----BEGIN EC PRIVATE KEY-----" "$DOMAIN_DIR/${DOMAIN}.key"; then + if grep -q -- "-----BEGIN EC PRIVATE KEY-----" "$DOMAIN_DIR/${DOMAIN}.key"; then rm -f "$DOMAIN_DIR/${DOMAIN}.key" _FORCE_RENEW=1 fi ;; prime256v1|secp384r1|secp521r1) - if grep --silent -- "-----BEGIN RSA PRIVATE KEY-----" "$DOMAIN_DIR/${DOMAIN}.key"; then + if grep -q -- "-----BEGIN RSA PRIVATE KEY-----" "$DOMAIN_DIR/${DOMAIN}.key"; then rm -f "$DOMAIN_DIR/${DOMAIN}.key" _FORCE_RENEW=1 fi ;; From f6d48acc72fdfc109ea39ae8b1c09a306c177102 Mon Sep 17 00:00:00 2001 From: Tim Kimber Date: Thu, 30 Sep 2021 20:42:52 +0100 Subject: [PATCH 4/4] Fix code location --- getssl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/getssl b/getssl index ab3afce..5cf32f9 100755 --- a/getssl +++ b/getssl @@ -820,7 +820,7 @@ check_getssl_upgrade() { # check if a more recent release is available if [ "$TEMP_UPGRADE_FILE" == "" ]; then error_exit "mktemp failed" fi - CODE_LOCATION=$(sed -e"s/master/${release_tag}/" <<<"$CODE_LOCATION") + CODE_LOCATION=$(sed -e"s/getssl\/master/${release_tag}/" <<<"$CODE_LOCATION") # shellcheck disable=SC2086 debug curl ${_NOMETER:---silent} --user-agent "$CURL_USERAGENT" "$CODE_LOCATION" --output "$TEMP_UPGRADE_FILE" # shellcheck disable=SC2086