From 8cc92680c56be1af376863c20ea0f66cd33ab40f Mon Sep 17 00:00:00 2001 From: Aubrey Hewes Date: Wed, 6 Jan 2021 22:06:14 +0100 Subject: [PATCH 1/2] feat(nginx-proxy): allow disabling hsts globally and explicitly enabling it per app https://github.com/dokku/dokku/pull/3843 --- docs/configuration/nginx.md | 14 +++++++++ plugins/nginx-vhosts/command-functions | 2 ++ plugins/nginx-vhosts/internal-functions | 14 ++++++++- tests/unit/nginx-vhosts_6.bats | 38 +++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 1 deletion(-) diff --git a/docs/configuration/nginx.md b/docs/configuration/nginx.md index 1966715f5..27a2cd6dd 100644 --- a/docs/configuration/nginx.md +++ b/docs/configuration/nginx.md @@ -61,6 +61,20 @@ The following options are also available via the `nginx:set` command: Beware that if you enable the header and a subsequent deploy of your application results in an HTTP deploy (for whatever reason), the way the header works means that a browser will not attempt to request the HTTP version of your site if the HTTPS version fails until the max-age is reached. +#### Globally disabling the HSTS Header + +HSTS Header can be disabled for all apps by setting the `hsts` property to false after passing the `--global` flag to `nginx:set`. Changing this value globally or on a per-app basis will require rebuilding the nginx config via the `nginx:build-config` command. + +```shell +dokku nginx:set --global hsts false +``` + +Once the HSTS setting is disabled globally, it can be re-enabled on a per-app basis by setting the `hsts` property as normal. + +```shell +dokku nginx:set node-js-app hsts true +``` + ### Checking access logs You may check nginx access logs via the `nginx:access-logs` command. This assumes that app access logs are being stored in `/var/log/nginx/$APP-access.log`, as is the default in the generated `nginx.conf`. diff --git a/plugins/nginx-vhosts/command-functions b/plugins/nginx-vhosts/command-functions index 1f190c4f5..4a78838a2 100755 --- a/plugins/nginx-vhosts/command-functions +++ b/plugins/nginx-vhosts/command-functions @@ -45,6 +45,8 @@ cmd-nginx-report-single() { "--nginx-client-max-body-size: $(fn-plugin-property-get-default "nginx" "$APP" "client-max-body-size" "")" "--nginx-disable-custom-config: $(fn-plugin-property-get-default "nginx" "$APP" "disable-custom-config" "false")" "--nginx-error-log-path: $(fn-nginx-error-log-path "$APP")" + "--nginx-global-hsts: $(fn-plugin-property-get-default "nginx" "--global" "hsts" "true")" + "--nginx-applied-hsts: $(fn-plugin-property-get-default "nginx" "$APP" "hsts" "$(fn-plugin-property-get-default "nginx" "--global" "hsts" "true")")" "--nginx-hsts: $(fn-plugin-property-get-default "nginx" "$APP" "hsts" "true")" "--nginx-hsts-include-subdomains: $(fn-plugin-property-get-default "nginx" "$APP" "hsts-include-subdomains" "true")" "--nginx-hsts-max-age: $(fn-plugin-property-get-default "nginx" "$APP" "hsts-max-age" "15724800")" diff --git a/plugins/nginx-vhosts/internal-functions b/plugins/nginx-vhosts/internal-functions index 0c5dcd807..f969a00de 100755 --- a/plugins/nginx-vhosts/internal-functions +++ b/plugins/nginx-vhosts/internal-functions @@ -13,9 +13,21 @@ fn-nginx-vhosts-last-visited-at() { fi } +fn-nginx-hsts-is-applied() { + declare APP="$1" + # if hsts is disabled globally then only apply it if an app has it set explicitly + if [[ "$(fn-plugin-property-get-default "nginx" "--global" "hsts" "true")" == "false" ]] && \ + [[ "$(fn-plugin-property-get-default "nginx" "$APP" "hsts" "false")" == "false" ]]; then + echo "false"; + return; + fi + # if app has explicitly turned it off else it is default on + echo $(fn-plugin-property-get-default "nginx" "$APP" "hsts" "true"); +} + fn-nginx-vhosts-manage-hsts() { declare APP="$1" SSL_ENABLED="$2" - local HSTS="$(fn-plugin-property-get-default "nginx" "$APP" "hsts" "true")" + local HSTS="$(fn-nginx-hsts-is-applied "$APP")" local HSTS_INCLUDE_SUBDOMAINS="$(fn-plugin-property-get-default "nginx" "$APP" "hsts-include-subdomains" "true")" local HSTS_MAX_AGE="$(fn-plugin-property-get-default "nginx" "$APP" "hsts-max-age" "15724800")" local HSTS_PRELOAD="$(fn-plugin-property-get-default "nginx" "$APP" "hsts-preload" "false")" diff --git a/tests/unit/nginx-vhosts_6.bats b/tests/unit/nginx-vhosts_6.bats index 1e49fa352..824d142ea 100644 --- a/tests/unit/nginx-vhosts_6.bats +++ b/tests/unit/nginx-vhosts_6.bats @@ -107,6 +107,44 @@ teardown() { assert_failure } +@test "(nginx-vhosts) nginx:set --global hsts" { + setup_test_tls wildcard + local HSTS_CONF="/home/dokku/${TEST_APP}/nginx.conf.d/hsts.conf" + + run deploy_app + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "Enabling HSTS" + + # disable hsts globally + run /bin/bash -c "dokku nginx:set --global hsts false" + # check it is now not applied + run /bin/bash -c "dokku nginx:build-config $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "Enabling HSTS" 0 + + # apply on app + run /bin/bash -c "dokku nginx:set $TEST_APP hsts true" + # check it is now applied + run /bin/bash -c "dokku nginx:build-config $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "Enabling HSTS" 1 + + # disable globally + run /bin/bash -c "dokku nginx:set --global hsts" + # check it is still applied + run /bin/bash -c "dokku nginx:build-config $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "Enabling HSTS" 1 +} + @test "(nginx-vhosts) nginx:set bind-address" { run deploy_app echo "output: $output" From 5cac43fe55465dd72dee5cd6577a1f9c620bc3dc Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sat, 23 Jan 2021 15:49:40 -0500 Subject: [PATCH 2/2] refactor: fix computation test and remove the app-specific hsts value The default for an app is empty, meaning it will fallback to the global value (which defaults to true). This will allow users to properly override the global value, regardless of what it is. --- plugins/nginx-vhosts/command-functions | 4 ++-- plugins/nginx-vhosts/internal-functions | 17 ++++++++--------- plugins/nginx-vhosts/subcommands/set | 9 ++++++++- tests/unit/nginx-vhosts_6.bats | 20 ++++++++++++++++---- 4 files changed, 34 insertions(+), 16 deletions(-) diff --git a/plugins/nginx-vhosts/command-functions b/plugins/nginx-vhosts/command-functions index 4a78838a2..d410de3e3 100755 --- a/plugins/nginx-vhosts/command-functions +++ b/plugins/nginx-vhosts/command-functions @@ -46,8 +46,8 @@ cmd-nginx-report-single() { "--nginx-disable-custom-config: $(fn-plugin-property-get-default "nginx" "$APP" "disable-custom-config" "false")" "--nginx-error-log-path: $(fn-nginx-error-log-path "$APP")" "--nginx-global-hsts: $(fn-plugin-property-get-default "nginx" "--global" "hsts" "true")" - "--nginx-applied-hsts: $(fn-plugin-property-get-default "nginx" "$APP" "hsts" "$(fn-plugin-property-get-default "nginx" "--global" "hsts" "true")")" - "--nginx-hsts: $(fn-plugin-property-get-default "nginx" "$APP" "hsts" "true")" + "--nginx-computed-hsts: $(fn-nginx-hsts-is-enabled "$APP")" + "--nginx-hsts: $(fn-plugin-property-get-default "nginx" "$APP" "hsts" "")" "--nginx-hsts-include-subdomains: $(fn-plugin-property-get-default "nginx" "$APP" "hsts-include-subdomains" "true")" "--nginx-hsts-max-age: $(fn-plugin-property-get-default "nginx" "$APP" "hsts-max-age" "15724800")" "--nginx-hsts-preload: $(fn-plugin-property-get-default "nginx" "$APP" "hsts-preload" "false")" diff --git a/plugins/nginx-vhosts/internal-functions b/plugins/nginx-vhosts/internal-functions index f969a00de..91e7ea528 100755 --- a/plugins/nginx-vhosts/internal-functions +++ b/plugins/nginx-vhosts/internal-functions @@ -13,21 +13,20 @@ fn-nginx-vhosts-last-visited-at() { fi } -fn-nginx-hsts-is-applied() { +fn-nginx-hsts-is-enabled() { declare APP="$1" - # if hsts is disabled globally then only apply it if an app has it set explicitly - if [[ "$(fn-plugin-property-get-default "nginx" "--global" "hsts" "true")" == "false" ]] && \ - [[ "$(fn-plugin-property-get-default "nginx" "$APP" "hsts" "false")" == "false" ]]; then - echo "false"; - return; + local hsts_is_enabled="$(fn-plugin-property-get-default "nginx" "$APP" "hsts" "")" + + if [[ "$hsts_is_enabled" == "" ]]; then + hsts_is_enabled="$(fn-plugin-property-get-default "nginx" "--global" "hsts" "true")" fi - # if app has explicitly turned it off else it is default on - echo $(fn-plugin-property-get-default "nginx" "$APP" "hsts" "true"); + + echo "$hsts_is_enabled" } fn-nginx-vhosts-manage-hsts() { declare APP="$1" SSL_ENABLED="$2" - local HSTS="$(fn-nginx-hsts-is-applied "$APP")" + local HSTS="$(fn-nginx-hsts-is-enabled "$APP")" local HSTS_INCLUDE_SUBDOMAINS="$(fn-plugin-property-get-default "nginx" "$APP" "hsts-include-subdomains" "true")" local HSTS_MAX_AGE="$(fn-plugin-property-get-default "nginx" "$APP" "hsts-max-age" "15724800")" local HSTS_PRELOAD="$(fn-plugin-property-get-default "nginx" "$APP" "hsts-preload" "false")" diff --git a/plugins/nginx-vhosts/subcommands/set b/plugins/nginx-vhosts/subcommands/set index 34925ed99..c65fbe78f 100755 --- a/plugins/nginx-vhosts/subcommands/set +++ b/plugins/nginx-vhosts/subcommands/set @@ -10,7 +10,7 @@ cmd-nginx-set() { [[ "$1" == "$cmd" ]] && shift 1 declare APP="$1" KEY="$2" VALUE="$3" local VALID_KEYS=("access-log-format" "access-log-path" "bind-address-ipv4" "bind-address-ipv6" "client-max-body-size" "disable-custom-config" "error-log-path" "hsts" "hsts-include-subdomains" "hsts-preload" "hsts-max-age" "proxy-read-timeout" "proxy-buffer-size" "proxy-buffering" "proxy-buffers" "proxy-busy-buffers-size" "x-forwarded-for-value" "x-forwarded-port-value" "x-forwarded-proto-value") - verify_app_name "$APP" + local GLOBAL_KEYS=("hsts") [[ -z "$KEY" ]] && dokku_log_fail "No key specified" @@ -18,6 +18,13 @@ cmd-nginx-set() { dokku_log_fail "Invalid key specified, valid keys include: access-log-format, access-log-path, bind-address-ipv4, bind-address-ipv6, client-max-body-size, disable-custom-config, error-log-path, hsts, hsts-include-subdomains, hsts-preload, hsts-max-age, proxy-read-timeout, proxy-buffer-size, proxy-buffering, proxy-buffers, proxy-busy-buffers-size, x-forwarded-for-value, x-forwarded-port-value, x-forwarded-proto-value" fi + if ! fn-in-array "$KEY" "${GLOBAL_KEYS[@]}"; then + if [[ "$APP" == "--global" ]]; then + dokku_log_fail "The key '$KEY' cannot be set globally" + fi + verify_app_name "$APP" + fi + if [[ -n "$VALUE" ]]; then dokku_log_info2_quiet "Setting ${KEY} to ${VALUE}" fn-plugin-property-write "nginx" "$APP" "$KEY" "$VALUE" diff --git a/tests/unit/nginx-vhosts_6.bats b/tests/unit/nginx-vhosts_6.bats index 824d142ea..1ce7b384f 100644 --- a/tests/unit/nginx-vhosts_6.bats +++ b/tests/unit/nginx-vhosts_6.bats @@ -119,8 +119,12 @@ teardown() { # disable hsts globally run /bin/bash -c "dokku nginx:set --global hsts false" + echo "output: $output" + echo "status: $status" + assert_success + # check it is now not applied - run /bin/bash -c "dokku nginx:build-config $TEST_APP" + run /bin/bash -c "dokku proxy:build-config $TEST_APP" echo "output: $output" echo "status: $status" assert_success @@ -128,17 +132,25 @@ teardown() { # apply on app run /bin/bash -c "dokku nginx:set $TEST_APP hsts true" + echo "output: $output" + echo "status: $status" + assert_success + # check it is now applied - run /bin/bash -c "dokku nginx:build-config $TEST_APP" + run /bin/bash -c "dokku proxy:build-config $TEST_APP" echo "output: $output" echo "status: $status" assert_success assert_output_contains "Enabling HSTS" 1 - # disable globally + # set global value to default run /bin/bash -c "dokku nginx:set --global hsts" + echo "output: $output" + echo "status: $status" + assert_success + # check it is still applied - run /bin/bash -c "dokku nginx:build-config $TEST_APP" + run /bin/bash -c "dokku proxy:build-config $TEST_APP" echo "output: $output" echo "status: $status" assert_success