diff --git a/doc/sources/nghttpx-howto.rst b/doc/sources/nghttpx-howto.rst index e7549de8..7c56984a 100644 --- a/doc/sources/nghttpx-howto.rst +++ b/doc/sources/nghttpx-howto.rst @@ -435,6 +435,15 @@ such PSK cipher suite with HTTP/2, disable HTTP/2 cipher black list by using :option:`--client-no-http2-cipher-black-list` option. But you should understand its implications. +Migration from nghttpx v1.21.x or earlier +----------------------------------------- + +As of nghttpx v1.22.0, it does not strip and append X-Forwarded-Proto +header field by default. In order to recover the previous behaviour, +that is always strip incoming X-Forwarded-Proto and append its own, +use :option:`--add-x-forwarded-proto` and +:option:`--strip-incoming-x-forwarded-proto` options. + Migration from nghttpx v1.18.x or earlier ----------------------------------------- diff --git a/gennghttpxfun.py b/gennghttpxfun.py index b10d7463..172d9f03 100755 --- a/gennghttpxfun.py +++ b/gennghttpxfun.py @@ -163,6 +163,8 @@ OPTIONS = [ "redirect-https-port", "frontend-max-requests", "single-thread", + "add-x-forwarded-proto", + "strip-incoming-x-forwarded-proto", ] LOGVARS = [ diff --git a/integration-tests/nghttpx_http2_test.go b/integration-tests/nghttpx_http2_test.go index a4c46c8f..11f2b0f3 100644 --- a/integration-tests/nghttpx_http2_test.go +++ b/integration-tests/nghttpx_http2_test.go @@ -35,6 +35,105 @@ func TestH2H1PlainGET(t *testing.T) { } } +// TestH2H1AddXfp tests that server appends :scheme to the existing +// x-forwarded-proto header field. +func TestH2H1AddXfp(t *testing.T) { + st := newServerTester([]string{"--add-x-forwarded-proto"}, t, func(w http.ResponseWriter, r *http.Request) { + xfp := r.Header.Get("X-Forwarded-Proto") + if got, want := xfp, "foo, http"; got != want { + t.Errorf("X-Forwarded-Proto = %q; want %q", got, want) + } + }) + defer st.Close() + + res, err := st.http2(requestParam{ + name: "TestH2H1AddXfp", + header: []hpack.HeaderField{ + pair("x-forwarded-proto", "foo"), + }, + }) + if err != nil { + t.Fatalf("Error st.http2() = %v", err) + } + if got, want := res.status, 200; got != want { + t.Errorf("status = %v; want %v", got, want) + } +} + +// TestH2H1NoAddXfp tests that server does not append :scheme to the +// existing x-forwarded-proto header field. +func TestH2H1NoAddXfp(t *testing.T) { + st := newServerTester(nil, t, func(w http.ResponseWriter, r *http.Request) { + xfp := r.Header.Get("X-Forwarded-Proto") + if got, want := xfp, "foo"; got != want { + t.Errorf("X-Forwarded-Proto = %q; want %q", got, want) + } + }) + defer st.Close() + + res, err := st.http2(requestParam{ + name: "TestH2H1NoAddXfp", + header: []hpack.HeaderField{ + pair("x-forwarded-proto", "foo"), + }, + }) + if err != nil { + t.Fatalf("Error st.http2() = %v", err) + } + if got, want := res.status, 200; got != want { + t.Errorf("status = %v; want %v", got, want) + } +} + +// TestH2H1StripXfp tests that server strips incoming +// x-forwarded-proto header field. +func TestH2H1StripXfp(t *testing.T) { + st := newServerTester([]string{"--add-x-forwarded-proto", "--strip-incoming-x-forwarded-proto"}, t, func(w http.ResponseWriter, r *http.Request) { + xfp := r.Header.Get("X-Forwarded-Proto") + if got, want := xfp, "http"; got != want { + t.Errorf("X-Forwarded-Proto = %q; want %q", got, want) + } + }) + defer st.Close() + + res, err := st.http2(requestParam{ + name: "TestH2H1StripXfp", + header: []hpack.HeaderField{ + pair("x-forwarded-proto", "foo"), + }, + }) + if err != nil { + t.Fatalf("Error st.http2() = %v", err) + } + if got, want := res.status, 200; got != want { + t.Errorf("status = %v; want %v", got, want) + } +} + +// TestH2H1StripNoAddXfp tests that server strips incoming +// x-forwarded-proto header field, and does not add another. +func TestH2H1StripNoAddXfp(t *testing.T) { + st := newServerTester([]string{"--strip-incoming-x-forwarded-proto"}, t, func(w http.ResponseWriter, r *http.Request) { + if got, found := r.Header["X-Forwarded-Proto"]; found { + t.Errorf("X-Forwarded-Proto = %q; want nothing", got) + } + }) + defer st.Close() + + res, err := st.http2(requestParam{ + name: "TestH2H1StripNoAddXfp", + header: []hpack.HeaderField{ + pair("x-forwarded-proto", "foo"), + }, + }) + if err != nil { + t.Fatalf("Error st.http2() = %v", err) + } + if got, want := res.status, 200; got != want { + t.Errorf("status = %v; want %v", got, want) + } +} + // TestH2H1AddXff tests that server generates X-Forwarded-For header // field when forwarding request to backend. func TestH2H1AddXff(t *testing.T) { @@ -741,7 +840,7 @@ func TestH2H1SNI(t *testing.T) { // with http value since :scheme is http, even if the frontend // connection is encrypted. func TestH2H1TLSXfp(t *testing.T) { - st := newServerTesterTLS(nil, t, func(w http.ResponseWriter, r *http.Request) { + st := newServerTesterTLS([]string{"--add-x-forwarded-proto"}, t, func(w http.ResponseWriter, r *http.Request) { if got, want := r.Header.Get("x-forwarded-proto"), "http"; got != want { t.Errorf("x-forwarded-proto: want %v; got %v", want, got) } @@ -1635,7 +1734,7 @@ func TestH2H2NoHostRewrite(t *testing.T) { // with http value since :scheme is http, even if the frontend // connection is encrypted. func TestH2H2TLSXfp(t *testing.T) { - st := newServerTesterTLS([]string{"--http2-bridge"}, t, func(w http.ResponseWriter, r *http.Request) { + st := newServerTesterTLS([]string{"--http2-bridge", "--add-x-forwarded-proto"}, t, func(w http.ResponseWriter, r *http.Request) { if got, want := r.Header.Get("x-forwarded-proto"), "http"; got != want { t.Errorf("x-forwarded-proto: want %v; got %v", want, got) } @@ -1653,6 +1752,105 @@ func TestH2H2TLSXfp(t *testing.T) { } } +// TestH2H2AddXfp tests that server appends :scheme to the existing +// x-forwarded-proto header field. +func TestH2H2AddXfp(t *testing.T) { + st := newServerTesterTLS([]string{"--http2-bridge", "--add-x-forwarded-proto"}, t, func(w http.ResponseWriter, r *http.Request) { + xfp := r.Header.Get("X-Forwarded-Proto") + if got, want := xfp, "foo, http"; got != want { + t.Errorf("X-Forwarded-Proto = %q; want %q", got, want) + } + }) + defer st.Close() + + res, err := st.http2(requestParam{ + name: "TestH2H2AddXfp", + header: []hpack.HeaderField{ + pair("x-forwarded-proto", "foo"), + }, + }) + if err != nil { + t.Fatalf("Error st.http2() = %v", err) + } + if got, want := res.status, 200; got != want { + t.Errorf("status = %v; want %v", got, want) + } +} + +// TestH2H2NoAddXfp tests that server does not append :scheme to the +// existing x-forwarded-proto header field. +func TestH2H2NoAddXfp(t *testing.T) { + st := newServerTesterTLS([]string{"--http2-bridge"}, t, func(w http.ResponseWriter, r *http.Request) { + xfp := r.Header.Get("X-Forwarded-Proto") + if got, want := xfp, "foo"; got != want { + t.Errorf("X-Forwarded-Proto = %q; want %q", got, want) + } + }) + defer st.Close() + + res, err := st.http2(requestParam{ + name: "TestH2H2NoAddXfp", + header: []hpack.HeaderField{ + pair("x-forwarded-proto", "foo"), + }, + }) + if err != nil { + t.Fatalf("Error st.http2() = %v", err) + } + if got, want := res.status, 200; got != want { + t.Errorf("status = %v; want %v", got, want) + } +} + +// TestH2H2StripXfp tests that server strips incoming +// x-forwarded-proto header field. +func TestH2H2StripXfp(t *testing.T) { + st := newServerTesterTLS([]string{"--http2-bridge", "--strip-incoming-x-forwarded-proto", "--add-x-forwarded-proto"}, t, func(w http.ResponseWriter, r *http.Request) { + xfp := r.Header.Get("X-Forwarded-Proto") + if got, want := xfp, "http"; got != want { + t.Errorf("X-Forwarded-Proto = %q; want %q", got, want) + } + }) + defer st.Close() + + res, err := st.http2(requestParam{ + name: "TestH2H2StripXfp", + header: []hpack.HeaderField{ + pair("x-forwarded-proto", "foo"), + }, + }) + if err != nil { + t.Fatalf("Error st.http2() = %v", err) + } + if got, want := res.status, 200; got != want { + t.Errorf("status = %v; want %v", got, want) + } +} + +// TestH2H2StripNoAddXfp tests that server strips incoming +// x-forwarded-proto header field, and does not add another. +func TestH2H2StripNoAddXfp(t *testing.T) { + st := newServerTesterTLS([]string{"--http2-bridge", "--strip-incoming-x-forwarded-proto"}, t, func(w http.ResponseWriter, r *http.Request) { + if got, found := r.Header["X-Forwarded-Proto"]; found { + t.Errorf("X-Forwarded-Proto = %q; want nothing", got) + } + }) + defer st.Close() + + res, err := st.http2(requestParam{ + name: "TestH2H2StripNoAddXfp", + header: []hpack.HeaderField{ + pair("x-forwarded-proto", "foo"), + }, + }) + if err != nil { + t.Fatalf("Error st.http2() = %v", err) + } + if got, want := res.status, 200; got != want { + t.Errorf("status = %v; want %v", got, want) + } +} + // TestH2H2AddXff tests that server generates X-Forwarded-For header // field when forwarding request to backend. func TestH2H2AddXff(t *testing.T) { diff --git a/src/shrpx.cc b/src/shrpx.cc index 5d31b6c6..38594155 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -2466,6 +2466,12 @@ HTTP: --strip-incoming-x-forwarded-for Strip X-Forwarded-For header field from inbound client requests. + --add-x-forwarded-proto + Append X-Forwarded-Proto header field to the backend + request. + --strip-incoming-x-forwarded-proto + Strip X-Forwarded-Proto header field from inbound client + requests. --add-forwarded= Append RFC 7239 Forwarded header field with parameters specified in comma delimited list . The supported @@ -3300,6 +3306,9 @@ int main(int argc, char **argv) { {SHRPX_OPT_FRONTEND_MAX_REQUESTS.c_str(), required_argument, &flag, 155}, {SHRPX_OPT_SINGLE_THREAD.c_str(), no_argument, &flag, 156}, + {SHRPX_OPT_ADD_X_FORWARDED_PROTO.c_str(), no_argument, &flag, 157}, + {SHRPX_OPT_STRIP_INCOMING_X_FORWARDED_PROTO.c_str(), no_argument, &flag, + 158}, {nullptr, 0, nullptr, 0}}; int option_index = 0; @@ -4036,6 +4045,16 @@ int main(int argc, char **argv) { cmdcfgs.emplace_back(SHRPX_OPT_SINGLE_THREAD, StringRef::from_lit("yes")); break; + case 157: + // --add-x-forwarded-proto + cmdcfgs.emplace_back(SHRPX_OPT_ADD_X_FORWARDED_PROTO, + StringRef::from_lit("yes")); + break; + case 158: + // --strip-incoming-x-forwarded-proto + cmdcfgs.emplace_back(SHRPX_OPT_STRIP_INCOMING_X_FORWARDED_PROTO, + StringRef::from_lit("yes")); + break; default: break; } diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index fba0309b..8914a63c 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -1816,6 +1816,11 @@ int option_lookup_token(const char *name, size_t namelen) { return SHRPX_OPTID_TLS_MIN_PROTO_VERSION; } break; + case 'o': + if (util::strieq_l("add-x-forwarded-prot", name, 20)) { + return SHRPX_OPTID_ADD_X_FORWARDED_PROTO; + } + break; case 'r': if (util::strieq_l("tls-ticket-key-ciphe", name, 20)) { return SHRPX_OPTID_TLS_TICKET_KEY_CIPHER; @@ -2048,6 +2053,11 @@ int option_lookup_token(const char *name, size_t namelen) { return SHRPX_OPTID_BACKEND_CONNECTIONS_PER_FRONTEND; } break; + case 'o': + if (util::strieq_l("strip-incoming-x-forwarded-prot", name, 31)) { + return SHRPX_OPTID_STRIP_INCOMING_X_FORWARDED_PROTO; + } + break; } break; case 33: @@ -3356,6 +3366,14 @@ int parse_config(Config *config, int optid, const StringRef &opt, case SHRPX_OPTID_SINGLE_THREAD: config->single_thread = util::strieq_l("yes", optarg); + return 0; + case SHRPX_OPTID_ADD_X_FORWARDED_PROTO: + config->http.xfp.add = util::strieq_l("yes", optarg); + + return 0; + case SHRPX_OPTID_STRIP_INCOMING_X_FORWARDED_PROTO: + config->http.xfp.strip_incoming = 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 bcad5bc8..ba6f0cf2 100644 --- a/src/shrpx_config.h +++ b/src/shrpx_config.h @@ -336,6 +336,10 @@ constexpr auto SHRPX_OPT_REDIRECT_HTTPS_PORT = constexpr auto SHRPX_OPT_FRONTEND_MAX_REQUESTS = StringRef::from_lit("frontend-max-requests"); constexpr auto SHRPX_OPT_SINGLE_THREAD = StringRef::from_lit("single-thread"); +constexpr auto SHRPX_OPT_ADD_X_FORWARDED_PROTO = + StringRef::from_lit("add-x-forwarded-proto"); +constexpr auto SHRPX_OPT_STRIP_INCOMING_X_FORWARDED_PROTO = + StringRef::from_lit("strip-incoming-x-forwarded-proto"); constexpr size_t SHRPX_OBFUSCATED_NODE_LENGTH = 8; @@ -638,6 +642,10 @@ struct HttpConfig { bool add; bool strip_incoming; } xff; + struct { + bool add; + bool strip_incoming; + } xfp; std::vector altsvcs; std::vector error_pages; HeaderRefs add_request_headers; @@ -928,6 +936,7 @@ enum { SHRPX_OPTID_ADD_REQUEST_HEADER, SHRPX_OPTID_ADD_RESPONSE_HEADER, SHRPX_OPTID_ADD_X_FORWARDED_FOR, + SHRPX_OPTID_ADD_X_FORWARDED_PROTO, SHRPX_OPTID_ALTSVC, SHRPX_OPTID_API_MAX_REQUEST_BODY, SHRPX_OPTID_BACKEND, @@ -1045,6 +1054,7 @@ enum { SHRPX_OPTID_STREAM_WRITE_TIMEOUT, SHRPX_OPTID_STRIP_INCOMING_FORWARDED, SHRPX_OPTID_STRIP_INCOMING_X_FORWARDED_FOR, + SHRPX_OPTID_STRIP_INCOMING_X_FORWARDED_PROTO, SHRPX_OPTID_SUBCERT, SHRPX_OPTID_SYSLOG_FACILITY, SHRPX_OPTID_TLS_DYN_REC_IDLE_TIMEOUT, diff --git a/src/shrpx_http2_downstream_connection.cc b/src/shrpx_http2_downstream_connection.cc index 019f3f9d..99856a5c 100644 --- a/src/shrpx_http2_downstream_connection.cc +++ b/src/shrpx_http2_downstream_connection.cc @@ -371,8 +371,24 @@ int Http2DownstreamConnection::push_request_headers() { } if (!config->http2_proxy && req.method != HTTP_CONNECT) { - // We use same protocol with :scheme header field - nva.push_back(http2::make_nv_ls_nocopy("x-forwarded-proto", req.scheme)); + auto &xfpconf = httpconf.xfp; + auto xfp = xfpconf.strip_incoming + ? nullptr + : req.fs.header(http2::HD_X_FORWARDED_PROTO); + + if (xfpconf.add) { + StringRef xfp_value; + // We use same protocol with :scheme header field + if (xfp) { + xfp_value = concat_string_ref(balloc, xfp->value, + StringRef::from_lit(", "), req.scheme); + } else { + xfp_value = req.scheme; + } + nva.push_back(http2::make_nv_ls_nocopy("x-forwarded-proto", xfp_value)); + } else if (xfp) { + nva.push_back(http2::make_nv_ls_nocopy("x-forwarded-proto", xfp->value)); + } } auto via = req.fs.header(http2::HD_VIA); diff --git a/src/shrpx_http_downstream_connection.cc b/src/shrpx_http_downstream_connection.cc index 70460347..aec721be 100644 --- a/src/shrpx_http_downstream_connection.cc +++ b/src/shrpx_http_downstream_connection.cc @@ -630,10 +630,25 @@ int HttpDownstreamConnection::push_request_headers() { buf->append("\r\n"); } if (!config->http2_proxy && !connect_method) { - buf->append("X-Forwarded-Proto: "); - assert(!req.scheme.empty()); - buf->append(req.scheme); - buf->append("\r\n"); + auto &xfpconf = httpconf.xfp; + auto xfp = xfpconf.strip_incoming + ? nullptr + : req.fs.header(http2::HD_X_FORWARDED_PROTO); + + if (xfpconf.add) { + buf->append("X-Forwarded-Proto: "); + if (xfp) { + buf->append((*xfp).value); + buf->append(", "); + } + assert(!req.scheme.empty()); + buf->append(req.scheme); + buf->append("\r\n"); + } else if (xfp) { + buf->append("X-Forwarded-Proto: "); + buf->append((*xfp).value); + buf->append("\r\n"); + } } auto via = req.fs.header(http2::HD_VIA); if (httpconf.no_via) {