Merge pull request #17 from iamxvbaba/dev
fix: sync lenient DC-ID validation for key exchange
This commit is contained in:
commit
5dcdb57e4c
5 changed files with 82 additions and 6 deletions
|
|
@ -1060,6 +1060,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,
|
||||
|
|
|
|||
|
|
@ -31,6 +31,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
|
||||
// many client forks 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
|
||||
|
|
@ -465,6 +472,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),
|
||||
|
|
|
|||
|
|
@ -27,9 +27,12 @@ import (
|
|||
//
|
||||
// DrKLO Android marks media temporary auth-key exchange with a negative DC in
|
||||
// p_q_inner_data_temp_dc (for example DC 2 -> -2). gotd v0.158.0 validates this
|
||||
// field by exact equality and rejects that legitimate media-temp path. Keep the
|
||||
// permanent-key check strict, but allow temp-key DC values whose absolute value
|
||||
// matches this server DC.
|
||||
// field by exact equality and rejects that legitimate media-temp path. The
|
||||
// temp-key DC check accepts any value whose absolute value matches this
|
||||
// server DC. The permanent-key check is lenient by default too (see
|
||||
// Options.StrictDC doc) — self-hosted single-server deployments commonly
|
||||
// have clients that alias dc_id 1..5 to the one backend, so a client-chosen
|
||||
// dc_id that isn't our configured DC is expected, not an error.
|
||||
func (s *Server) runServerExchange(ctx context.Context, conn transport.Conn) (exchange.ServerExchangeResult, error) {
|
||||
ex := serverExchangeCompat{
|
||||
conn: conn,
|
||||
|
|
@ -38,6 +41,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 +69,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 +350,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
|
||||
// self-hosted client forks commonly 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 {
|
||||
|
|
|
|||
|
|
@ -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; self-hosted client forks commonly 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},
|
||||
|
|
|
|||
|
|
@ -303,6 +303,21 @@ 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 self-hosted client
|
||||
// forks commonly run in "single-server backend" mode, where dc_id 1..5 all
|
||||
// alias to this one server so that any old data referencing a specific
|
||||
// dc_id still resolves correctly. When a client 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。默认内存实现。
|
||||
|
|
@ -482,6 +497,7 @@ type Server struct {
|
|||
outboundScratchPool *outboundScratchPool
|
||||
|
||||
dc int
|
||||
strictDC bool
|
||||
key exchange.PrivateKey
|
||||
authKeys store.AuthKeyStore
|
||||
conns *SessionManager
|
||||
|
|
@ -533,6 +549,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,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue