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

Invalidate Session on Password Change #18410

Closed
LukasReschke opened this issue Aug 18, 2015 · 22 comments
Closed

Invalidate Session on Password Change #18410

LukasReschke opened this issue Aug 18, 2015 · 22 comments

Comments

@LukasReschke
Copy link
Member

Imported from https://github.com/owncloud/security-tracker/issues/31:

Hi,

While testing password changing on the client,
I noticed that changing it does not invalidate the
users sessions. I am writing to this list because
this behavior constitutes a security issue,
depending on the users assumptions.

I would presume that if a user changes his password,
he might do so because:

- he does so routinely (e.g. enforced password policy
  in a company)

or

- his old password got compromised

In the latter, he definitely wants to expire his sessions
 (Web and WebDAV) as well.

In conclusion, I suggest we either introduce a patch to
invalidate the users session on password change, or
add an additional action "invalidate session" in the
user management UI (note that this would mean the
functionality would not accessible from alternative user
management interfaces, such as LDAP/AD).

Also related to #13727 (comment) and #17866 (comment)

@MorrisJobke
Copy link
Contributor

  • invalidate session for deleted LDAP users that still have an associated owncloud user

@felixboehm
Copy link
Contributor

@MTRichards Please provide a milestone here.

@MTRichards
Copy link
Contributor

Is there a ticket?

@MTRichards
Copy link
Contributor

@LukasReschke Are we planning on doing anything here, or is the urgency such that it is not required soon?

@MorrisJobke
Copy link
Contributor

Is there a ticket?

owncloud/client#3912 ;) (always check the last backlink at around the time of adding the label ;))

@MorrisJobke
Copy link
Contributor

Is there a ticket?

00003501

@MTRichards
Copy link
Contributor

This seems to be because the session is still valid.
So, how do we invalidate the session without querying the LDAP/AD database again?
I don't think you can.

So we should invalidate the session, and then require us to hit the LDAP/AD again.
That subsequently fails on sync.

@MorrisJobke

Isn't there a session keep alive setting somewhere? (if not, this would be good).

Is there an event or something when a user's external (LDAP/AD) password is changed? Do we even know when this happens (I think not)?

Alternatively, a manual "invalidate session" option might also work, but as an admin this is undesirable behavior.

@MorrisJobke
Copy link
Contributor

Isn't there a session keep alive setting somewhere? (if not, this would be good).

For LDAP there is a TTL

Is there an event or something when a user's external (LDAP/AD) password is changed? Do we even know when this happens (I think not)?

cc @blizzz

Alternatively, a manual "invalidate session" option might also work, but as an admin this is undesirable behavior.

It's not about the manual way, but about being able to do so (also from a developers point of view).

@blizzz
Copy link
Contributor

blizzz commented Nov 17, 2015

Isn't there a session keep alive setting somewhere? (if not, this would be good).

For LDAP there is a TTL

Only used for caching, nothing to do with sessions.

Is there an event or something when a user's external (LDAP/AD) password is changed? Do we even know when this happens (I think not)?

cc @blizzz

No, we don't.

@MTRichards
Copy link
Contributor

right, so now I understand the feature request - we need a TTL for session information.

@bboule I need to understand if this makes the top ten and the urgency there, and then @cmonteroluque can discuss delivery. No later than 9.1 in my head at this time, but not confirmed.

@karlitschek
Copy link
Contributor

Do we really have to invalidate the session or is it enough to invalidate the data in the session?

@DeepDiver1975
Copy link
Member

Do we really have to invalidate the session or is it enough to invalidate the data in the session?

from my understanding we need to invalidate the session - which can be done by marking the session as invalid and do a forced logout.

but the interesting point here is: we have no understanding when the password is changed on the ldap server - we basically have no other choice then let the session live until it expires.

we might need to readjust the session renewal mechanism - afaik a session will continue to live for ever (as in no password reentry by the user) if the session is reused often enough

@bboule
Copy link

bboule commented Jan 7, 2016

@felixboehm Could we add this to next weeks discussion as I dont seem to have a firm grasp of the priority here

MorrisJobke added a commit that referenced this issue Jan 10, 2016
* allows to drop the session from the database and then kill the active session automatically
* this check is done at most once every 5 seconds
* ref #18410
@MorrisJobke MorrisJobke mentioned this issue Jan 10, 2016
3 tasks
@MTRichards
Copy link
Contributor

Folks, I need a question answered.

Should this be an OCC command, such as occ: kill user_1 - this requires the admin to run an OCC command with the username specifically
OR
Should this be automatic in some way, so that the session (or equivalent) is killed within 60 min (or so) of the LDAP deletion?

It has an impact on the design and architecture.

@MorrisJobke
Copy link
Contributor

It has an impact on the design and architecture.

I guess both, but at most the second one. Also something like killing your own other sessions would be nice. See how github handles it: https://github.com/settings/security

@cdamken
Copy link
Contributor

cdamken commented Mar 29, 2016

For my specific request the user has no more access to the LDAP, but its still logged in the sync-client and continue downloading and uploading files, the user can't log over the webUI because of the credentials.

Should this be automatic in some way, so that the session (or equivalent) is killed within 60 min (or so) of the LDAP deletion?

That would be enough for my customer.

@MorrisJobke
Copy link
Contributor

That would be enough for my customer.

This nevertheless requires the ownCloud knows that user <-> session relationship which is currently not possible.

@MTRichards
Copy link
Contributor

Work package:

  1. Complete Morris' workaround (see above)
  2. add occ command to handle the reset users
  3. connect this into LDAP so it works for LDAP users
    a) for delete users (<-- this is the critical use case)
    b) for change password

@MorrisJobke

@MTRichards
Copy link
Contributor

Estimate: 2 weeks.

DeepDiver1975 pushed a commit that referenced this issue Apr 7, 2016
* allows to drop the session from the database and then kill the active session automatically
* this check is done at most once every 5 seconds
* ref #18410
@MorrisJobke
Copy link
Contributor

@ChristophWurst Is this already covered by your pluggable auth

@ChristophWurst
Copy link
Contributor

@MorrisJobke yes, that is covered. The session token contains the encrypted login password which is re-checked about once per 5 minutes and the user is logged out if the password was changed.

I did not test with LDAP, but the implementation should work with any user backend the user manager queries.

@MorrisJobke MorrisJobke removed their assignment May 17, 2016
@lock
Copy link

lock bot commented Aug 4, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests