Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prevention: do not accept block from someone who is not active sequencer #1037

Open
danwt opened this issue Aug 22, 2024 · 9 comments
Open
Assignees
Labels

Comments

@danwt
Copy link
Contributor

danwt commented Aug 22, 2024

In general it should not be possible for some third party to come and gossip bullshit blocks, if they are not sequencer.

It might already be implemented, need to check!

Note: this applies also to sequencers who were recently sequencer - so should double check any possible impact of latency in the system.

In general need to reason through, and explain why this attack cannot happen.

@danwt danwt self-assigned this Aug 22, 2024
@danwt danwt added the security label Aug 22, 2024
@omritoptix omritoptix assigned srene and unassigned danwt Aug 23, 2024
@danwt danwt unassigned srene Sep 2, 2024
@omritoptix
Copy link
Contributor

this should be already supported. can we close it? @danwt

@danwt
Copy link
Contributor Author

danwt commented Sep 9, 2024

Right, I've just seen the code, for gossip

dymint/types/validation.go

Lines 98 to 110 in 88ba1fe

func (c *Commit) ValidateWithHeader(proposerPubKey tmcrypto.PubKey, header *Header) error {
if err := c.ValidateBasic(); err != nil {
return err
}
abciHeaderPb := ToABCIHeaderPB(header)
abciHeaderBytes, err := abciHeaderPb.Marshal()
if err != nil {
return err
}
// commit is validated to have single signature
if !proposerPubKey.VerifySignature(abciHeaderBytes, c.Signatures[0]) {
return ErrInvalidSignature
}

dymint/block/block.go

Lines 166 to 169 in e9709f6

// This function validates the block and commit against the state before applying it.
func (m *Manager) validateBlockBeforeApply(block *types.Block, commit *types.Commit) error {
return types.ValidateProposedTransition(m.State, block, commit, m.GetProposerPubKey())
}

so no security issue

@danwt danwt closed this as completed Sep 9, 2024
@danwt danwt reopened this Sep 24, 2024
@danwt
Copy link
Contributor Author

danwt commented Sep 24, 2024

Reopening, this needs some thought IMO

@danwt
Copy link
Contributor Author

danwt commented Sep 24, 2024

I think the question here is latency in knowing the most up to date sequencer

Blocked by

Keeping open for now

@danwt
Copy link
Contributor Author

danwt commented Sep 24, 2024

@danwt danwt self-assigned this Sep 27, 2024
@danwt danwt changed the title prevention: make sure to not accept block from someone who is not active sequencer prevention: do not accept block from someone who is not active sequencer Sep 27, 2024
@danwt
Copy link
Contributor Author

danwt commented Sep 27, 2024

Quote "we should validate sequencer pubkey on block against hub state upon event on each new event received from the hub "

@omritoptix
Copy link
Contributor

I think this can be closed cause I blv this issue is about protecting against random party blocks and not about faulty sequecner rotation. in that case, I think that issue is covered.

@danwt
Copy link
Contributor Author

danwt commented Sep 27, 2024

Ok
May be we can discuss on Monday because thinking what if seq A is rotating to seq B correctly between H,H+1, but seq A gossips some blocks H+10 to another full node before the FN gets H or H+1, then the FN will have it cached

@danwt
Copy link
Contributor Author

danwt commented Sep 27, 2024

My point is I think it's worth due dilligence

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants