Reintroducing authz messages in a v1.0.15 fork

[zenodeapp]

Hi,

So I don't know if you remember this, but wanted to express my gratitude for being super helpful back when I was setting up the chain upgrade for GenesisL1 (see discussion: #1280).

Everything worked out well!

Now I noticed authz messages are getting rejected in the mempool, causing the chain to not handle i.e. auto-compounding (restaking).

I saw in another issue (#1289) that this was a security vulnerability as to why in a later version you guys removed it completely. We're on v1.0.15 so in our version we still only have the mempool rejection.

Is there any way to get my hands on these mentioned security reports? Also, is it wise to reintroduce it back in order for us to get restake working again? Finally, would this be considered a non-breaking change?

See my prepped ethermint fork to comment out the message rejection part: https://github.com/zenodeapp/ethermint/tree/reenable-authz-messages

If you recommend we shouldn't or know of a way to get this back safely, I'd be super grateful to hear from you :)!

Have a great start of the week!
ZEN

[yihuang]

Just be careful with the ante handlers, because MsgEthereumTx relies on many logics executed in ante handler, so it can't be executed with authz, which don't run the ante handler logics.
and we had some other custom messages which rely on ante handler logic, it's fixed later though, so technically we don't have to remove the authz module, we do it just to play safe, in case in the future someone forget about the tricky issues here when developing new feature.
So in short, you can have the authz module back safely, just keep the mempool rejections of the MsgEthereumTx message.

[zenodeapp]

Just be careful with the ante handlers, because MsgEthereumTx relies on many logics executed in ante handler, so it can't be executed with authz, which don't run the ante handler logics.
and we had some other custom messages which rely on ante handler logic, it's fixed later though, so technically we don't have to remove the authz module, we do it just to play safe, in case in the future someone forget about the tricky issues here when developing new feature.
So in short, you can have the authz module back safely, just keep the mempool rejections of the MsgEthereumTx message.

Got it, thank you for such a swift and thorough reply! Won't be touching the other rejection indeed!

[zenodeapp]

Just be careful with the ante handlers, because MsgEthereumTx relies on many logics executed in ante handler, so it can't be executed with authz, which don't run the ante handler logics.
and we had some other custom messages which rely on ante handler logic, it's fixed later though, so technically we don't have to remove the authz module, we do it just to play safe, in case in the future someone forget about the tricky issues here when developing new feature.
So in short, you can have the authz module back safely, just keep the mempool rejections of the MsgEthereumTx message.

One more thing, this doesn't require an upgrade proposal, right?

[yihuang]

Just be careful with the ante handlers, because MsgEthereumTx relies on many logics executed in ante handler, so it can't be executed with authz, which don't run the ante handler logics.
and we had some other custom messages which rely on ante handler logic, it's fixed later though, so technically we don't have to remove the authz module, we do it just to play safe, in case in the future someone forget about the tricky issues here when developing new feature.
So in short, you can have the authz module back safely, just keep the mempool rejections of the MsgEthereumTx message.

One more thing, this doesn't require an upgrade proposal, right?

Do you mean re-add the authz module when it's already removed? that'd need a upgrade proposal.

[zenodeapp]

Just be careful with the ante handlers, because MsgEthereumTx relies on many logics executed in ante handler, so it can't be executed with authz, which don't run the ante handler logics.
and we had some other custom messages which rely on ante handler logic, it's fixed later though, so technically we don't have to remove the authz module, we do it just to play safe, in case in the future someone forget about the tricky issues here when developing new feature.
So in short, you can have the authz module back safely, just keep the mempool rejections of the MsgEthereumTx message.

One more thing, this doesn't require an upgrade proposal, right?

Do you mean re-add the authz module when it's already removed? that'd need a upgrade proposal.

No, no. We're on v1.0.15. I believe that version only has the mempool rejection in the anteHandler? One of the later rc-versions had the complete removal of the authz module.

[yihuang]

Just be careful with the ante handlers, because MsgEthereumTx relies on many logics executed in ante handler, so it can't be executed with authz, which don't run the ante handler logics.
and we had some other custom messages which rely on ante handler logic, it's fixed later though, so technically we don't have to remove the authz module, we do it just to play safe, in case in the future someone forget about the tricky issues here when developing new feature.
So in short, you can have the authz module back safely, just keep the mempool rejections of the MsgEthereumTx message.

One more thing, this doesn't require an upgrade proposal, right?

Do you mean re-add the authz module when it's already removed? that'd need a upgrade proposal.

No, no. We're on v1.0.15. I believe that version only has the mempool rejection in the anteHandler? One of the later rc-versions had the complete removal of the authz module.

From 1.0.x to 1.1.x is a big breaking change.

[zenodeapp]

Just be careful with the ante handlers, because MsgEthereumTx relies on many logics executed in ante handler, so it can't be executed with authz, which don't run the ante handler logics.
and we had some other custom messages which rely on ante handler logic, it's fixed later though, so technically we don't have to remove the authz module, we do it just to play safe, in case in the future someone forget about the tricky issues here when developing new feature.
So in short, you can have the authz module back safely, just keep the mempool rejections of the MsgEthereumTx message.

One more thing, this doesn't require an upgrade proposal, right?

Do you mean re-add the authz module when it's already removed? that'd need a upgrade proposal.

No, no. We're on v1.0.15. I believe that version only has the mempool rejection in the anteHandler? One of the later rc-versions had the complete removal of the authz module.

From 1.0.x to 1.1.x is a big breaking change.

Got it, we'll remain on 1.0.15 for now, but will I be able to safely do this: https://github.com/zenodeapp/ethermint/tree/reenable-authz-messages? I've made sure to continue from the commit hash in the v1.0.15 go.mod. Or will this version not be secure enough to re-enable the Authz messages?

[yihuang]

Just be careful with the ante handlers, because MsgEthereumTx relies on many logics executed in ante handler, so it can't be executed with authz, which don't run the ante handler logics.

and we had some other custom messages which rely on ante handler logic, it's fixed later though, so technically we don't have to remove the authz module, we do it just to play safe, in case in the future someone forget about the tricky issues here when developing new feature.

So in short, you can have the authz module back safely, just keep the mempool rejections of the MsgEthereumTx message.

One more thing, this doesn't require an upgrade proposal, right?

Do you mean re-add the authz module when it's already removed? that'd need a upgrade proposal.

No, no. We're on v1.0.15. I believe that version only has the mempool rejection in the anteHandler? One of the later rc-versions had the complete removal of the authz module.

From 1.0.x to 1.1.x is a big breaking change.

Got it, we'll remain on 1.0.15 for now, but will I be able to safely do this: https://github.com/zenodeapp/ethermint/tree/reenable-authz-messages? I've made sure to continue from the commit hash in the v1.0.15 go.mod. Or will this version not be secure enough to re-enable the Authz messages?

i see what you mean, you can reject specifically the MsgEthereumTx inside of MsgExec instead of reject the whole MsgExec.

EDIT: backport this decorator, and pass pamateter:

DisabledAuthzMsgs: []string{
			sdk.MsgTypeURL(&evmtypes.MsgEthereumTx{}),
			sdk.MsgTypeURL(&vestingtypes.MsgCreateVestingAccount{}),
		}

[zenodeapp]

Just be careful with the ante handlers, because MsgEthereumTx relies on many logics executed in ante handler, so it can't be executed with authz, which don't run the ante handler logics.

and we had some other custom messages which rely on ante handler logic, it's fixed later though, so technically we don't have to remove the authz module, we do it just to play safe, in case in the future someone forget about the tricky issues here when developing new feature.

So in short, you can have the authz module back safely, just keep the mempool rejections of the MsgEthereumTx message.

One more thing, this doesn't require an upgrade proposal, right?

Do you mean re-add the authz module when it's already removed? that'd need a upgrade proposal.

No, no. We're on v1.0.15. I believe that version only has the mempool rejection in the anteHandler? One of the later rc-versions had the complete removal of the authz module.

From 1.0.x to 1.1.x is a big breaking change.

Got it, we'll remain on 1.0.15 for now, but will I be able to safely do this: https://github.com/zenodeapp/ethermint/tree/reenable-authz-messages? I've made sure to continue from the commit hash in the v1.0.15 go.mod. Or will this version not be secure enough to re-enable the Authz messages?

i see what you mean, you can reject specifically the MsgEthereumTx inside of MsgExec instead of reject the whole MsgExec.

EDIT: backport this decorator, and pass pamateter:

DisabledAuthzMsgs: []string{
			sdk.MsgTypeURL(&evmtypes.MsgEthereumTx{}),
			sdk.MsgTypeURL(&vestingtypes.MsgCreateVestingAccount{}),
		}

Hey thanks! I'll check this out in a bit and show the changes I made if that's okay with you!

Again, thank you for being so helpful man.

[zenodeapp]

@yihuang

Does it look okay now? https://github.com/zenodeapp/ethermint/tree/reenable-authz-messages

For quick overview, these are the 3 commits (The last commit was for rectifying a mistake of mine for removing the Blacklist option. I perhaps could have replaced this with the new ExtraDecorators[]-HandlerOption, but left it as is).

zenodeapp/ethermint@a69fadc
zenodeapp/ethermint@009f942
zenodeapp/ethermint@6561a2c

Also, this is non-breaking?

[yihuang]

Also, this is non-breaking?

AuthzLimiterDecorator is "breaking" because it works on consensus logic, if you want non-breaking, add a ctx.IsCheckTx condition.