Rod Hynes 1 год назад
Родитель
Сommit
7b008af50f

+ 33 - 12
psiphon/common/parameters/frontingSpec.go

@@ -43,12 +43,16 @@ type FrontingSpecs []*FrontingSpec
 // ServerEntry.MeekFrontingAddresses: multiple candidates are supported, and
 // each candidate may be a regex, or a static value (with regex syntax).
 type FrontingSpec struct {
+
+	// Optional/new fields use omitempty to minimize tactics tag churn.
+
 	FrontingProviderID string
-	Transports         protocol.FrontingTransports
+	Transports         protocol.FrontingTransports `json:",omitempty"`
 	Addresses          []string
-	DisableSNI         bool
-	VerifyServerName   string
-	VerifyPins         []string
+	DisableSNI         bool     `json:",omitempty"`
+	SkipVerify         bool     `json:",omitempty"`
+	VerifyServerName   string   `json:",omitempty"`
+	VerifyPins         []string `json:",omitempty"`
 	Host               string
 }
 
@@ -94,6 +98,11 @@ func (specs FrontingSpecs) SelectParameters() (
 		SNIServerName = ""
 	}
 
+	// When SkipVerify is true, VerifyServerName and VerifyPins must be empty,
+	// as checked in Validate. When dialing in any mode, MeekConn will set
+	// CustomTLSConfig.SkipVerify to true as long as VerifyServerName is "".
+	// So SkipVerify does not need to be explicitly returned.
+
 	return spec.FrontingProviderID,
 		transport,
 		frontingDialAddr,
@@ -105,7 +114,7 @@ func (specs FrontingSpecs) SelectParameters() (
 }
 
 // Validate checks that the JSON values are well-formed.
-func (specs FrontingSpecs) Validate() error {
+func (specs FrontingSpecs) Validate(allowSkipVerify bool) error {
 
 	// An empty FrontingSpecs is allowed as a tactics setting, but
 	// SelectParameters will fail at runtime: code that uses FrontingSpecs must
@@ -128,13 +137,25 @@ func (specs FrontingSpecs) Validate() error {
 				return errors.TraceNew("empty fronting address")
 			}
 		}
-		if len(spec.VerifyServerName) == 0 {
-			return errors.TraceNew("empty verify server name")
-		}
-		// An empty VerifyPins is allowed.
-		for _, pin := range spec.VerifyPins {
-			if len(pin) == 0 {
-				return errors.TraceNew("empty verify pin")
+		if spec.SkipVerify {
+			if !allowSkipVerify {
+				return errors.TraceNew("invalid skip verify")
+			}
+			if len(spec.VerifyServerName) != 0 {
+				return errors.TraceNew("unexpected verify server name")
+			}
+			if len(spec.VerifyPins) != 0 {
+				return errors.TraceNew("unexpected verify pins")
+			}
+		} else {
+			if len(spec.VerifyServerName) == 0 {
+				return errors.TraceNew("empty verify server name")
+			}
+			// An empty VerifyPins is allowed.
+			for _, pin := range spec.VerifyPins {
+				if len(pin) == 0 {
+					return errors.TraceNew("empty verify pin")
+				}
 			}
 		}
 		if len(spec.Host) == 0 {

+ 5 - 1
psiphon/common/parameters/inproxy.go

@@ -52,7 +52,11 @@ func (specs InproxyBrokerSpecsValue) Validate(checkBrokerPublicKeyList *[]string
 		if len(spec.BrokerFrontingSpecs) == 0 {
 			return errors.TraceNew("missing broker fronting spec")
 		}
-		err := spec.BrokerFrontingSpecs.Validate()
+		// Broker fronting specs may specify SkipVerify, since the meek
+		// payload has it's own transport security layer, the Noise sessions.
+		// Broker fronting dials use MeekModeWrappedPlaintextRoundTrip.
+		allowSkipVerify := true
+		err := spec.BrokerFrontingSpecs.Validate(allowSkipVerify)
 		if err != nil {
 			return errors.Trace(err)
 		}

+ 5 - 1
psiphon/common/parameters/parameters.go

@@ -1328,7 +1328,11 @@ func (p *Parameters) Set(
 					return nil, errors.Trace(err)
 				}
 			case FrontingSpecs:
-				err := v.Validate()
+				// By default, FrontingSpecs are not permitted to specify
+				// SkipVerify. This includes the ConjureAPIRegistrarFrontingSpecs
+				// case which uses MeekModePlaintextRoundTrip.
+				allowSkipVerify := false
+				err := v.Validate(allowSkipVerify)
 				if err != nil {
 					if skipOnError {
 						continue

+ 7 - 9
psiphon/inproxy.go

@@ -821,11 +821,9 @@ func MakeInproxyBrokerDialParameters(
 	// At this time, the broker client, the transport is limited to fronted
 	// HTTPS.
 	//
-	// MeekModePlaintextRoundTrip currently disallows HTTP, as it must for
-	// Conjure's request payloads, but the in-proxy broker session payload is
-	// obfuscated. As a future enhancement, allow HTTP for the in-proxy
-	// broker case, skip selecting TLS tactics and select HTTP tactics such
-	// as HTTPTransformerParameters.
+	// As a future enhancement, allow HTTP for the in-proxy broker case, skip
+	// selecting TLS tactics and select HTTP tactics such as
+	// HTTPTransformerParameters.
 
 	if brokerDialParams.BrokerTransport == protocol.FRONTING_TRANSPORT_HTTP {
 		return nil, errors.TraceNew("unsupported fronting transport")
@@ -1007,9 +1005,9 @@ func (brokerDialParams *InproxyBrokerDialParameters) prepareDialConfigs(
 
 	// MeekDialConfig
 	//
-	// The broker round trips use MeekModePlaintextRoundTrip without meek
-	// cookies, so meek obfuscation is not configured. The in-proxy broker
-	// session payloads have their own obfuscation layer.
+	// The broker round trips use MeekModeWrappedPlaintextRoundTrip without
+	// meek cookies, so meek obfuscation is not configured. The in-proxy
+	// broker session payloads have their own obfuscation layer.
 
 	addPsiphonFrontingHeader := false
 	if brokerDialParams.FrontingProviderID != "" {
@@ -1021,7 +1019,7 @@ func (brokerDialParams *InproxyBrokerDialParameters) prepareDialConfigs(
 	}
 
 	brokerDialParams.meekConfig = &MeekConfig{
-		Mode:                     MeekModePlaintextRoundTrip,
+		Mode:                     MeekModeWrappedPlaintextRoundTrip,
 		DiagnosticID:             brokerDialParams.FrontingProviderID,
 		Parameters:               config.GetParameters(),
 		DialAddress:              brokerDialParams.DialAddress,

+ 24 - 12
psiphon/meekConn.go

@@ -66,6 +66,7 @@ const (
 	MeekModeRelay = iota
 	MeekModeObfuscatedRoundTrip
 	MeekModePlaintextRoundTrip
+	MeekModeWrappedPlaintextRoundTrip
 )
 
 // MeekConfig specifies the behavior of a MeekConn.
@@ -99,9 +100,16 @@ type MeekConfig struct {
 	// HTTP level; TLS and server certificate verification is required; the
 	// origin server may be any HTTP(S) server.
 	//
-	// As with the other modes, MeekModePlaintextRoundTrip supports HTTP/2 with
-	// utls, and integration with DialParameters for replay -- which are not
-	// otherwise implemented if using just CustomTLSDialer and net.http.
+	// MeekModeWrappedPlaintextRoundTrip: is equivilent to
+	// MeekModePlaintextRoundTrip, except skipping of server certificate
+	// verification is permitted. In this mode, the caller is asserting that
+	// the HTTP plaintext payload is wrapped in its own transport security
+	// layer.
+	//
+	// As with the other modes, MeekMode[Wrapped]PlaintextRoundTrip supports
+	// HTTP/2 with utls, and integration with DialParameters for replay --
+	// which are not otherwise implemented if using just CustomTLSDialer and
+	// net.http.
 	Mode MeekMode
 
 	// DialAddress is the actual network address to dial to establish a
@@ -148,7 +156,8 @@ type MeekConfig struct {
 	RandomizedTLSProfileSeed *prng.Seed
 
 	// UseObfuscatedSessionTickets indicates whether to use obfuscated session
-	// tickets. Assumes UseHTTPS is true. Ignored for MeekModePlaintextRoundTrip.
+	// tickets. Assumes UseHTTPS is true.
+	// Ignored for MeekMode[Wrapped]PlaintextRoundTrip.
 	UseObfuscatedSessionTickets bool
 
 	// SNIServerName is the value to place in the TLS/QUIC SNI server_name field
@@ -191,8 +200,8 @@ type MeekConfig struct {
 	// ClientTunnelProtocol is the protocol the client is using. It's included in
 	// the meek cookie for optional use by the server, in cases where the server
 	// cannot unambiguously determine the tunnel protocol. ClientTunnelProtocol
-	// is used when selecting tactics targeted at specific protocols. Ignored for
-	// MeekModePlaintextRoundTrip.
+	// is used when selecting tactics targeted at specific protocols.
+	// Ignored for MeekMode[Wrapped]PlaintextRoundTrip.
 	ClientTunnelProtocol string
 
 	// NetworkLatencyMultiplier specifies a custom network latency multiplier to
@@ -200,7 +209,7 @@ type MeekConfig struct {
 	NetworkLatencyMultiplier float64
 
 	// The following values are used to create the obfuscated meek cookie.
-	// Ignored for MeekModePlaintextRoundTrip.
+	// Ignored for MeekMode[Wrapped]PlaintextRoundTrip.
 
 	MeekCookieEncryptionPublicKey string
 	MeekObfuscatedKey             string
@@ -470,6 +479,7 @@ func DialMeek(
 		}
 
 		if meekConfig.Mode != MeekModePlaintextRoundTrip &&
+			meekConfig.Mode != MeekModeWrappedPlaintextRoundTrip &&
 			meekConfig.MeekObfuscatedKey != "" {
 
 			// As the passthrough message is unique and indistinguishable from a normal
@@ -747,12 +757,13 @@ func DialMeek(
 		meek.meekObfuscatorPaddingSeed = meekConfig.MeekObfuscatorPaddingSeed
 		meek.clientTunnelProtocol = meekConfig.ClientTunnelProtocol
 
-	} else if meek.mode == MeekModePlaintextRoundTrip {
+	} else if meek.mode == MeekModePlaintextRoundTrip ||
+		meek.mode == MeekModeWrappedPlaintextRoundTrip {
 
 		// MeekModeRelay and MeekModeObfuscatedRoundTrip set the Host header
-		// implicitly via meek.url; MeekModePlaintextRoundTrip does not use
-		// meek.url; it uses the RoundTrip input request.URL instead. So the
-		// Host header is set to meekConfig.HostHeader explicitly here.
+		// implicitly via meek.url; MeekMode[Wrapped]PlaintextRoundTrip does
+		// not use meek.url; it uses the RoundTrip input request.URL instead.
+		// So the Host header is set to meekConfig.HostHeader explicitly here.
 		meek.additionalHeaders.Add("Host", meekConfig.HostHeader)
 	}
 
@@ -1056,7 +1067,8 @@ func (meek *MeekConn) ObfuscatedRoundTrip(
 // response body is read, idle connections may be left open.
 func (meek *MeekConn) RoundTrip(request *http.Request) (*http.Response, error) {
 
-	if meek.mode != MeekModePlaintextRoundTrip {
+	if meek.mode != MeekModePlaintextRoundTrip &&
+		meek.mode != MeekModeWrappedPlaintextRoundTrip {
 		return nil, errors.TraceNew("operation unsupported")
 	}
 

+ 16 - 4
psiphon/server/server_test.go

@@ -3476,6 +3476,15 @@ func generateInproxyTestConfig(
 	address := net.JoinHostPort(brokerIPAddress, strconv.Itoa(brokerPort))
 	addressRegex := strings.ReplaceAll(address, ".", "\\\\.")
 
+	skipVerify := false
+	verifyServerName := brokerFrontingHostName
+	verifyPins := fmt.Sprintf("[\"%s\"]", brokerVerifyPin)
+	if prng.FlipCoin() {
+		skipVerify = true
+		verifyServerName = ""
+		verifyPins = "[]"
+	}
+
 	brokerSpecsJSONFormat := `
             [{
                 "BrokerPublicKey": "%s",
@@ -3484,8 +3493,9 @@ func generateInproxyTestConfig(
                     "FrontingProviderID": "%s",
                     "Addresses": ["%s"],
                     "DisableSNI": true,
+                    "SkipVerify": %v,
                     "VerifyServerName": "%s",
-                    "VerifyPins": ["%s"],
+                    "VerifyPins": %s,
                     "Host": "%s"
                 }]
             }]
@@ -3497,8 +3507,9 @@ func generateInproxyTestConfig(
 		brokerRootObfuscationSecretStr,
 		brokerFrontingProviderID,
 		addressRegex,
-		brokerFrontingHostName,
-		brokerVerifyPin,
+		skipVerify,
+		verifyServerName,
+		verifyPins,
 		brokerFrontingHostName)
 
 	otherSessionPrivateKey, _ := inproxy.GenerateSessionPrivateKey()
@@ -3511,8 +3522,9 @@ func generateInproxyTestConfig(
 		otherRootObfuscationSecret.String(),
 		prng.HexString(16),
 		prng.HexString(16),
+		false,
 		prng.HexString(16),
-		prng.HexString(16),
+		fmt.Sprintf("[\"%s\"]", prng.HexString(16)),
 		prng.HexString(16))
 
 	var brokerSpecsJSON, proxyBrokerSpecsJSON, clientBrokerSpecsJSON string