Best practices of crypt 2fa user data

[alexander-schranz]

Bundle version: 6.0.2 Symfony version: 6.1.1 PHP version: 8.1.6 Using authenticators (enable_authenticator_manager: true): YES

Description

I'm currently implementing a bridge for scheb/2fa to @sulu CMS: https://github.com/sulu/sulu/pull/6589

We had some discussion about the stored data and we did got into the discussion that the 2fa data should be encrypted. All things are stored so in a single array field (recovery codes, otp settings, ...): https://github.com/sulu/sulu/blob/7754047e47fef8f40f4207beda6192b00f385be9/src/Sulu/Bundle/SecurityBundle/Entity/UserTwoFactor.php#L30-L36.

Are their any example facing this problem about crypting this data? Thought maybe I'm not the first trying to achieve this. Maybe I missed also some out of the box functionality maybe for crypting critical data like recovery codes or otp settings, so maybe I dont need to handle it manually. I already did have a look at DoctrineEncryptBundle but did look like an overhead to me. Already tried a custom dbal type but did get into other problems their. I'm happy for any opinion on this topic, maybe I'm seeing it to critical that such things should by crypted 🤔.

Additional Context

Currenlty implementation can be found here: https://github.com/sulu/sulu/pull/6589/files

[scheb]

I'm not having a good recommendation for this. Until now, no one (including me) has really considered encrypting that data on the database level. My rationale is, why bother with encrypting the 2fa secret. On its own, it's worthless, because you'd still need the ability to authenticate for that account. And the other thing is, sensitive user data that shouldn't be leaking from your system, and when it is actually leaking, then you're having a whole different problem.

I can understand, under the premise of reducing the impact of a potential data leakage, it makes sense to encrypt sensitive data. Though I'm not seeing an easy way how this could be done with the current implementation.

In my opinion, the best approach would be to provide an interface for secret decryption with the bundle, add a configuration option to define an optional service for secret decryption, then this service can be injected and used in these places:

https://github.com/scheb/2fa/blob/548122ac3154136942a97e00eb9c3f2863f9e3f7/src/google-authenticator/Security/TwoFactor/Provider/Google/GoogleTotpFactory.php#L27 https://github.com/scheb/2fa/blob/548122ac3154136942a97e00eb9c3f2863f9e3f7/src/totp/Security/TwoFactor/Provider/Totp/TotpFactory.php#L35

If you want to contribute this feature, I'm open for PRs.

[alexander-schranz]

@scheb Thank you for your detailed feedback. I think I will currently going with not encrypting the data.

On its own, it's worthless, because you'd still need the ability to authenticate for that account.

That something I must say I totally did not keep in mind, and yeah they would first need the password of the account to get to the 2fa steps. Think that is a great hint and let me go with a better feeling the unencrypted way.

sensitive user data that shouldn't be leaking from your system

Thats for sure, worked also on projects where every user data was encrypted on the level database. But yeah mostly it is not even worst of it, if somebody got the DB credentials they mostly would also get the encryptions keys files for this data. Hopefully nobody does quote this to store passwords plainly in dbs 🙈, thats another topic!

In my opinion, the best approach would be to provide an interface for secret decryption

Sounds good that way. At current state as said we will keep it like you and others are currently using it and store them plain.

Still we maybe could convert this issue into a feature request and see if other people are interested into the same. I'm happy to have a look at it in the future. So we make sure not implement something or making the code base complexer just for a feature a single project is using.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.