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]: hash_hkdf(): Argument #2 ($key) cannot be empty #34012

Open
6 of 9 tasks
j-ed opened this issue Sep 10, 2022 · 55 comments
Open
6 of 9 tasks

[Bug]: hash_hkdf(): Argument #2 ($key) cannot be empty #34012

j-ed opened this issue Sep 10, 2022 · 55 comments
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap 25-feedback bug feature: external storage

Comments

@j-ed
Copy link
Contributor

j-ed commented Sep 10, 2022

⚠️ 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

After upgrading my server from Nextcloud v24.0.4 to v24.0.5 and also upgrading to PHP8 (I don't know if this is of importance), the Nextcloud cron job generates the following error output:

ValueError: hash_hkdf(): Argument #2 ($key) cannot be empty in .../lib/private/Security/Crypto.php:97
Stack trace:
#0 .../nextcloud/lib/private/Security/Crypto.php(97): hash_hkdf('sha512', '')
#1 .../nextcloud/lib/private/Security/IdentityProof/Manager.php(110): OC\Security\Crypto->encrypt('-----BEGIN PRIV...')
#2 .../nextcloud/lib/private/Security/IdentityProof/Manager.php(133): OC\Security\IdentityProof\Manager->generateKey('user-juergen')
#3 .../nextcloud/lib/private/Security/IdentityProof/Manager.php(146): OC\Security\IdentityProof\Manager->retrieveKey('user-juergen')
#4 .../nextcloud/lib/private/Security/IdentityProof/Signer.php(64): OC\Security\IdentityProof\Manager->getKey(Object(OC\User\User))
#5 .../nextcloud/apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php(150): OC\Security\IdentityProof\Signer->sign('lookupserver', Array, Object(OC\User\User))
#6 .../nextcloud/lib/public/BackgroundJob/Job.php(79): OCA\LookupServerConnector\BackgroundJobs\RetryJob->run(Array)
#7 .../nextcloud/apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php(113): OCP\BackgroundJob\Job->execute(Object(OC\BackgroundJob\JobList), Object(OC\Log))
#8 .../nextcloud/cron.php(151): OCA\LookupServerConnector\BackgroundJobs\RetryJob->execute(Object(OC\BackgroundJob\JobList), Object(OC\Log))
#9 {main}

Steps to reproduce

I don't know how to reproduce it at the moment but it seems to occur if notification should be send-out.

Expected behavior

Execute background job without running into an error condition.

Installation method

Community Manual installation with Archive

Operating system

No response

PHP engine version

PHP 8.0

Web server

Apache (supported)

Database engine version

MariaDB

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

Updated from a minor version (ex. 22.2.3 to 22.2.4)

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

## Environment
#### Server Configuration
OS: Linux 5.15.64
Web server: Apache2 2.4.53
Database: MariaDB 10.3.35
PHP version: 8.0.20
Nextcloud version: 24.0.5

#### Client Configuration
Browser: Mozilla Firefox 104.0.2
Operating system: Windows 10

List of activated Apps

Enabled:
  - accessibility: 1.10.0
  - activity: 2.16.0
  - admin_audit: 1.14.0
  - announcementcenter: 6.3.1
  - apporder: 0.15.0
  - audioplayer: 3.3.0
  - bookmarks: 11.0.1
  - bruteforcesettings: 2.4.0
  - calendar: 3.5.0
  - circles: 24.0.1
  - cloud_federation_api: 1.7.0
  - comments: 1.14.0
  - contacts: 4.2.0
  - contactsinteraction: 1.5.0
  - dav: 1.22.0
  - event_update_notification: 1.5.0
  - external: 4.0.0
  - federatedfilesharing: 1.14.0
  - federation: 1.14.0
  - files: 1.19.0
  - files_accesscontrol: 1.14.1
  - files_antivirus: 3.3.1
  - files_automatedtagging: 1.14.0
  - files_downloadactivity: 1.13.0
  - files_external: 1.16.1
  - files_pdfviewer: 2.5.0
  - files_photospheres: 1.24.1
  - files_retention: 1.13.2
  - files_rightclick: 1.3.0
  - files_sharing: 1.16.2
  - files_trashbin: 1.14.0
  - files_versions: 1.17.0
  - files_videoplayer: 1.13.0
  - firstrunwizard: 2.13.0
  - groupfolders: 12.0.1
  - guests: 2.2.0
  - impersonate: 1.11.0
  - logreader: 2.9.0
  - lookup_server_connector: 1.12.0
  - mail: 1.13.8
  - maps: 0.2.1
  - metadata: 0.16.0
  - news: 18.1.1
  - nextcloud_announcements: 1.13.0
  - notes: 4.5.1
  - notifications: 2.12.1
  - notify_push: 0.4.0
  - oauth2: 1.12.0
  - password_policy: 1.14.0
  - photos: 1.6.0
  - previewgenerator: 5.0.0
  - privacy: 1.8.0
  - provisioning_api: 1.14.0
  - serverinfo: 1.14.0
  - settings: 1.6.0
  - sharebymail: 1.14.0
  - smb_test: 0.3.4
  - spreed: 14.0.4
  - suspicious_login: 4.2.0
  - systemtags: 1.14.0
  - tasks: 0.14.4
  - text: 3.5.1
  - theming: 1.15.0
  - twofactor_backupcodes: 1.13.0
  - twofactor_gateway: 0.20.0
  - twofactor_totp: 6.4.0
  - twofactor_webauthn: 0.3.1
  - unsplash: 1.2.5
  - updatenotification: 1.14.0
  - user_status: 1.4.0
  - viewer: 1.8.0
  - workflow_script: 1.9.0
  - workflowengine: 2.6.0
Disabled:
  - dashboard: 7.0.0
  - encryption
  - epubreader: 1.4.7
  - files_trackdownloads: 1.11.0
  - recommendations: 0.6.0
  - support: 1.4.0
  - survey_client: 1.1.0
  - twofactor_admin: 3.2.0
  - user_ldap
  - weather_status: 1.0.0

Nextcloud Signing status

No errors have been found.

Nextcloud Logs

.

Additional info

.

@j-ed j-ed added bug 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Sep 10, 2022
@solracsf
Copy link
Member

Do you have a 'secret' value on your config.php file?

@j-ed
Copy link
Contributor Author

j-ed commented Sep 11, 2022

@solracsf Thank you, it seems that the parameter got lost somehow during one of the last couple of updates. I've now added the parameter again and will report if the error reappears again.

@CarlSchwan
Copy link
Member

If secret is missing, you will see this error. Once you add it back, it should work again.

Please reopen this issue if it happens again after adding the secret parameter

@j-ed
Copy link
Contributor Author

j-ed commented Sep 12, 2022

@CarlSchwan The problem seems to be more complex than I expected at the beginning. I found out that the secret parameter hasn't been set in the past at all on the server so that an empty value was used for encrypting passwords etc. The system excepted that setting and never complained about an empty value before.
If I set 'secret' => '', in the configuration the displayed exception changes as follow - what is understandable somehow.

... -","app":"cron","method":"","url":"/cron.php","message":"hash_hkdf(): Argument #2 ($key) cannot be empty",

Next I've provided a value to the secret parameter to get rid of the reported messages. As expected this had a bigger impact on the server because all application passwords and mount passwords hd to be changed. I tried to get this done and was able to successfully set new app passwords but I was unable to access my personal external storage anymore. As soon as I access the external storage configuration page I get an exception displayed on the screen with the recommendation to check the server log file.
The server log files shows a HMAC does not match. exception, independently if mounts exists or not. So by setting a value for the secret parameter external storages got unusable. Now I'm sitting between two chairs and can only tell you that the incompatibility seems to have been introduced with NC 24.0.5.

@j-ed j-ed reopened this Sep 12, 2022
@solracsf
Copy link
Member

solracsf commented Sep 12, 2022

Secret is used in many places.
There are PRs ongoing about this:

#31499
#31492

@CarlSchwan
Copy link
Member

#31499 is working and you can apply the patch. The reason why it is wip is that we wanted to have a migration step in the background adding the secret if it was missing :(

I'll try to revive the branches

@CarlSchwan CarlSchwan self-assigned this Sep 13, 2022
@j-ed
Copy link
Contributor Author

j-ed commented Sep 16, 2022

@CarlSchwan Unfortunately I've already fixed the problem on my server. After setting the 'secret' parameter in the configuration I truncated the `oc_storages_credentials' table, which allowed me to access the external storage settings again and to enter new credentials. After that I had to set all application passwords again because the previously generated once didn't work anymore. Additionally I had to reconnect the TOTP applications of my users again. Finally I had to drop all mail tables and reinstalled the mail app to get it up-and-running again. For now it seems to work. Nevertheless the provided patch will be the right way to go, because otherwise many systems will most likely left in an insecure state.

@CarlSchwan
Copy link
Member

I merged the related PR

@denics

This comment was marked as duplicate.

@fritzchentastig
Copy link

same problem here
i checked and the value is here

@Dazgard
Copy link

Dazgard commented Dec 19, 2022

Hello,
same issue, secret has always been there, and still failing.

@mattiosystems
Copy link

mattiosystems commented Dec 22, 2022

I found a temporary solution. In the exception part, put $secret instead of empty ''. After the change, the code should look like this:
[...]
catch (Exception $e) {
if ($password === '') {
// Retry with empty secret as a fallback for instances...
return $this->decryptWithoutSecret($authenticatedCiphertext, $secret);
}
[...]
File is located in nextcloud/lib/private/Security/Crypto.php, edit line 134.

@denics
Copy link

denics commented Dec 22, 2022

Thank you! apparently it works quite well!

@denics

This comment was marked as resolved.

@denics
Copy link

denics commented Feb 3, 2023

I don't know if it is linked or not, but I am facing the same error when I click on external storages:

{
  "reqId": "",
  "level": 3,
  "time": "2023-02-03T00:10:22+00:00",
  "remoteAddr": "192.168.1.200",
  "user": "denix",
  "app": "index",
  "method": "GET",
  "url": "/settings/admin/externalstorages",
  "message": "HMAC does not match.",
  "userAgent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:109.0) Gecko/20100101 Firefox/109.0",
  "version": "25.0.3.2",
  "exception": {
    "Exception": "Exception",
    "Message": "HMAC does not match.",
    "Code": 0,
    "Trace": [
      {
        "file": "/var/www/nextcloud/lib/private/Security/Crypto.php",
        "line": 134,
        "function": "decryptWithoutSecret",
        "class": "OC\\Security\\Crypto",
        "type": "->",
        "args": [
          "*** sensitive parameters replaced ***"
        ]
      },
      {
        "file": "/var/www/nextcloud/lib/private/Security/CredentialsManager.php",
        "line": 104,
        "function": "decrypt",
        "class": "OC\\Security\\Crypto",
        "type": "->",
        "args": [
          "*** sensitive parameters replaced ***"
        ]
      },
      {
        "file": "/var/www/nextcloud/apps/files_external/lib/Lib/Auth/Password/GlobalAuth.php",
        "line": 55,
        "function": "retrieve",
        "class": "OC\\Security\\CredentialsManager",
        "type": "->"
      },
      {
        "file": "/var/www/nextcloud/apps/files_external/lib/Settings/Admin.php",
        "line": 71,
        "function": "getAuth",
        "class": "OCA\\Files_External\\Lib\\Auth\\Password\\GlobalAuth",
        "type": "->"
      },
      {
        "file": "/var/www/nextcloud/apps/settings/lib/Controller/CommonSettingsTrait.php",
        "line": 129,
        "function": "getForm",
        "class": "OCA\\Files_External\\Settings\\Admin",
        "type": "->"
      },
      {
        "file": "/var/www/nextcloud/apps/settings/lib/Controller/AdminSettingsController.php",
        "line": 83,
        "function": "formatSettings",
        "class": "OCA\\Settings\\Controller\\AdminSettingsController",
        "type": "->"
      },
      {
        "file": "/var/www/nextcloud/apps/settings/lib/Controller/CommonSettingsTrait.php",
        "line": 149,
        "function": "getSettings",
        "class": "OCA\\Settings\\Controller\\AdminSettingsController",
        "type": "->"
      },
      {
        "file": "/var/www/nextcloud/apps/settings/lib/Controller/AdminSettingsController.php",
        "line": 68,
        "function": "getIndexResponse",
        "class": "OCA\\Settings\\Controller\\AdminSettingsController",
        "type": "->"
      },
      {
        "file": "/var/www/nextcloud/lib/private/AppFramework/Http/Dispatcher.php",
        "line": 225,
        "function": "index",
        "class": "OCA\\Settings\\Controller\\AdminSettingsController",
        "type": "->"
      },
      {
        "file": "/var/www/nextcloud/lib/private/AppFramework/Http/Dispatcher.php",
        "line": 133,
        "function": "executeController",
        "class": "OC\\AppFramework\\Http\\Dispatcher",
        "type": "->"
      },
      {
        "file": "/var/www/nextcloud/lib/private/AppFramework/App.php",
        "line": 172,
        "function": "dispatch",
        "class": "OC\\AppFramework\\Http\\Dispatcher",
        "type": "->"
      },
      {
        "file": "/var/www/nextcloud/lib/private/Route/Router.php",
        "line": 298,
        "function": "main",
        "class": "OC\\AppFramework\\App",
        "type": "::"
      },
      {
        "file": "/var/www/nextcloud/lib/base.php",
        "line": 1047,
        "function": "match",
        "class": "OC\\Route\\Router",
        "type": "->"
      },
      {
        "file": "/var/www/nextcloud/index.php",
        "line": 36,
        "function": "handleRequest",
        "class": "OC",
        "type": "::"
      }
    ],
    "File": "/var/www/nextcloud/lib/private/Security/Crypto.php",
    "Line": 169,
    "CustomMessage": "--"
  }
}

@solracsf
Copy link
Member

About HMAC there is a report at #32258

@denics
Copy link

denics commented Mar 16, 2023

In version 27.0.0 there is still this issue. However, Crypto.php changed and now you should apply #34012 (comment) to line 132

You should have:

	/**
	 * Decrypts a value and verifies the HMAC (Encrypt-Then-Mac)
	 * @param string $authenticatedCiphertext
	 * @param string $password Password to encrypt, if not specified the secret from config.php will be taken
	 * @return string plaintext
	 * @throws Exception If the HMAC does not match
	 * @throws Exception If the decryption failed
	 */
	public function decrypt(string $authenticatedCiphertext, string $password = ''): string {
		$secret = $this->config->getSystemValue('secret');
		try {
			if ($password === '') {
				return $this->decryptWithoutSecret($authenticatedCiphertext, $secret);
			}
			return $this->decryptWithoutSecret($authenticatedCiphertext, $password);
		} catch (Exception $e) {
			if ($password === $secret) {
				// Retry with empty secret as a fallback for instances where the secret might not have been set by accident
				return $this->decryptWithoutSecret($authenticatedCiphertext, '');
			}
			throw $e;
		}
	}

I will not commit a patch as I do not have studied the full implications of this change, but please have a look!

@solracsf solracsf changed the title [Bug]: NC 24.0.5 hash_hkdf(): Argument #2 ($key) cannot be empty in .../lib/private/Security/Crypto.php:97 [Bug]: hash_hkdf(): Argument #2 ($key) cannot be empty Mar 17, 2023
@denics
Copy link

denics commented Mar 22, 2023

I confirm the issue and the solution in #34012 (comment) for version 26.0.0

@tguenth
Copy link

tguenth commented Mar 29, 2023

I have the same issue opening externalstorages in the administration panel for nectcloud 25.0.5.1.

[index] Fehler: Exception: hash_hkdf(): Argument #2 ($key) cannot be empty in file '/var/www/html/lib/private/Security/Crypto.php' line 160 at <<closure>>

0. /var/www/html/lib/private/AppFramework/App.php line 172
   OC\AppFramework\Http\Dispatcher->dispatch(["OCA\\Settings\ ... "], "index")
1. /var/www/html/lib/private/Route/Router.php line 298
   OC\AppFramework\App::main("OCA\\Settings\\ ... r", "index", ["OC\\AppFramewo ... "], ["externalstorag ... "])
2. /var/www/html/lib/base.php line 1047
   OC\Route\Router->match("/settings/user/externalstorages")
3. /var/www/html/index.php line 36
   OC::handleRequest()

GET /settings/user/externalstorages
from XXX.XXX.XXX.XXX by xxx at 2023-03-29T08:04:15+00:00

First I've checked my config.php to have a non-empty secret set.

I confirm the issue and the solution in #34012 (comment) for version 26.0.0

After manually applying the change proposed here #34012 (comment) i do get the issue #32258 when opening the externalstorages panel.

[index] Fehler: Exception: HMAC does not match. at <<closure>>

 0. /var/www/html/lib/private/Security/Crypto.php line 128
    OC\Security\Crypto->decryptWithoutSecret("*** sensitive parameters replaced ***")
 1. /var/www/html/lib/private/Security/CredentialsManager.php line 104
    OC\Security\Crypto->decrypt("*** sensitive parameters replaced ***")
 2. /var/www/html/apps/files_external/lib/Lib/Auth/Password/GlobalAuth.php line 55
    OC\Security\CredentialsManager->retrieve("tguenther", "password::global")
 3. /var/www/html/apps/files_external/lib/Settings/Personal.php line 79
    OCA\Files_External\Lib\Auth\Password\GlobalAuth->getAuth("tguenther")
 4. /var/www/html/apps/settings/lib/Controller/CommonSettingsTrait.php line 129
    OCA\Files_External\Settings\Personal->getForm()
 5. /var/www/html/apps/settings/lib/Controller/PersonalSettingsController.php line 73
    OCA\Settings\Controller\PersonalSettingsController->formatSettings([[["OCA\\Files_E ... ]])
 6. /var/www/html/apps/settings/lib/Controller/CommonSettingsTrait.php line 149
    OCA\Settings\Controller\PersonalSettingsController->getSettings("externalstorages")
 7. /var/www/html/apps/settings/lib/Controller/PersonalSettingsController.php line 64
    OCA\Settings\Controller\PersonalSettingsController->getIndexResponse("personal", "externalstorages")
 8. /var/www/html/lib/private/AppFramework/Http/Dispatcher.php line 225
    OCA\Settings\Controller\PersonalSettingsController->index("externalstorages")
 9. /var/www/html/lib/private/AppFramework/Http/Dispatcher.php line 133
    OC\AppFramework\Http\Dispatcher->executeController(["OCA\\Settings\ ... "], "index")
10. /var/www/html/lib/private/AppFramework/App.php line 172
    OC\AppFramework\Http\Dispatcher->dispatch(["OCA\\Settings\ ... "], "index")
11. /var/www/html/lib/private/Route/Router.php line 298
    OC\AppFramework\App::main("OCA\\Settings\\ ... r", "index", ["OC\\AppFramewo ... "], ["externalstorag ... "])
12. /var/www/html/lib/base.php line 1047
    OC\Route\Router->match("/settings/user/externalstorages")
13. /var/www/html/index.php line 36
    OC::handleRequest()

GET /settings/user/externalstorages
from XXX.XXX.XXX by xxx at 2023-03-29T08:26:59+00:00

@solracsf @denics as the the proposed solution #34012 (comment) does yield another issue #32258

Thanks

@enoch85
Copy link
Member

enoch85 commented Nov 18, 2023

I have both passwordsalt and secret defined in config.php but still seeing the same error on 27.1.3.

It needs to be the same as the original if you migrated your server in the past.

@bilibilistack
Copy link

If I cannot find the original secret, how could I reset? What kind of data will be lost?

@toglader
Copy link

I have exactly same issue with Nextcloud 27 from Truescale (TrueNAS scale)

@denics
Copy link

denics commented Dec 22, 2023

Still the same error in NC 28.0.1 and still the same solution #34012 (comment)

@AndyXheli
Copy link

IS this the same as #42157

@toglader
Copy link

toglader commented Jan 3, 2024

Solution provided, didn't help for me. Problem still exists

@fotosik
Copy link

fotosik commented Jan 31, 2024

The same for me, I even transferred the old values (passwordsalt, secret) from the previous installation into the config.php, but the issue is still the same,

Nextcloud 28.0.1.1

Strangely enough, I can access all of my data both via the Android app and via the web. That's why I don't really care. The only question is whether it is relevant to safety?

@smaxl
Copy link

smaxl commented Feb 23, 2024

I have both passwordsalt and secret defined in config.php but still seeing the same error on 27.1.3.

Same here 28.0.2

@fotosik
Copy link

fotosik commented Feb 23, 2024

In version 27.0.0 there is still this issue. However, Crypto.php changed and now you should apply #34012 (comment) to line 132

You should have:

	/**
	 * Decrypts a value and verifies the HMAC (Encrypt-Then-Mac)
	 * @param string $authenticatedCiphertext
	 * @param string $password Password to encrypt, if not specified the secret from config.php will be taken
	 * @return string plaintext
	 * @throws Exception If the HMAC does not match
	 * @throws Exception If the decryption failed
	 */
	public function decrypt(string $authenticatedCiphertext, string $password = ''): string {
		$secret = $this->config->getSystemValue('secret');
		try {
			if ($password === '') {
				return $this->decryptWithoutSecret($authenticatedCiphertext, $secret);
			}
			return $this->decryptWithoutSecret($authenticatedCiphertext, $password);
		} catch (Exception $e) {
			if ($password === $secret) {
				// Retry with empty secret as a fallback for instances where the secret might not have been set by accident
				return $this->decryptWithoutSecret($authenticatedCiphertext, '');
			}
			throw $e;
		}
	}

I will not commit a patch as I do not have studied the full implications of this change, but please have a look!

I used it in Nextcloud 28.0.2 and I have to say it no longer shows nextcloud.log errors, it works very well.
Apply to line [107]

I also have to say that the issue only occurred when I was doing activities with the Android Nextcloud.app, everything else like different browser connections or Windows Nextcloud.app and Linux Nextcloud.app didn't cause this error.

Thanks @denics

@enoch85
Copy link
Member

enoch85 commented Feb 24, 2024

@szaimen could you push for this fix amongst the core devs, or is it more complicated than that? (#34012 (comment))

@szaimen
Copy link
Contributor

szaimen commented Feb 26, 2024

@szaimen could you push for this fix amongst the core devs, or is it more complicated than that? (#34012 (comment))

Unfortunately I cannot tell. Pinging @nextcloud/server-backend for help on this

@come-nc
Copy link
Contributor

come-nc commented Feb 27, 2024

@szaimen could you push for this fix amongst the core devs, or is it more complicated than that? (#34012 (comment))

This solution is hiding the problem instead of fixing it.
The first step would be to log the Exception there to understand what is happening.
Add:

\OCP\Server:get(\Psr\Log\LoggerInterface::class)->error('Decryption failed: '.$e->getMessage(), ['exception' => $e]);

At the beginning of the catch of the decrypt method in Crypto.php.
Then post here the stacktrace for this error, on a 28 instance ideally.

@fotosik
Copy link

fotosik commented Feb 27, 2024

@come-nc you motivated me to activate the original crypto.php, strangely enough, there are no more errors in the nextcloud.log when I connect to the server with the android nextcloud.app, I also restarted the server. I'm not a developer but the behavior is somehow mysterious?

@skjnldsv
Copy link
Member

Code works in mysterious ways 🪄🎩

@fotosik
Copy link

fotosik commented Feb 27, 2024

Seems strange but injecting the code into Line 107 seems to have cured the issue...
Devil's stuff !

@enoch85
Copy link
Member

enoch85 commented Feb 28, 2024

@szaimen could you push for this fix amongst the core devs, or is it more complicated than that? (#34012 (comment))

This solution is hiding the problem instead of fixing it. The first step would be to log the Exception there to understand what is happening. Add:

\OCP\Server:get(\Psr\Log\LoggerInterface::class)->error('Decryption failed: '.$e->getMessage(), ['exception' => $e]);

At the beginning of the catch of the decrypt method in Crypto.php. Then post here the stacktrace for this error, on a 28 instance ideally.

Ok, so shouldn't this line be in master? I mean since it provides useful info it might be a good idea?

@come-nc
Copy link
Contributor

come-nc commented Feb 29, 2024

To be clear, from my understanding of the issue and the code, this can only be triggered by invalid encrypted data, most likely old encrypted data from previous versions of Nextcloud.

Ok, so shouldn't this line be in master? I mean since it provides useful info it might be a good idea?

Yeah we could try to improve the error report for this case, not with this line directly I think.

@AMN001
Copy link

AMN001 commented May 27, 2024

Still the same error in NC 28.0.5 and still the same solution #34012

@joshtrichards
Copy link
Member

There are different code paths to these situation. Full stack traces and history of your instance are important.

At a high level, there are basically two ways to get this error:

  • your secret has changed at some point
  • your secret truly is empty

In some cases, this isn't a bug and is expected behavior:

Only you know the history of your Nextcloud instance, but maybe you recognize one of these scenarios:

  • your secret changed at some point (e.g. a server migration/restore without copying/restoring config.php before going live with the new/replacement instance) [expected behavior because if you don't migrate/restore your old secret a different one will be used against your restored database]
  • you migrated/restored w/o copying/restoring your secret and ran things for awhile, but then copied over your secret from the old environment later on [expected behavior because this would result in some things in your database using one secret and others using a completely different secret]

The original report was hitting this from the encrypt side based on their stack trace:

public function encrypt(string $plaintext, string $password = ''): string {
if ($password === '') {
$password = $this->config->getSystemValueString('secret');
}
$keyMaterial = hash_hkdf('sha512', $password);
$this->cipher->setPassword(substr($keyMaterial, 0, 32));
$iv = \random_bytes($this->ivLength);
$this->cipher->setIV($iv);
/** @var string|false $encrypted */
$encrypted = $this->cipher->encrypt($plaintext);
if ($encrypted === false) {
throw new Exception('Encrypting failed.');
}
$ciphertext = bin2hex($encrypted);
$iv = bin2hex($iv);
$hmac = bin2hex($this->calculateHMAC($ciphertext.$iv, substr($keyMaterial, 32)));
return $ciphertext.'|'.$iv.'|'.$hmac.'|3';
}

That scenario can only happen when the secret is empty.

There are only ~3 other stack traces provided in this entire issue by anyone else and all are are coming via the decrypt() side of things, which is different from the original reporter.

(And they're not all reaching decrypt() via the same paths either.)

Full stack traces from all reporters are important to even determine whether what you're encountering has the same underlying root cause or not

Some possibilities I can think:

  • your secret is indeed empty right now in your config.php [in theory there is handling for this today but maybe we missed something]
  • your secret was empty at some point but now it isn't [in theory there was going to be handling for this as part of Add fallback routines for empty secret cases #31499 too I think but maybe not?]
  • some bug introduced via other code paths that reach here

But please think through the history of your instance as well since this may just be telling us your secret changed at some point because it got overlooked during a migration/restore

And even if it is a bug, the history may be relevant. Since reproducing this (outside of the scenarios it's expected behavior) is the ideal way to address things, if there is indeed a bug.

@denics
Copy link

denics commented Jun 25, 2024

thanks @joshtrichards , very useful indeed and I think I am one of those with a secret that changed during a system migration. However, I think that what most people would like to know here is: is there a way to regenerate a secret ?

@srijansaxena11
Copy link

thanks @joshtrichards , very useful indeed and I think I am one of those with a secret that changed during a system migration. However, I think that what most people would like to know here is: is there a way to regenerate a secret ?

Exactly. Also, if External Storage is somehow using old secret which is lost now and cannot be recovered, can I reset External Storage only so that it starts using the new secret even if it results in data loss?

@joshtrichards
Copy link
Member

joshtrichards commented Jun 27, 2024

However, I think that what most people would like to know here is: is there a way to regenerate a secret ?

As in get back the previous one? Only from your config.php backups. Otherwise you're kind of stuck with your new one. :)

Also, if External Storage is somehow using old secret which is lost now and cannot be recovered, can I reset External Storage only so that it starts using the new secret even if it results in data loss?

It somewhat depends on the state of your configuration today.

If your situation is basically: "I'm not using Server-side Encryption and moved to a new server, but didn't transfer over my old config.php so my new server generated a new secret"... I believe that it should be sufficient to remove/replace the existing credentials associated with the involved External Storage mount(s). The new credentials will then be associated with your new server's current secret when they're saved into your database (replacing the old ones associated with the prior secret).

Though, beware, this is a massive oversimplification on my part since there are other factors that impact your individual situation, since External Storage supports multiple authentication mechanisms and I've not thought through all the various combinations. I think should work for Username and password, Globally Credentials, User Entered, but not for either of the Log-in credentials mechanisms. For Log-in credentials, save in database you could probably toggle to a different mechanism, save things, then maybe switch back.

Keep in mind I'm making an educated guess here: there are a lot of variations and as I said I haven't tested (nor reviewed) every potential path.

EDIT: As an aside, since I'm looking over the code again right now anyway, the hash_hkdf() error noted in the title of this issue is sort of a red herring, but it does tell us one thing: It's only being seen because of some fallback code that makes a last ditch effort to see if it can make things work for you. It's intended for older environments that had empty secrets at some point. Chances are if you're hitting that area of code and seeing the hash_hkdf() error, you do have a secret (or at least the ciphertext being processed was created on an instance that did have a secret), but the secret you currently have isn't the correct (matching) one. Without the fallback code, you'd likely only see HMAC does not match. You'd still have the same situation/underlying cause, but the error would be limited to that. Currently working on a tiny PR that will change the logic for the fallback to avoid the extra PHP ValueError, but it can't fix the root problem reported in this Issue.

@enoch85
Copy link
Member

enoch85 commented Jun 27, 2024

@joshtrichards I think the FR here is about creating an occ command which replaces the secret everywhere through some kind of DB magic.

I would for sure appreciate if that was possible. I mean even if I keep backups for several years (yes I do), it would be nice with something like ./occ db:generate_new_secret which would replace the secret in DB with a new value, and above that put it in config.php.

Such a command shouldn't be run easily though, but like a last resort if you messed up recently or a long time ago.

@MrMeeb
Copy link

MrMeeb commented Aug 3, 2024

I believe I have this issue, following a migration of Nextcloud from bare metal to docker that I did just over a year ago. The log alerts in 28 must have drawn my attention to the errors. I guess I copied elements of the config.php but for whatever reason, did not include the secret. Maybe I did not know of the importance. In any case, luck would have it I have a backup of the old config, and as expected the secret in my current config and the old config do not match. The advice here seems to be 'replace the secret with the old one'. But does that still apply if the new secret has been in use for a year now? What is supposed to be done in that scenario?

@schnillerman
Copy link

This bug still persists in 29.0.4 (apache-docker) after a migration (from nginx-fmp-docker 29.0.4). In my case, it's related to external storage: The user's settings do not open, instead, there's an internal server error.

What solved it for me was taking over the secret from the config.php of the pre-migration-docker. However, migration scenarios and loss of the secret should be accounted for (generation of new secrets, passwordsalts and even instanceid should be a feature for future releases imho - especially for migration scenarios).

The migration (manual & more detailed manual) took me ~3hrs working out all file system chowns and chmods as well as appdata migration etc with only 1 user and pretty much only the default set of files (it was almost a fresh install, but I wanted to test migration).

@olegchensky
Copy link

30.0.0 the same

@ViperTecCorporation
Copy link

In my case, I migrated my server with an error to a new installation, importing the database from the old one, restoring the postgres volume, but I ended up leaving the new secret in the php config where I must adjust it in the database for everything to work again because I cannot use any sharing resource that comes with the error, update to version 30 did not solve it, any tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap 25-feedback bug feature: external storage
Projects
None yet
Development

No branches or pull requests