Fix HTTP/2 support

- Split meek server fronted/direct TLS configuration
- Fronted meek server now supports HTTP/2
- Log meek_server_http_version in server_tunnel
- Add transport log fields (meek_server_http_version) to broker request logs
- Use correct type assertions in psiphon.IsTLSConnUsingHTTP2
- Comment typo

psiphon/common/inproxy/broker.go

@@ -321,6 +321,7 @@ func (b *Broker) SetLimits(
 func (b *Broker) HandleSessionPacket(
 	ctx context.Context,
 	extendTransportTimeout ExtendTransportTimeout,
+	transportLogFields common.LogFields,
 	brokerClientIP string,
 	geoIPData common.GeoIPData,
 	inPacket []byte) ([]byte, error) {
@@ -339,25 +340,48 @@ func (b *Broker) HandleSessionPacket(
 		switch recordType {
 		case recordTypeAPIProxyAnnounceRequest:
 			responsePayload, err = b.handleProxyAnnounce(
-				ctx, extendTransportTimeout, brokerClientIP, geoIPData, initiatorID, unwrappedRequestPayload)
+				ctx,
+				extendTransportTimeout,
+				transportLogFields,
+				brokerClientIP,
+				geoIPData,
+				initiatorID,
+				unwrappedRequestPayload)
 			if err != nil {
 				return nil, errors.Trace(err)
 			}
 		case recordTypeAPIProxyAnswerRequest:
 			responsePayload, err = b.handleProxyAnswer(
-				ctx, extendTransportTimeout, brokerClientIP, geoIPData, initiatorID, unwrappedRequestPayload)
+				ctx,
+				extendTransportTimeout,
+				transportLogFields,
+				brokerClientIP,
+				geoIPData,
+				initiatorID,
+				unwrappedRequestPayload)
 			if err != nil {
 				return nil, errors.Trace(err)
 			}
 		case recordTypeAPIClientOfferRequest:
 			responsePayload, err = b.handleClientOffer(
-				ctx, extendTransportTimeout, brokerClientIP, geoIPData, initiatorID, unwrappedRequestPayload)
+				ctx,
+				extendTransportTimeout,
+				transportLogFields,
+				brokerClientIP,
+				geoIPData,
+				initiatorID,
+				unwrappedRequestPayload)
 			if err != nil {
 				return nil, errors.Trace(err)
 			}
 		case recordTypeAPIClientRelayedPacketRequest:
 			responsePayload, err = b.handleClientRelayedPacket(
-				ctx, extendTransportTimeout, geoIPData, initiatorID, unwrappedRequestPayload)
+				ctx,
+				extendTransportTimeout,
+				transportLogFields,
+				geoIPData,
+				initiatorID,
+				unwrappedRequestPayload)
 			if err != nil {
 				return nil, errors.Trace(err)
 			}
@@ -392,6 +416,7 @@ func (b *Broker) HandleSessionPacket(
 func (b *Broker) handleProxyAnnounce(
 	ctx context.Context,
 	extendTransportTimeout ExtendTransportTimeout,
+	transportLogFields common.LogFields,
 	proxyIP string,
 	geoIPData common.GeoIPData,
 	initiatorID ID,
@@ -459,6 +484,7 @@ func (b *Broker) handleProxyAnnounce(
 		if retErr != nil {
 			logFields["error"] = retErr.Error()
 		}
+		logFields.Add(transportLogFields)
 		b.config.Logger.LogMetric(brokerMetricName, logFields)
 	}()
 
@@ -620,6 +646,7 @@ func (b *Broker) handleProxyAnnounce(
 func (b *Broker) handleClientOffer(
 	ctx context.Context,
 	extendTransportTimeout ExtendTransportTimeout,
+	transportLogFields common.LogFields,
 	clientIP string,
 	geoIPData common.GeoIPData,
 	initiatorID ID,
@@ -677,7 +704,7 @@ func (b *Broker) handleClientOffer(
 		if retErr != nil {
 			logFields["error"] = retErr.Error()
 		}
-
+		logFields.Add(transportLogFields)
 		b.config.Logger.LogMetric(brokerMetricName, logFields)
 	}()
 
@@ -861,6 +888,7 @@ func (b *Broker) handleClientOffer(
 func (b *Broker) handleProxyAnswer(
 	ctx context.Context,
 	extendTransportTimeout ExtendTransportTimeout,
+	transportLogFields common.LogFields,
 	proxyIP string,
 	geoIPData common.GeoIPData,
 	initiatorID ID,
@@ -894,6 +922,7 @@ func (b *Broker) handleProxyAnswer(
 		if retErr != nil {
 			logFields["error"] = retErr.Error()
 		}
+		logFields.Add(transportLogFields)
 		b.config.Logger.LogMetric(brokerMetricName, logFields)
 	}()
 
@@ -965,6 +994,7 @@ func (b *Broker) handleProxyAnswer(
 func (b *Broker) handleClientRelayedPacket(
 	ctx context.Context,
 	extendTransportTimeout ExtendTransportTimeout,
+	transportLogFields common.LogFields,
 	geoIPData common.GeoIPData,
 	initiatorID ID,
 	requestPayload []byte) (retResponse []byte, retErr error) {
@@ -991,6 +1021,7 @@ func (b *Broker) handleClientRelayedPacket(
 		if retErr != nil {
 			logFields["error"] = retErr.Error()
 		}
+		logFields.Add(transportLogFields)
 		b.config.Logger.LogMetric(brokerMetricName, logFields)
 	}()
 

psiphon/common/inproxy/inproxy_test.go

@@ -792,6 +792,7 @@ func runHTTPServer(listener net.Listener, broker *Broker) error {
 		responsePayload, err := broker.HandleSessionPacket(
 			r.Context(),
 			extendTimeout,
+			nil,
 			clientIP,
 			geoIPData,
 			requestPayload)

psiphon/common/inproxy/server.go

@@ -143,7 +143,9 @@ func (s *ServerBrokerSessions) HandlePacket(
 		logFields["inproxy_client_nat_type"] = brokerReport.ClientNATType
 		logFields["inproxy_client_port_mapping_types"] = brokerReport.ClientPortMappingTypes
 
-		// TODO: log IPv4 vs. IPv6 information.
+		// TODO:
+		// - log IPv4 vs. IPv6 information
+		// - relay and log broker transport stats, such as meek HTTP version
 
 		ok := true
 

psiphon/config.go

@@ -631,7 +631,7 @@ type Config struct {
 	// ephemeral key will be generated.
 	InproxyProxySessionPrivateKey string
 
-	// InproxyMaxClients specifies the maximum number of in-rpxoy clients to
+	// InproxyMaxClients specifies the maximum number of in-proxy clients to
 	// be proxied concurrently.
 	InproxyMaxClients int
 

psiphon/server/meek.go

@@ -24,6 +24,7 @@ import (
 	"context"
 	"crypto/rand"
 	"crypto/subtle"
+	"crypto/tls"
 	"encoding/base64"
 	"encoding/hex"
 	"encoding/json"
@@ -40,7 +41,7 @@ import (
 	"sync/atomic"
 	"time"
 
-	tls "github.com/Psiphon-Labs/psiphon-tls"
+	psiphon_tls "github.com/Psiphon-Labs/psiphon-tls"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/errors"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/inproxy"
@@ -119,7 +120,8 @@ type MeekServer struct {
 	skipExtendedTurnAroundThreshold int
 	maxSessionStaleness             time.Duration
 	httpClientIOTimeout             time.Duration
-	tlsConfig                       *tls.Config
+	stdTLSConfig                    *tls.Config
+	psiphonTLSConfig                *psiphon_tls.Config
 	obfuscatorSeedHistory           *obfuscator.SeedHistory
 	clientHandler                   func(clientConn net.Conn, data *additionalTransportData)
 	openConns                       *common.Conns
@@ -248,12 +250,45 @@ func NewMeekServer(
 	}
 
 	if useTLS {
-		tlsConfig, err := meekServer.makeMeekTLSConfig(
-			isFronted, useObfuscatedSessionTickets)
-		if err != nil {
-			return nil, errors.Trace(err)
+
+		// For fronted meek servers, crypto/tls is used to ensure that
+		// net/http.Server.Serve will find *crypto/tls.Conn types, as
+		// required for enabling HTTP/2. The fronted case does not not
+		// support or require the TLS passthrough or obfuscated session
+		// ticket mechanisms, which are implemented in psiphon-tls. HTTP/2 is
+		// preferred for fronted meek servers in order to multiplex many
+		// concurrent requests, either from many tunnel clients or
+		// many/individual in-proxy broker clients, over a single network
+		// connection.
+		//
+		// For direct meek servers, psiphon-tls is used to provide the TLS
+		// passthrough or obfuscated session ticket obfuscation mechanisms.
+		// Direct meek servers do not enable HTTP/1.1 Each individual meek
+		// tunnel client will have its own network connection and each client
+		// has only a single in-flight meek request at a time.
+
+		if isFronted {
+
+			if useObfuscatedSessionTickets {
+				return nil, errors.TraceNew("obfuscated session tickets unsupported")
+			}
+			if meekServer.passthroughAddress != "" {
+				return nil, errors.TraceNew("passthrough unsupported")
+			}
+			tlsConfig, err := meekServer.makeFrontedMeekTLSConfig()
+			if err != nil {
+				return nil, errors.Trace(err)
+			}
+			meekServer.stdTLSConfig = tlsConfig
+		} else {
+
+			tlsConfig, err := meekServer.makeDirectMeekTLSConfig(
+				useObfuscatedSessionTickets)
+			if err != nil {
+				return nil, errors.Trace(err)
+			}
+			meekServer.psiphonTLSConfig = tlsConfig
 		}
-		meekServer.tlsConfig = tlsConfig
 	}
 
 	if useHTTPNormalizer && protocol.TunnelProtocolUsesMeekHTTPNormalizer(listenerTunnelProtocol) {
@@ -404,14 +439,17 @@ func (server *MeekServer) Run() error {
 		},
 	}
 
-	// Note: Serve() will be interrupted by listener.Close() call
-	var err error
-	if server.tlsConfig != nil {
-		httpsServer := HTTPSServer{Server: httpServer}
-		err = httpsServer.ServeTLS(server.listener, server.tlsConfig)
-	} else {
-		err = httpServer.Serve(server.listener)
+	// Note: Serve() will be interrupted by server.listener.Close() call
+	listener := server.listener
+	if server.stdTLSConfig != nil {
+		listener = tls.NewListener(server.listener, server.stdTLSConfig)
+	} else if server.psiphonTLSConfig != nil {
+		listener = psiphon_tls.NewListener(server.listener, server.psiphonTLSConfig)
+
+		// Disable auto HTTP/2
+		httpServer.TLSNextProto = make(map[string]func(*http.Server, *tls.Conn, http.Handler))
 	}
+	err := httpServer.Serve(listener)
 
 	// Can't check for the exact error that Close() will cause in Accept(),
 	// (see: https://code.google.com/p/go/issues/detail?id=4373). So using an
@@ -462,6 +500,7 @@ func (server *MeekServer) ServeHTTP(responseWriter http.ResponseWriter, request
 					"header": header,
 					"value":  requestValue,
 				}).Warning("invalid required meek header")
+
 				common.TerminateHTTPConnection(responseWriter, request)
 				return
 			}
@@ -1094,12 +1133,17 @@ func (server *MeekServer) getSessionOrEndpoint(
 	}
 	cachedResponse := NewCachedResponse(bufferLength, server.bufferPool)
 
+	// The cookie name, Content-Type, and HTTP version of the first request in
+	// the session are recorded for stats. It's possible, but not expected,
+	// that later requests will have different values.
+
 	session := &meekSession{
 		meekProtocolVersion: clientSessionData.MeekProtocolVersion,
 		sessionIDSent:       false,
 		cachedResponse:      cachedResponse,
 		cookieName:          meekCookie.Name,
 		contentType:         request.Header.Get("Content-Type"),
+		httpVersion:         request.Proto,
 	}
 
 	session.touch()
@@ -1412,12 +1456,7 @@ func (server *MeekServer) getMeekCookiePayload(
 	return payload, nil
 }
 
-// makeMeekTLSConfig creates a TLS config for a meek HTTPS listener.
-// Currently, this config is optimized for fronted meek where the nature
-// of the connection is non-circumvention; it's optimized for performance
-// assuming the peer is an uncensored CDN.
-func (server *MeekServer) makeMeekTLSConfig(
-	isFronted bool, useObfuscatedSessionTickets bool) (*tls.Config, error) {
+func (server *MeekServer) getWebServerCertificate() ([]byte, []byte, error) {
 
 	var certificate, privateKey string
 
@@ -1429,10 +1468,22 @@ func (server *MeekServer) makeMeekTLSConfig(
 		var err error
 		certificate, privateKey, _, err = common.GenerateWebServerCertificate(values.GetHostName())
 		if err != nil {
-			return nil, errors.Trace(err)
+			return nil, nil, errors.Trace(err)
 		}
 	}
 
+	return []byte(certificate), []byte(privateKey), nil
+}
+
+// makeFrontedMeekTLSConfig creates a TLS config for a fronted meek HTTPS
+// listener.
+func (server *MeekServer) makeFrontedMeekTLSConfig() (*tls.Config, error) {
+
+	certificate, privateKey, err := server.getWebServerCertificate()
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+
 	tlsCertificate, err := tls.X509KeyPair(
 		[]byte(certificate), []byte(privateKey))
 	if err != nil {
@@ -1444,38 +1495,71 @@ func (server *MeekServer) makeMeekTLSConfig(
 	minVersionCandidates := []uint16{tls.VersionTLS10, tls.VersionTLS11, tls.VersionTLS12}
 	minVersion := minVersionCandidates[prng.Intn(len(minVersionCandidates))]
 
+	// This is a reordering of the supported CipherSuites in golang 1.6[*]. Non-ephemeral key
+	// CipherSuites greatly reduce server load, and we try to select these since the meek
+	// protocol is providing obfuscation, not privacy/integrity (this is provided by the
+	// tunneled SSH), so we don't benefit from the perfect forward secrecy property provided
+	// by ephemeral key CipherSuites.
+	// https://github.com/golang/go/blob/1cb3044c9fcd88e1557eca1bf35845a4108bc1db/src/crypto/tls/cipher_suites.go#L75
+	//
+	// This optimization is applied only when there's a CDN in front of the meek server; in
+	// unfronted cases we prefer a more natural TLS handshake.
+	//
+	// [*] the list has since been updated, removing CipherSuites using RC4 and 3DES.
+	cipherSuites := []uint16{
+		tls.TLS_RSA_WITH_AES_128_GCM_SHA256,
+		tls.TLS_RSA_WITH_AES_256_GCM_SHA384,
+		tls.TLS_RSA_WITH_AES_128_CBC_SHA,
+		tls.TLS_RSA_WITH_AES_256_CBC_SHA,
+		tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
+		tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
+		tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
+		tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
+		tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,
+		tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,
+		tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,
+		tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,
+	}
+
 	config := &tls.Config{
 		Certificates: []tls.Certificate{tlsCertificate},
-		NextProtos:   []string{"http/1.1"},
+		// Offer and prefer "h2" for HTTP/2 support.
+		NextProtos:   []string{"h2", "http/1.1"},
 		MinVersion:   minVersion,
+		CipherSuites: cipherSuites,
 	}
 
-	if isFronted {
-		// This is a reordering of the supported CipherSuites in golang 1.6[*]. Non-ephemeral key
-		// CipherSuites greatly reduce server load, and we try to select these since the meek
-		// protocol is providing obfuscation, not privacy/integrity (this is provided by the
-		// tunneled SSH), so we don't benefit from the perfect forward secrecy property provided
-		// by ephemeral key CipherSuites.
-		// https://github.com/golang/go/blob/1cb3044c9fcd88e1557eca1bf35845a4108bc1db/src/crypto/tls/cipher_suites.go#L75
-		//
-		// This optimization is applied only when there's a CDN in front of the meek server; in
-		// unfronted cases we prefer a more natural TLS handshake.
-		//
-		// [*] the list has since been updated, removing CipherSuites using RC4 and 3DES.
-		config.CipherSuites = []uint16{
-			tls.TLS_RSA_WITH_AES_128_GCM_SHA256,
-			tls.TLS_RSA_WITH_AES_256_GCM_SHA384,
-			tls.TLS_RSA_WITH_AES_128_CBC_SHA,
-			tls.TLS_RSA_WITH_AES_256_CBC_SHA,
-			tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
-			tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
-			tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
-			tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
-			tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,
-			tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,
-			tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,
-			tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,
-		}
+	return config, nil
+}
+
+// makeDirectMeekTLSConfig creates a TLS config for a direct meek HTTPS
+// listener.
+func (server *MeekServer) makeDirectMeekTLSConfig(
+	useObfuscatedSessionTickets bool) (*psiphon_tls.Config, error) {
+
+	certificate, privateKey, err := server.getWebServerCertificate()
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+
+	tlsCertificate, err := psiphon_tls.X509KeyPair(
+		[]byte(certificate), []byte(privateKey))
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+
+	// Vary the minimum version to frustrate scanning/fingerprinting of unfronted servers.
+	// Limitation: like the certificate, this value changes on restart.
+	minVersionCandidates := []uint16{tls.VersionTLS10, tls.VersionTLS11, tls.VersionTLS12}
+	minVersion := minVersionCandidates[prng.Intn(len(minVersionCandidates))]
+
+	config := &psiphon_tls.Config{
+		Certificates: []psiphon_tls.Certificate{tlsCertificate},
+		// Omit "h2", so HTTP/2 is not negotiated. Note that the
+		// negotiated-ALPN extension in the ServerHello is plaintext, even in
+		// TLS 1.3.
+		NextProtos: []string{"http/1.1"},
+		MinVersion: minVersion,
 	}
 
 	if useObfuscatedSessionTickets {
@@ -1820,9 +1904,14 @@ func (server *MeekServer) inproxyBrokerHandler(
 	// many clients, it is expected that CDNs will perform an HTTP/3 request
 	// cancellation in this scenario.
 
+	transportLogFields := common.LogFields{
+		"meek_server_http_version": r.Proto,
+	}
+
 	packet, err = server.inproxyBroker.HandleSessionPacket(
 		r.Context(),
 		extendTimeout,
+		transportLogFields,
 		clientIP,
 		geoIPData,
 		packet)
@@ -1860,6 +1949,7 @@ type meekSession struct {
 	cachedResponse                   *CachedResponse
 	cookieName                       string
 	contentType                      string
+	httpVersion                      string
 }
 
 func (session *meekSession) touch() {
@@ -1930,6 +2020,7 @@ func (session *meekSession) GetMetrics() common.LogFields {
 	logFields["meek_underlying_connection_count"] = atomic.LoadInt64(&session.metricUnderlyingConnCount)
 	logFields["meek_cookie_name"] = session.cookieName
 	logFields["meek_content_type"] = session.contentType
+	logFields["meek_server_http_version"] = session.httpVersion
 	return logFields
 }
 

psiphon/server/server_test.go

@@ -1649,6 +1649,14 @@ func runServer(t *testing.T, runConfig *runServerConfig) {
 		expectQUICVersion = limitQUICVersions[0]
 	}
 	expectDestinationBytesFields := runConfig.doDestinationBytes && !runConfig.doChangeBytesConfig
+	expectMeekHTTPVersion := ""
+	if protocol.TunnelProtocolUsesMeek(runConfig.tunnelProtocol) {
+		if protocol.TunnelProtocolUsesFrontedMeek(runConfig.tunnelProtocol) {
+			expectMeekHTTPVersion = "HTTP/2.0"
+		} else {
+			expectMeekHTTPVersion = "HTTP/1.1"
+		}
+	}
 
 	// The client still reports zero domain_bytes when no port forwards are
 	// allowed (expectTrafficFailure).
@@ -1672,6 +1680,7 @@ func runServer(t *testing.T, runConfig *runServerConfig) {
 			expectQUICVersion,
 			expectDestinationBytesFields,
 			passthroughAddress,
+			expectMeekHTTPVersion,
 			inproxyTestConfig,
 			logFields)
 		if err != nil {
@@ -1840,6 +1849,7 @@ func checkExpectedServerTunnelLogFields(
 	expectQUICVersion string,
 	expectDestinationBytesFields bool,
 	expectPassthroughAddress *string,
+	expectMeekHTTPVersion string,
 	inproxyTestConfig *inproxyTestConfig,
 	fields map[string]interface{}) error {
 
@@ -2035,6 +2045,7 @@ func checkExpectedServerTunnelLogFields(
 			"meek_cookie_size",
 			"meek_limit_request",
 			"meek_underlying_connection_count",
+			"meek_server_http_version",
 			tactics.APPLIED_TACTICS_TAG_PARAMETER_NAME,
 		} {
 			if fields[name] == nil || fmt.Sprintf("%s", fields[name]) == "" {
@@ -2045,6 +2056,10 @@ func checkExpectedServerTunnelLogFields(
 		if !common.Contains(testUserAgents, fields["user_agent"].(string)) {
 			return fmt.Errorf("unexpected user_agent '%s'", fields["user_agent"])
 		}
+
+		if fields["meek_server_http_version"].(string) != expectMeekHTTPVersion {
+			return fmt.Errorf("unexpected meek_server_http_version '%s'", fields["meek_server_http_version"])
+		}
 	}
 
 	if protocol.TunnelProtocolUsesMeekHTTP(runConfig.tunnelProtocol) {

psiphon/tlsDialer.go

@@ -723,10 +723,12 @@ func verifyCertificatePins(pins []string, verifiedChains [][]*x509.Certificate)
 }
 
 func IsTLSConnUsingHTTP2(conn net.Conn) bool {
-	if c, ok := conn.(*utls.UConn); ok {
-		state := c.ConnectionState()
-		return state.NegotiatedProtocolIsMutual &&
-			state.NegotiatedProtocol == "h2"
+	if t, ok := conn.(*tlsConn); ok {
+		if u, ok := t.Conn.(*utls.UConn); ok {
+			state := u.ConnectionState()
+			return state.NegotiatedProtocolIsMutual &&
+				state.NegotiatedProtocol == "h2"
+		}
 	}
 	return false
 }