diff --git a/gennghttpxfun.py b/gennghttpxfun.py index a388b119..97ec0f69 100755 --- a/gennghttpxfun.py +++ b/gennghttpxfun.py @@ -198,6 +198,7 @@ OPTIONS = [ "max-worker-processes", "worker-process-grace-shutdown-period", "frontend-quic-initial-rtt", + "require-http-scheme", ] LOGVARS = [ diff --git a/integration-tests/nghttpx_http2_test.go b/integration-tests/nghttpx_http2_test.go index a856f8fc..f31f1878 100644 --- a/integration-tests/nghttpx_http2_test.go +++ b/integration-tests/nghttpx_http2_test.go @@ -2870,3 +2870,88 @@ func TestH2H1ChunkedEndsPrematurely(t *testing.T) { t.Errorf("res.errCode = %v; want %v", got, want) } } + +// TestH2H1RequireHTTPSchemeHTTPSWithoutEncryption verifies that https +// scheme in non-encrypted connection is treated as error. +func TestH2H1RequireHTTPSchemeHTTPSWithoutEncryption(t *testing.T) { + st := newServerTester([]string{"--require-http-scheme"}, t, func(w http.ResponseWriter, r *http.Request) { + t.Errorf("server should not forward this request") + }) + defer st.Close() + + res, err := st.http2(requestParam{ + name: "TestH2H1RequireHTTPSchemeHTTPSWithoutEncryption", + scheme: "https", + }) + if err != nil { + t.Fatalf("Error st.http2() = %v", err) + } + + if got, want := res.status, 400; got != want { + t.Errorf("status = %v; want %v", got, want) + } +} + +// TestH2H1RequireHTTPSchemeHTTPWithEncryption verifies that http +// scheme in encrypted connection is treated as error. +func TestH2H1RequireHTTPSchemeHTTPWithEncryption(t *testing.T) { + st := newServerTesterTLS([]string{"--require-http-scheme"}, t, func(w http.ResponseWriter, r *http.Request) { + t.Errorf("server should not forward this request") + }) + defer st.Close() + + res, err := st.http2(requestParam{ + name: "TestH2H1RequireHTTPSchemeHTTPWithEncryption", + scheme: "http", + }) + if err != nil { + t.Fatalf("Error st.http2() = %v", err) + } + + if got, want := res.status, 400; got != want { + t.Errorf("status = %v; want %v", got, want) + } +} + +// TestH2H1RequireHTTPSchemeUnknownSchemeWithoutEncryption verifies +// that unknown scheme in non-encrypted connection is treated as +// error. +func TestH2H1RequireHTTPSchemeUnknownSchemeWithoutEncryption(t *testing.T) { + st := newServerTester([]string{"--require-http-scheme"}, t, func(w http.ResponseWriter, r *http.Request) { + t.Errorf("server should not forward this request") + }) + defer st.Close() + + res, err := st.http2(requestParam{ + name: "TestH2H1RequireHTTPSchemeUnknownSchemeWithoutEncryption", + scheme: "unknown", + }) + if err != nil { + t.Fatalf("Error st.http2() = %v", err) + } + + if got, want := res.status, 400; got != want { + t.Errorf("status = %v; want %v", got, want) + } +} + +// TestH2H1RequireHTTPSchemeUnknownSchemeWithEncryption verifies that +// unknown scheme in encrypted connection is treated as error. +func TestH2H1RequireHTTPSchemeUnknownSchemeWithEncryption(t *testing.T) { + st := newServerTesterTLS([]string{"--require-http-scheme"}, t, func(w http.ResponseWriter, r *http.Request) { + t.Errorf("server should not forward this request") + }) + defer st.Close() + + res, err := st.http2(requestParam{ + name: "TestH2H1RequireHTTPSchemeUnknownSchemeWithEncryption", + scheme: "unknown", + }) + if err != nil { + t.Fatalf("Error st.http2() = %v", err) + } + + if got, want := res.status, 400; got != want { + t.Errorf("status = %v; want %v", got, want) + } +} diff --git a/src/shrpx-unittest.cc b/src/shrpx-unittest.cc index 24a1acdc..81591ec5 100644 --- a/src/shrpx-unittest.cc +++ b/src/shrpx-unittest.cc @@ -137,6 +137,8 @@ int main(int argc, char *argv[]) { shrpx::test_shrpx_http_create_affinity_cookie) || !CU_add_test(pSuite, "http_create_atlsvc_header_field_value", shrpx::test_shrpx_http_create_altsvc_header_value) || + !CU_add_test(pSuite, "http_check_http_scheme", + shrpx::test_shrpx_http_check_http_scheme) || !CU_add_test(pSuite, "router_match", shrpx::test_shrpx_router_match) || !CU_add_test(pSuite, "router_match_wildcard", shrpx::test_shrpx_router_match_wildcard) || diff --git a/src/shrpx.cc b/src/shrpx.cc index 9bdcb34c..74b65934 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -3237,6 +3237,13 @@ HTTP: "redirect-if-not-tls" parameter in --backend option. Default: )" << config->http.redirect_https_port << R"( + --require-http-scheme + Always require http or https scheme in HTTP request. It + also requires that https scheme must be used for an + encrypted connection. Otherwise, http scheme must be + used. This option is recommended for a server + deployment which directly faces clients and the services + it provides only require http or https scheme. API: --api-max-request-body= @@ -4238,6 +4245,7 @@ int main(int argc, char **argv) { required_argument, &flag, 189}, {SHRPX_OPT_FRONTEND_QUIC_INITIAL_RTT.c_str(), required_argument, &flag, 190}, + {SHRPX_OPT_REQUIRE_HTTP_SCHEME.c_str(), no_argument, &flag, 191}, {nullptr, 0, nullptr, 0}}; int option_index = 0; @@ -5142,6 +5150,11 @@ int main(int argc, char **argv) { cmdcfgs.emplace_back(SHRPX_OPT_FRONTEND_QUIC_INITIAL_RTT, StringRef{optarg}); break; + case 191: + // --require-http-scheme + cmdcfgs.emplace_back(SHRPX_OPT_REQUIRE_HTTP_SCHEME, + StringRef::from_lit("yes")); + break; default: break; } diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index 19c34180..d7f3a71c 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -2210,6 +2210,9 @@ int option_lookup_token(const char *name, size_t namelen) { if (util::strieq_l("no-location-rewrit", name, 18)) { return SHRPX_OPTID_NO_LOCATION_REWRITE; } + if (util::strieq_l("require-http-schem", name, 18)) { + return SHRPX_OPTID_REQUIRE_HTTP_SCHEME; + } if (util::strieq_l("tls-ticket-key-fil", name, 18)) { return SHRPX_OPTID_TLS_TICKET_KEY_FILE; } @@ -4166,6 +4169,9 @@ int parse_config(Config *config, int optid, const StringRef &opt, return 0; } + case SHRPX_OPTID_REQUIRE_HTTP_SCHEME: + config->http.require_http_scheme = util::strieq_l("yes", optarg); + return 0; case SHRPX_OPTID_CONF: LOG(WARN) << "conf: ignored"; diff --git a/src/shrpx_config.h b/src/shrpx_config.h index b5871fff..29ac81e8 100644 --- a/src/shrpx_config.h +++ b/src/shrpx_config.h @@ -401,6 +401,8 @@ constexpr auto SHRPX_OPT_WORKER_PROCESS_GRACE_SHUTDOWN_PERIOD = StringRef::from_lit("worker-process-grace-shutdown-period"); constexpr auto SHRPX_OPT_FRONTEND_QUIC_INITIAL_RTT = StringRef::from_lit("frontend-quic-initial-rtt"); +constexpr auto SHRPX_OPT_REQUIRE_HTTP_SCHEME = + StringRef::from_lit("require-http-scheme"); constexpr size_t SHRPX_OBFUSCATED_NODE_LENGTH = 8; @@ -857,6 +859,7 @@ struct HttpConfig { bool no_location_rewrite; bool no_host_rewrite; bool no_server_rewrite; + bool require_http_scheme; }; struct Http2Config { @@ -1295,6 +1298,7 @@ enum { SHRPX_OPTID_READ_RATE, SHRPX_OPTID_REDIRECT_HTTPS_PORT, SHRPX_OPTID_REQUEST_HEADER_FIELD_BUFFER, + SHRPX_OPTID_REQUIRE_HTTP_SCHEME, SHRPX_OPTID_RESPONSE_HEADER_FIELD_BUFFER, SHRPX_OPTID_RLIMIT_MEMLOCK, SHRPX_OPTID_RLIMIT_NOFILE, diff --git a/src/shrpx_http.cc b/src/shrpx_http.cc index 0627250f..ad32dc9e 100644 --- a/src/shrpx_http.cc +++ b/src/shrpx_http.cc @@ -271,6 +271,10 @@ StringRef create_altsvc_header_value(BlockAllocator &balloc, return StringRef{iov.base, p}; } +bool check_http_scheme(const StringRef &scheme, bool encrypted) { + return encrypted ? scheme == "https" : scheme == "http"; +} + } // namespace http } // namespace shrpx diff --git a/src/shrpx_http.h b/src/shrpx_http.h index 7b9cbb17..18935d82 100644 --- a/src/shrpx_http.h +++ b/src/shrpx_http.h @@ -83,6 +83,12 @@ bool require_cookie_secure_attribute(SessionAffinityCookieSecure secure, StringRef create_altsvc_header_value(BlockAllocator &balloc, const std::vector &altsvcs); +// Returns true if either of the following conditions holds: +// - scheme is https and encrypted is true +// - scheme is http and encrypted is false +// Otherwise returns false. +bool check_http_scheme(const StringRef &scheme, bool encrypted); + } // namespace http } // namespace shrpx diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 8859ec37..919cdb9e 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -419,10 +419,16 @@ int Http2Upstream::on_request_headers(Downstream *downstream, downstream->set_request_state(DownstreamState::HEADER_COMPLETE); + if (config->http.require_http_scheme && + !http::check_http_scheme(req.scheme, handler_->get_ssl() != nullptr)) { + if (error_reply(downstream, 400) != 0) { + return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; + } + return 0; + } + #ifdef HAVE_MRUBY - auto upstream = downstream->get_upstream(); - auto handler = upstream->get_client_handler(); - auto worker = handler->get_worker(); + auto worker = handler_->get_worker(); auto mruby_ctx = worker->get_mruby_context(); if (mruby_ctx->run_on_request_proc(downstream) != 0) { diff --git a/src/shrpx_http3_upstream.cc b/src/shrpx_http3_upstream.cc index 12299360..62ad9220 100644 --- a/src/shrpx_http3_upstream.cc +++ b/src/shrpx_http3_upstream.cc @@ -2325,10 +2325,15 @@ int Http3Upstream::http_end_request_headers(Downstream *downstream, int fin) { downstream->set_request_state(DownstreamState::HEADER_COMPLETE); + if (config->http.require_http_scheme && + !http::check_http_scheme(req.scheme, /* encrypted = */true) { + if (error_reply(downstream, 400) != 0) { + return -1; + } + } + #ifdef HAVE_MRUBY - auto upstream = downstream->get_upstream(); - auto handler = upstream->get_client_handler(); - auto worker = handler->get_worker(); + auto worker = handler_->get_worker(); auto mruby_ctx = worker->get_mruby_context(); if (mruby_ctx->run_on_request_proc(downstream) != 0) { diff --git a/src/shrpx_http_test.cc b/src/shrpx_http_test.cc index 7a9c63ab..3ace8702 100644 --- a/src/shrpx_http_test.cc +++ b/src/shrpx_http_test.cc @@ -154,4 +154,15 @@ void test_shrpx_http_create_altsvc_header_value(void) { } } +void test_shrpx_http_check_http_scheme(void) { + CU_ASSERT(http::check_http_scheme(StringRef::from_lit("https"), true)); + CU_ASSERT(!http::check_http_scheme(StringRef::from_lit("https"), false)); + CU_ASSERT(!http::check_http_scheme(StringRef::from_lit("http"), true)); + CU_ASSERT(http::check_http_scheme(StringRef::from_lit("http"), false)); + CU_ASSERT(!http::check_http_scheme(StringRef::from_lit("foo"), true)); + CU_ASSERT(!http::check_http_scheme(StringRef::from_lit("foo"), false)); + CU_ASSERT(!http::check_http_scheme(StringRef{}, true)); + CU_ASSERT(!http::check_http_scheme(StringRef{}, false)); +} + } // namespace shrpx diff --git a/src/shrpx_http_test.h b/src/shrpx_http_test.h index 8c3b1d93..d50ab533 100644 --- a/src/shrpx_http_test.h +++ b/src/shrpx_http_test.h @@ -35,6 +35,7 @@ void test_shrpx_http_create_forwarded(void); void test_shrpx_http_create_via_header_value(void); void test_shrpx_http_create_affinity_cookie(void); void test_shrpx_http_create_altsvc_header_value(void); +void test_shrpx_http_check_http_scheme(void); } // namespace shrpx diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc index d9bef5e2..bf7bf114 100644 --- a/src/shrpx_https_upstream.cc +++ b/src/shrpx_https_upstream.cc @@ -433,12 +433,18 @@ int htp_hdrs_completecb(llhttp_t *htp) { downstream->set_request_state(DownstreamState::HEADER_COMPLETE); + auto &resp = downstream->response(); + + if (config->http.require_http_scheme && + !http::check_http_scheme(req.scheme, handler->get_ssl() != nullptr)) { + resp.http_status = 400; + return -1; + } + #ifdef HAVE_MRUBY auto worker = handler->get_worker(); auto mruby_ctx = worker->get_mruby_context(); - auto &resp = downstream->response(); - if (mruby_ctx->run_on_request_proc(downstream) != 0) { resp.http_status = 500; return -1;