Skip to content

Commit 0257aa7

Browse files
shreyasbhat0shreyasbhat0
authored
fix: signatures replayed across different networks (#331)
* fix: signature using chain id * chore: update regression testing * chore: update changelog
1 parent 7fcc1d8 commit 0257aa7

13 files changed

+126
-38
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ Contains all the PRs that improved the code without changing the behaviors.
4646
- Fixed bug allowing double claims with Thorchain claims.
4747
- Fixed non deterministic map iteration to sorted iteration
4848
- Fixed fee stuck in arkeo module by moving fees to arkeo-reserve
49+
- Fixed signature replay on different network
4950

5051
---
5152

‎arkeocli/claim.go

+13-1
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,19 @@ func runClaimCmd(cmd *cobra.Command, args []string) (err error) {
8080
return err
8181
}
8282

83-
signBytes := types.GetBytesToSign(contract.Id, nonce)
83+
chainId, err := cmd.Flags().GetString("chain-id")
84+
if err != nil {
85+
return err
86+
}
87+
88+
if len(chainId) == 0 {
89+
chainId, err = promptForArg(cmd, "specify the chain id:")
90+
if err != nil {
91+
return err
92+
}
93+
}
94+
95+
signBytes := types.GetBytesToSign(contract.Id, nonce, chainId)
8496
signature, _, err := clientCtx.Keyring.Sign(key.Name, signBytes, signing.SignMode_SIGN_MODE_DIRECT)
8597
if err != nil {
8698
return errors.Wrapf(err, "error signing")

‎sentinel/auth.go

+21-11
Original file line numberDiff line numberDiff line change
@@ -32,33 +32,35 @@ type ContractAuth struct {
3232
ContractId uint64
3333
Timestamp int64
3434
Signature []byte
35+
ChainId string
3536
}
3637

3738
type ArkAuth struct {
3839
ContractId uint64
3940
Spender common.PubKey
4041
Nonce int64
4142
Signature []byte
43+
ChainId string
4244
}
4345

4446
// String implement fmt.Stringer
4547
func (aa ArkAuth) String() string {
46-
return GenerateArkAuthString(aa.ContractId, aa.Nonce, aa.Signature)
48+
return GenerateArkAuthString(aa.ContractId, aa.Nonce, aa.Signature, aa.ChainId)
4749
}
4850

49-
func GenerateArkAuthString(contractId uint64, nonce int64, signature []byte) string {
50-
return fmt.Sprintf("%s:%s", GenerateMessageToSign(contractId, nonce), hex.EncodeToString(signature))
51+
func GenerateArkAuthString(contractId uint64, nonce int64, signature []byte, chainId string) string {
52+
return fmt.Sprintf("%s:%s", GenerateMessageToSign(contractId, nonce, chainId), hex.EncodeToString(signature))
5153
}
5254

53-
func GenerateMessageToSign(contractId uint64, nonce int64) string {
54-
return fmt.Sprintf("%d:%d", contractId, nonce)
55+
func GenerateMessageToSign(contractId uint64, nonce int64, chainId string) string {
56+
return fmt.Sprintf("%d:%d:%s", contractId, nonce, chainId)
5557
}
5658

5759
func parseContractAuth(raw string) (ContractAuth, error) {
5860
var auth ContractAuth
5961
var err error
6062

61-
parts := strings.SplitN(raw, ":", 3)
63+
parts := strings.SplitN(raw, ":", 4)
6264

6365
if len(parts) > 0 {
6466
auth.ContractId, err = strconv.ParseUint(parts[0], 10, 64)
@@ -74,8 +76,12 @@ func parseContractAuth(raw string) (ContractAuth, error) {
7476
}
7577
}
7678

77-
if len(parts) > 2 {
78-
auth.Signature, err = hex.DecodeString(parts[2])
79+
if len(parts) > 0 {
80+
auth.ChainId = parts[2]
81+
}
82+
83+
if len(parts) > 3 {
84+
auth.Signature, err = hex.DecodeString(parts[3])
7985
if err != nil {
8086
return auth, err
8187
}
@@ -87,7 +93,7 @@ func parseArkAuth(raw string) (ArkAuth, error) {
8793
var aa ArkAuth
8894
var err error
8995

90-
parts := strings.SplitN(raw, ":", 3)
96+
parts := strings.SplitN(raw, ":", 4)
9197

9298
if len(parts) > 0 {
9399
aa.ContractId, err = strconv.ParseUint(parts[0], 10, 64)
@@ -104,7 +110,11 @@ func parseArkAuth(raw string) (ArkAuth, error) {
104110
}
105111

106112
if len(parts) > 2 {
107-
aa.Signature, err = hex.DecodeString(parts[2])
113+
aa.ChainId = parts[2]
114+
}
115+
116+
if len(parts) > 3 {
117+
aa.Signature, err = hex.DecodeString(parts[3])
108118
if err != nil {
109119
return aa, err
110120
}
@@ -134,7 +144,7 @@ func (auth ContractAuth) Validate(lastTimestamp int64, client common.PubKey) err
134144
if err != nil {
135145
return err
136146
}
137-
msg := fmt.Sprintf("%d:%d", auth.ContractId, auth.Timestamp)
147+
msg := fmt.Sprintf("%d:%d:%s", auth.ContractId, auth.Timestamp, auth.ChainId)
138148
if !pk.VerifySignature([]byte(msg), auth.Signature) {
139149
return fmt.Errorf("invalid signature")
140150
}

‎sentinel/auth_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,12 @@ func TestArkAuth(t *testing.T) {
5050
require.NoError(t, err)
5151

5252
// happy path
53-
raw := GenerateArkAuthString(contractId, nonce, signature)
53+
raw := GenerateArkAuthString(contractId, nonce, signature, "arkeo")
5454
_, err = parseArkAuth(raw)
5555
require.NoError(t, err)
5656

5757
// bad signature
58-
raw = GenerateArkAuthString(contractId, nonce, signature)
58+
raw = GenerateArkAuthString(contractId, nonce, signature, "arkeo")
5959
_, err = parseArkAuth(raw + "randome not hex!")
6060
require.Error(t, err)
6161
}

‎test/regression/cmd/operations.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ func createContractAuth(input map[string]string) (string, bool, error) {
290290
}
291291

292292
// sign our msg
293-
msg := sentinel.GenerateMessageToSign(id, timestamp)
293+
msg := sentinel.GenerateMessageToSign(id, timestamp, "arkeo")
294294
auth, err := signThis(msg, input["signer"])
295295
return auth, true, err
296296
}
@@ -323,7 +323,7 @@ func createAuth(input map[string]string) (string, bool, error) {
323323
if err != nil {
324324
return "", true, fmt.Errorf("failed to parse nonce: %s", err)
325325
}
326-
msg := sentinel.GenerateMessageToSign(id, nonce)
326+
msg := sentinel.GenerateMessageToSign(id, nonce, "arkeo")
327327
auth, err := signThis(msg, input["signer"])
328328
return auth, true, err
329329
}

‎test/regression/suites/contracts/pay-as-you-go.yaml

+2-2
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ description: ensure contract shows up in active contracts
101101
endpoint: http://localhost:3636/open-claims
102102
asserts:
103103
- length == 1
104-
- .[0].signature == "1332d913e9bd87df5b11964a2c6abc770e3fe5bf5228d38e59f23aed97708c4a669c8e050baadca897ad7bec74b7951009b38d477bccdc2427eae8cec90dce39"
104+
- .[0].signature == "0e2ad0a02bf31ca722d67ef1af03e3f955f878e48fbc476e658b320ebbe1e9364b397a57328cf4ee6ed4cbb0cbb2bb4d538c165a3950a02ae2912c0ef370d766"
105105
- .[0].claimed == false
106106
---
107107
########################################################################################
@@ -143,7 +143,7 @@ type: check
143143
description: ensure contract shows up in active contracts
144144
endpoint: http://localhost:3636/claim/1
145145
asserts:
146-
- .signature == "1332d913e9bd87df5b11964a2c6abc770e3fe5bf5228d38e59f23aed97708c4a669c8e050baadca897ad7bec74b7951009b38d477bccdc2427eae8cec90dce39"
146+
- .signature == "0e2ad0a02bf31ca722d67ef1af03e3f955f878e48fbc476e658b320ebbe1e9364b397a57328cf4ee6ed4cbb0cbb2bb4d538c165a3950a02ae2912c0ef370d766"
147147
- .claimed == false
148148
- .contract_id == 1
149149
---

‎tools/curleo/main.go

+7-4
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,15 @@ func main() {
7272
}
7373
metadata := curl.parseMetadata()
7474
spender := curl.getSpender(*user)
75+
76+
chainID := flag.String("chain-id", "arkeo", "chain ID to use for signing")
77+
7578
contract := curl.getActiveContract(metadata.Configuration.ProviderPubKey.String(), service, spender)
7679
if contract.Height == 0 {
77-
println(fmt.Sprintf("no active contract found for provider:%s cbhain:%s - will attempt free tier", metadata.Configuration.ProviderPubKey.String(), service))
80+
println(fmt.Sprintf("no active contract found for provider:%s chain:%s - will attempt free tier", metadata.Configuration.ProviderPubKey.String(), service))
7881
} else {
7982
claim := curl.getClaim(contract.Id)
80-
auth := curl.sign(*user, contract.Id, claim.Nonce+1)
83+
auth := curl.sign(*user, contract.Id, claim.Nonce+1, *chainID)
8184
values.Add(sentinel.QueryArkAuth, auth)
8285
}
8386
u.RawQuery = values.Encode()
@@ -189,7 +192,7 @@ func (c Curl) parseMetadata() sentinel.Metadata {
189192
return meta
190193
}
191194

192-
func (c Curl) sign(user string, contractId uint64, nonce int64) string {
195+
func (c Curl) sign(user string, contractId uint64, nonce int64, chainId string) string {
193196
interfaceRegistry := codectypes.NewInterfaceRegistry()
194197
std.RegisterInterfaces(interfaceRegistry)
195198
ModuleBasics.RegisterInterfaces(interfaceRegistry)
@@ -203,7 +206,7 @@ func (c Curl) sign(user string, contractId uint64, nonce int64) string {
203206
if err != nil {
204207
log.Fatal(err)
205208
}
206-
msg := sentinel.GenerateMessageToSign(contractId, nonce)
209+
msg := sentinel.GenerateMessageToSign(contractId, nonce, chainId)
207210

208211
println("invoking Sign...")
209212
signature, pk, err := kb.Sign(user, []byte(msg), signing.SignMode_SIGN_MODE_DIRECT)

‎x/arkeo/keeper/keeper_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func SetupKeeper(t testing.TB) (cosmos.Context, Keeper) {
9292
"ArkeoParams",
9393
)
9494

95-
ctx := sdk.NewContext(stateStore, tmproto.Header{}, false, log.NewNopLogger())
95+
ctx := sdk.NewContext(stateStore, tmproto.Header{}, false, log.NewNopLogger()).WithChainID("arkeo")
9696

9797
_ = paramskeeper.NewKeeper(cdc, amino, keyParams, tkeyParams)
9898
govModuleAddr := authtypes.NewModuleAddress(govtypes.ModuleName).String()
@@ -217,7 +217,7 @@ func SetupKeeperWithStaking(t testing.TB) (cosmos.Context, Keeper, stakingkeeper
217217
"ArkeoParams",
218218
)
219219

220-
ctx := sdk.NewContext(stateStore, tmproto.Header{}, false, log.NewNopLogger())
220+
ctx := sdk.NewContext(stateStore, tmproto.Header{}, false, log.NewNopLogger()).WithChainID("arkeo")
221221

222222
govModuleAddr := "tarkeo1krj9ywwmqcellgunxg66kjw5dtt402kq0uf6pu"
223223
_ = paramskeeper.NewKeeper(cdc, amino, keyParams, tkeyParams)

‎x/arkeo/keeper/msg_server_claim_contract_income.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func (k msgServer) HandlerClaimContractIncome(ctx cosmos.Context, msg *types.Msg
5656
if err != nil {
5757
return err
5858
}
59-
if !pk.VerifySignature(msg.GetBytesToSign(), msg.Signature) {
59+
if !pk.VerifySignature(msg.GetBytesToSign(ctx.ChainID()), msg.Signature) {
6060
return errors.Wrap(types.ErrClaimContractIncomeInvalidSignature, "signature mismatch")
6161
}
6262
}

‎x/arkeo/keeper/msg_server_claim_contract_income_test.go

+69-7
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func TestValidate(t *testing.T) {
6565
Nonce: 20,
6666
}
6767

68-
message := msg.GetBytesToSign()
68+
message := msg.GetBytesToSign("arkeo")
6969
msg.Signature, _, err = kb.Sign("whatever", message, signing.SignMode_SIGN_MODE_DIRECT)
7070
require.NoError(t, err)
7171
require.NoError(t, s.HandlerClaimContractIncome(ctx, &msg))
@@ -127,7 +127,7 @@ func TestHandlePayAsYouGo(t *testing.T) {
127127
Nonce: 20,
128128
}
129129

130-
message := msg.GetBytesToSign()
130+
message := msg.GetBytesToSign("arkeo")
131131
msg.Signature, _, err = kb.Sign("whatever", message, signing.SignMode_SIGN_MODE_DIRECT)
132132
require.NoError(t, err)
133133

@@ -143,7 +143,7 @@ func TestHandlePayAsYouGo(t *testing.T) {
143143
Nonce: 21,
144144
}
145145

146-
message = msg.GetBytesToSign()
146+
message = msg.GetBytesToSign("arkeo")
147147
msg.Signature, _, err = kb.Sign("whatever", message, signing.SignMode_SIGN_MODE_DIRECT)
148148
require.NoError(t, err)
149149

@@ -157,7 +157,7 @@ func TestHandlePayAsYouGo(t *testing.T) {
157157
msg.Nonce = 25
158158

159159
// update signature for message
160-
message = msg.GetBytesToSign()
160+
message = msg.GetBytesToSign("arkeo")
161161
msg.Signature, _, err = kb.Sign("whatever", message, signing.SignMode_SIGN_MODE_DIRECT)
162162
require.NoError(t, err)
163163

@@ -174,7 +174,7 @@ func TestHandlePayAsYouGo(t *testing.T) {
174174
msg.Nonce = contract.Deposit.Int64() / contract.Rate.Amount.Int64() * 1000000000000
175175

176176
// update signature for message
177-
message = msg.GetBytesToSign()
177+
message = msg.GetBytesToSign("arkeo")
178178
msg.Signature, _, err = kb.Sign("whatever", message, signing.SignMode_SIGN_MODE_DIRECT)
179179
require.NoError(t, err)
180180

@@ -311,7 +311,7 @@ func TestClaimContractIncomeHandler(t *testing.T) {
311311
Nonce: 20,
312312
}
313313

314-
message := msg.GetBytesToSign()
314+
message := msg.GetBytesToSign("arkeo")
315315
msg.Signature, _, err = kb.Sign("whatever", message, signing.SignMode_SIGN_MODE_DIRECT)
316316
require.NoError(t, err)
317317

@@ -324,7 +324,7 @@ func TestClaimContractIncomeHandler(t *testing.T) {
324324
// bad nonce
325325
msg.Nonce = 0
326326

327-
message = msg.GetBytesToSign()
327+
message = msg.GetBytesToSign("arkeo")
328328
msg.Signature, _, err = kb.Sign("whatever", message, signing.SignMode_SIGN_MODE_DIRECT)
329329
require.NoError(t, err)
330330

@@ -381,3 +381,65 @@ func TestClaimContractIncomeHandlerSignatureVerification(t *testing.T) {
381381
err = s.HandlerClaimContractIncome(ctx, &msg)
382382
require.Error(t, err, types.ErrClaimContractIncomeInvalidSignature)
383383
}
384+
385+
func TestSignatureReplay(t *testing.T) {
386+
var err error
387+
// Setup keeper, context, and staking
388+
ctx, k, sk := SetupKeeperWithStaking(t)
389+
ctx = ctx.WithBlockHeight(20)
390+
391+
s := newMsgServer(k, sk)
392+
393+
// Set up necessary codec and keyring for signing.
394+
interfaceRegistry := codectypes.NewInterfaceRegistry()
395+
std.RegisterInterfaces(interfaceRegistry)
396+
module.NewBasicManager().RegisterInterfaces(interfaceRegistry)
397+
types.RegisterInterfaces(interfaceRegistry)
398+
cdc := codec.NewProtoCodec(interfaceRegistry)
399+
400+
pubkey := types.GetRandomPubKey()
401+
acc := types.GetRandomBech32Addr()
402+
service := common.BTCService
403+
kb := cKeys.NewInMemory(cdc)
404+
info, _, err := kb.NewMnemonic("whatever", cKeys.English, `m/44'/931'/0'/0/0`, "", hd.Secp256k1)
405+
require.NoError(t, err)
406+
pk, err := info.GetPubKey()
407+
require.NoError(t, err)
408+
client, err := common.NewPubKeyFromCrypto(pk)
409+
require.NoError(t, err)
410+
rate, err := cosmos.ParseCoin("10uarkeo")
411+
require.NoError(t, err)
412+
413+
contract := types.NewContract(pubkey, service, client)
414+
contract.Duration = 100
415+
contract.Rate = rate
416+
contract.Height = 10
417+
contract.Nonce = 0
418+
contract.Type = types.ContractType_PAY_AS_YOU_GO
419+
contract.Deposit = cosmos.NewInt(contract.Duration * contract.Rate.Amount.Int64())
420+
contract.Id = 1
421+
require.NoError(t, k.SetContract(ctx, contract))
422+
423+
require.NoError(t, k.MintToModule(ctx, types.ReserveName, getCoin(common.Tokens(10000*100*2))))
424+
require.NoError(t, k.SendFromModuleToModule(ctx, types.ReserveName, types.ContractName, getCoins(1000*100)))
425+
426+
// Create a claim message
427+
msg := types.MsgClaimContractIncome{
428+
ContractId: contract.Id,
429+
Creator: acc.String(),
430+
Nonce: 20,
431+
}
432+
433+
// Sign the message using chain ID "arkeo"
434+
messageBytes := msg.GetBytesToSign("arkeo")
435+
msg.Signature, _, err = kb.Sign("whatever", messageBytes, signing.SignMode_SIGN_MODE_DIRECT)
436+
require.NoError(t, err)
437+
438+
require.NoError(t, s.HandlerClaimContractIncome(ctx, &msg))
439+
440+
ctxReplay := ctx.WithChainID("otherchain")
441+
msg.Nonce = 21
442+
err = s.HandlerClaimContractIncome(ctxReplay, &msg)
443+
require.Error(t, err)
444+
require.Contains(t, err.Error(), "signature mismatch", "expected signature mismatch error when using a replayed signature on a different chain")
445+
}

‎x/arkeo/keeper/msg_server_open_contract_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ func TestOpenContractWithSettlementPeriod(t *testing.T) {
344344
Creator: clientAddress.String(),
345345
Nonce: 20,
346346
}
347-
message := claimMsg.GetBytesToSign()
347+
message := claimMsg.GetBytesToSign("arkeo")
348348
claimMsg.Signature, _, err = kb.Sign("whatever", message, signing.SignMode_SIGN_MODE_DIRECT)
349349
require.NoError(t, err)
350350
_, err = s.ClaimContractIncome(ctx, &claimMsg)

‎x/arkeo/types/message_claim_contract_income.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,12 @@ func (msg *MsgClaimContractIncome) GetSignBytes() []byte {
4444
return sdk.MustSortJSON(bz)
4545
}
4646

47-
func (msg *MsgClaimContractIncome) GetBytesToSign() []byte {
48-
return GetBytesToSign(msg.ContractId, msg.Nonce)
47+
func (msg *MsgClaimContractIncome) GetBytesToSign(chainId string) []byte {
48+
return GetBytesToSign(msg.ContractId, msg.Nonce, chainId)
4949
}
5050

51-
func GetBytesToSign(contractId uint64, nonce int64) []byte {
52-
return []byte(fmt.Sprintf("%d:%d", contractId, nonce))
51+
func GetBytesToSign(contractId uint64, nonce int64, chainID string) []byte {
52+
return []byte(fmt.Sprintf("%d:%d:%s", contractId, nonce, chainID))
5353
}
5454

5555
func (msg *MsgClaimContractIncome) ValidateBasic() error {

‎x/arkeo/types/message_claim_contract_income_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func TestClaimContractIncomeValidateBasic(t *testing.T) {
4242
Nonce: 24,
4343
}
4444

45-
message := msg.GetBytesToSign()
45+
message := msg.GetBytesToSign("arkeo")
4646
msg.Signature, _, err = kb.Sign("whatever", message, signing.SignMode_SIGN_MODE_DIRECT)
4747
require.NoError(t, err)
4848
err = msg.ValidateBasic()

0 commit comments

Comments
 (0)