Merge branch 'nghttpx-xfp'

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
 -----------------------------------------
 

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 = [

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) {

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=<LIST>
               Append RFC  7239 Forwarded header field  with parameters
               specified in comma delimited list <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;
       }

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";

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<AltSvc> altsvcs;
   std::vector<ErrorPage> 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,

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
+    auto &xfpconf = httpconf.xfp;
-    nva.push_back(http2::make_nv_ls_nocopy("x-forwarded-proto", req.scheme));
+    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);

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: ");
+    auto &xfpconf = httpconf.xfp;
-    assert(!req.scheme.empty());
+    auto xfp = xfpconf.strip_incoming
-    buf->append(req.scheme);
+                   ? nullptr
-    buf->append("\r\n");
+                   : 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) {