fix(mtproto): leave clients mtg cannot parse out of the generated config

One mtg-multi process serves every client of an MTProto inbound from a
single TOML file and refuses the whole file over one value it cannot
parse. InstanceFromInbound already guarded the ad tag for that reason
but copied secret, quota and expiry as stored, so a malformed secret, a
year-10000 expiry or a near-MaxInt64 quota on one client took the
sidecar down for everyone on the inbound.

A client whose secret mtg cannot parse is left out with a warning, and
quota and expiry are capped at what mtg accepts.

internal/mtproto/manager.go

@@ -169,8 +169,9 @@ func GetManager() *Manager {
 // InstanceFromInbound derives a desired Instance from an mtproto inbound,
 // building one named secret per active client. Secrets are healed on save (see
 // normalizeMtprotoSecret) and by the migration, so they are read as-is here to
-// keep the fingerprint stable across reconciles. Returns false when the inbound
-// is not a usable mtproto inbound or has no active client secret to serve.
+// keep the fingerprint stable across reconciles; a client whose stored values
+// mtg could not parse is left out (see usableAdTag). Returns false when the
+// inbound is not a usable mtproto inbound or has no active client secret to serve.
 func InstanceFromInbound(ib *model.Inbound) (Instance, bool) {
 	if ib == nil || ib.Protocol != model.MTProto {
 		return Instance{}, false
@@ -205,15 +206,15 @@ func InstanceFromInbound(ib *model.Inbound) (Instance, bool) {
 	}
 	secrets := make([]SecretEntry, 0, len(parsed.Clients))
 	for _, c := range parsed.Clients {
-		if !c.Enable || c.Secret == "" || c.Email == "" {
+		if !c.Enable || c.Email == "" || !model.ValidMtprotoSecret(c.Secret) {
 			continue
 		}
 		entry := SecretEntry{Name: c.Email, Secret: c.Secret, AdTag: usableAdTag(c.AdTag)}
 		if c.TotalGB > 0 {
-			entry.QuotaBytes = c.TotalGB
+			entry.QuotaBytes = min(c.TotalGB, maxQuotaBytes)
 		}
 		if c.ExpiryTime > 0 {
-			entry.ExpiresUnix = c.ExpiryTime / 1000
+			entry.ExpiresUnix = min(c.ExpiryTime/1000, maxExpiresUnix)
 		}
 		secrets = append(secrets, entry)
 	}
@@ -253,6 +254,13 @@ func usableAdTag(tag string) string {
 	return tag
 }
 
+// The largest limits mtg parses: its byte parser overflows just short of
+// MaxInt64 and RFC 3339 ends with the year 9999. Larger stored values are capped.
+const (
+	maxQuotaBytes  = int64(9_000_000_000_000_000_000)
+	maxExpiresUnix = int64(253402300799)
+)
+
 // Ensure starts the mtg process for an instance, or restarts it when its
 // configuration changed. A no-op when the desired process is already running.
 func (m *Manager) Ensure(inst Instance) error {

internal/mtproto/manager_test.go

@@ -76,7 +76,7 @@ func TestInstanceFromInbound(t *testing.T) {
 		t.Fatal("an inbound with no active secret should not produce an instance")
 	}
 
-	badTags := &model.Inbound{Protocol: model.MTProto, Settings: `{"clients":[{"email":"x","secret":"ee00","adTag":"deadbeef","enable":true}]}`}
+	badTags := &model.Inbound{Protocol: model.MTProto, Settings: `{"clients":[{"email":"x","secret":"` + aliceSecret + `","adTag":"deadbeef","enable":true}]}`}
 	badInst, ok := InstanceFromInbound(badTags)
 	if !ok {
 		t.Fatal("expected a usable instance despite a malformed ad tag")
@@ -86,6 +86,42 @@ func TestInstanceFromInbound(t *testing.T) {
 	}
 }
 
+// mtg refuses its whole config over one secret, quota or expiry it cannot
+// parse, which would take every client of the inbound down with the one holding it.
+func TestInstanceFromInboundLeavesOutWhatMtgCannotParse(t *testing.T) {
+	const good = "ee0123456789abcdef0123456789abcdef6578616d706c652e636f6d"
+	inst, ok := InstanceFromInbound(&model.Inbound{Id: 1, Protocol: model.MTProto, Port: 443, Settings: `{"clients":[` +
+		`{"email":"hex","enable":true,"secret":"` + good + `","totalGB":1000,"expiryTime":4102444800000},` +
+		`{"email":"base64","enable":true,"secret":"7gEjRWeJq83vASNFZ4mrze9leGFtcGxlLmNvbQ"},` +
+		`{"email":"truncated","enable":true,"secret":"zz"},` +
+		`{"email":"not-faketls","enable":true,"secret":"dd0123456789abcdef0123456789abcdef6578616d706c652e636f6d"},` +
+		`{"email":"no-host","enable":true,"secret":"ee0123456789abcdef0123456789abcdef"},` +
+		`{"email":"year-10000","enable":true,"secret":"` + good + `","expiryTime":253402300800000},` +
+		`{"email":"huge-quota","enable":true,"secret":"` + good + `","totalGB":9223372036854775806}]}`})
+	if !ok {
+		t.Fatal("expected a usable instance")
+	}
+	got := map[string]SecretEntry{}
+	for _, e := range inst.Secrets {
+		got[e.Name] = e
+	}
+	if len(got) != 4 {
+		t.Fatalf("only the clients mtg can parse should be served, got %+v", inst.Secrets)
+	}
+	if e := got["hex"]; e.Secret != good || e.QuotaBytes != 1000 || e.ExpiresUnix != 4102444800 {
+		t.Fatalf("parsable values must be kept as stored, got %+v", e)
+	}
+	if _, served := got["base64"]; !served {
+		t.Fatal("mtg parses a base64 secret, so its client must be served")
+	}
+	if e := got["year-10000"]; expiresString(e.ExpiresUnix) != "9999-12-31T23:59:59Z" {
+		t.Fatalf("an expiry past RFC 3339 must be capped, got %q", expiresString(e.ExpiresUnix))
+	}
+	if e := got["huge-quota"]; e.QuotaBytes != 9_000_000_000_000_000_000 {
+		t.Fatalf("a quota mtg's parser overflows on must be capped, got %d", e.QuotaBytes)
+	}
+}
+
 func TestRenderConfig(t *testing.T) {
 	// A bare instance emits only the required keys, api-bind-to, and the
 	// [secrets] section, with no optional keys and no [domain-fronting].