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

Check timestamps on more API calls to avoid replay attacks #43

Closed
adamstallard opened this issue Jan 10, 2019 · 3 comments
Closed

Check timestamps on more API calls to avoid replay attacks #43

adamstallard opened this issue Jan 10, 2019 · 3 comments
Assignees

Comments

@adamstallard
Copy link
Member

adamstallard commented Jan 10, 2019

As a working example, PUT and DELETE calls to /connections check the timestamp from the most recent PUT or DELETE operation and only update it if the timestamp is newer. It uses the 'removed' collection to record timestamps to DELETE /connections operations.

If we want the timestamps included in calls to
PUT /membership
DELETE /membership
POST /groups
DELETE /groups
POST /fetchUserInfo
to be useful, we need to store previous timestamps in the DB and do similar checks

Otherwise replay attacks are possible, e.g. I can repeatedly remove someone from a group if they left it at some point in the past and rejoined; I can fetchUserInfo for another user.

@adamstallard adamstallard added this to To Do in Node Jun 5, 2019
@abramsymons abramsymons self-assigned this Dec 1, 2019
@abramsymons
Copy link
Collaborator

To make sure I have a correct understanding, you use removed collection to have access to timestamp of delete connection request. Otherwise if someone connect to another one and then delete that connection, as we don't have timestamp of delete connection request, someone can replay the first connection request.

Otherwise replay attacks are possible, e.g. I can repeatedly remove someone from a group if they left it at some point in the past and rejoined

We can simply solve this by comparing the remove request timestamp by join timestamp that is recorded in the usersInGroups. But just like in connection, more challenging scenario is when you leave a group after joining and there is no timestamp data for leaving as record is deleted. Therefor we need another collection for recording timestamps for leaving group requests.

If I understand the issue correctly, I think this issue is related to the issue #38 .
I think if we record all requests from the users with their signature in one place, we can avoid replay attacks without adding a new collection for removed records for any create/delete connection/membership/group.

@adamstallard
Copy link
Member Author

adamstallard commented Dec 2, 2019

To make sure I have a correct understanding, you use removed collection to have access to timestamp of delete connection request. Otherwise if someone connect to another one and then delete that connection, as we don't have timestamp of delete connection request, someone can replay the first connection request.

Exactly right

We can simply solve this by comparing the remove request timestamp by join timestamp that is recorded in the usersInGroups. But just like in connection, more challenging scenario is when you leave a group after joining and there is no timestamp data for leaving as record is deleted. Therefor we need another collection for recording timestamps for leaving group requests.

Yes.

If I understand the issue correctly, I think this issue is related to the issue #38 .
I think if we record all requests from the users with their signature in one place, we can avoid replay attacks without adding a new collection for removed records for any create/delete connection/membership/group.

I think so, too. We may even be able to deprecate the "removed" collection, or we may keep such extra collections around just to have better performance for these kind of checks.

@abramsymons
Copy link
Collaborator

We are checking operation hashes instead of timestamp to avoid any operation be replayed.
The hash is calculated for current operations based on operation_name + message where message includes timestamp too.

@abramsymons abramsymons moved this from To Do to Review in Node Dec 5, 2019
@adamstallard adamstallard added this to Review in Everything Jun 15, 2020
@adamstallard adamstallard moved this from Review to Complete in Everything Jun 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Everything
  
Complete
Node
Review
Development

No branches or pull requests

2 participants