From d63b4c10347e04b885dfd74d3720a823d0da797a Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Tue, 25 Apr 2017 23:41:56 +0900 Subject: [PATCH] nghttpx: Forward multiple via, xff, and xfp header fields Previously, for Via, X-Forwarded-For, and X-Forwarded-Proto header field, nghttpx only forwarded the last header field of each. With this commit, nghttpx forwards all of them if it is configured to do so. --- src/http2.cc | 158 +++++++++++++++++++---- src/http2.h | 34 ++++- src/http2_test.cc | 58 ++++++++- src/shrpx_http2_downstream_connection.cc | 18 ++- src/shrpx_http2_upstream.cc | 8 +- src/shrpx_http_downstream_connection.cc | 19 ++- src/shrpx_https_upstream.cc | 11 +- 7 files changed, 256 insertions(+), 50 deletions(-) diff --git a/src/http2.cc b/src/http2.cc index 04dfb7ad..5d677ade 100644 --- a/src/http2.cc +++ b/src/http2.cc @@ -358,15 +358,21 @@ nghttp2_nv make_nv_nocopy(const StringRef &name, const StringRef &value, namespace { void copy_headers_to_nva_internal(std::vector &nva, - const HeaderRefs &headers, uint8_t nv_flags) { - for (auto &kv : headers) { - if (kv.name.empty() || kv.name[0] == ':') { + const HeaderRefs &headers, uint8_t nv_flags, + uint32_t flags) { + auto it_forwarded = std::end(headers); + auto it_xff = std::end(headers); + auto it_xfp = std::end(headers); + auto it_via = std::end(headers); + + for (auto it = std::begin(headers); it != std::end(headers); ++it) { + auto kv = &(*it); + if (kv->name.empty() || kv->name[0] == ':') { continue; } - switch (kv.token) { + switch (kv->token) { case HD_COOKIE: case HD_CONNECTION: - case HD_FORWARDED: case HD_HOST: case HD_HTTP2_SETTINGS: case HD_KEEP_ALIVE: @@ -375,51 +381,157 @@ void copy_headers_to_nva_internal(std::vector &nva, case HD_TE: case HD_TRANSFER_ENCODING: case HD_UPGRADE: - case HD_VIA: - case HD_X_FORWARDED_FOR: - case HD_X_FORWARDED_PROTO: continue; + case HD_FORWARDED: + if (flags & HDOP_STRIP_FORWARDED) { + continue; + } + + if (it_forwarded == std::end(headers)) { + it_forwarded = it; + continue; + } + + kv = &(*it_forwarded); + it_forwarded = it; + break; + case HD_X_FORWARDED_FOR: + if (flags & HDOP_STRIP_X_FORWARDED_FOR) { + continue; + } + + if (it_xff == std::end(headers)) { + it_xff = it; + continue; + } + + kv = &(*it_xff); + it_xff = it; + break; + case HD_X_FORWARDED_PROTO: + if (flags & HDOP_STRIP_X_FORWARDED_PROTO) { + continue; + } + + if (it_xfp == std::end(headers)) { + it_xfp = it; + continue; + } + + kv = &(*it_xfp); + it_xfp = it; + break; + case HD_VIA: + if (flags & HDOP_STRIP_VIA) { + continue; + } + + if (it_via == std::end(headers)) { + it_via = it; + continue; + } + + kv = &(*it_via); + it_via = it; + break; } - nva.push_back(make_nv_internal(kv.name, kv.value, kv.no_index, nv_flags)); + nva.push_back( + make_nv_internal(kv->name, kv->value, kv->no_index, nv_flags)); } } } // namespace void copy_headers_to_nva(std::vector &nva, - const HeaderRefs &headers) { - copy_headers_to_nva_internal(nva, headers, NGHTTP2_NV_FLAG_NONE); + const HeaderRefs &headers, uint32_t flags) { + copy_headers_to_nva_internal(nva, headers, NGHTTP2_NV_FLAG_NONE, flags); } void copy_headers_to_nva_nocopy(std::vector &nva, - const HeaderRefs &headers) { + const HeaderRefs &headers, uint32_t flags) { copy_headers_to_nva_internal(nva, headers, NGHTTP2_NV_FLAG_NO_COPY_NAME | - NGHTTP2_NV_FLAG_NO_COPY_VALUE); + NGHTTP2_NV_FLAG_NO_COPY_VALUE, + flags); } void build_http1_headers_from_headers(DefaultMemchunks *buf, - const HeaderRefs &headers) { - for (auto &kv : headers) { - if (kv.name.empty() || kv.name[0] == ':') { + const HeaderRefs &headers, + uint32_t flags) { + auto it_forwarded = std::end(headers); + auto it_xff = std::end(headers); + auto it_xfp = std::end(headers); + auto it_via = std::end(headers); + + for (auto it = std::begin(headers); it != std::end(headers); ++it) { + auto kv = &(*it); + if (kv->name.empty() || kv->name[0] == ':') { continue; } - switch (kv.token) { + switch (kv->token) { case HD_CONNECTION: case HD_COOKIE: - case HD_FORWARDED: case HD_HOST: case HD_HTTP2_SETTINGS: case HD_KEEP_ALIVE: case HD_PROXY_CONNECTION: case HD_SERVER: case HD_UPGRADE: - case HD_VIA: - case HD_X_FORWARDED_FOR: - case HD_X_FORWARDED_PROTO: continue; + case HD_FORWARDED: + if (flags & HDOP_STRIP_FORWARDED) { + continue; + } + + if (it_forwarded == std::end(headers)) { + it_forwarded = it; + continue; + } + + kv = &(*it_forwarded); + it_forwarded = it; + break; + case HD_X_FORWARDED_FOR: + if (flags & HDOP_STRIP_X_FORWARDED_FOR) { + continue; + } + + if (it_xff == std::end(headers)) { + it_xff = it; + continue; + } + + kv = &(*it_xff); + it_xff = it; + break; + case HD_X_FORWARDED_PROTO: + if (flags & HDOP_STRIP_X_FORWARDED_PROTO) { + continue; + } + + if (it_xfp == std::end(headers)) { + it_xfp = it; + continue; + } + + kv = &(*it_xfp); + it_xfp = it; + break; + case HD_VIA: + if (flags & HDOP_STRIP_VIA) { + continue; + } + + if (it_via == std::end(headers)) { + it_via = it; + continue; + } + + kv = &(*it_via); + it_via = it; + break; } - capitalize(buf, kv.name); + capitalize(buf, kv->name); buf->append(": "); - buf->append(kv.value); + buf->append(kv->value); buf->append("\r\n"); } } diff --git a/src/http2.h b/src/http2.h index 9859930b..db2983f1 100644 --- a/src/http2.h +++ b/src/http2.h @@ -187,24 +187,50 @@ nghttp2_nv make_nv_ls_nocopy(const char (&name)[N], const StringRef &value) { NGHTTP2_NV_FLAG_NO_COPY_NAME | NGHTTP2_NV_FLAG_NO_COPY_VALUE}; } +enum HeaderBuildOp { + HDOP_NONE, + // Forwarded header fields must be stripped. If this flag is not + // set, all Forwarded header fields other than last one are added. + HDOP_STRIP_FORWARDED = 1, + // X-Forwarded-For header fields must be stripped. If this flag is + // not set, all X-Forwarded-For header fields other than last one + // are added. + HDOP_STRIP_X_FORWARDED_FOR = 1 << 1, + // X-Forwarded-Proto header fields must be stripped. If this flag + // is not set, all X-Forwarded-Proto header fields other than last + // one are added. + HDOP_STRIP_X_FORWARDED_PROTO = 1 << 2, + // Via header fields must be stripped. If this flag is not set, all + // Via header fields other than last one are added. + HDOP_STRIP_VIA = 1 << 3, + // Strip above all header fields. + HDOP_STRIP_ALL = HDOP_STRIP_FORWARDED | HDOP_STRIP_X_FORWARDED_FOR | + HDOP_STRIP_X_FORWARDED_PROTO | HDOP_STRIP_VIA, +}; + // Appends headers in |headers| to |nv|. |headers| must be indexed // before this call (its element's token field is assigned). Certain // headers, including disallowed headers in HTTP/2 spec and headers -// which require special handling (i.e. via), are not copied. +// which require special handling (i.e. via), are not copied. |flags| +// is one or more of HeaderBuildOp flags. They tell function that +// certain header fields should not be added. void copy_headers_to_nva(std::vector &nva, - const HeaderRefs &headers); + const HeaderRefs &headers, uint32_t flags); // Just like copy_headers_to_nva(), but this adds // NGHTTP2_NV_FLAG_NO_COPY_NAME and NGHTTP2_NV_FLAG_NO_COPY_VALUE. void copy_headers_to_nva_nocopy(std::vector &nva, - const HeaderRefs &headers); + const HeaderRefs &headers, uint32_t flags); // Appends HTTP/1.1 style header lines to |buf| from headers in // |headers|. |headers| must be indexed before this call (its // element's token field is assigned). Certain headers, which // requires special handling (i.e. via and cookie), are not appended. +// |flags| is one or more of HeaderBuildOp flags. They tell function +// that certain header fields should not be added. void build_http1_headers_from_headers(DefaultMemchunks *buf, - const HeaderRefs &headers); + const HeaderRefs &headers, + uint32_t flags); // Return positive window_size_increment if WINDOW_UPDATE should be // sent for the stream |stream_id|. If |stream_id| == 0, this function diff --git a/src/http2_test.cc b/src/http2_test.cc index d2be6230..04c26110 100644 --- a/src/http2_test.cc +++ b/src/http2_test.cc @@ -150,11 +150,33 @@ auto headers = HeaderRefs{ {StringRef::from_lit("zulu"), StringRef::from_lit("12")}}; } // namespace +namespace { +auto headers2 = HeaderRefs{ + {StringRef::from_lit("x-forwarded-for"), StringRef::from_lit("xff1"), false, + http2::HD_X_FORWARDED_FOR}, + {StringRef::from_lit("x-forwarded-for"), StringRef::from_lit("xff2"), false, + http2::HD_X_FORWARDED_FOR}, + {StringRef::from_lit("x-forwarded-proto"), StringRef::from_lit("xfp1"), + false, http2::HD_X_FORWARDED_PROTO}, + {StringRef::from_lit("x-forwarded-proto"), StringRef::from_lit("xfp2"), + false, http2::HD_X_FORWARDED_PROTO}, + {StringRef::from_lit("forwarded"), StringRef::from_lit("fwd1"), false, + http2::HD_FORWARDED}, + {StringRef::from_lit("forwarded"), StringRef::from_lit("fwd2"), false, + http2::HD_FORWARDED}, + {StringRef::from_lit("via"), StringRef::from_lit("via1"), false, + http2::HD_VIA}, + {StringRef::from_lit("via"), StringRef::from_lit("via2"), false, + http2::HD_VIA}, +}; +} // namespace + void test_http2_copy_headers_to_nva(void) { auto ans = std::vector{0, 1, 4, 5, 6, 7, 12}; std::vector nva; - http2::copy_headers_to_nva_nocopy(nva, headers); + http2::copy_headers_to_nva_nocopy(nva, headers, + http2::HDOP_STRIP_X_FORWARDED_FOR); CU_ASSERT(7 == nva.size()); for (size_t i = 0; i < ans.size(); ++i) { check_nv(headers[ans[i]], &nva[i]); @@ -169,7 +191,7 @@ void test_http2_copy_headers_to_nva(void) { } nva.clear(); - http2::copy_headers_to_nva(nva, headers); + http2::copy_headers_to_nva(nva, headers, http2::HDOP_STRIP_X_FORWARDED_FOR); CU_ASSERT(7 == nva.size()); for (size_t i = 0; i < ans.size(); ++i) { check_nv(headers[ans[i]], &nva[i]); @@ -180,12 +202,27 @@ void test_http2_copy_headers_to_nva(void) { CU_ASSERT(NGHTTP2_NV_FLAG_NONE == nva[i].flags); } } + + nva.clear(); + + auto ans2 = std::vector{0, 2, 4, 6}; + http2::copy_headers_to_nva(nva, headers2, http2::HDOP_NONE); + CU_ASSERT(ans2.size() == nva.size()); + for (size_t i = 0; i < ans2.size(); ++i) { + check_nv(headers2[ans2[i]], &nva[i]); + } + + nva.clear(); + + http2::copy_headers_to_nva(nva, headers2, http2::HDOP_STRIP_ALL); + CU_ASSERT(nva.empty()); } void test_http2_build_http1_headers_from_headers(void) { MemchunkPool pool; DefaultMemchunks buf(&pool); - http2::build_http1_headers_from_headers(&buf, headers); + http2::build_http1_headers_from_headers(&buf, headers, + http2::HDOP_STRIP_X_FORWARDED_FOR); auto hdrs = std::string(buf.head->pos, buf.head->last); CU_ASSERT("Alpha: 0\r\n" "Bravo: 1\r\n" @@ -196,6 +233,21 @@ void test_http2_build_http1_headers_from_headers(void) { "Te: 8\r\n" "Te: 9\r\n" "Zulu: 12\r\n" == hdrs); + + buf.reset(); + + http2::build_http1_headers_from_headers(&buf, headers2, http2::HDOP_NONE); + hdrs = std::string(buf.head->pos, buf.head->last); + CU_ASSERT("X-Forwarded-For: xff1\r\n" + "X-Forwarded-Proto: xfp1\r\n" + "Forwarded: fwd1\r\n" + "Via: via1\r\n" == hdrs); + + buf.reset(); + + http2::build_http1_headers_from_headers(&buf, headers2, + http2::HDOP_STRIP_ALL); + CU_ASSERT(0 == buf.rleft()); } void test_http2_lws(void) { diff --git a/src/shrpx_http2_downstream_connection.cc b/src/shrpx_http2_downstream_connection.cc index 99856a5c..d2264862 100644 --- a/src/shrpx_http2_downstream_connection.cc +++ b/src/shrpx_http2_downstream_connection.cc @@ -203,7 +203,7 @@ ssize_t http2_data_read_callback(nghttp2_session *session, int32_t stream_id, nva.reserve(trailers.size()); // We cannot use nocopy version, since nva may be touched after // Downstream object is deleted. - http2::copy_headers_to_nva(nva, trailers); + http2::copy_headers_to_nva(nva, trailers, http2::HDOP_STRIP_ALL); if (!nva.empty()) { rv = nghttp2_submit_trailer(session, stream_id, nva.data(), nva.size()); if (rv != 0) { @@ -310,7 +310,16 @@ int Http2DownstreamConnection::push_request_headers() { nva.push_back(http2::make_nv_ls_nocopy(":authority", authority)); } - http2::copy_headers_to_nva_nocopy(nva, req.fs.headers()); + auto &fwdconf = httpconf.forwarded; + auto &xffconf = httpconf.xff; + auto &xfpconf = httpconf.xfp; + + uint32_t build_flags = + (fwdconf.strip_incoming ? http2::HDOP_STRIP_FORWARDED : 0) | + (xffconf.strip_incoming ? http2::HDOP_STRIP_X_FORWARDED_FOR : 0) | + (xfpconf.strip_incoming ? http2::HDOP_STRIP_X_FORWARDED_PROTO : 0); + + http2::copy_headers_to_nva_nocopy(nva, req.fs.headers(), build_flags); if (!http2conf.no_cookie_crumbling) { downstream_->crumble_request_cookie(nva); @@ -319,8 +328,6 @@ int Http2DownstreamConnection::push_request_headers() { auto upstream = downstream_->get_upstream(); auto handler = upstream->get_client_handler(); - auto &fwdconf = httpconf.forwarded; - auto fwd = fwdconf.strip_incoming ? nullptr : req.fs.header(http2::HD_FORWARDED); @@ -351,8 +358,6 @@ int Http2DownstreamConnection::push_request_headers() { nva.push_back(http2::make_nv_ls_nocopy("forwarded", fwd->value)); } - auto &xffconf = httpconf.xff; - auto xff = xffconf.strip_incoming ? nullptr : req.fs.header(http2::HD_X_FORWARDED_FOR); @@ -371,7 +376,6 @@ int Http2DownstreamConnection::push_request_headers() { } if (!config->http2_proxy && req.method != HTTP_CONNECT) { - auto &xfpconf = httpconf.xfp; auto xfp = xfpconf.strip_incoming ? nullptr : req.fs.header(http2::HD_X_FORWARDED_PROTO); diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 83a0ca62..36b975d1 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -1418,7 +1418,7 @@ ssize_t downstream_data_read_callback(nghttp2_session *session, if (!trailers.empty()) { std::vector nva; nva.reserve(trailers.size()); - http2::copy_headers_to_nva_nocopy(nva, trailers); + http2::copy_headers_to_nva_nocopy(nva, trailers, http2::HDOP_STRIP_ALL); if (!nva.empty()) { rv = nghttp2_submit_trailer(session, stream_id, nva.data(), nva.size()); @@ -1667,7 +1667,8 @@ int Http2Upstream::on_downstream_header_complete(Downstream *downstream) { nva.push_back(http2::make_nv_ls_nocopy(":status", response_status)); if (downstream->get_non_final_response()) { - http2::copy_headers_to_nva_nocopy(nva, resp.fs.headers()); + http2::copy_headers_to_nva_nocopy(nva, resp.fs.headers(), + http2::HDOP_STRIP_ALL); if (LOG_ENABLED(INFO)) { log_response_headers(downstream, nva); @@ -1687,7 +1688,8 @@ int Http2Upstream::on_downstream_header_complete(Downstream *downstream) { return 0; } - http2::copy_headers_to_nva_nocopy(nva, resp.fs.headers()); + http2::copy_headers_to_nva_nocopy( + nva, resp.fs.headers(), http2::HDOP_STRIP_ALL & ~http2::HDOP_STRIP_VIA); if (!config->http2_proxy && !httpconf.no_server_rewrite) { nva.push_back(http2::make_nv_ls_nocopy("server", httpconf.server_name)); diff --git a/src/shrpx_http_downstream_connection.cc b/src/shrpx_http_downstream_connection.cc index 8c82b3df..905de09f 100644 --- a/src/shrpx_http_downstream_connection.cc +++ b/src/shrpx_http_downstream_connection.cc @@ -538,7 +538,16 @@ int HttpDownstreamConnection::push_request_headers() { buf->append(authority); buf->append("\r\n"); - http2::build_http1_headers_from_headers(buf, req.fs.headers()); + auto &fwdconf = httpconf.forwarded; + auto &xffconf = httpconf.xff; + auto &xfpconf = httpconf.xfp; + + uint32_t build_flags = + (fwdconf.strip_incoming ? http2::HDOP_STRIP_FORWARDED : 0) | + (xffconf.strip_incoming ? http2::HDOP_STRIP_X_FORWARDED_FOR : 0) | + (xfpconf.strip_incoming ? http2::HDOP_STRIP_X_FORWARDED_PROTO : 0); + + http2::build_http1_headers_from_headers(buf, req.fs.headers(), build_flags); auto cookie = downstream_->assemble_request_cookie(); if (!cookie.empty()) { @@ -577,8 +586,6 @@ int HttpDownstreamConnection::push_request_headers() { auto upstream = downstream_->get_upstream(); auto handler = upstream->get_client_handler(); - auto &fwdconf = httpconf.forwarded; - auto fwd = fwdconf.strip_incoming ? nullptr : req.fs.header(http2::HD_FORWARDED); @@ -611,8 +618,6 @@ int HttpDownstreamConnection::push_request_headers() { buf->append("\r\n"); } - auto &xffconf = httpconf.xff; - auto xff = xffconf.strip_incoming ? nullptr : req.fs.header(http2::HD_X_FORWARDED_FOR); @@ -630,7 +635,6 @@ int HttpDownstreamConnection::push_request_headers() { buf->append("\r\n"); } if (!config->http2_proxy && !connect_method) { - auto &xfpconf = httpconf.xfp; auto xfp = xfpconf.strip_incoming ? nullptr : req.fs.header(http2::HD_X_FORWARDED_PROTO); @@ -740,7 +744,8 @@ int HttpDownstreamConnection::end_upload_data() { output->append("0\r\n\r\n"); } else { output->append("0\r\n"); - http2::build_http1_headers_from_headers(output, trailers); + http2::build_http1_headers_from_headers(output, trailers, + http2::HDOP_STRIP_ALL); output->append("\r\n"); } diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc index 0fca27fa..e5239d7d 100644 --- a/src/shrpx_https_upstream.cc +++ b/src/shrpx_https_upstream.cc @@ -1059,9 +1059,10 @@ int HttpsUpstream::on_downstream_header_complete(Downstream *downstream) { get_client_handler()->get_upstream_scheme()); } - http2::build_http1_headers_from_headers(buf, resp.fs.headers()); - if (downstream->get_non_final_response()) { + http2::build_http1_headers_from_headers(buf, resp.fs.headers(), + http2::HDOP_STRIP_ALL); + buf->append("\r\n"); if (LOG_ENABLED(INFO)) { @@ -1073,6 +1074,9 @@ int HttpsUpstream::on_downstream_header_complete(Downstream *downstream) { return 0; } + http2::build_http1_headers_from_headers( + buf, resp.fs.headers(), http2::HDOP_STRIP_ALL & ~http2::HDOP_STRIP_VIA); + auto worker = handler_->get_worker(); // after graceful shutdown commenced, add connection: close header @@ -1205,7 +1209,8 @@ int HttpsUpstream::on_downstream_body_complete(Downstream *downstream) { output->append("0\r\n\r\n"); } else { output->append("0\r\n"); - http2::build_http1_headers_from_headers(output, trailers); + http2::build_http1_headers_from_headers(output, trailers, + http2::HDOP_STRIP_ALL); output->append("\r\n"); } }