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

[Bug]: passwordsalt migration missing #34780

Closed
6 of 9 tasks
feanor12 opened this issue Oct 24, 2022 · 39 comments · Fixed by #35368
Closed
6 of 9 tasks

[Bug]: passwordsalt migration missing #34780

feanor12 opened this issue Oct 24, 2022 · 39 comments · Fixed by #35368
Assignees
Labels
1. to develop Accepted and waiting to be taken care of 25-feedback bug

Comments

@feanor12
Copy link

⚠️ This issue respects the following points: ⚠️

  • This is a bug, not a question or a configuration/webserver/proxy issue.
  • This issue is not already reported on Github (I've searched it).
  • Nextcloud Server is up to date. See Maintenance and Release Schedule for supported versions.
  • Nextcloud Server is running on 64bit capable CPU, PHP and OS.
  • I agree to follow Nextcloud's Code of Conduct.

Bug description

Upgrading to 25 with a config.php that has no or an empty passwordsalt results in an error message.
There is no clear solution on how to introduce a passwordsalt setting to an older setup.

Adiddional description on discord: https://help.nextcloud.com/t/passwordsalt-missing-from-config-php/148081/2

Steps to reproduce

1.install 24
2.use a config.php without a passwordsalt
3.upgrade to 25

Expected behavior

A warning before the upgrade or migration/guide to use salted password hashes.

Possible questions:
Does setting passwordsalt make passwords unusable?
What must be done to migrate this setting?

Installation method

Community Docker image

Operating system

Other

PHP engine version

PHP 8.1

Web server

Nginx

Database engine version

MariaDB

Is this bug present after an update or on a fresh install?

Updated to a major version (ex. 22.2.3 to 23.0.1)

Are you using the Nextcloud Server Encryption module?

Encryption is Disabled

What user-backends are you using?

  • Default user-backend (database)
  • LDAP/ Active Directory
  • SSO - SAML
  • Other

Configuration report

{
    "system": {
        "overwrite.cli.url": "https:\/\/nextcloud.domain.at",
        "enable_previews": false,
        "overwriteprotocol": "https",
        "datadirectory": "***REMOVED SENSITIVE VALUE***",
        "dbtype": "mysql",
        "version": "25.0.0.18",
        "dbname": "***REMOVED SENSITIVE VALUE***",
        "dbhost": "***REMOVED SENSITIVE VALUE***",
        "dbtableprefix": "oc_",
        "dbuser": "***REMOVED SENSITIVE VALUE***",
        "dbpassword": "***REMOVED SENSITIVE VALUE***",
        "installed": true,
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "maintenance": false,
        "theme": "",
        "forcessl": true,
        "trusted_domains": [
            "owncloud.domain.at",
            "nextcloud.domain.at"
        ],
        "trusted_proxies": "***REMOVED SENSITIVE VALUE***",
        "overwritehost": "nextcloud.domain.at",
        "share_folder": "\/Shared",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "loglevel": 0,
        "updater.release.channel": "stable",
        "htaccess.RewriteBase": "\/",
        "memcache.local": "\\OC\\Memcache\\APCu",
        "memcache.locking": "\\OC\\Memcache\\Redis",
        "filelocking.enabled": true,
        "redis": {
            "host": "***REMOVED SENSITIVE VALUE***",
            "port": 6379,
            "password": "***REMOVED SENSITIVE VALUE***"
        },
        "apps_paths": [
            {
                "path": "\/var\/www\/html\/apps",
                "url": "\/apps",
                "writable": false
            },
            {
                "path": "\/var\/www\/html\/custom_apps",                "url": "\/custom_apps",
                "writable": true
            }
        ],
        "mail_smtpmode": "smtp",
        "mail_smtphost": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpport": "25",
        "mail_sendmailmode": "smtp",
        "mail_domain": "***REMOVED SENSITIVE VALUE***",
        "mail_from_address": "***REMOVED SENSITIVE VALUE***",
        "default_phone_region": "AT",
        "mysql.utf8mb4": true
    }
}

List of activated Apps

Enabled:
  - activity: 2.17.0
  - admin_audit: 1.15.0
  - calendar: 4.0.1
  - circles: 25.0.0
  - cloud_federation_api: 1.8.0
  - comments: 1.15.0
  - contacts: 5.0.1
  - contactsinteraction: 1.6.0
  - dashboard: 7.5.0
  - dav: 1.24.0
  - deck: 1.8.0
  - federatedfilesharing: 1.15.0
  - federation: 1.15.0
  - files: 1.20.1
  - files_external: 1.17.0
  - files_pdfviewer: 2.6.0
  - files_rightclick: 1.4.0
  - files_sharing: 1.17.0
  - files_trashbin: 1.15.0
  - files_versions: 1.18.0
  - firstrunwizard: 2.14.0
  - logreader: 2.10.0
  - lookup_server_connector: 1.13.0
  - nextcloud_announcements: 1.14.0
  - notes: 4.6.0
  - notifications: 2.13.1
  - oauth2: 1.13.0
  - password_policy: 1.15.0
  - photos: 2.0.0
  - privacy: 1.9.0
  - provisioning_api: 1.15.0
  - recommendations: 1.4.0
  - related_resources: 1.0.1
  - serverinfo: 1.15.0
  - settings: 1.7.0
  - sharebymail: 1.15.0
  - spreed: 15.0.0
  - support: 1.8.0
  - survey_client: 1.13.0
  - systemtags: 1.15.0
  - tasks: 0.14.5
  - text: 3.6.0
  - theming: 2.0.0
  - twofactor_backupcodes: 1.14.0
  - updatenotification: 1.15.0
  - user_status: 1.5.0
  - viewer: 1.9.0
  - weather_status: 1.5.0
  - workflowengine: 2.7.0
Disabled:
  - bruteforcesettings: 2.4.0
  - encryption
  - files_texteditor: 2.14.0
  - suspicious_login
  - twofactor_totp
  - user_ldap

Nextcloud Signing status

No response

Nextcloud Logs

No response

Additional info

No response

@feanor12 feanor12 added 0. Needs triage Pending check for reproducibility or if it fits our roadmap bug labels Oct 24, 2022
@llucps
Copy link

llucps commented Oct 28, 2022

It happened to me too after upgrading to Nextcloud 25

@szaimen
Copy link
Contributor

szaimen commented Oct 28, 2022

cc @juliushaertl @CarlSchwan

@rakekniven
Copy link
Member

rakekniven commented Oct 28, 2022

Same here after upgrading from v24.0.6 to v25.0.0

Bildschirmfoto 2022-10-28 um 17 09 19

My original installation is very old and based on OC3. Did every update (major or minor) since then.
Just checked my backups and there has been no passwordsalt variable set in config.php at all.

@rakekniven
Copy link
Member

I just added the value to the config.php by using
php /var/www/nextcloud/occ config:system:set passwordsalt --type=string --value=''

But still no change.

@CarlSchwan
Copy link
Member

try:

 php /var/www/nextcloud/occ config:system:set passwordsalt --type=string --value='ReplaceThisTest'

the passwordsalt needs to be not empty

@rakekniven
Copy link
Member

rakekniven commented Oct 28, 2022

@CarlSchwan Ok, but setting a salt on an existing installation with users will not prevent users to login?

Minutes ago I tried https://help.nextcloud.com/t/passwordsalt-missing-from-config-php/148081/2 and for sure this works as a workaround.

@FrenchHope
Copy link

FrenchHope commented Oct 29, 2022

Oddly the patch doesn't work on my instance...

edit : it works. Cache problem.

But I can't confirm actions like updating apps, the button is greyed out (maybe a browser problem)

@feanor12
Copy link
Author

What patch did you apply?

  • Set a non empty passwordsalt
  • Remove config check for passwordsalt

@FrenchHope
Copy link

FrenchHope commented Oct 29, 2022

I removed config check for passwordsalt as described here :

https://help.nextcloud.com/t/passwordsalt-missing-from-config-php/148081/2

(not a bug in my browser as I thought)

edit : it's a display bug, pressing enter works ! #34828

@feanor12
Copy link
Author

As far as I understand hashes, changing the salt would invalidate them.
Therefore, to migrate to a new salt all hashes must be updated and because this would require the actually passwords this has to be triggered by the user.

A procedure like this might work, but I know very little about the inner workings of nextcloud:

  • Admin forces a password hash update on login
  • User uses the old/empty salt for the next login
  • The entered password value is used to generate a new hash using the new salt
  • To allow this to work for more than one user the migration has to be tracked for all users

However, I don't know how to trigger such a password change.

Furthermore, it could be that the hashes are also used for other things that are not user passwords, making migration more complicated.

@llucps
Copy link

llucps commented Nov 1, 2022

Sorry but I'm still confused what the outcome of all this is,

As far as I know I've never had passwordsalt in my config file and my Nextcloud installation is as old as the first Nextcloud version releases in 2015-2016?

It would be great that someone could point out what needs to be done.. for the moment I just removed the passwordsalt from:

foreach (['secret', 'instanceid', 'passwordsalt'] as $requiredConfig) { if ($config->getValue($requiredConfig, '') === '' && !\OC::$CLI && $config->getValue('installed', false)) { $errors[] = [

But messing with the code it's not obviously a solution.

Thanks,

@rakekniven
Copy link
Member

rakekniven commented Nov 1, 2022

My understanding so far:
Old installations does not have a passwordsalt in their config.
New ones have one.

NC v25.0.0 is checking for it and here the trouble started for old installations.
Workaround is to modify code.

My question already added to a previous post is "Will setting a salt on an existing installation with users prevent these users to login?"
If so it needs a solution.

@wolegis
Copy link

wolegis commented Nov 2, 2022

This passwordsalt parameter is a password pepper. A superficial research in the code revealed that it has been around since at least 2014 - but was not enforced until recently. So apparently we have a lot of installations without using passwordsalt. Setting this parameter subsequently is all but trivial. This would require to recalculate all hashes in the database. This cannot be done automatically since it requires knowledge of all cleartext passwords.

Mind that just setting this parameter without a migration will render all your accounts inaccessible.

IMHO the only option is to drop the enforcing of passwordsalt. In case someone forgot to set this parameter during initial setup he/she has to live with it. The security impact is negligible since according to comments in config.sample.php there is also a per-user salt.

@szaimen
Copy link
Contributor

szaimen commented Nov 2, 2022

IMHO the only option is to drop the enforcing of passwordsalt. In case someone forgot to set this parameter during initial setup he/she has to live with it. The security impact is negligible since according to comments in config.sample.php there is also a per-user salt.

cc @juliushaertl @PVince81 @juliushaertl for opinions on that

@wolegis
Copy link

wolegis commented Nov 2, 2022

IMHO the only option is to drop the enforcing of passwordsalt.

Let me clarify my statement:

passwordsalt should not be enforced during normal operation. But it should be enforced during initial setup, e.g. when running occ maintenance:install.

@CarlSchwan
Copy link
Member

Passwordsalt is used in lib/private/Security/Hasher.php only, and this includes a fallback if it was not set previously to try with an empty salt

@rakekniven Did you actually try to setup a passwordsalt and then you couldn't login again?

@rakekniven
Copy link
Member

rakekniven commented Nov 3, 2022

Did you actually try to setup a passwordsalt and then you couldn't login again?

@CarlSchwan No, how to create such an salt?
I will happily crash my prod installation 😄 and report back.

@wolegis
Copy link

wolegis commented Nov 6, 2022

I gave it a try. Generated a password salt with

base64 </dev/urandom | head -c 30

and inserted that into /etc/webapps/nextcloud/config/config.php. To my surprise login was still possible. Changing passwords also worked.

After that I upgraded to 25.0.1(from 24.0.6). Still logging in and changing passwords is possible.

Meanwhile I'm pretty sure that passwordsalt is not taken into account at the moment - at least not for the Argon2ID password hashes in table oc_users. I would strongly appreciate that the Nextcloud developers enlighten the community about:

  • Why the config parameter passwordsalt became mandatory?
  • Where and under which circumstances the parameter is actually used?
  • Whether there are any plans to use passwordsalt for the hashes in table oc_users?

@cripton
Copy link

cripton commented Nov 7, 2022

I gave it a try. Generated a password salt with

dd if=/dev/urandom of=/dev/stdout count=1 2>/dev/null | base64 | head -c 30

and inserted that into /etc/webapps/nextcloud/config/config.php. To my surprise login was still possible. Changing passwords also worked.

After that I upgraded to 25.0.1(from 24.0.6). Still logging in and changing passwords is possible.

Meanwhile I'm pretty sure that passwordsalt is not taken into account at the moment - at least not for the Argon2ID password hashes in table oc_users. I would strongly appreciate that the Nextcloud developers enlighten the community about:

* Why the config parameter `passwordsalt` became mandatory?

* Where and under which circumstances the parameter is actually used?

* Whether there are any plans to use `passwordsalt` for the hashes in table `oc_users`?

i have to add the "secret" parameter also, and everything works now

@feanor12
Copy link
Author

feanor12 commented Nov 8, 2022

I also tested it and adding the salt like #34780 (comment) seems to work.

@llucps
Copy link

llucps commented Nov 8, 2022

Yes it worked for me to @feanor12.

Thanks.

@knfoo
Copy link

knfoo commented Nov 12, 2022

@cripton

i have to add the "secret" parameter also, and everything works now

I had the passwordsalt but not the secret what did you do to make it work ?
I have this problem now

                                      Error
    The required secret config variable is not configured in the config.php file.
    Please ask your server administrator to check the Nextcloud configuration.

If I put something into secret it is not working 🤔

@wolegis
Copy link

wolegis commented Nov 13, 2022

                                      Error
    The required secret config variable is not configured in the config.php file.
    Please ask your server administrator to check the Nextcloud configuration.

If I put something into secret it is not working thinking

Maybe a typo. Try this and put the output in your config.php:

printf "'secret' => '%s',\n" "$(base64 </dev/urandom | head -c 48)"

Make sure there are no other lines starting with 'secret' =>.

@knfoo
Copy link

knfoo commented Nov 14, 2022

@wolegis Thank you for your hint.
I tried to do this and now I get a different error.


Internal Server Error

The server was unable to complete your request.

If this happens again, please send the technical details below to the server administrator.

More details can be found in the server log.
Technical details

    Remote Address: 192.168.1.56
    Request ID: OOCZBFO4Qy66cmdcNXSK
    Type: Exception
    Code: 0
    Message: HMAC does not match.
    File: /var/www/html/lib/private/Security/Crypto.php
    Line: 156


Trace

#0 /var/www/html/apps/twofactor_totp/lib/Service/Totp.php(138): OC\Security\Crypto->decrypt('3424239a680f5a3...')
#1 /var/www/html/apps/twofactor_totp/lib/Provider/TotpProvider.php(105): OCA\TwoFactorTOTP\Service\Totp->validateSecret(Object(OC\User\User), '105285')
#2 /var/www/html/lib/private/Authentication/TwoFactorAuth/Manager.php(268): OCA\TwoFactorTOTP\Provider\TotpProvider->verifyChallenge(Object(OC\User\User), '105285')
#3 /var/www/html/core/Controller/TwoFactorChallengeController.php(182): OC\Authentication\TwoFactorAuth\Manager->verifyChallenge('totp', Object(OC\User\User), '105285')
#4 /var/www/html/lib/private/AppFramework/Http/Dispatcher.php(225): OC\Core\Controller\TwoFactorChallengeController->solveChallenge('totp', '105285', '/apps/dashboard...')
#5 /var/www/html/lib/private/AppFramework/Http/Dispatcher.php(133): OC\AppFramework\Http\Dispatcher->executeController(Object(OC\Core\Controller\TwoFactorChallengeController), 'solveChallenge')
#6 /var/www/html/lib/private/AppFramework/App.php(172): OC\AppFramework\Http\Dispatcher->dispatch(Object(OC\Core\Controller\TwoFactorChallengeController), 'solveChallenge')
#7 /var/www/html/lib/private/Route/Router.php(298): OC\AppFramework\App::main('OC\\Core\\Control...', 'solveChallenge', Object(OC\AppFramework\DependencyInjection\DIContainer), Array)
#8 /var/www/html/lib/base.php(1047): OC\Route\Router->match('/login/challeng...')
#9 /var/www/html/index.php(36): OC::handleRequest()
#10 {main}

I guess it is because my secret was not set when I initial installed my nextcloud years back 🤔

@wolegis
Copy link

wolegis commented Nov 14, 2022

@knfoo I think you have no other option than to switch back to the empty secret and remove the string 'secret' from the list in lib/private/legacy/OC_Util.php:1229.

Hopefully the Nextcloud developers come up with a migration path soon (not only for passwordsalt, but also for secret).

@knfoo
Copy link

knfoo commented Nov 14, 2022

@wolegis I did that.

cat /usr/src/nextcloud/lib/private/legacy/OC_Util.php | grep -A4 -B4 "variable is not configured in the config.php file"

                foreach ([ 'instanceid', 'passwordsalt'] as $requiredConfig) {
                        if ($config->getValue($requiredConfig, '') === '' && !\OC::$CLI && $config->getValue('installed', false)) {
                                $errors[] = [
                                        'error' => $l->t('The required %s config variable is not configured in the config.php file.', [$requiredConfig]),
                                        'hint' => $l->t('Please ask your server administrator to check the Nextcloud configuration.')
                                ];
                        }
                }

but still it says the value is missing 🤔 and my php skills is getting a little challenged to dig deeper.

I tried with 'secret' => '', and with no secret in the config.php file but with same result.

The log for this is:

{
  "reqId": "lcnnXiukbWMWqBLiD4D6",
  "level": 1,
  "time": "2022-11-14T12:02:57+00:00",
  "remoteAddr": "192.71.69.45",
  "user": "--",
  "app": "no app in context",
  "method": "GET",
  "url": "/",
  "message": "Unable to generate a URL for the named route \"settings.Help.help\" as such route does not exist.",
  "userAgent": "Mozilla/5.0 (X11; Linux x86_64; rv:106.0) Gecko/20100101 Firefox/106.0",
  "version": "25.0.1.1",
  "exception": {
    "Exception": "Symfony\\Component\\Routing\\Exception\\RouteNotFoundException",
    "Message": "Unable to generate a URL for the named route \"settings.Help.help\" as such route does not exist.",
    "Code": 0,
    "Trace": [
      {
        "file": "/var/www/html/lib/private/Route/Router.php",
        "line": 361,
        "function": "generate",
        "class": "Symfony\\Component\\Routing\\Generator\\UrlGenerator",
        "type": "->",
        "args": [
          "settings.Help.help",
          [],
          1
        ]
      },
      {
        "file": "/var/www/html/lib/private/Route/CachingRouter.php",
        "line": 58,
        "function": "generate",
        "class": "OC\\Route\\Router",
        "type": "->",
        "args": [
          "settings.Help.help",
          [],
          false
        ]
      },
      {
        "file": "/var/www/html/lib/private/URLGenerator.php",
        "line": 103,
        "function": "generate",
        "class": "OC\\Route\\CachingRouter",
        "type": "->",
        "args": [
          "settings.Help.help",
          []
        ]
      },
      {
        "file": "/var/www/html/lib/private/NavigationManager.php",
        "line": 197,
        "function": "linkToRoute",
        "class": "OC\\URLGenerator",
        "type": "->",
        "args": [
          "settings.Help.help"
        ]
      },
      {
        "file": "/var/www/html/lib/private/NavigationManager.php",
        "line": 113,
        "function": "init",
        "class": "OC\\NavigationManager",
        "type": "->",
        "args": []
      },
      {
        "file": "/var/www/html/apps/theming/lib/ThemingDefaults.php",
        "line": 188,
        "function": "getAll",
        "class": "OC\\NavigationManager",
        "type": "->",
        "args": [
          "guest"
        ]
      },
      {
        "file": "/var/www/html/lib/private/legacy/OC_Defaults.php",
        "line": 280,
        "function": "getShortFooter",
        "class": "OCA\\Theming\\ThemingDefaults",
        "type": "->",
        "args": []
      },
      {
        "file": "/var/www/html/lib/public/Defaults.php",
        "line": 177,
        "function": "getLongFooter",
        "class": "OC_Defaults",
        "type": "->",
        "args": []
      },
      {
        "file": "/var/www/html/core/templates/layout.guest.php",
        "line": 53,
        "function": "getLongFooter",
        "class": "OCP\\Defaults",
        "type": "->",
        "args": []
      },
      {
        "file": "/var/www/html/lib/private/Template/Base.php",
        "line": 180,
        "args": [
          "/var/www/html/core/templates/layout.guest.php"
        ],
        "function": "include"
      },
      {
        "file": "/var/www/html/lib/private/Template/Base.php",
        "line": 150,
        "function": "load",
        "class": "OC\\Template\\Base",
        "type": "->",
        "args": [
          "/var/www/html/core/templates/layout.guest.php",
          null
        ]
      },
      {
        "file": "/var/www/html/lib/private/legacy/OC_Template.php",
        "line": 181,
        "function": "fetchPage",
        "class": "OC\\Template\\Base",
        "type": "->",
        "args": [
          null
        ]
      },
      {
        "file": "/var/www/html/lib/private/legacy/OC_Template.php",
        "line": 212,
        "function": "fetchPage",
        "class": "OC_Template",
        "type": "->",
        "args": [
          null
        ]
      },
      {
        "file": "/var/www/html/lib/private/Template/Base.php",
        "line": 132,
        "function": "fetchPage",
        "class": "OC_Template",
        "type": "->",
        "args": []
      },
      {
        "file": "/var/www/html/lib/private/legacy/OC_Template.php",
        "line": 274,
        "function": "printPage",
        "class": "OC\\Template\\Base",
        "type": "->",
        "args": []
      },
      {
        "file": "/var/www/html/lib/base.php",
        "line": 693,
        "function": "printGuestPage",
        "class": "OC_Template",
        "type": "::",
        "args": [
          "",
          "error",
          [
            [
              [
                "The required secret config variable is not configured in the config.php file.",
                "Please ask your server administrator to check the Nextcloud configuration."
              ]
            ]
          ]
        ]
      },
      {
        "file": "/var/www/html/lib/base.php",
        "line": 1144,
        "function": "init",
        "class": "OC",
        "type": "::",
        "args": []
      },
      {
        "file": "/var/www/html/index.php",
        "line": 34,
        "args": [
          "/var/www/html/lib/base.php"
        ],
        "function": "require_once"
      }
    ],
    "File": "/var/www/html/3rdparty/symfony/routing/Generator/UrlGenerator.php",
    "Line": 143,
    "message": "Unable to generate a URL for the named route \"settings.Help.help\" as such route does not exist.",
    "exception": {},
    "CustomMessage": "Unable to generate a URL for the named route \"settings.Help.help\" as such route does not exist."
  }
}

@wolegis
Copy link

wolegis commented Nov 14, 2022

Restart your server. Old code is still present in your OPcache.

I quit here. This is not the place for personal support.

@knfoo
Copy link

knfoo commented Nov 14, 2022

Thank you for your input. I run Nextcloud in a container and build a new one to fix the foreach loop. The logs is from this newly build container so I would say that the OPcache is cleared.

Hopefully others can help me debug this issue.

@modzilla99
Copy link

@knfoo just went through into the same issue and obviously need to edit the same file in /var/www/html as well!

@knfoo
Copy link

knfoo commented Nov 15, 2022

@modzilla99 thank you! It has clearly been to long ago since I have installed Nextcloud, I totally forgot about that. It has just been working to well. Now it is working again for me 🙏

@jejanim
Copy link

jejanim commented Nov 15, 2022

Also running nextcloud in a container and got hit by this. @knfoo did you succeed yet?

@knfoo
Copy link

knfoo commented Nov 16, 2022

@jejanim I did with these steps. Not sure if the first is needed but did it non the less.

  1. build a new container based on upstream
FROM nextcloud:25.0.1-apache

RUN sed -i "s/'secret',//g" /usr/src/nextcloud/lib/private/legacy/OC_Util.php
  1. Since I have persisted the data (https://github.com/nextcloud/docker#persistent-data) I needed to fix /var/www/html/lib/private/legacy/OC_Util.php from within the container, the same way with sed or editor if you prefer that. You can also do it on the host system where the location will be different.

@invario
Copy link

invario commented Nov 19, 2022

Originally posted by @wolegis in #34780 (comment)

This worked perfectly for me thanks!

@ccoenen
Copy link

ccoenen commented Nov 21, 2022

upgrading 24 -> 25 has been the first rough update for years. Somewhat sad that this issue here is four weeks old, and it is still present in NC25.0.1.

In my case: i set a new, random passwordsalt to my existing config, and all old logins continued to work just fine. I just wish this was either automated or there was a warning somewhere along the way.

@PVince81
Copy link
Member

ah, interesting, so this is from old versions or coming from ownCloud.

I have an instance currently on NC 24 that was migrated two years ago from OC 10 and earlier, and indeed I don't have a password salt either.

@PVince81 PVince81 added 1. to develop Accepted and waiting to be taken care of and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Nov 22, 2022
@PVince81 PVince81 added this to the Nextcloud 25.0.2 milestone Nov 22, 2022
@CarlSchwan
Copy link
Member

I gave it a try. Generated a password salt with

base64 </dev/urandom | head -c 30

and inserted that into /etc/webapps/nextcloud/config/config.php. To my surprise login was still possible. Changing passwords also worked.

After that I upgraded to 25.0.1(from 24.0.6). Still logging in and changing passwords is possible.

Meanwhile I'm pretty sure that passwordsalt is not taken into account at the moment - at least not for the Argon2ID password hashes in table oc_users. I would strongly appreciate that the Nextcloud developers enlighten the community about:

* Why the config parameter `passwordsalt` became mandatory?

Because this increase the security as it allows new password to use it. Also this caused some application (e.g end to end encryption) to fail when secretwas not set

* Where and under which circumstances the parameter is actually used?

When hashing a password and in general using the Security\Hasher service. When comparing the hash with a previous stored hash we compare with both am empty passwordsalt and an the passwordsalt set in the config.php

* Whether there are any plans to use `passwordsalt` for the hashes in table `oc_users`?

It is used already but only when setting new passwords. We don't rehash old passwords

@malteger
Copy link

@CarlSchwan if passwordsalt is actually used (again) it might also be a good idea to remove the deprecation notice in the sample config file, as this was very confusing to me at first while looking at the docs.

The config entry has been officially deprecated in 2014 with 726626b

@wolegis
Copy link

wolegis commented Nov 23, 2022

@CarlSchwan

Many thanks for taking care of this issue and answering my questions.

  • Why the config parameter passwordsalt became mandatory?

Because this increase the security as it allows new password to use it. Also this caused some application (e.g end to end encryption) to fail when secretwas not set

  • Where and under which circumstances the parameter is actually used?

When hashing a password and in general using the Security\Hasher service. When comparing the hash with a previous stored hash we compare with both am empty passwordsalt and an the passwordsalt set in the config.php

  • Whether there are any plans to use passwordsalt for the hashes in table oc_users?

It is used already but only when setting new passwords. We don't rehash old passwords

I had a closer look at Security/Hasher. From what I understand your statements are not correct. Actually the passwordsalt is only used to verify very old (legacy) password hashes. These are recognised by their length of 60 characters (and absence of |). See function legacyHashVerify.

New password hashes (anything that has |s and a preceding version marker) are verified by using PHP's function password_verify in function verifyHash without ever using the password salt.

Hashes of new passwords are generated by applying PHP's function password_hash again without consideration of the password salt. See function hash.

Bottom line: For new passwords passwordsalt is not taken into account. Neither when calculating the hashes, nor when verifying the password hashes. There is no fallback to verify these password hashes first with and then without the password salt. There is no security gain whatsoever when passwordsalt is set in the configuration.

The only case when passwordsalt is taken into account is for verification of legacy password hashes of length 60 (and without any |). The enforcement of passwordsalt only makes sense in case there is at least one such password hash in table oc_users.

Please correct me if I'm wrong.

@j-ed
Copy link
Contributor

j-ed commented Jan 23, 2023

@PVince81 Based on the administrator documentation and the comment in the config.sample.php file this parameter has been deprecated and should never be used by a developer anymore. I remember that I removed it from the configuration file approximately 6 years ago (Nextcloud 11.0.0) and never had problems afterwards. I wonder why it has been reactivated now but the documentation haven't been updated?!

  @deprecated This salt is deprecated and only used for legacy-compatibility,
  developers should *NOT* use this value for anything nowadays.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of 25-feedback bug
Projects
None yet