Strip default scheme ports from Host headers (#4448)

Fixes #4447.

features/featureflag_string.go

@@ -36,11 +36,12 @@ func _() {
 	_ = x[V1DisableNewValidations-25]
 	_ = x[PrecertificateOCSP-26]
 	_ = x[PrecertificateRevocation-27]
+	_ = x[StripDefaultSchemePort-28]
 }
 
-const _FeatureFlag_name = "unusedPerformValidationRPCACME13KeyRolloverSimplifiedVAHTTPTLSSNIRevalidationAllowRenewalFirstRLSetIssuedNamesRenewalBitFasterRateLimitProbeCTLogsRevokeAtRACAAValidationMethodsCAAAccountURIHeadNonceStatusOKNewAuthorizationSchemaDisableAuthz2OrdersEarlyOrderRateLimitEnforceMultiVAMultiVAFullResultsRemoveWFE2AccountIDCheckRenewalFirstMandatoryPOSTAsGETFasterGetOrderForNamesAllowV1RegistrationParallelCheckFailedValidationDeleteUnusedChallengesV1DisableNewValidationsPrecertificateOCSPPrecertificateRevocation"
+const _FeatureFlag_name = "unusedPerformValidationRPCACME13KeyRolloverSimplifiedVAHTTPTLSSNIRevalidationAllowRenewalFirstRLSetIssuedNamesRenewalBitFasterRateLimitProbeCTLogsRevokeAtRACAAValidationMethodsCAAAccountURIHeadNonceStatusOKNewAuthorizationSchemaDisableAuthz2OrdersEarlyOrderRateLimitEnforceMultiVAMultiVAFullResultsRemoveWFE2AccountIDCheckRenewalFirstMandatoryPOSTAsGETFasterGetOrderForNamesAllowV1RegistrationParallelCheckFailedValidationDeleteUnusedChallengesV1DisableNewValidationsPrecertificateOCSPPrecertificateRevocationStripDefaultSchemePort"
 
-var _FeatureFlag_index = [...]uint16{0, 6, 26, 43, 59, 77, 96, 120, 135, 146, 156, 176, 189, 206, 228, 247, 266, 280, 298, 317, 334, 352, 374, 393, 422, 444, 467, 485, 509}
+var _FeatureFlag_index = [...]uint16{0, 6, 26, 43, 59, 77, 96, 120, 135, 146, 156, 176, 189, 206, 228, 247, 266, 280, 298, 317, 334, 352, 374, 393, 422, 444, 467, 485, 509, 531}
 
 func (i FeatureFlag) String() string {
 	if i < 0 || i >= FeatureFlag(len(_FeatureFlag_index)-1) {

features/features.go

@@ -75,6 +75,9 @@ const (
 	// PrecertificateRevocation allows revocation of precertificates with the
 	// ACMEv2 interface.
 	PrecertificateRevocation
+	// StripDefaultSchemePort enables stripping of default scheme ports from HTTP
+	// request Host headers
+	StripDefaultSchemePort
 )
 
 // List of features and their default value, protected by fMu
@@ -107,6 +110,7 @@ var features = map[FeatureFlag]bool{
 	V1DisableNewValidations:       false,
 	PrecertificateOCSP:            false,
 	PrecertificateRevocation:      false,
+	StripDefaultSchemePort:        false,
 }
 
 var fMu = new(sync.RWMutex)

test/config-next/wfe.json

@@ -46,7 +46,8 @@
       }
     },
     "features": {
-      "NewAuthorizationSchema": true
+      "NewAuthorizationSchema": true,
+      "StripDefaultSchemePort": true
     }
   },
 

test/config-next/wfe2.json

@@ -55,7 +55,8 @@
       "NewAuthorizationSchema": true,
       "RemoveWFE2AccountID": true,
       "MandatoryPOSTAsGET": true,
-      "PrecertificateRevocation": true
+      "PrecertificateRevocation": true,
+      "StripDefaultSchemePort": true
     }
   },
 

web/context.go

@@ -6,8 +6,10 @@ import (
 	"fmt"
 	"net"
 	"net/http"
+	"strings"
 	"time"
 
+	"github.com/letsencrypt/boulder/features"
 	blog "github.com/letsencrypt/boulder/log"
 )
 
@@ -101,6 +103,24 @@ func (th *TopHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		Extra:     make(map[string]interface{}, 0),
 	}
 
+	if features.Enabled(features.StripDefaultSchemePort) {
+		// Some clients will send a HTTP Host header that includes the default port
+		// for the scheme that they are using. Previously when we were fronted by
+		// Akamai they would rewrite the header and strip out the unnecessary port,
+		// now that they are not in our request path we need to strip these ports out
+		// ourselves.
+		//
+		// The main reason we want to strip these ports out is so that when this header
+		// is sent to the /directory endpoint we don't reply with directory URLs that
+		// also contain these ports, which would then in turn end up being sent in the JWS
+		// signature 'url' header, which we don't support.
+		if r.TLS != nil && strings.HasSuffix(r.Host, ":443") {
+			r.Host = strings.TrimSuffix(r.Host, ":443")
+		} else if r.TLS == nil && strings.HasSuffix(r.Host, ":80") {
+			r.Host = strings.TrimSuffix(r.Host, ":80")
+		}
+	}
+
 	begin := time.Now()
 	rwws := &responseWriterWithStatus{w, 0}
 	defer func() {

web/context_test.go

@@ -2,12 +2,16 @@ package web
 
 import (
 	"bytes"
+	"crypto/tls"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"strings"
 	"testing"
 
+	"github.com/letsencrypt/boulder/features"
 	blog "github.com/letsencrypt/boulder/log"
+	"github.com/letsencrypt/boulder/test"
 )
 
 type myHandler struct{}
@@ -70,3 +74,45 @@ func TestOrigin(t *testing.T) {
 			expected, strings.Join(mockLog.GetAllMatching(".*"), "\n"))
 	}
 }
+
+type hostHeaderHandler struct {
+	f func(*RequestEvent, http.ResponseWriter, *http.Request)
+}
+
+func (hhh hostHeaderHandler) ServeHTTP(e *RequestEvent, w http.ResponseWriter, r *http.Request) {
+	hhh.f(e, w, r)
+}
+
+func TestHostHeaderRewrite(t *testing.T) {
+	err := features.Set(map[string]bool{"StripDefaultSchemePort": true})
+	test.AssertNotError(t, err, "features.Set failed")
+	defer features.Reset()
+
+	mockLog := blog.UseMock()
+	hhh := hostHeaderHandler{f: func(_ *RequestEvent, _ http.ResponseWriter, r *http.Request) {
+		t.Helper()
+		test.AssertEquals(t, r.Host, "localhost")
+	}}
+	th := NewTopHandler(mockLog, &hhh)
+
+	req, err := http.NewRequest("GET", "/", &bytes.Reader{})
+	test.AssertNotError(t, err, "http.NewRequest failed")
+	req.Host = "localhost:80"
+	fmt.Println("here")
+	th.ServeHTTP(httptest.NewRecorder(), req)
+
+	req, err = http.NewRequest("GET", "/", &bytes.Reader{})
+	test.AssertNotError(t, err, "http.NewRequest failed")
+	req.Host = "localhost:443"
+	req.TLS = &tls.ConnectionState{}
+	th.ServeHTTP(httptest.NewRecorder(), req)
+
+	hhh.f = func(_ *RequestEvent, _ http.ResponseWriter, r *http.Request) {
+		t.Helper()
+		test.AssertEquals(t, r.Host, "localhost:123")
+	}
+	req, err = http.NewRequest("GET", "/", &bytes.Reader{})
+	test.AssertNotError(t, err, "http.NewRequest failed")
+	req.Host = "localhost:123"
+	th.ServeHTTP(httptest.NewRecorder(), req)
+}