From 8875cd33fde5e6384dd4f0966e4392a753bffde6 Mon Sep 17 00:00:00 2001 From: onysd Date: Sat, 18 Jul 2026 10:33:17 +0300 Subject: [PATCH] fixes --- cmd/telesrv/main.go | 1 + deploy/deploy_test.go | 25 +++++++++++++ ...4_owpengram_system_account_brand.down.sql} | 0 ...024_owpengram_system_account_brand.up.sql} | 0 ...957_widen_phone_for_email_signup.down.sql} | 0 ...00957_widen_phone_for_email_signup.up.sql} | 0 ...0260714003052_users_signup_email.down.sql} | 0 ... 20260714003052_users_signup_email.up.sql} | 0 deploy/migrations/README.md | 33 +++++++++++++++++ internal/config/config.go | 8 +++++ internal/domain/emailphone.go | 2 +- internal/mtprotoedge/exchange_compat.go | 17 ++++++++- internal/mtprotoedge/exchange_test.go | 36 +++++++++++++++++-- internal/mtprotoedge/server.go | 18 ++++++++++ 14 files changed, 136 insertions(+), 4 deletions(-) create mode 100644 deploy/deploy_test.go rename deploy/migrations/{0086_owpengram_system_account_brand.down.sql => 20260713103024_owpengram_system_account_brand.down.sql} (100%) rename deploy/migrations/{0086_owpengram_system_account_brand.up.sql => 20260713103024_owpengram_system_account_brand.up.sql} (100%) rename deploy/migrations/{0087_widen_phone_for_email_signup.down.sql => 20260713200957_widen_phone_for_email_signup.down.sql} (100%) rename deploy/migrations/{0087_widen_phone_for_email_signup.up.sql => 20260713200957_widen_phone_for_email_signup.up.sql} (100%) rename deploy/migrations/{0088_users_signup_email.down.sql => 20260714003052_users_signup_email.down.sql} (100%) rename deploy/migrations/{0088_users_signup_email.up.sql => 20260714003052_users_signup_email.up.sql} (100%) create mode 100644 deploy/migrations/README.md diff --git a/cmd/telesrv/main.go b/cmd/telesrv/main.go index e00314d7..cc13aba3 100644 --- a/cmd/telesrv/main.go +++ b/cmd/telesrv/main.go @@ -897,6 +897,7 @@ func run(logger *zap.Logger) error { srv := mtprotoedge.New(mtprotoedge.Options{ Logger: logger.Named("mtprotoedge"), DC: cfg.DC, + StrictDC: cfg.StrictDCCheck, RSAKey: rsaKey, LayerRPC: router, AuthKeys: authKeyStore, diff --git a/deploy/deploy_test.go b/deploy/deploy_test.go new file mode 100644 index 00000000..0a741c6e --- /dev/null +++ b/deploy/deploy_test.go @@ -0,0 +1,25 @@ +package deploy + +import ( + "testing" + + "github.com/golang-migrate/migrate/v4/source/iofs" +) + +// TestMigrationsLoad asserts the embedded migrations directory parses +// cleanly with no database connection involved. Two independently-added +// migrations landing on the same version (e.g. an upstream sync and a local +// feature branch both picking "next free sequential number" at merge-base +// time) don't show up as a git conflict — they're different files — so iofs +// only catches it at load time, and previously that meant "at server +// startup," crashing telesrv with "postgres migrate: iofs source: failed to +// init driver ... duplicate migration file." This is why our own migrations +// use a YYYYMMDDHHMMSS timestamp version instead of continuing upstream's +// plain sequential numbering (see deploy/migrations/README.md): a 14-digit +// timestamp never collides with upstream's numbers and always sorts after +// them, no renumbering needed on the next sync. +func TestMigrationsLoad(t *testing.T) { + if _, err := iofs.New(Migrations, "migrations"); err != nil { + t.Fatalf("iofs.New(migrations): %v", err) + } +} diff --git a/deploy/migrations/0086_owpengram_system_account_brand.down.sql b/deploy/migrations/20260713103024_owpengram_system_account_brand.down.sql similarity index 100% rename from deploy/migrations/0086_owpengram_system_account_brand.down.sql rename to deploy/migrations/20260713103024_owpengram_system_account_brand.down.sql diff --git a/deploy/migrations/0086_owpengram_system_account_brand.up.sql b/deploy/migrations/20260713103024_owpengram_system_account_brand.up.sql similarity index 100% rename from deploy/migrations/0086_owpengram_system_account_brand.up.sql rename to deploy/migrations/20260713103024_owpengram_system_account_brand.up.sql diff --git a/deploy/migrations/0087_widen_phone_for_email_signup.down.sql b/deploy/migrations/20260713200957_widen_phone_for_email_signup.down.sql similarity index 100% rename from deploy/migrations/0087_widen_phone_for_email_signup.down.sql rename to deploy/migrations/20260713200957_widen_phone_for_email_signup.down.sql diff --git a/deploy/migrations/0087_widen_phone_for_email_signup.up.sql b/deploy/migrations/20260713200957_widen_phone_for_email_signup.up.sql similarity index 100% rename from deploy/migrations/0087_widen_phone_for_email_signup.up.sql rename to deploy/migrations/20260713200957_widen_phone_for_email_signup.up.sql diff --git a/deploy/migrations/0088_users_signup_email.down.sql b/deploy/migrations/20260714003052_users_signup_email.down.sql similarity index 100% rename from deploy/migrations/0088_users_signup_email.down.sql rename to deploy/migrations/20260714003052_users_signup_email.down.sql diff --git a/deploy/migrations/0088_users_signup_email.up.sql b/deploy/migrations/20260714003052_users_signup_email.up.sql similarity index 100% rename from deploy/migrations/0088_users_signup_email.up.sql rename to deploy/migrations/20260714003052_users_signup_email.up.sql diff --git a/deploy/migrations/README.md b/deploy/migrations/README.md new file mode 100644 index 00000000..7f0125fd --- /dev/null +++ b/deploy/migrations/README.md @@ -0,0 +1,33 @@ +# Migration numbering + +Upstream (`iamxvbaba/gramsrv`) numbers its migrations sequentially +(`0001`, `0002`, ..., currently up to `0092`). This fork periodically merges +upstream, and upstream keeps adding its own sequentially-numbered migrations +between syncs. + +**Any new OwpenGram-specific migration must use a `YYYYMMDDHHMMSS` timestamp +as its version instead of continuing the sequential numbering** — e.g. +`20260713103024_owpengram_system_account_brand.up.sql`. + +Why: golang-migrate treats the version as a plain number and only cares +about ordering, not format. If we keep picking "the next free sequential +number" for our own migrations, the next upstream sync is very likely to +have independently picked the *same* number for one of its own new +migrations — two different new files, so git sees no conflict, and the +collision only surfaces at server startup (`iofs source: failed to init +driver ... duplicate migration file`, see `deploy/deploy_test.go`'s +`TestMigrationsLoad` for the regression test that now catches this at +`go test` time instead). This happened once already (our `0086`-`0088` +collided with upstream's own new `0086`-`0088`), and renumbering our +migrations to fit after upstream's (`0093`-`0095`) would only have deferred +the exact same problem to the *next* sync. + +A 14-digit timestamp is always numerically larger than upstream's 4-5 digit +sequential numbers, so it always sorts after everything upstream has today +or will plausibly reach, and it never collides with another OwpenGram +migration as long as two aren't created in the same second. No renumbering +is ever needed again on future syncs — just keep using the timestamp at the +time the migration is written. + +Upstream's own migrations keep their original sequential numbers untouched; +only migrations authored in this fork use the timestamp scheme. diff --git a/internal/config/config.go b/internal/config/config.go index 44d5c766..7428a0ab 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -30,6 +30,13 @@ type Config struct { RSAKeyPath string // DC 是本 server 的 DC ID。 DC int + // StrictDCCheck turns on exact DC-ID validation for the permanent-key + // exchange (default off = lenient). See mtprotoedge.Options.StrictDC doc + // for the full rationale: telesrv is always a single physical backend, + // but the OwpenGram client forks intentionally alias dc_id 1..5 to it, + // so a mismatched client-chosen dc_id is expected, not an attack — strict + // mode exists only for a hypothetical future real multi-DC deployment. + StrictDCCheck bool // MTProtoMaxConnections / PerIP 覆盖 raw Accept、codec sniff、握手到认证 session // 的完整物理连接生命周期；负数关闭对应 admission 上限。 MTProtoMaxConnections int @@ -444,6 +451,7 @@ func Load() (Config, error) { AdvertiseIP: envOr("TELESRV_ADVERTISE_IP", "127.0.0.1"), RSAKeyPath: envOr("TELESRV_RSA_KEY", "data/server_rsa.pem"), DC: envIntOr("TELESRV_DC", 2), + StrictDCCheck: envBoolOr("TELESRV_STRICT_DC_CHECK", false), MTProtoMaxConnections: envIntOr("TELESRV_MTPROTO_MAX_CONNECTIONS", 200000), MTProtoMaxConnectionsPerIP: envIntOr("TELESRV_MTPROTO_MAX_CONNECTIONS_PER_IP", 4096), MTProtoMaxConcurrentHandshakes: envIntOr("TELESRV_MTPROTO_MAX_CONCURRENT_HANDSHAKES", 256), diff --git a/internal/domain/emailphone.go b/internal/domain/emailphone.go index 0b6373a3..ef1692ae 100644 --- a/internal/domain/emailphone.go +++ b/internal/domain/emailphone.go @@ -15,7 +15,7 @@ import ( const EmailPhonePrefix = "888" // MaxEmailSignupPhoneLen mirrors the users.phone column width (see migration -// 0087) and ValidPhone's upper bound. +// widen_phone_for_email_signup) and ValidPhone's upper bound. const MaxEmailSignupPhoneLen = 200 // emailPhoneEscape marks a 2-character escape sequence standing in for one diff --git a/internal/mtprotoedge/exchange_compat.go b/internal/mtprotoedge/exchange_compat.go index 8856ab58..7f0f1051 100644 --- a/internal/mtprotoedge/exchange_compat.go +++ b/internal/mtprotoedge/exchange_compat.go @@ -38,6 +38,7 @@ func (s *Server) runServerExchange(ctx context.Context, conn transport.Conn) (ex timeout: exchange.DefaultTimeout, key: s.key, dc: s.dc, + strictDC: s.strictDC, log: s.log.Named("exchange"), rng: compatServerRNG{rand: s.rand}, commitKey: s.commitExchangeAuthKey, @@ -65,6 +66,7 @@ type serverExchangeCompat struct { timeout time.Duration key exchange.PrivateKey dc int + strictDC bool log *zap.Logger rng compatServerRNG commitKey func(context.Context, exchange.ServerExchangeResult, int) error @@ -345,10 +347,23 @@ func (s serverExchangeCompat) validatePQInnerDataDC(d mt.PQInnerDataClass) error switch innerDataDC := d.(type) { case *mt.PQInnerDataDC: if innerDataDC.DC != s.dc { + if !s.strictDC { + // Lenient by default (Options.StrictDC doc has the full + // rationale): telesrv is a single physical backend, and the + // OwpenGram client forks deliberately alias dc_id 1..5 to this + // one server, so a client-chosen dc_id that isn't our + // configured DC is expected, not an error. dc_id plays no + // role in key derivation, so accepting it doesn't weaken the + // exchange. + s.log.Debug("Accepted permanent auth key DC mismatch (lenient mode)", + zap.Int("server_dc", s.dc), + zap.Int("client_dc", innerDataDC.DC)) + return nil + } return wrongDCError(s.dc, innerDataDC.DC) } case *mt.PQInnerDataTempDC: - if !sameDCByAbs(innerDataDC.DC, s.dc) { + if !sameDCByAbs(innerDataDC.DC, s.dc) && s.strictDC { return wrongDCError(s.dc, innerDataDC.DC) } if innerDataDC.DC < 0 { diff --git a/internal/mtprotoedge/exchange_test.go b/internal/mtprotoedge/exchange_test.go index a61d6e6f..19aa17f3 100644 --- a/internal/mtprotoedge/exchange_test.go +++ b/internal/mtprotoedge/exchange_test.go @@ -366,8 +366,8 @@ func TestKeyExchangeAcceptsAndroidMediaTempNegativeDC(t *testing.T) { } } -func TestKeyExchangeRejectsWrongNegativeTempDC(t *testing.T) { - ex := serverExchangeCompat{dc: 2, log: zaptest.NewLogger(t)} +func TestKeyExchangeRejectsWrongNegativeTempDCWhenStrict(t *testing.T) { + ex := serverExchangeCompat{dc: 2, strictDC: true, log: zaptest.NewLogger(t)} err := ex.validatePQInnerDataDC(&mt.PQInnerDataTempDC{DC: -3}) var exErr *exchange.ServerExchangeError if !errors.As(err, &exErr) { @@ -378,6 +378,38 @@ func TestKeyExchangeRejectsWrongNegativeTempDC(t *testing.T) { } } +// TestKeyExchangeAcceptsMismatchedDCByDefault asserts that, in the default +// lenient mode, neither permanent nor temp key exchange requires dc_id to +// equal the server's configured DC. telesrv is always a single physical +// backend; the OwpenGram client forks deliberately alias dc_id 1..5 to it +// (see Options.StrictDC doc), so a mismatched client-chosen dc_id must not be +// rejected — doing so previously broke every account whose client picked a +// starting dc_id other than the server's. +func TestKeyExchangeAcceptsMismatchedDCByDefault(t *testing.T) { + ex := serverExchangeCompat{dc: 2, log: zaptest.NewLogger(t)} + if err := ex.validatePQInnerDataDC(&mt.PQInnerDataDC{DC: 3}); err != nil { + t.Fatalf("permanent DC mismatch: err = %v, want nil (lenient by default)", err) + } + if err := ex.validatePQInnerDataDC(&mt.PQInnerDataTempDC{DC: -3}); err != nil { + t.Fatalf("temp DC mismatch: err = %v, want nil (lenient by default)", err) + } +} + +// TestKeyExchangeRejectsMismatchedPermanentDCWhenStrict asserts that +// strictDC=true still enforces exact DC-ID equality for permanent-key +// exchange (kept for a hypothetical future real multi-DC deployment). +func TestKeyExchangeRejectsMismatchedPermanentDCWhenStrict(t *testing.T) { + ex := serverExchangeCompat{dc: 2, strictDC: true, log: zaptest.NewLogger(t)} + err := ex.validatePQInnerDataDC(&mt.PQInnerDataDC{DC: 3}) + var exErr *exchange.ServerExchangeError + if !errors.As(err, &exErr) { + t.Fatalf("err = %T %v, want ServerExchangeError", err, err) + } + if exErr.Code != codec.CodeWrongDC { + t.Fatalf("error code = %d, want %d", exErr.Code, codec.CodeWrongDC) + } +} + func TestDecodeCompatPQInnerDataTemp(t *testing.T) { want := mt.PQInnerData{ Pq: []byte{0x0f}, diff --git a/internal/mtprotoedge/server.go b/internal/mtprotoedge/server.go index d3665d6b..6358124e 100644 --- a/internal/mtprotoedge/server.go +++ b/internal/mtprotoedge/server.go @@ -301,6 +301,22 @@ type Options struct { // DC 是本 server 的 DC ID。默认 2。 DC int + // StrictDC turns on exact DC-ID validation for the permanent-key exchange + // (default off = lenient). telesrv is always a single physical backend — + // there is no real multi-DC federation behind it — but the OwpenGram + // client forks intentionally run in "single-server backend" mode, where + // dc_id 1..5 all alias to this one server (see owpengram_servers.cpp / + // ApplyServerToDcOptions in the desktop client) so that any old data + // referencing a specific dc_id still resolves correctly. When tdesktop + // adds a new local account it picks its own starting dc_id (its usual + // multi-DC load-spreading behavior, unrelated to which physical server + // it's actually talking to) — that choice is not guaranteed to equal our + // configured DC. Strict validation would reject those accounts with + // "-444 wrong dc_id" even though they are connecting to the right (and + // only) server; dc_id is a client-side routing label here, not part of + // key derivation, so accepting the mismatch does not weaken the exchange. + // The switch exists for a hypothetical future real multi-DC deployment. + StrictDC bool // RSAKey 是 server RSA 私钥，用于密钥交换。nil 时无法完成握手。 RSAKey *rsa.PrivateKey // AuthKeys 持久化 auth key。默认内存实现。 @@ -480,6 +496,7 @@ type Server struct { outboundScratchPool *outboundScratchPool dc int + strictDC bool key exchange.PrivateKey authKeys store.AuthKeyStore conns *SessionManager @@ -531,6 +548,7 @@ func New(opts Options) *Server { outboundControlBudget: newOutboundTrackedBudget(defaultOutboundControlMaxBytes), outboundScratchPool: newOutboundScratchPool(opts.OutboundWriteGlobalMaxBytes), dc: opts.DC, + strictDC: opts.StrictDC, key: exchange.PrivateKey{RSA: opts.RSAKey}, authKeys: opts.AuthKeys, conns: conns,