Merge pull request #674 from srvrco/fix-issue-673

Remove spaces and sort when checking if SANS list changed

getssl

@@ -2987,11 +2987,12 @@ if [[ -s "$CERT_FILE" ]]; then
   debug "certificate $CERT_FILE exists"
   enddate=$(openssl x509 -in "$CERT_FILE" -noout -enddate 2>/dev/null| cut -d= -f 2-)
   debug "local cert is valid until $enddate"
-  existing_sanlist=$(openssl x509 -in "$CERT_FILE" -noout -text | grep "DNS:" | sed 's/^ *//g')
+  existing_sanlist=$(openssl x509 -in "$CERT_FILE" -noout -text | grep "DNS:" | sed '{ s/ *DNS://g; y/,/\n/; }' | sort -u | xargs | sed 's/ /,/g')
+  sorted_sanlist=$(echo "$SANLIST" | sed '{ s/subjectAltName=//; s/ *DNS://g; y/,/\n/; }' | sort -u | xargs | sed 's/ /,/g')
   debug "local cert is for domains: ${existing_sanlist}"
   if [[ "$enddate" != "-" ]]; then
     enddate_s=$(date_epoc "$enddate")
-    if [[ $(date_renew) -lt "$enddate_s" ]] && [[ $_FORCE_RENEW -ne 1 ]] && [[ "subjectAltName=$existing_sanlist" == "$SANLIST" ]]; then
+    if [[ $(date_renew) -lt "$enddate_s" ]] && [[ $_FORCE_RENEW -ne 1 ]] && [[ "$existing_sanlist" == "$sorted_sanlist" ]]; then
       issuer=$(openssl x509 -in "$CERT_FILE" -noout -issuer 2>/dev/null)
       if [[ "$issuer" == *"Fake LE Intermediate"* ]] && [[ "$CA" == "https://acme-v02.api.letsencrypt.org" ]]; then
         debug "upgrading from fake cert to real"
@@ -3001,8 +3002,8 @@ if [[ -s "$CERT_FILE" ]]; then
         graceful_exit $_NOTIFY_VALID
       fi
     else
-      if [[ "subjectAltName=$existing_sanlist" != "$SANLIST" ]]; then
-        info "Domain list in existing certificate ($existing_sanlist) does not match domains requested ($SANLIST), so recreating certificate"
+      if [[ "$existing_sanlist" != "$sorted_sanlist" ]]; then
+        info "Domain list in existing certificate ($existing_sanlist) does not match domains requested ($sorted_sanlist), so recreating certificate"
       fi
       debug "${DOMAIN}: certificate needs renewal"
     fi

test/19-test-add-to-sans.bats

@@ -34,7 +34,37 @@ teardown() {
 }
 
 
-@test "Check we can add a new domain to SANS" {
+@test "Check that if the SANS doesn't change, we don't re-create the certificate (single domain)" {
+    if [ -n "$STAGING" ]; then
+        skip "Not trying on staging server yet"
+    fi
+    CONFIG_FILE="getssl-dns01.cfg"
+
+    . "${CODE_DIR}/test/test-config/${CONFIG_FILE}"
+    CERT=${INSTALL_DIR}/.getssl/${GETSSL_CMD_HOST}/${GETSSL_CMD_HOST}.crt
+
+    create_certificate
+    assert_success
+    check_output_for_errors
+
+    # As the SANS list changed, a new certificate is needed
+    refute_line --partial "does not match domains requested"
+    refute_line --partial "does not have the same domains as the config - re-create-csr"
+    refute_line --partial "certificate installed OK on server"
+    assert_line --partial 'certificate is valid for more than'
+
+    # Check that the SAN list in the certificate matches the expected value
+    SAN_IN_CERT=$(openssl x509 -in "$CERT" -noout -text | grep "DNS:" | sed 's/^ *//g')
+    SAN_EXPECTED="DNS:${GETSSL_HOST}"
+    if [[ "$SAN_IN_CERT" != "$SAN_EXPECTED" ]]; then
+        echo "# SAN_IN_CERT=$SAN_IN_CERT"
+        echo "# SAN_EXPECTED=$SAN_EXPECTED"
+    fi
+    [ "${SAN_IN_CERT}" = "$SAN_EXPECTED" ]
+}
+
+
+@test "Check certificate is recreated if we add a new domain to SANS" {
     if [ -n "$STAGING" ]; then
         skip "Not trying on staging server yet"
     fi
@@ -66,3 +96,72 @@ EOF
     fi
     [ "${SAN_IN_CERT}" = "$SAN_EXPECTED" ]
 }
+
+
+@test "Check that if the SANS doesn't change, we don't re-create the certificate (multiple domains)" {
+    if [ -n "$STAGING" ]; then
+        skip "Not trying on staging server yet"
+    fi
+    CONFIG_FILE="getssl-dns01.cfg"
+
+    cat <<- EOF > ${INSTALL_DIR}/.getssl/${GETSSL_CMD_HOST}/getssl_test_specific.cfg
+SANS="a.${GETSSL_HOST}"
+EOF
+
+    . "${CODE_DIR}/test/test-config/${CONFIG_FILE}"
+    CERT=${INSTALL_DIR}/.getssl/${GETSSL_CMD_HOST}/${GETSSL_CMD_HOST}.crt
+
+    create_certificate
+    assert_success
+    check_output_for_errors
+
+    # As the SANS list changed, a new certificate is needed
+    refute_line --partial "does not match domains requested"
+    refute_line --partial "does not have the same domains as the config - re-create-csr"
+    refute_line --partial "certificate installed OK on server"
+    assert_line --partial 'certificate is valid for more than'
+
+    # Check that the SAN list in the certificate matches the expected value
+    SAN_IN_CERT=$(openssl x509 -in "$CERT" -noout -text | grep "DNS:" | sed 's/^ *//g')
+    SAN_EXPECTED="DNS:${GETSSL_HOST}, DNS:a.${GETSSL_HOST}"
+    if [[ "$SAN_IN_CERT" != "$SAN_EXPECTED" ]]; then
+        echo "# SAN_IN_CERT=$SAN_IN_CERT"
+        echo "# SAN_EXPECTED=$SAN_EXPECTED"
+    fi
+    [ "${SAN_IN_CERT}" = "$SAN_EXPECTED" ]
+}
+
+
+@test "Check that if the SANS doesn't change, we don't re-create the certificate (reordered domains)" {
+    if [ -n "$STAGING" ]; then
+        skip "Not trying on staging server yet"
+    fi
+    CONFIG_FILE="getssl-dns01.cfg"
+
+    cat <<- EOF > ${INSTALL_DIR}/.getssl/${GETSSL_CMD_HOST}/getssl_test_specific.cfg
+IGNORE_DIRECTORY_DOMAIN="true"
+SANS="a.${GETSSL_HOST}, ${GETSSL_HOST}"
+EOF
+
+    . "${CODE_DIR}/test/test-config/${CONFIG_FILE}"
+    CERT=${INSTALL_DIR}/.getssl/${GETSSL_CMD_HOST}/${GETSSL_CMD_HOST}.crt
+
+    create_certificate
+    assert_success
+    check_output_for_errors
+
+    # As the SANS list changed, a new certificate is needed
+    refute_line --partial "does not match domains requested"
+    refute_line --partial "does not have the same domains as the config - re-create-csr"
+    refute_line --partial "certificate installed OK on server"
+    assert_line --partial 'certificate is valid for more than'
+
+    # Check that the SAN list in the certificate matches the expected value
+    SAN_IN_CERT=$(openssl x509 -in "$CERT" -noout -text | grep "DNS:" | sed 's/^ *//g')
+    SAN_EXPECTED="DNS:${GETSSL_HOST}, DNS:a.${GETSSL_HOST}"
+    if [[ "$SAN_IN_CERT" != "$SAN_EXPECTED" ]]; then
+        echo "# SAN_IN_CERT=$SAN_IN_CERT"
+        echo "# SAN_EXPECTED=$SAN_EXPECTED"
+    fi
+    [ "${SAN_IN_CERT}" = "$SAN_EXPECTED" ]
+}