Add Forwarded headers to the fetch request. (#382)

To be cautious re: privacy/security, specify only host. Note that this
changes behavior for anybody who was previously including "Forwarded" or
"X-Forwarded-Host" in their ForwardedRequestHeaders in the config.

Author: twifkak

packager/signer/signer.go

@@ -177,6 +177,17 @@ func (this *Signer) fetchURL(fetch *url.URL, serveHTTPReq *http.Request) (*http.
 		}
 		req.Header.Set("Via", via)
 	}
+	if quotedHost, err := util.QuotedString(serveHTTPReq.Host); err == nil {
+		// TODO(twifkak): Extract host from upstream Forwarded header
+		// and concatenate. (Do not include any other parameters, as
+		// they may lead to over-signing.)
+		req.Header.Set("Forwarded", `host=` + quotedHost)
+		xfh := serveHTTPReq.Host
+		if oldXFH := serveHTTPReq.Header.Get("X-Forwarded-Host"); oldXFH != "" {
+			xfh = oldXFH + "," + xfh
+		}
+		req.Header.Set("X-Forwarded-Host", xfh)
+	}
 	// Set conditional headers that were included in ServeHTTP's Request.
 	for header := range util.ConditionalRequestHeaders {
 		if value := GetJoined(serveHTTPReq.Header, header); value != "" {

packager/signer/signer_test.go

@@ -183,6 +183,8 @@ func (this *SignerSuite) TestSimple() {
 	this.Assert().Equal(fakePath, this.lastRequest.URL.String())
 	this.Assert().Equal(userAgent, this.lastRequest.Header.Get("User-Agent"))
 	this.Assert().Equal("1.1 amppkg", this.lastRequest.Header.Get("Via"))
+	this.Assert().Equal(`host="example.com"`, this.lastRequest.Header.Get("Forwarded"))
+	this.Assert().Equal("example.com", this.lastRequest.Header.Get("X-Forwarded-Host"))
 	this.Assert().Equal(http.StatusOK, resp.StatusCode, "incorrect status: %#v", resp)
 	this.Assert().Equal(fmt.Sprintf(`google;v="%d"`, transformer.SupportedVersions[0].Max), resp.Header.Get("AMP-Cache-Transform"))
 	this.Assert().Equal("nosniff", resp.Header.Get("X-Content-Type-Options"))
@@ -272,6 +274,25 @@ func (this *SignerSuite) TestFetchSignWithForwardedRequestHeaders() {
 	this.Assert().Equal(append(payloadPrefix.Bytes(), transformedBody...), exchange.Payload)
 }
 
+func (this *SignerSuite) TestForwardedHost() {
+	urlSets := []util.URLSet{{
+		Sign:  &util.URLPattern{[]string{"https"}, "", this.httpHost(), stringPtr("/amp/.*"), []string{}, stringPtr(""), false, 2000, nil},
+		Fetch: &util.URLPattern{[]string{"http"}, "", this.httpHost(), stringPtr("/amp/.*"), []string{}, stringPtr(""), false, 2000, boolPtr(true)},
+	}}
+	header := http.Header{
+		"AMP-Cache-Transform": {"google"}, "Accept": {"application/signed-exchange;v=" + accept.AcceptedSxgVersion},
+		"Forwarded": {`host="www.example.com";for=192.0.0.1`},
+		"X-Forwarded-For": {"192.0.0.1"},
+		"X-Forwarded-Host": {"www.example.com"}}
+	this.getFRH(this.T(), this.new(urlSets),
+		"/priv/doc?fetch="+url.QueryEscape(this.httpURL()+fakePath)+
+			"&sign="+url.QueryEscape(this.httpSignURL()+fakePath),
+		"example.com", header)
+
+	this.Assert().Equal(`host="example.com"`, this.lastRequest.Header.Get("Forwarded"))
+	this.Assert().Equal("www.example.com,example.com", this.lastRequest.Header.Get("X-Forwarded-Host"))
+}
+
 func (this *SignerSuite) TestEscapeQueryParamsInFetchAndSign() {
 	urlSets := []util.URLSet{{
 		Sign:  &util.URLPattern{[]string{"https"}, "", this.httpHost(), stringPtr("/amp/.*"), []string{}, stringPtr(".*"), false, 2000, nil},

packager/util/http.go

@@ -5,6 +5,8 @@ import (
 	"net/http"
 	"regexp"
 	"strings"
+
+	"github.com/pkg/errors"
 )
 
 // A comma, as defined in https://tools.ietf.org/html/rfc7230#section-7, with
@@ -91,3 +93,26 @@ func haveInvalidForwardedRequestHeader(h string) string {
 	}
 	return ""
 }
+
+// Escapes the input and surrounds it in quotes, so it's a valid quoted-string,
+// per https://tools.ietf.org/html/rfc7230#section-3.2.6. Returns error if the
+// input contains any chars outside of HTAB / SP / VCHAR
+// (https://tools.ietf.org/html/rfc5234#appendix-B.1) and thus isn't even
+// quotable.
+func QuotedString(input string) (string, error) {
+	var ret strings.Builder
+	ret.WriteByte('"')
+	for i := 0; i < len(input); i++ {
+		b := input[i]
+		if (b < 0x20 || b > 0x7e) && b != 0x09 {
+			return "", errors.New("contains non-printable char")
+		}
+		if b == '"' || b == '\\' {
+			ret.Write([]byte{'\\', b})
+		} else {
+			ret.WriteByte(b)
+		}
+	}
+	ret.WriteByte('"')
+	return ret.String(), nil
+}

packager/util/http_test.go

@@ -0,0 +1,16 @@
+package util
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestQuotedString(t *testing.T) {
+	valueFrom := func(s string, _ error) string { return s }
+	errorFrom := func(_ string, err error) error { return err }
+
+	assert.EqualError(t, errorFrom(QuotedString("abc\ndef")), "contains non-printable char")
+	assert.Equal(t, `"abc"`, valueFrom(QuotedString("abc")))
+	assert.Equal(t, `"abc\"\\"`, valueFrom(QuotedString(`abc"\`)))
+}