App password/tokens must not be invalidated on password change

[nickvergessen]

I found this while discussing https://github.com/nextcloud/server/pull/2563 with @schiessle

Steps

1. Login
2. Create an app password
3. Setup sync client with 2.
4. Change password in personal settings

Expected

1. App passwords should still work, or
2. you should be warned that you need to regenerate all your app passwords.

Actual

Sync client auth fails

Same applies to the reset password, so maybe we should add a message there, like with encryption.

cc @ChristophWurst

[rullzer]

I would expect token to continue working if I change my password.

Now it might make sense to delete them if I do password recovery. But even then. The whole idea is that is is linked to 1 specific device/app/whatever.

[nickvergessen]

I wouldn't right away delete them, but mark them invalid or whatever, so people can still see the names of the app passwords they had to remember changing them everywhere.

[ChristophWurst]

I wouldn't right away delete them, but mark them invalid or whatever, so people can still see the names of the app passwords they had to remember changing them everywhere.

What's the benefit there? Just that you know which clients you had connected?

I would expect token to continue working if I change my password.

That would be great, of course. However, we had/have the requirement to have access to the login password at any time to reuse it for stuff like external storage. To make it clean I'd like to give a brief overview of what we currently do:

• the token entry in the database belongs to either a browser session or a device
• for browser sesssion, we take its session id as the secret token, device passwords get a random one, which at the same time is the password you enter into your client.
• those tokens are hashed before we store them in the database
• the login password is encrypted and stored with the db token. we use the token (before hashing ofc) as encryption password.

This means: if you want to update the db entry of a device/browser session you'd have to know the secret token. Therefore this is simply impossible with the current implementation.

@LukasReschke thoughts? Anything we can do about that?

[nickvergessen]

What's the benefit there? Just that you know which clients you had connected?

Yes, it helps you to remember to create new tokens for all devices

[LukasReschke]

@LukasReschke thoughts? Anything we can do about that?

We can in fact do something about it, at least to some degree. So we would have to have an unique private key for every user which is encrypted with the secret token. The password is then encrypted in the DB to this private key and once a new password is detected it is getting updated.

This means we'd at max break the tokens until somebody logged-in again with password. Then all tokens would work again.

[mightyBroccoli]

I just noticed that this issue still happens.

[ChristophWurst]

@mightyBroccoli would you like to help implement this? Feel free to dig into it.

The rule of thumb in this issue tracker is: if the ticket is still open and there's no referenced pull request, nobody fixed the bug/implemented the feature.

[jaark]

This behaviour has a really bad interaction with brute force protection if a user has a number of app passwords

[ghost]

Is there any work around for this by any chance? This is causing Nextcloud to be almost unusable in our deployment. We use FreeIPA with LDAP+OTP to authenticate to nextcloud It seems that every 5-10 minutes nextcloud deletes all app passwords. I assume this is because the password is "UserPass+Token" the password changes every 30 seconds.

[jaark]

That is correct. The app passwords contain an encrypted form of what Nextcloud sees as the base authentication password so whenever that changes all existing app passwords become unusable.

The only workaround could be if you can change the way you authenticate. If you are able to remove the OTP component from LDAP and if there is a compatible 2FA plugin for Nextcloud that works with your OTP system then you can configure things that way.

[ghost]

Hey Jack,

Thanks for the information, just so I am clear. Do you know if this is something the nextcloud team is looking into fixing in the future ?

Thanks again!

Sent from Ninehttp://www.9folders.com/

From: "Jack" notifications@github.com Sent: Sunday, April 1, 2018, 12:08 PM To: nextcloud/server server@noreply.github.com Cc: Chris Schnobb; Comment comment@noreply.github.com Subject: Re: [nextcloud/server] App password/tokens need to be invalidated on password change (#2581)

That is correct. The app passwords contain an encrypted form of what Nextcloud sees as the base authentication password so whenever that changes all existing app passwords become unusable.

The only workaround could be if you can change the way you authenticate. If you are able to remove the OTP component from LDAP and if there is a compatible 2FA plugin for Nextcloud that works with your OTP system then you can configure things that way.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/nextcloud/server/issues/2581#issuecomment-377797127, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AVvmueYC-GOio9oHPhQqTDw9_Ir_SJLdks5tkPttgaJpZM4LIu-Y.

[jaark]

I have no idea, sorry. I certainly hope so.

[rullzer]

I have been thinking a bit about this. The best way I think (as a first step).

1. When generating an app password generate a public+private keypair
2. The privatekey is stored encrypted with the token (just like we store the password now)
3. The public key is just added to the table
4. The password is encrypted with the public key and also added to the table

Now the logic for the tokens now stays pretty similar. Just the way to obtain the data changes.

But now we can listen to the password changed hook and just update all the password fields (since we have the public key).

[rullzer]

An open question is still how to signal validity of the token. Assume the password is changed on an external system (LDAP). How can we get this info and mark the token as invalid? Because we don't want to keep hammering the LDAP with the wrong password.

@blizzz suggestions?

[blizzz]

@rullzer well, well. LDAP does not tell us, if a password changed (at least nothing that's a standard). We would need to keep track of credentials that do work. And once they stop we can say, that something has happened. Since there's no standardized account locking thing, it is possible that one account was just disabled, temporarily, and the password restored later. Not sure if this would be an issue, perhaps not.

Another edge case we did not look at before, I think, is that an LDAP account can have several valid passwords. Some other service was utilizing this mechanism for tokens. I don't think it is common, and probably it is not an issue at all (since user facing it'll always be just one main password i guess). Just came to my mind.

[tessus]

Is there an update on this?

It is a design problem that app tokens are invalidated when a user's password is changed. Why would someone ever use device tokens in such a case? The tokens are supposed to be independent (that's why a revoke token action exists). But if all tokens are invalidated when the user's password is changed, it renders the use of app tokens useless.

Please look at other systems that use tokens, e.g. github. I do not know a single system except Nextcloud that invalidates tokens on a password change.

[ChristophWurst]

@tessus we are aware of this limitation. @rullzer is working on a fix at the moment: https://github.com/nextcloud/server/pull/9485 :wink:

[tessus]

@ChristophWurst Thank you for the info. Awesome change @rullzer. 👍

Is there a reason why such a change is not included in a point release? why wait for 14?

[rullzer]

Because it is a big change ;) While annoying the the current behaviour was actually the way it was designed.

In general I'm not a fan of backporting anything unless it is a security risk or eats data.

Also our stabilization period for NC14 is a lot longer than for the point releases. So it gets hopefully more testing etc.

[tessus]

Because it is a big change ;) While annoying the the current behaviour was actually the way it was designed.

Fortunately you changed the design for the better. 👍

In general I'm not a fan of backporting anything unless it is a security risk or eats data.

Makes sense. However, sometimes changes take a very long time (especially, if they are not backported) before they are available in the product and this can be very problematic.

Also our stabilization period for NC14 is a lot longer than for the point releases. So it gets hopefully more testing etc.

This is true, but I've noticed that the last 2 major releases had still serious issues even though the stabilization period was a lot longer. e.g. Had I migrated right away, I'd have had a broken cloud system. I'm just mentioning this to show off that this particular change will only affect me and my users in about another 9-12 months, which is a long time to live with this problem.

[r2evans]

Not back-porting this is resulting in a perceived denial-of-service anytime a user changes their password. Within a couple of minutes, all of their devices have failed multiple times, locking them out (and forcing me to fix things manually). This isn't just a "big design change", it is a significant usability fix for admins and users alike.

It must not be the norm, but my idea of what an app-token is for is to provide stability in clients when the user changes their (normal) password. It's an assumption, I guess, but also the behavior I see in every other app-token I use (github, gitlab, etc). I'm not slinging mud here, just admitting that I mis-read the documentation on it. Now that I had recommended all my users set this up, I'm going to have consequences for months.

[brunt82]

Today we had an issue with a core-switch, so that there was a general network error. Therfore the Nextcloud server was unable to retrieve the LDAP server. After fixing the core-switch, I noticed that all app passwords of the connected clients / devices were deleted. But app passwords of shut down devices still work now.

That means the app passwords will not only deleted after password change but also if the LDAP server is unavailable. Are the app passwords deleted as soon as the login with the stored LDAP password is impossible? Therefore there may be additional situations like user is temporarily disabled,...

[ChristophWurst]

Are the app passwords deleted as soon as the login with the stored LDAP password is impossible?

Yes, exactly that. The encrypted login password cannot be re-verified and therefore these app passwords are considered invalid.

[rullzer]

This is something that we want to look into now that we have the new apppasswords. But it requires more logic than there currently is.

[tessus]

Shouldn't the title be updated to "App password/tokens must not be invalidated on password change"?

I think we established in earlier comments that invalidating tokens on a password change is the opposite of why tokens are used in the first place.

[tessus]

@rullzer I've only read now your algorithm for making this work.

When generating an app password generate a public+private keypair The privatekey is stored encrypted with the token (just like we store the password now) The public key is just added to the table The password is encrypted with the public key and also added to the table

But this means that anybody who has access to the database can decrypt all passwords. Is this really how you want to handle passwords? Or am I missing something here? I mean, it should be clearly stated in the documentation that passwords are not stored securely. There's a reason why secure systems use hash algorithms.

[ChristophWurst]

But this means that anybody who has access to the database can decrypt all passwords. Is this really how you want to handle passwords? Or am I missing something here? I mean, it should be clearly stated in the documentation that passwords are not stored securely. There's a reason why secure systems use hash algorithms.

From a simple database snapshot, you definitely can't. You need the token (app password) to decrypt the private key which in turn is used to decrypt the user password. Only Nextcloud (application server) knows your app passwords. But anybody having access to your app server can always read your passwords on login, so this isn't a new attack vector.

[ChristophWurst]

General note: if you think you've found security issues please report them at https://hackerone.com/nextcloud!

[rullzer]

@tessus your token is only stored hashed (and salted) in the DB

[tessus]

@ChristophWurst thanks for the reply.

From a simple database snapshot, you definitely can't. You need the token (app password) to decrypt the private key which in turn is used to decrypt the user password. Only Nextcloud (application server) knows your app passwords. But anybody having access to your app server can always read your passwords on login, so this isn't a new attack vector.

I have to disagree here, at least partly. There are ways (e.g. using scrypt on the client) that the server does not know the credentials at all. However, I understand that in other cases someone could change the code to access and read the passwords. A secure system should never make a password available to anyone - not even the app server. I also understand that there might be reasons for that (access for external storage) or whatnot, but in such cases one should rather work with tokens/oauth in the first place, which also implies 3rd party providers (dropbox, s3, ...) have to support these auth mechanisms (and in most cases they do).

... security issues please report them at ...

I actually thought about it, but since a developer actually wrote the algorithm in this thread it was rather moot.

@rullzer also thank you for the reply.

[friendlyspiderfromtheneighborhood]

Hi there, are there any updates on this issue?...

[ArnY]

Hello, As stated by @rullzer it should be fixed in nextcloud 15, so that would be for December (december 6th according to nextcloud's roadmap)

[tessus]

@ArnY I think you are mistaken. According to @rullzer's comments above it should have been made available with NC 14. The code was merged into master on June 19th, thus the code should be in NC14. I do not know why there's a Nextcloud 15 milestone on this issue.

Maybe @rullzer can shed some light on this.

[rullzer]

Because it doesn't work yet with external user backends. This should make it into 15.

[r2evans]

Does AD/LDAP count as an external user backend? SAML?

[tessus]

@rullzer ok, so just to claify: it works with local users on NC14. so, if a user changes their password in nc14 (for a local user), their tokens won't be invalidated, right?

[rullzer]

@r2evans yes LDAP is external as we don't get notified on password changes

@tessus correct

[tessus]

Thanks for the info. Btw, the topic name still says the exact opposite of what this issue is about. (I've mentioned this a month ago.)

[rullzer]

Fixed with https://github.com/nextcloud/server/issues/11390