fix: Remove Server header and allow all on port 80 (#29585)

Another attempt at #29550, which was reverted. Fallback is not happening
if cert provisioning fails _despite_ having the correct header. But with
the changes in this PR, since we'll listen on `:80`, fallback _will_
happen when cert provisioning fails due to incorrect domain
configuration.

We're also adding [Hurl](https://hurl.dev) based tests. They're not run
in any CI yet. That'll come in soon.

deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs

@@ -3,17 +3,20 @@ import {dirname} from "path"
 import {spawnSync} from "child_process"
 import {X509Certificate} from "crypto"
 
-const APPSMITH_CUSTOM_DOMAIN = process.env.APPSMITH_CUSTOM_DOMAIN ?? null
+// The custom domain is expected to only have the domain. So if it has a protocol, we ignore the whole value.
+// This was the effective behaviour before Caddy.
+const CUSTOM_DOMAIN = (process.env.APPSMITH_CUSTOM_DOMAIN || "").replace(/^https?:\/\/.+$/, "")
+
 const CaddyfilePath = process.env.TMP + "/Caddyfile"
 
 let certLocation = null
-if (APPSMITH_CUSTOM_DOMAIN != null) {
+if (CUSTOM_DOMAIN !== "") {
   try {
     fs.accessSync("/appsmith-stacks/ssl/fullchain.pem", fs.constants.R_OK)
     certLocation = "/appsmith-stacks/ssl"
-  } catch (_) {
+  } catch {
     // no custom certs, see if old certbot certs are there.
-    const letsEncryptCertLocation = "/etc/letsencrypt/live/" + APPSMITH_CUSTOM_DOMAIN
+    const letsEncryptCertLocation = "/etc/letsencrypt/live/" + CUSTOM_DOMAIN
     const fullChainPath = letsEncryptCertLocation + `/fullchain.pem`
     try {
       fs.accessSync(fullChainPath, fs.constants.R_OK)
@@ -21,7 +24,7 @@ if (APPSMITH_CUSTOM_DOMAIN != null) {
       if (!isCertExpired(fullChainPath)) {
         certLocation = letsEncryptCertLocation
       }
-    } catch (_) {
+    } catch {
       // no certs there either, ignore.
     }
   }
@@ -114,18 +117,31 @@ parts.push(`
 
   handle_errors {
     respond "{err.status_code} {err.status_text}" {err.status_code}
+    header -Server
   }
 }
 
-localhost:80 127.0.0.1:80 {
+# We bind to http on 80, so that localhost requests don't get redirected to https.
+:80 {
   import all-config
 }
+`)
 
-${APPSMITH_CUSTOM_DOMAIN || ":80"} {
+if (CUSTOM_DOMAIN !== "") {
+  // If no custom domain, no extra routing needed.
+  // We have to own the http-to-https redirect, since we need to remove the `Server` header from the response.
+  parts.push(`
+  https://${CUSTOM_DOMAIN} {
     import all-config
     ${tlsConfig}
   }
+  http://${CUSTOM_DOMAIN} {
+    redir https://{host}{uri}
+    header -Server
+    header Connection close
+  }
   `)
+}
 
 fs.mkdirSync(dirname(CaddyfilePath), { recursive: true })
 fs.writeFileSync(CaddyfilePath, parts.join("\n"))

deploy/docker/route-tests/Dockerfile

@@ -0,0 +1,9 @@
+FROM node:lts-alpine
+
+RUN apk add --no-cache bash caddy \
+  && apk add --no-cache hurl mkcert --repository=http://dl-cdn.alpinelinux.org/alpine/edge/testing/ \
+  && mkcert -install
+
+WORKDIR /code
+
+ENTRYPOINT [ "bash", "entrypoint.sh" ]

deploy/docker/route-tests/common-https/forwarded-headers.hurl

@@ -0,0 +1,33 @@
+GET https://custom-domain.com/oauth2/google/authorize
+HTTP 200
+```
+Scheme = 'http'
+X-Forwarded-Proto = 'https'
+Host = 'custom-domain.com'
+X-Forwarded-Host = 'custom-domain.com'
+Forwarded = ''
+```
+
+# This is the CloudRun test. That the `Forwarded` header should be removed when proxying to upstream.
+GET https://custom-domain.com/oauth2/google/authorize
+Forwarded: something something
+HTTP 200
+```
+Scheme = 'http'
+X-Forwarded-Proto = 'https'
+Host = 'custom-domain.com'
+X-Forwarded-Host = 'custom-domain.com'
+Forwarded = ''
+```
+
+GET https://custom-domain.com/oauth2/google/authorize
+X-Forwarded-Proto: http
+X-Forwarded-Host: overridden.com
+HTTP 200
+```
+Scheme = 'http'
+X-Forwarded-Proto = 'http'
+Host = 'custom-domain.com'
+X-Forwarded-Host = 'overridden.com'
+Forwarded = ''
+```

deploy/docker/route-tests/common/forwarded-headers.hurl

@@ -0,0 +1,33 @@
+GET http://local.com/oauth2/google/authorize
+HTTP 200
+```
+Scheme = 'http'
+X-Forwarded-Proto = 'http'
+Host = 'local.com'
+X-Forwarded-Host = 'local.com'
+Forwarded = ''
+```
+
+# This is the CloudRun test. That the `Forwarded` header should be removed when proxying to upstream.
+GET http://local.com/oauth2/google/authorize
+Forwarded: something something
+HTTP 200
+```
+Scheme = 'http'
+X-Forwarded-Proto = 'http'
+Host = 'local.com'
+X-Forwarded-Host = 'local.com'
+Forwarded = ''
+```
+
+GET http://local.com/oauth2/google/authorize
+X-Forwarded-Proto: https
+X-Forwarded-Host: overridden.com
+HTTP 200
+```
+Scheme = 'http'
+X-Forwarded-Proto = 'https'
+Host = 'local.com'
+X-Forwarded-Host = 'overridden.com'
+Forwarded = ''
+```

deploy/docker/route-tests/common/index-html-response.hurl

@@ -0,0 +1,41 @@
+GET http://localhost
+HTTP 200
+Content-Type: text/html; charset=utf-8
+[Asserts]
+header "Server" not exists
+body == "index.html body"
+
+GET http://127.0.0.1
+HTTP 200
+Content-Type: text/html; charset=utf-8
+[Asserts]
+header "Server" not exists
+body == "index.html body"
+
+GET http://local.com
+HTTP 200
+Content-Type: text/html; charset=utf-8
+[Asserts]
+header "Server" not exists
+body == "index.html body"
+
+GET http://localhost/some/non/handled/path
+HTTP 200
+Content-Type: text/html; charset=utf-8
+[Asserts]
+header "Server" not exists
+body == "index.html body"
+
+GET http://127.0.0.1/some/non/handled/path
+HTTP 200
+Content-Type: text/html; charset=utf-8
+[Asserts]
+header "Server" not exists
+body == "index.html body"
+
+GET http://local.com/some/non/handled/path
+HTTP 200
+Content-Type: text/html; charset=utf-8
+[Asserts]
+header "Server" not exists
+body == "index.html body"

deploy/docker/route-tests/common/static-404.hurl

@@ -0,0 +1,14 @@
+GET http://localhost/static/missing
+HTTP 404
+[Asserts]
+header "Server" not exists
+
+GET http://127.0.0.1/static/missing
+HTTP 404
+[Asserts]
+header "Server" not exists
+
+GET http://local.com/static/missing
+HTTP 404
+[Asserts]
+header "Server" not exists

deploy/docker/route-tests/echo.caddyfile

@@ -0,0 +1,12 @@
+{
+  admin off
+}
+
+http://:5050
+
+respond "Scheme = '{scheme}'
+X-Forwarded-Proto = '{header.x-forwarded-proto}'
+Host = '{host}'
+X-Forwarded-Host = '{header.x-forwarded-host}'
+Forwarded = '{header.forwarded}'
+"

deploy/docker/route-tests/entrypoint.sh

@@ -0,0 +1,83 @@
+#!/usr/bin/env bash
+
+set -o errexit
+set -o nounset
+set -o pipefail
+#set -o xtrace
+
+new-spec() {
+  echo "-----------" "$@" "-----------"
+  unset APPSMITH_CUSTOM_DOMAIN
+  mkdir -p /appsmith-stacks/ssl
+  find /appsmith-stacks/ssl -type f -delete
+}
+
+reload-caddy() {
+  sed -i 's/127.0.0.1:{args\[0]}/127.0.0.1:5050/' "$TMP/Caddyfile"
+  caddy fmt --overwrite "$TMP/Caddyfile"
+  caddy reload --config "$TMP/Caddyfile"
+  sleep 1
+}
+
+run-hurl() {
+  hurl --test \
+    --resolve local.com:80:127.0.0.1 \
+    --resolve custom-domain.com:80:127.0.0.1 \
+    --resolve custom-domain.com:443:127.0.0.1 \
+    "$@"
+}
+
+if [[ "${OPEN_SHELL-}" == 1 ]]; then
+  # Open shell for debugging after this script is done.
+  trap bash EXIT
+fi
+
+echo
+echo "caddy version: $(caddy --version)"
+echo "hurl version: $(hurl --version)"
+echo "mkcert version: $(mkcert --version)"
+echo
+
+export TMP=/tmp/appsmith
+export WWW_PATH="$TMP/www"
+
+mkdir -p "$WWW_PATH"
+echo -n 'index.html body' > "$WWW_PATH/index.html"
+mkcert -install
+
+# Start echo server
+(
+  export XDG_DATA_HOME="$TMP/echo-data"
+  export XDG_CONFIG_HOME="$TMP/echo-conf"
+  mkdir -p "$XDG_DATA_HOME" "$XDG_CONFIG_HOME"
+  caddy start --config echo.caddyfile --adapter caddyfile >> "$TMP/echo-caddy.log" 2>&1
+)
+
+# Start Caddy for use with our config to test
+echo localhost > "$TMP/Caddyfile"
+caddy start --config "$TMP/Caddyfile" >> "$TMP/caddy.log" 2>&1
+
+sleep 1
+
+
+new-spec "Spec 1: With no custom domain"
+node /caddy-reconfigure.mjs
+reload-caddy
+run-hurl common/*.hurl
+
+
+new-spec "Spec 2: With a custom domain, cert obtained (because of internal CA)"
+export APPSMITH_CUSTOM_DOMAIN=custom-domain.com
+node /caddy-reconfigure.mjs
+#sed -i '2i acme_ca https://acme-staging-v02.api.letsencrypt.org/directory' "$TMP/Caddyfile"
+sed -i '/https:\/\/'"$APPSMITH_CUSTOM_DOMAIN"' {$/a tls internal' "$TMP/Caddyfile"
+reload-caddy
+run-hurl common/*.hurl common-https/*.hurl spec-2/*.hurl
+
+
+new-spec "Spec 3: With a custom domain, certs given in ssl folder"
+export APPSMITH_CUSTOM_DOMAIN=custom-domain.com
+mkcert -cert-file "/appsmith-stacks/ssl/fullchain.pem" -key-file "/appsmith-stacks/ssl/privkey.pem" "$APPSMITH_CUSTOM_DOMAIN"
+node /caddy-reconfigure.mjs
+reload-caddy
+run-hurl common/*.hurl spec-3/*.hurl

deploy/docker/route-tests/run.sh

@@ -0,0 +1,16 @@
+#!/usr/bin/env bash
+
+set -o errexit
+set -o nounset
+
+loc="$(dirname "$0")"
+docker build -f "$loc/Dockerfile" --tag ar "$loc/.."
+docker run \
+  --name ar \
+  --rm \
+  -it \
+  --hostname ar \
+  -e OPEN_SHELL=${OPEN_SHELL-} \
+  --volume "$loc/../fs/opt/appsmith/caddy-reconfigure.mjs:/caddy-reconfigure.mjs:ro" \
+  --volume "$loc:/code:ro" \
+  ar

deploy/docker/route-tests/spec-2/custom-domain.hurl

@@ -0,0 +1,27 @@
+GET http://custom-domain.com
+HTTP 302
+Location: https://custom-domain.com/
+[Asserts]
+header "Server" not exists
+
+GET http://custom-domain.com/random/path
+HTTP 302
+Location: https://custom-domain.com/random/path
+[Asserts]
+header "Server" not exists
+
+GET https://custom-domain.com
+HTTP 200
+[Asserts]
+header "Server" not exists
+certificate "Issuer" == "CN = Caddy Local Authority - ECC Intermediate"
+
+GET https://custom-domain.com/random/path
+HTTP 200
+[Asserts]
+header "Server" not exists
+
+GET https://custom-domain.com/static/x
+HTTP 404
+[Asserts]
+header "Server" not exists

deploy/docker/route-tests/spec-3/custom-domain.hurl

@@ -0,0 +1,29 @@
+GET http://custom-domain.com
+HTTP 302
+Location: https://custom-domain.com/
+Connection: close
+[Asserts]
+header "Server" not exists
+
+GET http://custom-domain.com/random/path
+HTTP 302
+Location: https://custom-domain.com/random/path
+Connection: close
+[Asserts]
+header "Server" not exists
+
+GET https://custom-domain.com
+HTTP 200
+[Asserts]
+header "Server" not exists
+certificate "Issuer" == "O = mkcert development CA, OU = root@ar, CN = mkcert root@ar"
+
+GET https://custom-domain.com/random/path
+HTTP 200
+[Asserts]
+header "Server" not exists
+
+GET https://custom-domain.com/static/x
+HTTP 404
+[Asserts]
+header "Server" not exists