Skip to content

FEATURE: Add setting for encryption key #3426

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

Open
wants to merge 2 commits into
base: 8.3
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Neos.Flow/Classes/Security/Cryptography/HashService.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ class HashService
public function injectSettings(array $settings)
{
$this->strategySettings = $settings['security']['cryptography']['hashingStrategies'];
if (!empty($settings['security']['cryptography']['encryptionKey'])) {
$this->encryptionKey = $settings['security']['cryptography']['encryptionKey'];
}
}

/**
Expand Down
4 changes: 4 additions & 0 deletions Neos.Flow/Configuration/Settings.Security.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ Neos:

cryptography:

# A private, unique key used for encryption tasks. Normally 40 characters long and received from a persistent
# filesystem cache. If set to a non-empty string, the cache is not involved anymore.
encryptionKey: ''
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I digress – the schema says it's a required string, so either make it an empty string by default or adjust the schema. 😎

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, sure. But the schema is part of this PR, so we are open for both solutions, right? So question is, what do we expect the value for an not-set encryption key?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something empty… 🙃 Either way is fine.


hashingStrategies:

# The default strategy will be used to hash or validate passwords if no specific strategy is given
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ properties:
additionalProperties: false
required: true
properties:
'encryptionKey': { type: string, required: true }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'encryptionKey': { type: string, required: true }
'encryptionKey': { type: [string, 'null'], required: true }

I guess the schema sould look like this. (untested)


'hashingStrategies':
type: dictionary
additionalProperties: { type: string, format: class-name }
Expand Down