diff --git a/internal/database/model/model.go b/internal/database/model/model.go index 06120d9f9..302b0ef6a 100644 --- a/internal/database/model/model.go +++ b/internal/database/model/model.go @@ -4,6 +4,7 @@ package model import ( "bytes" "crypto/rand" + "encoding/base64" "encoding/hex" "encoding/json" "fmt" @@ -707,6 +708,16 @@ func ValidMtprotoAdTag(tag string) bool { return err == nil } +// ValidMtprotoSecret mirrors mtg's secret parser: hex, else unpadded URL-safe +// base64, of the 0xee FakeTLS marker, a 16-byte key and a non-empty host name. +func ValidMtprotoSecret(secret string) bool { + decoded, err := hex.DecodeString(secret) + if err != nil { + decoded, err = base64.RawURLEncoding.DecodeString(secret) + } + return err == nil && len(decoded) > 17 && decoded[0] == 0xee +} + // StripMtprotoInboundSecret removes the vestigial inbound-level `secret` from an // mtproto inbound's settings JSON. MTProto is multi-client: every secret lives on // a client, and mtg's [secrets] config plus every share link read only the diff --git a/internal/database/model/model_mtproto_test.go b/internal/database/model/model_mtproto_test.go index 674ee8373..b4031f683 100644 --- a/internal/database/model/model_mtproto_test.go +++ b/internal/database/model/model_mtproto_test.go @@ -25,6 +25,20 @@ func TestGenerateFakeTLSSecret(t *testing.T) { } } +func TestValidMtprotoSecret(t *testing.T) { + const good = "ee0123456789abcdef0123456789abcdef6578616d706c652e636f6d" + for _, s := range []string{good, strings.ToUpper(good), "ee0123456789abcdef0123456789abcdef61", "7gEjRWeJq83vASNFZ4mrze9leGFtcGxlLmNvbQ"} { + if !ValidMtprotoSecret(s) { + t.Fatalf("mtg parses %q, so it must be valid", s) + } + } + for _, s := range []string{"", "zz", "ee00", "dd0123456789abcdef0123456789abcdef6578616d706c652e636f6d", "ee0123456789abcdef0123456789abcdef", "7gEjRWeJq83vASNFZ4mrze9leGFtcGxlLmNvbQ==", "not a secret"} { + if ValidMtprotoSecret(s) { + t.Fatalf("mtg refuses %q, so it must be invalid", s) + } + } +} + func TestStripMtprotoInboundAdTag(t *testing.T) { in := `{"adTag":"0123456789abcdef0123456789abcdef","clients":[{"email":"a","adTag":"fedcba9876543210fedcba9876543210"}]}` out, changed := StripMtprotoInboundAdTag(in) diff --git a/internal/mtproto/manager.go b/internal/mtproto/manager.go index 44ace5622..fdd19127d 100644 --- a/internal/mtproto/manager.go +++ b/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 { diff --git a/internal/mtproto/manager_test.go b/internal/mtproto/manager_test.go index 079840507..412975aac 100644 --- a/internal/mtproto/manager_test.go +++ b/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].