Skip to content

Commit 39fc824

Browse files
tdakkotatdakkota
committed
fix(exchange): add some nonce checks
1 parent 9bc6d57 commit 39fc824

File tree

3 files changed

+68
-38
lines changed

3 files changed

+68
-38
lines changed

‎internal/exchange/client_flow.go‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,12 @@ import (
77
"math/big"
88

99
"go.uber.org/zap"
10-
11-
"github.com/gotd/td/internal/proto"
12-
1310
"golang.org/x/xerrors"
1411

1512
"github.com/gotd/td/bin"
1613
"github.com/gotd/td/internal/crypto"
1714
"github.com/gotd/td/internal/mt"
15+
"github.com/gotd/td/internal/proto"
1816
)
1917

2018
// Run runs client-side flow.
@@ -41,6 +39,7 @@ func (c ClientExchange) Run(ctx context.Context) (ClientExchangeResult, error) {
4139
if res.Nonce != nonce {
4240
return ClientExchangeResult{}, xerrors.New("ResPQ nonce mismatch")
4341
}
42+
serverNonce := res.ServerNonce
4443

4544
// Selecting first public key that match fingerprint.
4645
var selectedPubKey *rsa.PublicKey
@@ -89,7 +88,7 @@ Loop:
8988
Pq: res.Pq,
9089
Nonce: nonce,
9190
NewNonce: newNonce,
92-
ServerNonce: res.ServerNonce,
91+
ServerNonce: serverNonce,
9392
P: pBytes,
9493
Q: qBytes,
9594
}
@@ -105,7 +104,7 @@ Loop:
105104
}
106105
reqDHParams := &mt.ReqDHParamsRequest{
107106
Nonce: nonce,
108-
ServerNonce: res.ServerNonce,
107+
ServerNonce: serverNonce,
109108
P: pBytes,
110109
Q: qBytes,
111110
PublicKeyFingerprint: crypto.RSAFingerprint(selectedPubKey),
@@ -138,8 +137,11 @@ Loop:
138137
if p.Nonce != nonce {
139138
return ClientExchangeResult{}, xerrors.New("ServerDHParamsOk nonce mismatch")
140139
}
140+
if p.ServerNonce != serverNonce {
141+
return ClientExchangeResult{}, xerrors.New("ServerDHParamsOk server nonce mismatch")
142+
}
141143

142-
key, iv := crypto.TempAESKeys(newNonce.BigInt(), res.ServerNonce.BigInt())
144+
key, iv := crypto.TempAESKeys(newNonce.BigInt(), serverNonce.BigInt())
143145
// Decrypting inner data.
144146
data, err := crypto.DecryptExchangeAnswer(p.EncryptedAnswer, key, iv)
145147
if err != nil {
@@ -151,6 +153,12 @@ Loop:
151153
if err := innerData.Decode(b); err != nil {
152154
return ClientExchangeResult{}, err
153155
}
156+
if innerData.Nonce != nonce {
157+
return ClientExchangeResult{}, xerrors.New("ServerDHInnerData nonce mismatch")
158+
}
159+
if innerData.ServerNonce != serverNonce {
160+
return ClientExchangeResult{}, xerrors.New("ServerDHInnerData server nonce mismatch")
161+
}
154162

155163
dhPrime := big.NewInt(0).SetBytes(innerData.DhPrime)
156164
g := big.NewInt(int64(innerData.G))
@@ -215,6 +223,13 @@ Loop:
215223
}
216224
switch v := dhSetRes.(type) {
217225
case *mt.DhGenOk: // dh_gen_ok#3bcbf734
226+
if v.Nonce != nonce {
227+
return ClientExchangeResult{}, xerrors.New("DhGenOk nonce mismatch")
228+
}
229+
if v.ServerNonce != serverNonce {
230+
return ClientExchangeResult{}, xerrors.New("DhGenOk server nonce mismatch")
231+
}
232+
218233
var key crypto.Key
219234
authKey.FillBytes(key[:])
220235
authKeyID := key.ID()

‎internal/exchange/client_flow_test.go‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package exchange
2+
3+
import (
4+
"context"
5+
"crypto/rsa"
6+
"math/rand"
7+
"net"
8+
"testing"
9+
"time"
10+
11+
"github.com/stretchr/testify/require"
12+
"go.uber.org/zap/zaptest"
13+
14+
"github.com/gotd/td/internal/tdsync"
15+
"github.com/gotd/td/transport"
16+
)
17+
18+
func TestExchangeTimeout(t *testing.T) {
19+
a := require.New(t)
20+
21+
reader := rand.New(rand.NewSource(1))
22+
key, err := rsa.GenerateKey(reader, 2048)
23+
a.NoError(err)
24+
log := zaptest.NewLogger(t)
25+
26+
i := transport.Intermediate(nil)
27+
client, _ := i.Pipe()
28+
29+
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
30+
defer cancel()
31+
32+
grp := tdsync.NewCancellableGroup(ctx)
33+
grp.Go(func(groupCtx context.Context) error {
34+
_, err := NewExchanger(client).
35+
WithLogger(log.Named("client")).
36+
WithRand(reader).
37+
WithTimeout(1 * time.Second).
38+
Client([]*rsa.PublicKey{&key.PublicKey}).
39+
Run(groupCtx)
40+
return err
41+
})
42+
43+
err = grp.Wait()
44+
if err, ok := err.(net.Error); !ok || !err.Timeout() {
45+
require.NoError(t, err)
46+
}
47+
}

‎internal/exchange/flow_test.go‎

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"crypto/rsa"
66
"fmt"
77
"math/rand"
8-
"net"
98
"testing"
109
"time"
1110

@@ -53,37 +52,6 @@ func TestExchange(t *testing.T) {
5352
require.NoError(t, grp.Wait())
5453
}
5554

56-
func TestExchangeTimeout(t *testing.T) {
57-
a := require.New(t)
58-
59-
reader := rand.New(rand.NewSource(1))
60-
key, err := rsa.GenerateKey(reader, 2048)
61-
a.NoError(err)
62-
log := zaptest.NewLogger(t)
63-
64-
i := transport.Intermediate(nil)
65-
client, _ := i.Pipe()
66-
67-
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
68-
defer cancel()
69-
70-
grp := tdsync.NewCancellableGroup(ctx)
71-
grp.Go(func(groupCtx context.Context) error {
72-
_, err := NewExchanger(client).
73-
WithLogger(log.Named("client")).
74-
WithRand(reader).
75-
WithTimeout(1 * time.Second).
76-
Client([]*rsa.PublicKey{&key.PublicKey}).
77-
Run(groupCtx)
78-
return err
79-
})
80-
81-
err = grp.Wait()
82-
if err, ok := err.(net.Error); !ok || !err.Timeout() {
83-
require.NoError(t, err)
84-
}
85-
}
86-
8755
func TestExchangeCorpus(t *testing.T) {
8856
k := testutil.RSAPrivateKey()
8957

0 commit comments

Comments
 (0)