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.

Sign up for GitHub

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

Jump to bottom

[~] added token complexity for client-server interaction #417

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[~] added token complexity for client-server interaction #417

wants to merge 1 commit into from

Conversation

Ya-Pasha-364shy
Copy link
Contributor

@Ya-Pasha-364shy Ya-Pasha-364shy commented May 5, 2024 •
edited
Loading

Hello everyone, I decided to complicate the token by double-xor encryption with an initialization vector and a connection secret, this secret is unique for each connection. I will be glad to constructive criticism and reviews.

Fixes #266 issue

  • Best regards, Pavel Chernov

@Ya-Pasha-364shy
Copy link
Contributor Author

Hi all. Do you find these changes useful or can I close the pull request?

@Yanmei-Liu
Copy link
Collaborator

Hi all. Do you find these changes useful or can I close the pull request?

@Ya-Pasha-364shy Thanks for the PR, and sorry for the late reply. We find it useful but it can't be merged at the current state. There's an important issue here: The generated token need to be used during new connection handshaking in the future. While usually we used it in distributed cluster, and the servers in the cluster don't have to share state with each other. Then the current solution would cause the server can't validate the token in the future without current state.

We have discussed a new solution to solve this issue. Basically we'd like to have the modification upon your PR, and keep this PR unmerged until this new solution is merged into this branch.

@Ya-Pasha-364shy
Copy link
Contributor Author

@Ya-Pasha-364shy Thanks for the PR, and sorry for the late reply. We find it useful but it can't be merged at the current state. There's an important issue here: The generated token need to be used during new connection handshaking in the future. While usually we used it in distributed cluster, and the servers in the cluster don't have to share state with each other. Then the current solution would cause the server can't validate the token in the future without current state.

We have discussed a new solution to solve this issue. Basically we'd like to have the modification upon your PR, and keep this PR unmerged until this new solution is merged into this branch.

Thx for feedback. I understand you, I will waiting for decision of checking new connection handshaking by token in the future. Tag me here when the new solution is merged into main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Yanmei-Liu Yanmei-Liu Awaiting requested review from Yanmei-Liu

@yangfurong yangfurong Awaiting requested review from yangfurong

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: Address validation token is too easy.
2 participants
@Ya-Pasha-364shy @Yanmei-Liu