diff --git a/integration-tests/nghttpx_http2_test.go b/integration-tests/nghttpx_http2_test.go index a33d4c60..431c17ea 100644 --- a/integration-tests/nghttpx_http2_test.go +++ b/integration-tests/nghttpx_http2_test.go @@ -140,7 +140,7 @@ func TestH2H1StripAddXff(t *testing.T) { // Forwarded header field with obfuscated "by" and "for" parameters. func TestH2H1AddForwardedObfuscated(t *testing.T) { st := newServerTester([]string{"--add-forwarded=by,for,host,proto"}, t, func(w http.ResponseWriter, r *http.Request) { - pattern := fmt.Sprintf(`by=_[^;]+;for=_[^;]+;host="127.0.0.1:%v";proto=http`, serverPort) + pattern := fmt.Sprintf(`by=_[^;]+;for=_[^;]+;host="127\.0\.0\.1:%v";proto=http`, serverPort) validFwd := regexp.MustCompile(pattern) got := r.Header.Get("Forwarded") @@ -164,10 +164,11 @@ func TestH2H1AddForwardedObfuscated(t *testing.T) { // TestH2H1AddForwardedByIP tests that server generates Forwarded header // field with IP address in "by" parameter. func TestH2H1AddForwardedByIP(t *testing.T) { - st := newServerTester([]string{"--add-forwarded=by,for,host,proto", "--forwarded-by=ip", "--forwarded-for=_bravo"}, t, func(w http.ResponseWriter, r *http.Request) { - want := fmt.Sprintf(`by="127.0.0.1:%v";for=_bravo;host="127.0.0.1:%v";proto=http`, serverPort, serverPort) - if got := r.Header.Get("Forwarded"); got != want { - t.Errorf("Forwarded = %v; want %v", got, want) + st := newServerTester([]string{"--add-forwarded=by,for", "--forwarded-by=ip"}, t, func(w http.ResponseWriter, r *http.Request) { + pattern := fmt.Sprintf(`by="127\.0\.0\.1:%v";for=_[^;]+`, serverPort) + validFwd := regexp.MustCompile(pattern) + if got := r.Header.Get("Forwarded"); !validFwd.MatchString(got) { + t.Errorf("Forwarded = %v; want pattern %v", got, pattern) } }) defer st.Close() @@ -280,12 +281,14 @@ func TestH2H1StripForwarded(t *testing.T) { } // TestH2H1AddForwardedStatic tests that server generates Forwarded -// header field with the given static obfuscated strings for "by" and -// "for" parameters. +// header field with the given static obfuscated string for "by" +// parameter. func TestH2H1AddForwardedStatic(t *testing.T) { - st := newServerTester([]string{"--add-forwarded=by,for", "--forwarded-by=_alpha", "--forwarded-for=_bravo"}, t, func(w http.ResponseWriter, r *http.Request) { - if got, want := r.Header.Get("Forwarded"), `by=_alpha;for=_bravo`; got != want { - t.Errorf("Forwarded = %v; want %v", got, want) + st := newServerTester([]string{"--add-forwarded=by,for", "--forwarded-by=_alpha"}, t, func(w http.ResponseWriter, r *http.Request) { + pattern := `by=_alpha;for=_[^;]+` + validFwd := regexp.MustCompile(pattern) + if got := r.Header.Get("Forwarded"); !validFwd.MatchString(got) { + t.Errorf("Forwarded = %v; want pattern %v", got, pattern) } }) defer st.Close() @@ -1669,12 +1672,13 @@ func TestH2H2StripAddXff(t *testing.T) { } // TestH2H2AddForwarded tests that server generates Forwarded header -// field using static obfuscated "by" and "for" parameter. +// field using static obfuscated "by" parameter. func TestH2H2AddForwarded(t *testing.T) { - st := newServerTesterTLS([]string{"--http2-bridge", "--add-forwarded=by,for,host,proto", "--forwarded-by=_alpha", "--forwarded-for=_bravo"}, t, func(w http.ResponseWriter, r *http.Request) { - want := fmt.Sprintf(`by=_alpha;for=_bravo;host="127.0.0.1:%v";proto=https`, serverPort) - if got := r.Header.Get("Forwarded"); got != want { - t.Errorf("Forwarded = %v; want %v", got, want) + st := newServerTesterTLS([]string{"--http2-bridge", "--add-forwarded=by,for,host,proto", "--forwarded-by=_alpha"}, t, func(w http.ResponseWriter, r *http.Request) { + pattern := fmt.Sprintf(`by=_alpha;for=_[^;]+;host="127\.0\.0\.1:%v";proto=https`, serverPort) + validFwd := regexp.MustCompile(pattern) + if got := r.Header.Get("Forwarded"); !validFwd.MatchString(got) { + t.Errorf("Forwarded = %v; want pattern %v", got, pattern) } }) defer st.Close() @@ -1692,11 +1696,11 @@ func TestH2H2AddForwarded(t *testing.T) { } // TestH2H2AddForwardedMerge tests that server generates Forwarded -// header field using static obfuscated "by" and "for" parameter, and +// header field using static obfuscated "by" parameter, and // existing Forwarded header field. func TestH2H2AddForwardedMerge(t *testing.T) { - st := newServerTesterTLS([]string{"--http2-bridge", "--add-forwarded=by,for,host,proto", "--forwarded-by=_alpha", "--forwarded-for=_bravo"}, t, func(w http.ResponseWriter, r *http.Request) { - want := fmt.Sprintf(`host=foo, by=_alpha;for=_bravo;host="127.0.0.1:%v";proto=https`, serverPort) + st := newServerTesterTLS([]string{"--http2-bridge", "--add-forwarded=by,host,proto", "--forwarded-by=_alpha"}, t, func(w http.ResponseWriter, r *http.Request) { + want := fmt.Sprintf(`host=foo, by=_alpha;host="127.0.0.1:%v";proto=https`, serverPort) if got := r.Header.Get("Forwarded"); got != want { t.Errorf("Forwarded = %v; want %v", got, want) } @@ -1719,11 +1723,11 @@ func TestH2H2AddForwardedMerge(t *testing.T) { } // TestH2H2AddForwardedStrip tests that server generates Forwarded -// header field using static obfuscated "by" and "for" parameter, and +// header field using static obfuscated "by" parameter, and // existing Forwarded header field stripped. func TestH2H2AddForwardedStrip(t *testing.T) { - st := newServerTesterTLS([]string{"--http2-bridge", "--strip-incoming-forwarded", "--add-forwarded=by,for,host,proto", "--forwarded-by=_alpha", "--forwarded-for=_bravo"}, t, func(w http.ResponseWriter, r *http.Request) { - want := fmt.Sprintf(`by=_alpha;for=_bravo;host="127.0.0.1:%v";proto=https`, serverPort) + st := newServerTesterTLS([]string{"--http2-bridge", "--strip-incoming-forwarded", "--add-forwarded=by,host,proto", "--forwarded-by=_alpha"}, t, func(w http.ResponseWriter, r *http.Request) { + want := fmt.Sprintf(`by=_alpha;host="127.0.0.1:%v";proto=https`, serverPort) if got := r.Header.Get("Forwarded"); got != want { t.Errorf("Forwarded = %v; want %v", got, want) } diff --git a/src/shrpx.cc b/src/shrpx.cc index 29440fb6..4ce54338 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -1596,16 +1596,13 @@ HTTP: consists of character set [A-Za-z0-9._-], as described in RFC 7239. Default: obfuscated - --forwarded-for=(obfuscated|ip|) + --forwarded-for=(obfuscated|ip) Specify the parameter value sent out with "for" parameter of Forwarded header field. If "obfuscated" is given, the string is randomly generated for each client connection. If "ip" is given, the remote client address of the connection, without port number, is sent with - "for" parameter. User can also specify the static - obfuscated string. The limitation is that it must start - with "_", and only consists of character set - [A-Za-z0-9._-], as described in RFC 7239. + "for" parameter. Default: obfuscated --no-via Don't append to Via header field. If Via header field is received, it is left unaltered. diff --git a/src/shrpx_client_handler.cc b/src/shrpx_client_handler.cc index 8ed85c88..ed161db4 100644 --- a/src/shrpx_client_handler.cc +++ b/src/shrpx_client_handler.cc @@ -408,13 +408,9 @@ ClientHandler::ClientHandler(Worker *worker, int fd, SSL *ssl, if ((fwdconf.params & FORWARDED_FOR) && fwdconf.for_node_type == FORWARDED_NODE_OBFUSCATED) { - if (fwdconf.for_obfuscated.empty()) { - forwarded_for_obfuscated_ = "_"; - forwarded_for_obfuscated_ += util::random_alpha_digit( - worker_->get_randgen(), SHRPX_OBFUSCATED_NODE_LENGTH); - } else { - forwarded_for_obfuscated_ = fwdconf.for_obfuscated; - } + forwarded_for_obfuscated_ = "_"; + forwarded_for_obfuscated_ += util::random_alpha_digit( + worker_->get_randgen(), SHRPX_OBFUSCATED_NODE_LENGTH); } } diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index dcc7231e..9401013b 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -2123,7 +2123,8 @@ int parse_config(const char *opt, const char *optarg, case SHRPX_OPTID_FORWARDED_FOR: { auto type = parse_forwarded_node_type(optarg); - if (type == -1) { + if (type == -1 || + (optid == SHRPX_OPTID_FORWARDED_FOR && optarg[0] == '_')) { LOG(ERROR) << opt << ": unknown node type or illegal obfuscated string " << optarg; return -1; @@ -2142,11 +2143,6 @@ int parse_config(const char *opt, const char *optarg, break; case SHRPX_OPTID_FORWARDED_FOR: fwdconf.for_node_type = static_cast(type); - if (optarg[0] == '_') { - fwdconf.for_obfuscated = optarg; - } else { - fwdconf.for_obfuscated = ""; - } break; } diff --git a/src/shrpx_config.h b/src/shrpx_config.h index 41fd1714..339d5a4a 100644 --- a/src/shrpx_config.h +++ b/src/shrpx_config.h @@ -379,12 +379,9 @@ struct TLSConfig { struct HttpConfig { struct { // obfuscated value used in "by" parameter of Forwarded header - // field. - std::string by_obfuscated; - // obfuscated value used in "for" parameter of Forwarded header // field. This is only used when user defined static obfuscated // string is provided. - std::string for_obfuscated; + std::string by_obfuscated; // bitwise-OR of one or more of shrpx_forwarded_param values. uint32_t params; // type of value recorded in "by" parameter of Forwarded header