Limit signature duration based on max_age_secs.

This uses the new metadata field returned from the transformer library, which is based on the presence of any inline amp-scripts.
ampproject · Sep 17, 2019 · 75cb14d · 75cb14d
1 parent fe15fb8
commit 75cb14d
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 4 deletions.
diff --git a/packager/signer/signer.go b/packager/signer/signer.go
@@ -477,11 +477,17 @@ func (this *Signer) serveSignedExchange(resp http.ResponseWriter, fetchResp *htt
 	if err != nil {
 		util.NewHTTPError(http.StatusInternalServerError, "Error building validity href: ", err).LogAndRespond(resp)
 	}
+	// Expires - Date must be <= 604800 seconds, per
+	// https://tools.ietf.org/html/draft-yasskin-httpbis-origin-signed-exchanges-impl-00#section-3.5.
+	duration := 7*24*time.Hour
+	println("max-age=", metadata.MaxAgeSecs)
+	if maxAge := time.Duration(metadata.MaxAgeSecs) * time.Second; maxAge < duration {
+		duration = maxAge
+	}
+	date := now.Add(-24 * time.Hour)
 	signer := signedexchange.Signer{
-		// Expires - Date must be <= 604800 seconds, per
-		// https://tools.ietf.org/html/draft-yasskin-httpbis-origin-signed-exchanges-impl-00#section-3.5.
-		Date:        now.Add(-24 * time.Hour),
-		Expires:     now.Add(6 * 24 * time.Hour),
+		Date:        date,
+		Expires:     date.Add(duration),
 		Certs:       []*x509.Certificate{cert},
 		CertUrl:     certURL,
 		ValidityUrl: signURL.ResolveReference(validityHRef),

diff --git a/packager/signer/signer_test.go b/packager/signer/signer_test.go
@@ -29,6 +29,7 @@ import (
 	"testing"
 
 	"github.com/WICG/webpackage/go/signedexchange"
+	"github.com/WICG/webpackage/go/signedexchange/structuredheader"
 	"github.com/ampproject/amppackager/packager/accept"
 	"github.com/ampproject/amppackager/packager/mux"
 	"github.com/ampproject/amppackager/packager/rtv"
@@ -199,6 +200,16 @@ func (this *SignerSuite) TestSimple() {
 	certHash, _ := base64.RawURLEncoding.DecodeString(pkgt.CertName)
 	this.Assert().Contains(exchange.SignatureHeaderValue, "cert-sha256=*"+base64.StdEncoding.EncodeToString(certHash[:])+"*")
 	// TODO(twifkak): Control date, and test for expires and sig.
+
+	signatures, err := structuredheader.ParseParameterisedList(exchange.SignatureHeaderValue)
+	this.Require().NoError(err)
+	this.Require().NotEmpty(signatures)
+	date, ok := signatures[0].Params["date"].(int64)
+	this.Require().True(ok)
+	expires, ok := signatures[0].Params["expires"].(int64)
+	this.Require().True(ok)
+	this.Assert().Equal(int64(604800), expires-date)
+
 	// The response header values are untested here, as that is covered by signedexchange tests.
 
 	// For small enough bodies, the only thing that MICE does is add a record size prefix.
@@ -483,6 +494,50 @@ func (this *SignerSuite) TestRemovesHopByHopHeaders() {
 	this.Assert().NotContains(exchange.ResponseHeaders, http.CanonicalHeaderKey("Transfer-Encoding"))
 }
 
+func (this *SignerSuite) TestLimitsDuration() {
+	urlSets := []util.URLSet{{
+		Sign: &util.URLPattern{[]string{"https"}, "", this.httpsHost(), stringPtr("/amp/.*"), []string{}, stringPtr(""), false, 2000, nil}}}
+	this.fakeHandler = func(resp http.ResponseWriter, req *http.Request) {
+		resp.Header().Set("Content-Type", "text/html; charset=utf-8")
+		resp.Write([]byte("<html amp><body><amp-script script max-age=4000>"))
+	}
+	resp := this.get(this.T(), this.new(urlSets), "/priv/doc?sign="+url.QueryEscape(this.httpsURL()+fakePath))
+	this.Assert().Equal(http.StatusOK, resp.StatusCode, "incorrect status: %#v", resp)
+
+	exchange, err := signedexchange.ReadExchange(resp.Body)
+	this.Require().NoError(err)
+	signatures, err := structuredheader.ParseParameterisedList(exchange.SignatureHeaderValue)
+	this.Require().NoError(err)
+	this.Require().NotEmpty(signatures)
+	date, ok := signatures[0].Params["date"].(int64)
+	this.Require().True(ok)
+	expires, ok := signatures[0].Params["expires"].(int64)
+	this.Require().True(ok)
+	this.Assert().Equal(int64(4000), expires-date)
+}
+
+func (this *SignerSuite) TestDoesNotExtendDuration() {
+	urlSets := []util.URLSet{{
+		Sign: &util.URLPattern{[]string{"https"}, "", this.httpsHost(), stringPtr("/amp/.*"), []string{}, stringPtr(""), false, 2000, nil}}}
+	this.fakeHandler = func(resp http.ResponseWriter, req *http.Request) {
+		resp.Header().Set("Content-Type", "text/html; charset=utf-8")
+		resp.Write([]byte("<html amp><body><amp-script script max-age=700000>"))
+	}
+	resp := this.get(this.T(), this.new(urlSets), "/priv/doc?sign="+url.QueryEscape(this.httpsURL()+fakePath))
+	this.Assert().Equal(http.StatusOK, resp.StatusCode, "incorrect status: %#v", resp)
+
+	exchange, err := signedexchange.ReadExchange(resp.Body)
+	this.Require().NoError(err)
+	signatures, err := structuredheader.ParseParameterisedList(exchange.SignatureHeaderValue)
+	this.Require().NoError(err)
+	this.Require().NotEmpty(signatures)
+	date, ok := signatures[0].Params["date"].(int64)
+	this.Require().True(ok)
+	expires, ok := signatures[0].Params["expires"].(int64)
+	this.Require().True(ok)
+	this.Assert().Equal(int64(604800), expires-date)
+}
+
 func (this *SignerSuite) TestErrorNoCache() {
 	urlSets := []util.URLSet{{
 		Fetch: &util.URLPattern{[]string{"http"}, "", this.httpHost(), stringPtr("/amp/.*"), []string{}, stringPtr(""), false, 2000, boolPtr(true)},