LDAP authentication assumes passwords are reusable

nealey commented 6 years ago

### Steps to reproduce 1. Set up an LDAP server with non-reusable passwords, perhaps a corporate LDAP server backed by a RADIUS server linked with company-issued [time-based token generators](https://www.rsa.com/en-us/products/rsa-securid-suite) 2. Enable LDAP authentication in Nextcloud 3. Log in with generated token 4. Wait 5 minutes ### Expected behaviour I should stay logged in longer than 5 minutes at a time. ### Actual behaviour I am logged out after 5 minutes when Nextcloud tries to reauthenticate with my (non-reusable) login password. ### Workaround The following patch will skip the 5-minute password check: ``` --- lib/private/User/Session.php~ 2018-09-07 23:14:26.867485000 +0000 +++ lib/private/User/Session.php 2018-09-07 22:51:03.908411000 +0000 @@ -690,12 +690,14 @@ return true; } + if (false) { /* Kludge around LDAP with non-reusable passwords */ if ($this->manager->checkPassword($dbToken->getLoginName(), $pwd) === false || (!is_null($this->activeUser) && !$this->activeUser->isEnabled())) { $this->tokenProvider->invalidateToken($token); // Password has changed or user was disabled -> log user out return false; } + } $dbToken->setLastCheck($now); return true; } ``` Sorry, I'm having a lot of trouble pasting a tab character in here. Hopefully this patch is simple enough to recreate by hand. This code section is present in Nextcloud 14 as well. ### Server configuration **Operating system**: Container Linux by CoreOS 1800.7.0 (Rhyolite) **Web server**: Apache2 2.4.25-3+deb **Database**: MariaDB **PHP version**: 7.1.20 **Nextcloud version**: 13.0.4 **Updated from an older Nextcloud/ownCloud or fresh install**: Updated from an older Nextcloud **Where did you install Nextcloud from**: `docker run nextcloud:13.0.4` **Signing status:**

Signing status

``` No errors have been found. ```

**List of activated apps:**

App list

``` Enabled: - activity: 2.6.1 - bruteforcesettings: 1.1.0 - comments: 1.3.0 - dav: 1.4.7 - deck: 0.4.1 - federatedfilesharing: 1.3.1 - federation: 1.3.0 - files: 1.8.0 - files_external: 1.4.1 - files_pdfviewer: 1.2.1 - files_sharing: 1.5.0 - files_texteditor: 2.5.1 - files_trashbin: 1.3.0 - files_versions: 1.6.0 - files_videoplayer: 1.2.0 - firstrunwizard: 2.2.1 - gallery: 18.0.0 - logreader: 2.0.0 - lookup_server_connector: 1.1.0 - nextcloud_announcements: 1.2.0 - notes: 2.3.2 - notifications: 2.1.2 - oauth2: 1.1.1 - onlyoffice: 1.3.0 - passman: 2.1.4 - password_policy: 1.3.0 - provisioning_api: 1.3.0 - serverinfo: 1.3.0 - sharebymail: 1.3.0 - survey_client: 1.1.0 - systemtags: 1.3.0 - tasks: 0.9.6 - theming: 1.4.5 - twofactor_backupcodes: 1.2.3 - updatenotification: 1.3.0 - user_ldap: 1.3.1 - user_saml: 1.5.0 - workflowengine: 1.3.0 Disabled: - admin_audit - encryption - user_external ```

**Nextcloud configuration:**

Config report

``` I{ "system": { "instanceid": "***REMOVED SENSITIVE VALUE***", "passwordsalt": "***REMOVED SENSITIVE VALUE***", "secret": "***REMOVED SENSITIVE VALUE***", "trusted_domains": [ "nextcloud-main", "onlyoffice-document-server" ], "overwriteprotocol": "https", "overwritehost": "arcs.lanl.gov", "overwritewebroot": "\/nextcloud", "datadirectory": "***REMOVED SENSITIVE VALUE***", "lost_password_link": "disabled", "proxy": "proxyout.lanl.gov:8080", "dbtype": "mysql", "version": "13.0.4.0", "dbname": "***REMOVED SENSITIVE VALUE***", "dbhost": "***REMOVED SENSITIVE VALUE***", "dbtableprefix": "oc_", "mysql.utf8mb4": true, "dbuser": "***REMOVED SENSITIVE VALUE***", "dbpassword": "***REMOVED SENSITIVE VALUE***", "logtimezone": "UTC", "installed": true, "mail_smtpmode": "smtp", "mail_from_address": "***REMOVED SENSITIVE VALUE***", "mail_domain": "***REMOVED SENSITIVE VALUE***", "mail_smtphost": "***REMOVED SENSITIVE VALUE***", "mail_smtpport": "25", "memcache.local": "\\OC\\Memcache\\Redis", "memcache.locking": "\\OC\\Memcache\\Redis", "redis": { "host": "***REMOVED SENSITIVE VALUE***", "port": 6379 }, "theme": "", "loglevel": 0, "maintenance": false, "overwrite.cli.url": "https:\/\/arcs.lanl.gov\/nextcloud", "ldapIgnoreNamingRules": false, "ldapProviderFactory": "\\OCA\\User_LDAP\\LDAPProviderFactory" } } ```

**Are you using external storage, if yes which one**: local **Are you using encryption:** yes, with an haproxy front-end **Are you using an external user-backend, if yes which one:** LDAP #### LDAP configuration (delete this part if not used)

LDAP config

### Client configuration **Browser**: Chrome 68.0.3440.118 **Operating system**: ChromeOS 68.0.3440.118 ### Logs #### Web server error log

Web server error log

``` H00558: apache2: Could not reliably determine the server's fully qualified domain name, using 10.0.1.244. Set the 'ServerName' directive globally to suppress this message AH00558: apache2: Could not reliably determine the server's fully qualified domain name, using 10.0.1.244. Set the 'ServerName' directive globally to suppress this message [Fri Sep 07 18:15:44.748489 2018] [mpm_prefork:notice] [pid 1] AH00163: Apache/2.4.25 (Debian) PHP/7.1.20 configured -- resuming normal operations [Fri Sep 07 18:15:44.748582 2018] [core:notice] [pid 1] AH00094: Command line: 'apache2 -D FOREGROUND' [Fri Sep 07 18:20:21.355161 2018] [authz_core:error] [pid 29] [client 10.0.1.249:34068] AH01630: client denied by server configuration: /var/www/html/data/.ocdata [Fri Sep 07 18:20:24.690810 2018] [authz_core:error] [pid 30] [client 10.0.1.251:33810] AH01630: client denied by server configuration: /var/www/html/data/.ocdata [Fri Sep 07 18:28:29.828013 2018] [php7:warn] [pid 41] [client 10.0.1.251:34882] PHP Warning: Redis::connect(): php_network_getaddresses: getaddrinfo failed: Name or service not known in /var/www/html/lib/private/RedisFactory.php on line 84 [Fri Sep 07 18:28:29.828075 2018] [php7:warn] [pid 41] [client 10.0.1.251:34882] PHP Warning: Redis::connect(): connect() failed: php_network_getaddresses: getaddrinfo failed: Name or service not known in /var/www/html/lib/private/RedisFactory.php on line 84 [Fri Sep 07 18:28:29.829955 2018] [php7:error] [pid 41] [client 10.0.1.251:34882] PHP Fatal error: Uncaught RedisException: Redis server went away in /var/www/html/lib/private/Memcache/Redis.php:54\nStack trace:\n#0 /var/www/html/lib/private/Memcache/Redis.php(54): Redis->get('7b1b99d0725c301...')\n#1 /var/www/html/lib/autoloader.php(146): OC\\Memcache\\Redis->get('OC_User')\n#2 [internal function]: OC\\Autoloader->load('OC_User')\n#3 /var/www/html/lib/private/Log/File.php(104): spl_autoload_call('OC_User')\n#4 [internal function]: OC\\Log\\File::write('PHP', 'RedisException:...', 3)\n#5 /var/www/html/lib/private/Log.php(329): call_user_func(Array, 'PHP', 'RedisException:...', 3)\n#6 /var/www/html/lib/private/Log.php(179): OC\\Log->log(3, 'RedisException:...', Array)\n#7 /var/www/html/lib/private/Log/ErrorHandler.php(81): OC\\Log->critical('RedisException:...', Array)\n#8 [internal function]: OC\\Log\\ErrorHandler::onException(Object(RedisException))\n#9 {main}\n thrown in /var/www/html/lib/private/Memcache/Redis.php on line 54 [Fri Sep 07 18:28:29.830174 2018] [php7:error] [pid 41] [client 10.0.1.251:34882] PHP Fatal error: Uncaught RedisException: Redis server went away in /var/www/html/lib/private/Memcache/Redis.php:54\nStack trace:\n#0 /var/www/html/lib/private/Memcache/Redis.php(54): Redis->get('7b1b99d0725c301...')\n#1 /var/www/html/lib/autoloader.php(146): OC\\Memcache\\Redis->get('OC_User')\n#2 [internal function]: OC\\Autoloader->load('OC_User')\n#3 /var/www/html/lib/private/Log/File.php(104): spl_autoload_call('OC_User')\n#4 [internal function]: OC\\Log\\File::write('PHP', 'Uncaught RedisE...', 3)\n#5 /var/www/html/lib/private/Log.php(329): call_user_func(Array, 'PHP', 'Uncaught RedisE...', 3)\n#6 /var/www/html/lib/private/Log.php(179): OC\\Log->log(3, 'Uncaught RedisE...', Array)\n#7 /var/www/html/lib/private/Log/ErrorHandler.php(68): OC\\Log->critical('Uncaught RedisE...', Array)\n#8 [internal function]: OC\\Log\\ErrorHandler::onShutdown()\n#9 {main}\n thrown in /var/www/html/lib/private/Memcache/Redis.php on line 54 [Fri Sep 07 22:00:45.185345 2018] [authz_core:error] [pid 50] [client 10.0.1.250:46302] AH01630: client denied by server configuration: /var/www/html/data/.ocdata [Fri Sep 07 22:00:50.170253 2018] [authz_core:error] [pid 34] [client 10.0.1.249:41682] AH01630: client denied by server configuration: /var/www/html/data/.ocdata [Fri Sep 07 22:00:59.872722 2018] [authz_core:error] [pid 51] [client 10.0.1.250:46368] AH01630: client denied by server configuration: /var/www/html/data/.ocdata [Fri Sep 07 22:01:54.160050 2018] [authz_core:error] [pid 56] [client 10.0.1.251:52350] AH01630: client denied by server configuration: /var/www/html/data/.ocdata [Fri Sep 07 22:03:03.890282 2018] [authz_core:error] [pid 38] [client 10.0.1.251:52594] AH01630: client denied by server configuration: /var/www/html/data/.ocdata [Fri Sep 07 22:03:07.574277 2018] [authz_core:error] [pid 32] [client 10.0.1.249:41782] AH01630: client denied by server configuration: /var/www/html/data/.ocdata [Fri Sep 07 22:54:31.273827 2018] [authz_core:error] [pid 53] [client 10.0.1.251:56922] AH01630: client denied by server configuration: /var/www/html/data/.ocdata [Fri Sep 07 22:54:34.342268 2018] [authz_core:error] [pid 52] [client 10.0.1.250:56582] AH01630: client denied by server configuration: /var/www/html/data/.ocdata [Fri Sep 07 22:57:28.881287 2018] [authz_core:error] [pid 53] [client 10.0.1.251:57172] AH01630: client denied by server configuration: /var/www/html/data/.ocdata ```

#### Nextcloud log (data/nextcloud.log)

Nextcloud log

``` root@7c2ef2fb937d:/var/www/html/data# ls -lh nextcloud.log -rw-r----- 1 www-data www-data 641M Sep 7 23:25 nextcloud.log root@7c2ef2fb937d:/var/www/html/data# tail -n 4 nextcloud.log {"reqId":"4TYNJBJnX6BNc39qCqCV","level":0,"time":"2018-09-07T23:25:32+00:00","remoteAddr":"10.0.1.251","user":"--","app":"core","method":"GET","url":"\/nextcloud\/index.php\/login","message":"Scss is disabled for \/var\/www\/html\/core\/css\/jquery-ui-fixes.scss, ignoring","userAgent":"Go-http-client\/1.1","version":"13.0.4.0"} {"reqId":"4TYNJBJnX6BNc39qCqCV","level":0,"time":"2018-09-07T23:25:32+00:00","remoteAddr":"10.0.1.251","user":"--","app":"core","method":"GET","url":"\/nextcloud\/index.php\/login","message":"Scss is disabled for \/var\/www\/html\/core\/css\/server.scss, ignoring","userAgent":"Go-http-client\/1.1","version":"13.0.4.0"} {"reqId":"4TYNJBJnX6BNc39qCqCV","level":0,"time":"2018-09-07T23:25:32+00:00","remoteAddr":"10.0.1.251","user":"--","app":"core","method":"GET","url":"\/nextcloud\/index.php\/login","message":"Scss is disabled for \/var\/www\/html\/core\/css\/share.scss, ignoring","userAgent":"Go-http-client\/1.1","version":"13.0.4.0"} {"reqId":"4TYNJBJnX6BNc39qCqCV","level":0,"time":"2018-09-07T23:25:32+00:00","remoteAddr":"10.0.1.251","user":"--","app":"core","method":"GET","url":"\/nextcloud\/index.php\/login","message":"Scss is disabled for \/var\/www\/html\/core\/css\/jquery.ocdialog.scss, ignoring","userAgent":"Go-http-client\/1.1","version":"13.0.4.0"} ```

#### Browser log

Browser log

Not relevant

nextcloud-bot commented 6 years ago

GitMate.io thinks possibly related issues are https://github.com/nextcloud/server/issues/7156 (LDAP Invalid private key after password reset), https://github.com/nextcloud/server/issues/8704 (LDAP Authentication using start_tls), https://github.com/nextcloud/server/issues/7135 (LDAP password change not always working), https://github.com/nextcloud/server/issues/272 (LDAP Users not mapped and authentication fails), and https://github.com/nextcloud/server/issues/772 (LDAP Users not mapped and authentication fails).

blizzz commented 5 years ago

It's not the LDAP backend, but our token system assumes that they are. There's no way around currently.

nealey commented 5 years ago

Right now I'm looking at maintaining a one-off build, reapplying patches to each upstream release. If there's a preferred way out of this, please let me know and I'll start working on a merge request.

If nobody has a better way, turning my if false into something that checks a configuration value will be the commit I submit.

RumblyShip commented 5 years ago

Is this issue something that can be added as a goal for the main project? This timer causes any LDAP back end to be useless if configured with 2FA. I am attempting to integrate the Duo LDAP proxy but cannot deploy something that causes re-authentication every 5 minutes.

blizzz commented 5 years ago

@ChristophWurst your opinion?

ChristophWurst commented 5 years ago

If that's how some (LDAP) user back-ends work it's of course a problem.

I second https://github.com/nextcloud/server/issues/11113#issuecomment-420817079. We haven't considered this kind of setup so far, hence this does not work.

However, some Nextcloud components generally assume that the password is available (that's why we actually store and periodically check it), so this is just one of many parts that break apart if passwords are only used one time. I have no solution for this off the top of my head.

blizzz commented 5 years ago

However, some Nextcloud components generally assume that the password is available (that's why we actually store and periodically check it)

Isn't it essentially external storages when is in conjunction with login credentials? and at least server side encryption.

nealey commented 5 years ago

For what it's worth, nothing in my configuration appears to need a reusable password: my patch is working just fine in my organization. I would be okay with a configuration item like "This authentication backend does not use reusable passwords" and a big caveat saying that enabling the item would preclude using certain extended functionality that needs reusable passwords.

ChristophWurst commented 5 years ago

On a second thought, I remembered that we already support tokens/sessions without a password, hence the password is actually optional when a token is created: https://github.com/nextcloud/server/blob/0211e17e3fe7dd36ba9360de4cc70aaddfafa4c2/lib/private/Authentication/Token/IProvider.php#L35-L53. When these tokens are checked, the password check is omitted (obviously).

ChristophWurst commented 5 years ago

Isn't it essentially external storages when is in conjunction with login credentials? and at least server side encryption.

Yes, and also features in external apps like automatic account setup in Mail.

blizzz commented 5 years ago

When we just provide a flag in the config.php to not save passwords with the token, would it just work (i.e. apps not throwing exceptions)? that would be a rather cheap solution.

ChristophWurst commented 5 years ago

When we just provide a flag in the config.php to not save passwords with the token, would it just work (i.e. apps not throwing exceptions)? that would be a rather cheap solution.

That could be a cheap hack, yes. There are, however, other changes in this regard at #11390. Not sure if they make things easier or harder for this case.

rullzer commented 5 years ago

@nealey @RumblyShip

This seems like an enterprise use case to me. You might want to look into a Nextcloud subscription.

nealey commented 5 years ago

@rullzer can you explain to the people who are watching this bug how a Nextcloud subscription can help with one-time passwords in LDAP? I had offered to put in some serious time creating a patch that would be accepted upstream, and your request for money in response to that offer strikes me as offensive.

I had already added a Nextcloud subscription to my FY19 budget, and now that funds are available I was starting the process of doing this (my company moves very slowly). But now I'm reconsidering. Please help me get back to wanting to do unpaid work for your company by explaining how your sales pitch in a bug report on a DFSG-Free codebase shouldn't make me so angry.

rullzer commented 5 years ago

@nealey It was not my intend to offend you. I missed that you were offering to create a patch that would solve the issue in a sustainable way. Blame that on me maybe reading a bit to quickly. My apologies.

As for the general question. Issues our customers are having are of course higher prioritized to get solved. Which is one of the benefits of a subscription.

If you are still willing to move forward with this I'd be happy to give some pointers in the right direction and brainstorm on possible solutions.

devuan2 commented 5 years ago

Hi. I'm running into this issue as well using the Duo LDAP proxy as my auth backend (push request every 5 mins) and trying to figure out the best way to proceed. I can't really turn users loose on it the way it is. I'm thinking about trimming down checkTokenCredentials() to just automatically logout everyone (each individual login has a separate "timer") an hour after they login. Users removed from LDAP within an hour of their login could still have a valid session going which may not be ideal for some environments. Are there other reasons I may not want to do this?

--- Session.php.orig    2019-06-03 15:16:08.142037376 -0500
+++ Session.php 2019-06-03 15:23:31.810533938 -0500
@@ -672,34 +672,17 @@
                // This check is performed each 5 minutes
                $lastCheck = $dbToken->getLastCheck() ? : 0;
                $now = $this->timeFactory->getTime();
-               if ($lastCheck > ($now - 60 * 5)) {
+               if ($lastCheck > ($now - 60 * 60)) {
                        // Checked performed recently, nothing to do now
                        return true;
                }
-
-               try {
-                       $pwd = $this->tokenProvider->getPassword($dbToken, $token);
-               } catch (InvalidTokenException $ex) {
-                       // An invalid token password was used -> log user out
-                       return false;
-               } catch (PasswordlessTokenException $ex) {
-                       // Token has no password
-
-                       if (!is_null($this->activeUser) && !$this->activeUser->isEnabled()) {
-                               $this->tokenProvider->invalidateToken($token);
-                               return false;
-                       }
-
-                       $dbToken->setLastCheck($now);
-                       return true;
-               }
-
-               if ($this->manager->checkPassword($dbToken->getLoginName(), $pwd) === false
-                       || (!is_null($this->activeUser) && !$this->activeUser->isEnabled())) {
+      else
+      {
                        $this->tokenProvider->invalidateToken($token);
                        // Password has changed or user was disabled -> log user out
                        return false;
                }
+
                $dbToken->setLastCheck($now);
                return true;
        }

skjnldsv commented 4 years ago

@nealey have you tried the patch above? Cheers

ghost commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity and seems to be missing some essential information. It will be closed if no further activity occurs. Thank you for your contributions.

nealey commented 4 years ago

@nealey have you tried the patch above? Cheers

Which patch? The one I provided in the original 2018 bug report, or the one that logs everybody out unconditionally after an hour? I kind of like my solution better...

lu1as commented 4 years ago

I ran into the same problem as I'm using a FreeIPA LDAP backend with OTP. So the password has an OTP token as suffix. Would it be possible to have a setting which disables this check?

ghost commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity and seems to be missing some essential information. It will be closed if no further activity occurs. Thank you for your contributions.

nealey commented 4 years ago

Hello, bot! This issue still exists, how can we (the people affected by it) clarify things for you to help?

skjnldsv commented 4 years ago

It's my bad, the bot will keep pinging until the issue have been validated or not :) @rullzer @ChristophWurst @blizzz ?

devuan2 commented 4 years ago

I just ran into this issue again with a Duo implementation. Is there any hope of getting an official fix for this or is there a better way to implement one of the patches offered by myself or @nealey? Everyone is using MFA for everything now. If MFA isn't addressed in a newer version of Nextcloud (haven't looked yet) I would think it will need to be in the very near future.

PS: I should clarify, in my patch above automatically logout everyone shouldn't be interpreted as every currently logged in user is logged out on the hour or something like that. Each user has an hour after login to complete their upload/download before they are logged out. A prompt of somekind would be a nice addition I suppose but not sure how I would implement that.

ghost commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity and seems to be missing some essential information. It will be closed if no further activity occurs. Thank you for your contributions.

tuxick commented 2 years ago

Having the same problem using DUO Authentication Proxy.

ChristophWurst commented 2 years ago

The solution of the patch is to store the volatile password but never check it again. That way, if they were to use external storage, the storage would get a wrong password and fail. I rather like https://github.com/nextcloud/server/issues/11113#issuecomment-423916411 so that we don't store the password at all, with the implication that external storage and similar will not work if they require a user password.

The easiest implementation will be to let admins set this via config.php. A bit more sophisticated approach is to ask the user backend if the password is "stable".

alfonsrv commented 2 years ago

Unfortunately simply patching the Session.php doesn't work for v24 anymore.
Also: Integrity checks

PVince81 commented 2 years ago

For context: I think the purpose of the code in Session.php was to kick users out / disable them when their password has changed or their LDAP account gets disabled (which in some LDAP implementations requires setting a dummy password).

I guess having a switch to disable that behavior would be fine to cover for use cases where it doesn't make sense.

CarlSchwan commented 2 years ago

The solution where we don't store passwords can be found here: https://github.com/nextcloud/server/pull/32624 would anyone be willing to test it (on their test system) :)

manf0001 commented 1 year ago

Just wondering on the status for this issue? I have just setup a nextcloud Server (Nextcloud Hub 3 -25.0.2), using my FreeIPA with built in OTP. Which works great if I only login to the website.

But if I create a token for use on my android app, or in gnome, it will work but after a few minutes, the token no longer seems to work, and I'm being prompted to sign in again. Sounds like there are some interesting suggestions, just wondering if anything has been implemented for the next update?

Thanks

CarlSchwan commented 1 year ago

This was actually fixed with https://github.com/nextcloud/server/pull/33225

Put 'auth.storeCryptedPassword' => false, in your config.php and this should work

olewales commented 1 year ago

@CarlSchwan is this configuration variable documented anywhere? I found it only by sheer luck in this issue

joshtrichards commented 1 year ago

@olewales https://docs.nextcloud.com/server/latest/admin_manual/configuration_server/config_sample_php_parameters.html?highlight=storecryptedpassword#auth-storecryptedpassword

nextcloud / server

LDAP authentication assumes passwords are reusable #11113