EncryptBundle icon indicating copy to clipboard operation
EncryptBundle copied to clipboard

[Malfunction] `onEncryptionLoadKey` purpose gets violated by `postLoad`

Open Volmarg opened this issue 2 years ago • 4 comments

Since I want to use Your package in next project I will try to explain the issue the best I can right now.

Flow of data - sending between projects

image

Details

  • I'm using the event, that allows to change the key on fly,
  • Since Your package requires any key to be defined i need to have a dummy key to prevent exceptions, so I have this
parameters:
    encrypt_key: "0000000000000000000000000000000000000000000" # (random key - not used - needs to be set to suppress the encryption bundle error)

Flow of data - user logs in

image

Cron call no 1. - Insert Passwords Groups

foreach($insertRequest->getPasswordsGroupsArrays() as $passwordGroupArray){
   $passwordGroupEntity = PasswordGroup::fromArray($passwordGroupArray);
   ...
   $this->passwordGroupController->save($passwordGroupEntity);
}

Status: Works

Cron call no 2. - Insert Passwords and assign them to groups

foreach($insertRequest->getPasswordsArrays() as $passwordArray){
    $passwordEntity      = Password::fromArray($passwordArray);
    $passwordGroupEntity = $this->passwordGroupController->getOneForId($passwordEntity->getGroupId());
    ...
    $this->passwordController->save($passwordEntity);
}

Status: Does not work

Under which circumstances the problem happens

  1. The passwordGroupEntity is already saved with first cron call, and has correct encryption string from PMS (1),
  2. Now I fetch the passwordGroupEntity from DB
  3. Seems like Your bundle decrypts the values in there
  4. I set the relation between passwordGroupEntity and passwordEntity
  5. I save the passwordEntity
  6. Doctrine messes up encrypts the passwordGroupEntity fields with key stored in yaml which is 0000

Where is Your code related to the issue

  • Class: DoctrineEncryptSubscriber,
  • Method: processFields
  • Code block:
            if($isEncryptOperation) {
                $encryptedValue = $this->encryptor->encrypt($value);
                $refProperty->setValue($entity, $encryptedValue);
            } else {
                $decryptedValue = $this->decryptValue($value);
                $refProperty->setValue($entity, $decryptedValue); 
                //we don't want the object to be dirty immediately after reading
                $unitOfWork->setOriginalEntityProperty($oid, $refProperty->getName(), $value);
            }
  • Specifically - this one line causes the problem, when commented out works fine
$refProperty->setValue($entity, $decryptedValue); 
  • Additionally - this problem makes the overwriting of encryption key via event no longer usable, because right now Your bundle doesn't know that the key will be replaced and uses the one hardcoded.

This makes sense because "Hello how will Your bundle know that key is replaced otherwise?". Yet the current situation makes the onEncryptionLoadKey kinda useless.

End note

I've spent tones of time trying to find out why / where this happens. I cannot propose the solution as I don't fully understand why such thing has place. I have my own solution in project which is saving via plain sql - I can use that in current project, but next one is a no since I want to encrypt entire DB.

Volmarg avatar Jun 26 '21 06:06 Volmarg

Hi Volmarg,

The encryptor/decryptor is constructed with the default configuration key.

When getSecretKey is called by an encryption or decryption process it allows you to insert an alternative key using the event dispatcher. If the event returns null then the defined configuration key is used.

Take a look at the following in the OpenSSLEncryptor. Check if the subscriber you are using in the second system is returning a key.

 private function getSecretKey(){

        // Throw an event to allow encryption keys to be defined during runtime.
        $getKeyEvent = new EncryptKeyEvent();
        $this->dispatcher->dispatch($getKeyEvent, EncryptKeyEvents::LOAD_KEY);

        // If the event is returned with a key, then override the parameter defined key.
        if(null !== $getKeyEvent->getKey()){
            $this->secretKey = $getKeyEvent->getKey();
        }

        // If the key is still empty, then throw an exception.
        if(empty($this->secretKey)){
            throw new EncryptException('The bundle specshaper\encrypt-bundle requires a parameter.yml value for "encrypt_key"
            Use cli command "php bin/console encrypt:genkey" to create a key, or set via a listener on the EncryptKeyEvents::LOAD_KEY event');
        }

        // Decode the key
        $key = base64_decode($this->secretKey);

        $keyLengthOctet = mb_strlen($key, '8bit');

        if (32 !== $keyLengthOctet) {
            throw new \Exception("Needs a 256-bit key, '".($keyLengthOctet * 8)."'bit given!");
        }

mogilvie avatar Jun 28 '21 08:06 mogilvie

Hi,

I know what You mean, and I agree with what You wrote. The thing here is that the second system has a default key 000000..., and the real key is being set only upon logging-in by the user. Sending key via API is not allowed.

Therefore the api-endpoint in PMS-IO (target project which takes the encrypted data), does not have the real key. I would expect the data just to be inserted in DB without the decrypt, and then encrypt again.

Volmarg avatar Jun 28 '21 16:06 Volmarg

Forgive the repetition, I'm trying to get my head around the intent.

Project No1. - PMS (some sort of back end admin system?) A user is created, a password is entered, and a unique key is generated to encrypt the password. The user, complete with encrypted password and key string is persisted to PMS-DB. These details are then communicated back to a user somehow? Is that a concern that the credentials need to be transmitted together?

Project No2 - PMS-Bridge I'm not following this so well. Is this a separate symfony project? or does it exist in the same environment as Project No 1 as the green shading might indicate?

Does project 1 send the new user+password+key to Project 3 using a the cron in Project 2? Or is it just the basic user details and an encrypted password?

Are user credentials transmitted just on a scheduled cron job to fetch new users and transfer them across projects? The Symfony Messenger Component might be something to consider.

Why is this bidirectional (black arrows between project 2 and project 1) Does a new user registration in Project 3 also get persisted back to Project 1?

Project No3 - PMS-IO Some user facing application where the user logs in with username, password, and encryption key. Are new user details registered in Project 3 persisted back into project 1?

Suggestions I think the issue is in the highlighted box below.

image

In your authentication you have a form that gets submitted with a username, password and encryption key. The normal guard authentication will query the DB for any user credentials to match the username. If no match, then the authentication will fail.

However, if the username exists, then the authentication will load the user entity in order to get the DB stored hashed password. The authentication system then hashes the user supplied login form password and compares it against the DB stored hashed password. If they don't match then authentication fails.

The problem is that when the authenticator is loading the user entity, if you have marked the password as @encrypted via this bundle, then the postLoad doctrine event listener will try and decrypt the password. If the user specific encrypt key doesn't exist in project 3 then the encryptKeyEvent object returns nothing and the decryptor will default to the parameter defined encryption key. It will decrypt the password incorrectly. The comparison between the login form password/key combination and the DB stored encrypted password will not be successful.

My suggestion is to create your own authenticator in project 3 that queries the PMS-IO DB with a select statement, instead of hydrating the entire user entity. This would avoid triggering the doctrine postLoad event and will allow you to encrypt your login form password with the login form supplied encryption key, and compare that against the DB stored encrypted password. If this is successful then you grant the user authentication, and persist or store the encryption key for future use in Project 3.

Having written all the above, I'd be concerned about a public login form where the user can enter their password, and an encryption key, and get confirmation back that the password/key combination works or doesn't.

I would also be concerned that if the Project 1 or 3 DB was compromised, and the encryption key and the password are in the same set of data, then it could be reverse engineered to decrypt the rest of the encrypted fields. Or perhaps you are only temporarily storing the encrypt key in session for Project 3 somehow?

mogilvie avatar Jun 29 '21 10:06 mogilvie

Yeah, it’s normal – get a headaches at work sometimes as well trying to understand what’s going on.

Project No1. - PMS

  • backend with management panel,
  • user must register, there can be multiple accounts but all share the same encryption key,
  • the key is stored in services.yaml
  • this key is never, ever transmitted, it stays in the project,
  • only the encrypted DB data is moved to the Project No3 – PMS-IO
  • this project has no input api logic,
  • has strict access rules,
  • stays on home based raspberry (it’s actually this one - https://github.com/Volmarg/personal-management-system)

Project No. 2 - PMS-Bridge

  • this is a seprate project included into the Project No. 1 - PMS via composer,
  • and yes it’s a part of Project No. 1 – PMS this way
  • Project No. 1 – PMS sends only the database entries other than credentials (so all kind of notes, password etc),
  • the cron job runs each 1min and checks if is allowed to insert something in the Project No3 – PMS-IO (consider it a ping hey- can I get in?!),
  • Symfony Messenger Component never heard of it – way to many components to know :D
  • The bidirectional arrow is misleading (my bad here) this should by jest a text instead of that arrows (just an info that Bridge sends data to PMS-IO No.3)

Project No3 – PMS-IO

  • this project is read-only
  • nothing is ever sent back to the Project No1. - PMS,

Validation form

This works a bit differently.

  • user entity is not marked with @Encrypted at all,
  • the login / password is validated with almost standard symfony logic,
  • if user is found and matches – then it follows further,
  • it checks if the encryption key was provided in the last input field,
    • if not then will fail,
    • if yes then will continue to decrypt data from DB,
      • if key is matching the key used to encrypt then user sees the data,
      • if not, then empty strings are returned which is fine for me

So actually this what You wrote here

My suggestion is to create your own authenticator in project 3 that queries the PMS-IO DB with a select statement, instead of hydrating the entire user entity. This would avoid triggering the doctrine postLoad event and will allow you to encrypt your login form password with the login form supplied encryption key, and compare that against the DB stored encrypted password. If this is successful then you grant the user authentication, and persist or store the encryption key for future use in Project 3.

Is not actually needed because the user login-in is fine.

Concerns

  • user (which is me in this case only – but in case someone would find the url and tried to get in) will only see information like “unauthorized” in cases such us:
    • no matching user found,
    • combination of key/user is invalid
    • encryption key is either to long or to short (not that it is incorrect)

Data compromising

  • You are partially correct, while both projects share the same key, the Project No1. - PMS stores the key in yaml while the Project No3. PMS-IO stores it only in session.

More highlight

Generally both projects are for private usage only – I created them on my own for personal usage – the code source of both is accessible for everyone, but I write this all for myself

Project No1. PMS

  • available only on local raspberry,
  • ip filtered access to the project in local network,
  • full sd card encryption is planned soon,

Project No3. PMS-IO

  • available over internet connection,
  • geolocalized IP restriction,
  • encrypted DB,
  • encryption key stored in session only,
  • data removed upon logout,
  • data removal triggered after user is inactive X minutes,
  • data insertion from project 1 only when certain flag in DB is set to allow insertion,

and so on – I’m a bit obsessed with that because like You said data breach and I don’t want this.

So still, there is some problem with that code that I mentioned up there, I wonder if it would be somehow possible to save the data into DB using standard persist / flush without triggering decryption and encryption with local key. In my opinion the data should only be saved in DB in that logic, nothing else should happen there.

Volmarg avatar Jul 03 '21 07:07 Volmarg