Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 11 commits into from
Dec 18, 2023
36 changes: 26 additions & 10 deletions deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,28 @@ 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?:\/\/.+$/, "")
sharat87 marked this conversation as resolved.
Show resolved Hide resolved

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)
console.log("Old Let's Encrypt cert file exists, now checking if it's expired.")
if (!isCertExpired(fullChainPath)) {
certLocation = letsEncryptCertLocation
}
} catch (_) {
} catch {
// no certs there either, ignore.
}
sharat87 marked this conversation as resolved.
Show resolved Hide resolved
}
Expand Down Expand Up @@ -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"} {
import all-config
${tlsConfig}
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"))
Expand Down
9 changes: 9 additions & 0 deletions deploy/docker/route-tests/Dockerfile
Original file line number Diff line number Diff line change
@@ -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" ]
33 changes: 33 additions & 0 deletions deploy/docker/route-tests/common-https/forwarded-headers.hurl
Original file line number Diff line number Diff line change
@@ -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 = ''
```
33 changes: 33 additions & 0 deletions deploy/docker/route-tests/common/forwarded-headers.hurl
Original file line number Diff line number Diff line change
@@ -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 = ''
```
41 changes: 41 additions & 0 deletions deploy/docker/route-tests/common/index-html-response.hurl
Original file line number Diff line number Diff line change
@@ -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"
14 changes: 14 additions & 0 deletions deploy/docker/route-tests/common/static-404.hurl
Original file line number Diff line number Diff line change
@@ -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
12 changes: 12 additions & 0 deletions deploy/docker/route-tests/echo.caddyfile
Original file line number Diff line number Diff line change
@@ -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}'
"
83 changes: 83 additions & 0 deletions deploy/docker/route-tests/entrypoint.sh
Original file line number Diff line number Diff line change
@@ -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
sharat87 marked this conversation as resolved.
Show resolved Hide resolved

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
16 changes: 16 additions & 0 deletions deploy/docker/route-tests/run.sh
Original file line number Diff line number Diff line change
@@ -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
27 changes: 27 additions & 0 deletions deploy/docker/route-tests/spec-2/custom-domain.hurl
Original file line number Diff line number Diff line change
@@ -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
29 changes: 29 additions & 0 deletions deploy/docker/route-tests/spec-3/custom-domain.hurl
Original file line number Diff line number Diff line change
@@ -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
Loading