genecommerce / module-encryption-key-manager

Tooling to help generate and invalidate magento encryption keys
GNU Lesser General Public License v3.0
54 stars 14 forks source link

Feature request: re-encrypt passwords inside env.php #12

Closed davekleijn closed 1 month ago

davekleijn commented 2 months ago

Hi,

I have saved a lot of passwords inside the env.php file. bin/magento config:set path password --lock-env

I have a question regarding these passwords. It looks like Magento is not changing this values when rotating the encryption key. The passwords still start with 0:3. After invalidating the original encryption key I get a lot of errors because these passwords are incorrect.

I propose adding a feature to the module that automatically re-encrypts passwords stored in env.php. Or is there another way to do this? I know it is possible with magerun, but that can only be done manually.

Maybe I'm overlooking something. If that is the case please let me know.

Thank you for considering this feature request.

convenient commented 2 months ago

I would definitely accept a PR to achieve this

Here is how you write to env.php https://github.com/genecommerce/module-encryption-key-manager/blob/58ce868669242af7a9ff5c5c0bca1739fb4f70e2/Console/InvalidateOldEncryptionKeys.php#L117-L119

Here is how you reencrypt values https://github.com/genecommerce/module-encryption-key-manager/blob/58ce868669242af7a9ff5c5c0bca1739fb4f70e2/Service/ChangeEncryptionKey.php#L68

I think we could have a command to perform this action

Seeing as some people may have already installed this module and generated a new encryption key (to get secured against the JWT threat) then I think it makes sense to have this as a supplemental command that we can run to work through this data? 🤔

davekleijn commented 1 month ago

Hi @convenient,

I think we can close this issue.

"Review your env.php, if you store any encrypted values there they will need to be reissued by the provider as they may have been leaked."

We need to manually update the passwords in env.php after they are reissued. So it's not needed that they are automatically updated?

Kind regards, Dave Kleijn

convenient commented 1 month ago

Good morning @davekleijn

For this specific cosmicsting security issue I think it is not necessary to have this feature. If we're assuming that the env.php has been leaked, then all credentials need to be re-issued from payment gateways etc and re-added so that will be the effect of rotating those.

However, I think this issue should remain open as an enhancement to improve the completeness of this tool. It may be in the future that someone wants to rotate their encryption key for some other purpose, and in those scenarios the other values in env.php may still be secure so I think having some capability to do this is still warranted 👍 Currently I think this tool is the most complete offering of how to safely rotate keys in Magento, so having coverage of env.php still feels important.

Would you agree?

davekleijn commented 1 month ago

Good morning @convenient,

Now that you mention it, I do agree with that. It would definitely be useful to implement this automatically in the future. If I have some spare time in the coming days, I will look into it. Have not had much time lately due to all the security patches, haha.

convenient commented 1 month ago

Agreed, i think in the context of the current cosmicsting situation, people don't need to worry about the additional process of automatically re-encrypting env.php because it all needs to be done manually with new creds.

This means the process of re-encrypting the env.php data can be bundled into the generate command, where we're already reading/writing from the env.php file anyway :)

I'd really appreciate a PR with these changes thank you, and I very much know what you mean about being busy haha.