Merge pull request #11 from getlantern/fisk/preserve-baked-sni

config: apply default frontingsnis without a country code; preserve baked-in SNI

Author: myleshorton

config.go

@@ -27,11 +27,11 @@ type CA struct {
 
 // Provider is a domain fronting provider (e.g. Akamai, CloudFront).
 type Provider struct {
-	HostAliases         map[string]string    `yaml:"hostaliases"`
-	PassthroughPatterns []string             `yaml:"passthrupatterns"`
-	TestURL             string               `yaml:"testurl"`
-	Masquerades         []*Masquerade        `yaml:"masquerades"`
-	VerifyHostname      *string              `yaml:"verifyhostname"`
+	HostAliases         map[string]string `yaml:"hostaliases"`
+	PassthroughPatterns []string          `yaml:"passthrupatterns"`
+	TestURL             string            `yaml:"testurl"`
+	Masquerades         []*Masquerade     `yaml:"masquerades"`
+	VerifyHostname      *string           `yaml:"verifyhostname"`
 	// Pipeline-emitted YAML keys are lowercase-concatenated, not
 	// snake_case (the upstream generator uses lowercased Go field
 	// names with no yaml tag); the tag here must match the wire
@@ -133,9 +133,11 @@ func (cfg *Config) CertPool() (*x509.CertPool, error) {
 	return pool, nil
 }
 
-// ExpandedProvider returns a copy of the provider with masquerades expanded
-// with SNI based on the country code. Host aliases are lowercased.
-// Passthrough patterns are also lowercased for efficient lookup.
+// ExpandedProvider returns a copy of the provider with each masquerade's SNI
+// resolved: a country-specific or "default" arbitrary SNI if the provider
+// configures one (the "default" strategy applies even with no country code),
+// otherwise the masquerade's baked-in SNI, otherwise empty (SNI omitted). Host
+// aliases and passthrough patterns are lowercased for efficient lookup.
 func ExpandedProvider(p *Provider, countryCode string) *Provider {
 	ep := &Provider{
 		HostAliases:         make(map[string]string, len(p.HostAliases)),
@@ -154,8 +156,13 @@ func ExpandedProvider(p *Provider, countryCode string) *Provider {
 		ep.PassthroughPatterns[i] = strings.ToLower(pt)
 	}
 
+	// Select the SNI strategy: a country-specific entry if one matches, else the
+	// "default" entry. The default applies even when no country code is set, so a
+	// provider's default arbitrary-SNI strategy is active for every client — the
+	// production client passes no country code, and gating "default" behind one
+	// would leave the strategy permanently inert.
 	var sniCfg *SNIConfig
-	if countryCode != "" && p.FrontingSNIs != nil {
+	if p.FrontingSNIs != nil {
 		var ok bool
 		sniCfg, ok = p.FrontingSNIs[countryCode]
 		if !ok {
@@ -164,13 +171,41 @@ func ExpandedProvider(p *Provider, countryCode string) *Provider {
 	}
 
 	for _, m := range p.Masquerades {
-		sni := GenerateSNI(sniCfg, m.IpAddress)
-		ep.Masquerades = append(ep.Masquerades, &Masquerade{
+		// A generated SNI (country-specific or "default" arbitrary-SNI strategy)
+		// takes precedence. Otherwise keep any SNI baked into the masquerade by
+		// the config — this lets a provider whose edges require a specific front
+		// SNI pin one per masquerade without depending on a country code being
+		// set (the production client sets none). Empty stays empty (SNI omitted).
+		sni := m.SNI
+		if g := GenerateSNI(sniCfg, m.IpAddress); g != "" {
+			sni = g
+		}
+		nm := &Masquerade{
 			Domain:         m.Domain,
 			IpAddress:      m.IpAddress,
 			SNI:            sni,
-			VerifyHostname: p.VerifyHostname,
-		})
+			VerifyHostname: m.VerifyHostname,
+		}
+		// Resolve the hostname the edge cert is verified against on the SNI path
+		// (dialFront): a per-masquerade value wins, then the provider default,
+		// and finally the front Domain. Defaulting to Domain matters because the
+		// SNI path otherwise falls back to chain-only verification when no
+		// hostname is set — accepting any cert that chains to a trusted root
+		// (for a single-CA pool like aliyun's GlobalSign R3, any R3-issued cert,
+		// which a network MITM could present). We verify against Domain, NOT the
+		// SNI: the SNI is often a decoy the served cert doesn't cover (akamai
+		// edges send SNI=crunchbase.com but serve their a248.e.akamai.net cert),
+		// whereas the cert IS valid for the front Domain — the same check the
+		// no-SNI path already does.
+		if nm.VerifyHostname == nil {
+			nm.VerifyHostname = p.VerifyHostname
+		}
+		if nm.VerifyHostname == nil && sni != "" && nm.Domain != "" {
+			// Point at the new masquerade's own Domain field rather than a
+			// loop-local copy, avoiding a per-iteration heap allocation.
+			nm.VerifyHostname = &nm.Domain
+		}
+		ep.Masquerades = append(ep.Masquerades, nm)
 	}
 	return ep
 }

config_test.go

@@ -73,6 +73,76 @@ func TestExpandedProvider(t *testing.T) {
 		ep := ExpandedProvider(p, "")
 		assert.Equal(t, "api.cdn.com", ep.HostAliases["api.example.com"])
 	})
+
+	t.Run("default arbitrary SNI applies without a country code", func(t *testing.T) {
+		// The production client passes no country code; a provider's "default"
+		// arbitrary-SNI strategy must still apply (e.g. akamai sending SNI
+		// globally), not be gated behind a country code.
+		dp := &Provider{
+			Masquerades: []*Masquerade{{Domain: "cdn.example.com", IpAddress: "1.2.3.4"}},
+			FrontingSNIs: map[string]*SNIConfig{
+				"default": {UseArbitrarySNIs: true, ArbitrarySNIs: []string{"crunchbase.com"}},
+			},
+		}
+		ep := ExpandedProvider(dp, "")
+		require.Len(t, ep.Masquerades, 1)
+		assert.Equal(t, "crunchbase.com", ep.Masquerades[0].SNI)
+	})
+
+	t.Run("baked-in masquerade SNI is preserved when no SNI is generated", func(t *testing.T) {
+		// A provider whose edges require a specific front SNI can pin one per
+		// masquerade; with no arbitrary-SNI strategy it must survive expansion.
+		bp := &Provider{
+			Masquerades: []*Masquerade{{Domain: "img.alicdn.com", IpAddress: "1.2.3.4", SNI: "www.mobgslb.tbcache.com"}},
+		}
+		ep := ExpandedProvider(bp, "")
+		require.Len(t, ep.Masquerades, 1)
+		assert.Equal(t, "www.mobgslb.tbcache.com", ep.Masquerades[0].SNI)
+	})
+
+	t.Run("VerifyHostname defaults to the front Domain when none is configured", func(t *testing.T) {
+		// Avoids chain-only verification on the SNI path. It defaults to Domain
+		// (the front domain the edge cert is actually for), NOT the SNI, which is
+		// often a decoy the served cert doesn't cover.
+		vp := &Provider{
+			Masquerades: []*Masquerade{{Domain: "img.alicdn.com", IpAddress: "1.2.3.4", SNI: "www.mobgslb.tbcache.com"}},
+		}
+		ep := ExpandedProvider(vp, "")
+		require.Len(t, ep.Masquerades, 1)
+		require.NotNil(t, ep.Masquerades[0].VerifyHostname)
+		assert.Equal(t, "img.alicdn.com", *ep.Masquerades[0].VerifyHostname)
+	})
+
+	t.Run("no SNI leaves VerifyHostname unset (no-SNI path verifies Domain)", func(t *testing.T) {
+		np := &Provider{Masquerades: []*Masquerade{{Domain: "cdn.example.com", IpAddress: "1.2.3.4"}}}
+		ep := ExpandedProvider(np, "")
+		require.Len(t, ep.Masquerades, 1)
+		assert.Empty(t, ep.Masquerades[0].SNI)
+		assert.Nil(t, ep.Masquerades[0].VerifyHostname)
+	})
+
+	t.Run("per-masquerade VerifyHostname wins over the SNI default", func(t *testing.T) {
+		pinned := "pinned.example"
+		pp := &Provider{
+			Masquerades: []*Masquerade{{Domain: "img.alicdn.com", IpAddress: "1.2.3.4", SNI: "www.mobgslb.tbcache.com", VerifyHostname: &pinned}},
+		}
+		ep := ExpandedProvider(pp, "")
+		require.Len(t, ep.Masquerades, 1)
+		require.NotNil(t, ep.Masquerades[0].VerifyHostname)
+		assert.Equal(t, "pinned.example", *ep.Masquerades[0].VerifyHostname)
+	})
+
+	t.Run("generated SNI overrides a baked-in SNI", func(t *testing.T) {
+		op := &Provider{
+			Masquerades: []*Masquerade{{Domain: "cdn.example.com", IpAddress: "1.2.3.4", SNI: "baked.example"}},
+			FrontingSNIs: map[string]*SNIConfig{
+				"default": {UseArbitrarySNIs: true, ArbitrarySNIs: []string{"generated.example"}},
+			},
+		}
+		ep := ExpandedProvider(op, "")
+		require.Len(t, ep.Masquerades, 1)
+		assert.Equal(t, "generated.example", ep.Masquerades[0].SNI)
+	})
 }
 
 func TestParseConfigYAML(t *testing.T) {