feat: add request verification on send

lumtis commented 1 year ago

Close https://github.com/ignite/cli/issues/3257

Add the verification logic for requests when they are sent in addition to approve.

For this some refactoring have been done:

There is a single method to send requests
Request content is initialized outside of it and used in a method to simulate chain from request contents
Join is no longer sending requests, it returns request content for simulation

Test

Try to join the chain as a coordinator with a gentx with an invalid signature (it used to work)

ignite network chain join 10 --amount 95000000stake
? Peer's address ...
Source code fetched
Blockchain set up
Source code fetched
Blockchain set up
Requests format verified
Blockchain initialized
Genesis initialized
Genesis built
✘ The chain failed to start

tbruyelle commented 1 year ago

Try to join the chain as a coordinator with a gentx with an invalid signature (it used to work)

Sorry but how to use an invalid signature ^^' ?

lumtis commented 1 year ago

Try to join the chain as a coordinator with a gentx with an invalid signature (it used to work)

Sorry but how to use an invalid signature ^^' ?

By opening the gentx file generated from the init command and manually modifying data in the signatures field

tbruyelle commented 1 year ago

@lubtd What I did so far :

With the main version:

ignite network chain init 10

Alter gentx signature in $HOME/spn/10/config/gentx/gentx.json

{
   "body":{
      "messages":[
         {
            "@type":"/cosmos.staking.v1beta1.MsgCreateValidator",
            "description":{
               "moniker":"moniker",
               "identity":"",
               "website":"",
               "security_contact":"",
               "details":""
            },
            "commission":{
               "rate":"0.100000000000000000",
               "max_rate":"0.200000000000000000",
               "max_change_rate":"0.010000000000000000"
            },
            "min_self_delegation":"1",
            "delegator_address":"cosmos13gdkcvyz8us4u5lwdh7n5adldydrzmmgt0rfsw",
            "validator_address":"cosmosvaloper13gdkcvyz8us4u5lwdh7n5adldydrzmmgwmhuua",
            "pubkey":{
               "@type":"/cosmos.crypto.ed25519.PubKey",
               "key":"xYkKtYSNzod+l+fn0wBMmDD7aQZhCz5NCT/nFmk2JYQ="
            },
            "value":{
               "denom":"stake",
               "amount":"95000000"
            }
         }
      ],
      "memo":"418a85866d4d34806337d15a6abd8a2799aa2466@192.168.0.31:26656",
      "timeout_height":"0",
      "extension_options":[

      ],
      "non_critical_extension_options":[

      ]
   },
   "auth_info":{
      "signer_infos":[
         {
            "public_key":{
               "@type":"/cosmos.crypto.secp256k1.PubKey",
               "key":"ApllJatAUGW+po90temdo1IVfyByvkxAuw2ME0G3MDE7"
            },
            "mode_info":{
               "single":{
                  "mode":"SIGN_MODE_DIRECT"
               }
            },
            "sequence":"0"
         }
      ],
      "fee":{
         "amount":[

         ],
         "gas_limit":"200000",
         "payer":"",
         "granter":""
      },
      "tip":null
   },
   "signatures":[
-      "mn2D051PW7l7vXLOxFbzmWjbnZ6p/dDBhTlxi0RmZQMUDBO0fyYWyyvfL5EWeV+CJEaOXOV/rvxU+Sk8iQwX0A=="
+      "mn2D051PW7l7vXLOxFbzmWjbnZ6p/dDBhTlxi0RmZQMUDBO0fyYWyyvfL5EWeV+CJEaOXOV/rvxU+Sk8iQwX0B=="
   ]
}

Run ignite network chain join 10 --amount 95000000stake, as expected it worked because it's the main version. Then I rejected these requests.

Changed my .ignite/plugins/plugins.yml :

plugins:
-- path: github.com/ignite/cli-plugin-network@main
+- path: github.com/ignite/cli-plugin-network@feat/request-verification

Run ignite network chain join 10 --amount 95000000stake again, and it also worked, despite the bad signature. Not sure if I did something wrong.

lumtis commented 1 year ago

@tbruyelle can't reproduce

The command fails with the change:

"signatures":["qQTPrt4U6IrYbTWuvLLBVvw892z28waNd06/cG9zUHwgweMgQAvkFe6kMgInk3hKGYMGoPNywEvTcBt/jKnSkQ=="]
--->
"signatures":["qQTPrt4U6IrYbTWuvLLBVvw892z28waNd06/cG9zUHwgweMgQAvkFe6kMgInk3hKGYMGoPNywEvTcBt/jKnSkA=="]

tbruyelle commented 1 year ago

@lubtd OK I have also a panic "signature verification failed" if I replace the A with a Q (and that doesn't happen with main).

I'm just wondering why that doesn't happen if I replace the A with a B :thinking:

lumtis commented 1 year ago

When I replace the Q by a R (following letter) this is also working. This is very strange

I try to see if this a normal part of the signature since the failing example shows it goes into the same workflow that tries to initialize the genesis and therefore verifies the signatures

lumtis commented 1 year ago

@tbruyelle what about creating a separate issue to investigate this? At this point it's the appd start command that works with a modified signature

tbruyelle commented 1 year ago

At this point it's the appd start command that works with a modified signature

so you mean the issue should be created in the CLI?

aljo242 commented 1 year ago

Will approve once new issue is opened

lumtis commented 1 year ago

so you mean the issue should be created in the CLI?

No, the logic that verifies transaction signature in the SDK, since a gentx is a regular Cosmos tx

lumtis commented 1 year ago

Will approve once new issue is opened

https://github.com/ignite/cli-plugin-network/issues/12

ignite / cli-plugin-network

feat: add request verification on send #7

Test