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

Confirm Retire Prior To via Acknowledgement #3548

Closed
wants to merge 1 commit into from
Closed

Confirm Retire Prior To via Acknowledgement #3548

wants to merge 1 commit into from

Conversation

nibanks
Copy link
Member

@nibanks nibanks commented Mar 28, 2020

Another proposal to fix #3509. Summary of design changes:

  • CIDs retired via RPT are not individually, explicitly retired by the receiver of the NCID frame.
  • The sender of the RPT gets its confirmation of the retirement when it receives an ACK for the packet that contained the NCID frame.

This proposal eliminates the extra state required to retire the CIDs requested by their peer because the CIDs can immediately be discarded. The only state required by the receiver is what was already required to acknowledge the packet.

@martinthomson
Copy link
Member

I don't think that we should go against established principles to solve something that can be solved more trivially.

@martinthomson martinthomson added -transport design An issue that affects the design of the protocol; resolution requires consensus. labels Mar 28, 2020
Copy link
Contributor

@ianswett ianswett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this direction is untenable, but I think we need to not rely on acknowledgements to drive the state machine.

Instead, I'd suggest keying off use of a CID issued after the Retire Prior To as indication that the RPT was received.

In that world, I think the model is: a receiver of Retire Prior To doesn't need to use RCID to explicitly retire prior CIDs and instead as soon as it starts using a CID issued after the Retire Prior To, it's assumed all the prior CIDs have been retired. Given there's already a MUST about no longer using the new CID immediately, this doesn't seem that problematic.

I think that's a fairly solid direction, now that I write it down, are there any issues I'm not thinking of?

@nibanks
Copy link
Member Author

nibanks commented Mar 28, 2020

I don't think that we should go against established principles to solve something that can be solved more trivially.

IMO, #3547 is a heuristic that mitigates the problem, but doesn't quite solve it. Additionally, it is more complex to implement than this proposal, as it requires tracking all the additional state to retire the CIDs.

Instead, I'd suggest keying off use of a CID issued after the Retire Prior To as indication that the RPT was received.

I still prefer the ACK approach, personally, but I do think I'd prefer this suggestion over #3547. I think it would work pretty much the same as the ACK approach, but just uses a different implicit signal.

@kazuho
Copy link
Member

kazuho commented Mar 28, 2020

Does this PR give us protection against the other attack, that uses intentional migration as the attack vector (#3509 (comment))? The problem is not limited to the use of RPT.

@MikeBishop
Copy link
Contributor

Instead, I'd suggest keying off use of a CID issued after the Retire Prior To as indication that the RPT was received.

In that world, I think the model is: a receiver of Retire Prior To doesn't need to use RCID to explicitly retire prior CIDs and instead as soon as it starts using a CID issued after the Retire Prior To, it's assumed all the prior CIDs have been retired. Given there's already a MUST about no longer using the new CID immediately, this doesn't seem that problematic.

The complication is that RPT doesn't have to immediately get rid of all old CIDs and start fresh; that's just one scenario. Consider the load-balancer key rotation case:

  • CIDs contain a bit to indicate if they're from key phase 0 or key phase 1.
  • I've issued you CIDs under phase 0 and phase 1; your active CIDs include a mix
  • I'm about to rotate key 0, so all the outstanding phase 0 keys need to get scrapped
  • I send you NCID(<a CID from the new phase 0>, RPT=<your newest phase 0 CID>).
  • There's no need for you to skip the phase 1 CIDs and start using your shiny new phase 0 key, but if you don't, I can't know for sure that you've dropped the old-phase-0 keys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Required state for retaining unacked RETIRE_CONNECTION_ID frames is unbound
5 participants