Validate Checkpoint in ChangePack for PushPull API requests

[hackerwins]

Description:

When the Client calls the PushPull API and sends the request ChangePack, Checkpoint in the request may be tampered with due to various reasons such as network delays causing duplicate requests, new SDK bugs, or intentional tampering by malicious clients.

Therefore, it is beneficial for the stability and security of the system to validate Checkpoint.

Consider the following validation checks:

• Changes in the request ChangePack passed to PushPull API are created by a single Client, so Change.ID.Checkpoint.ClientSeq should increment sequentially by one.
• Checkpoint.ServerSeq in the request ChangePack for PushPull API cannot be greater than the server's Checkpoint.ServerSeq since it is set when the server saves the Change to the database.

If the Checkpoint is invalid, consider the following exception handling:

• If duplicate requests caused by network delays treat them as OK
• For other reasons, return INVALID_ARGUMENT

Why:

This validation will help ensure the integrity and security of the system.

[binary-ho]

I'm interested in this issue. may I try this?

[sejongk]

I'm interested in this issue. may I try this?

@binary-ho Sure. If you have any questions, feel free to ask.

[binary-ho]

Hello, I have some questions while trying to resolve the issue.

• Issue: Validate Checkpoint in ChangePack for PushPull API requests
• Reference: PushPull function in yorkie_server.go file

1. Where do we obtain the server's ClientSeq and ServerSeq?

To resolve this issue, I believe comparing the ClientSeq and ServerSeq included in the client request with the ClientSeq and ServerSeq stored by the server in the Document of the project would be appropriate.

I think we can use the ClientSeq and ServerSeq of the Checkpoint in the ClientInfo struct, which can be obtained through the ClientId of the Request Message. Is my understanding correct?

2. Where do we obtain the client's ClientSeq and ServerSeq?

The ChangePack of the Request Msg contains a Checkpoint and an array of Changes.

• The Checkpoint has its own ClientSeq and ServerSeq.
• Each Change in Changes also has its own ClientSeq and ServerSeq in its ChangeID.

According to the issue description, "so Change.ID.Checkpoint.ClientSeq should increment sequentially by one." Should the client's ClientSeq refer to the Seq values of each Change in this context? (Not the ClientSeq in the Checkpoint of the ChangePack in the Msg.)

Additionally, you mentioned "Checkpoint.ServerSeq in the request ChangePack for PushPull API cannot be greater than the server's Checkpoint.ServerSeq since it is set when the server saves the Change to the database."

For the client's ServerSeq, should we use the ServerSeq in the Checkpoint of the ChangePack in the Msg? (Not the ServerSeq values of each Change.)

3. How do we determine if the ClientSeq is invalid?

You mentioned "Change.ID.Checkpoint.ClientSeq should increment sequentially by one." Does this mean it is problematic if ClientSeq_sent_by_client ≤ ClientSeq_of_clientInfo?

(I am likely confused because I don't know exactly where and how the ClientSeq is generated.)

4. How do we determine if a request is a duplicate?

In the case where ClientSeq_sent_by_client == ClientSeq_of_clientInfo, can we consider this as a duplicate request due to network delays? I seem to be unclear about the criteria for determining a "duplicate request."

5. How do we determine if the ServerSeq is invalid?

If ServerSeq_sent_by_client > ClientSeq_of_clientInfo, is this an invalid situation?