FEATURE: Add setting for encryption key

[christophlehmann]

With this the encryption key can be defined in a setting. When defined it is not received from cache anymore.

Neos:
  Flow:
    security:
      cryptography:
        encryptionKey: 'something-random-usually-40-chars-long'

Ideally set this from an environment variable, so it doesn't have to be in your codebase…

Resolves: #3425

Upgrade instructions

None – but feel free to use the new feature. For existing projects, set the encryption key to the value found in the cache, Data/Persistent/Cache/Data/Flow_Security_Cryptography_HashService/encryptionKey usually.

Review instructions

Set the new setting to a non-empty string and see that \Neos\Flow\Security\Cryptography\HashService::generateHmac() returns a Hmac generated with your new encryption key.

Checklist

• Code follows the PSR-2 coding style
• Tests have been created, run and adjusted as needed
• The PR is created against the lowest maintained branch
• Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
• Reviewer - The first section explains the change briefly for change-logs
• Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

[kdambekalns]

Usually we put the cache for this into the database, in case the filesystem does not suffice (e.g. if it's non-persistent or the setup scales over multiple server)…

[kdambekalns]

I am not opposed to this. I wonder, though, if there should be some check for the key, to make sure it's not just 123 when it is set…

[christophlehmann]

Plaintext secrets in the database might be a way, but have a bad touch to me.

wonder, though, if there should be some check for the key

Would keep it simple stupid. Its comment states the default length, which is sufficient in my eyes. Ok?

[kdambekalns]

Plaintext secrets in the database might be a way, but have a bad touch to me.

True. Point taken.

Would keep it simple stupid. Its comment states the default length, which is sufficient in my eyes. Ok?

Yeah, fine. I guess anyone caring enough to set it, will do it right.

[kdambekalns]

I tweaked the PR description a bit. It would be awesome if you could rebase this on Flow 8.3 though, as it's clearly not in scope for the security-only versions before that.

Oh, and – thanks for the PR! I don't mean to sound hostile. 😇

[christophlehmann]

I tweaked the PR description a bit. It would be awesome if you could rebase this on Flow 8.3 though, as it's clearly not in scope for the security-only versions before that.

Okay, I interpreted 7.3 as lowest maintained branch. Is rebased!

I don't mean to sound hostile.

No, you don't. Questions and discussions are absolutely fine 👍

[kdambekalns]

Ok, fine with me.

I requested review from a few people and have one last question (to those): In theory this must go into 8.4, as it is marked as a feature. But… is it? WDYT?

[dlubitz]

Thank you for your PR. This makes sence. I left one comment to discuss.

I'd rather vote for 8.4 but also fine with sneaking it into 8.3.

[dlubitz]

Wouldn't it make sence to set this to null instead? Would clearly say, that this property is not set.

Suggested change

	encryptionKey: ''
	encryptionKey: null