magespecialist/m2-MSP_TwoFactorAuth

isn't enabling/disabling 2FA from the admin a security issue?

Closed this issue · 4 comments

piotrekkaminski commented

I think ability to disable/enable 2FA from admin is a possible security risk. We already had hackers entering admin and changing symlinks config to use RCE. In this case, if there is CSRF vulnerability in Magento, admin can be tricked to click on link that would disable 2FA.

I think all enable/disable/provider list should be in command line or XML file, not exposed via admin.

phoenix128 commented

Hello @piotrekkaminski ,
I think you are right and this should be one point of concern.
More than removing the admin section to enable or disable 2FA and providers, what do you think about adding a kind of configuration lock?
We could add a backend button or a CLI script to put the configuration in read only mode.

piotrekkaminski commented

While a lock would be a secure idea, i think there is no such pattern in the admin already and it would be confusing.

phoenix128 commented

We should handle this at user level too. 2FA allows individual users to enable/disable their 2FA config.
Do you think we should remove individual user's selection and leave the global one?
If we remove individual users configuration we could move 2FA global configuration at XML level.

giacmir commented

@piotrekkaminski Actually a lock mechanism for admin configs exists on versions >= 2.2.

http://devdocs.magento.com/guides/v2.2/config-guide/cli/config-cli-subcommands-config-mgmt-set.html

It's not 100% secure in case of RCE or similar (but if you have RCE then nothing is secure anymore) since you can force code execution to change it, but its safe from the XSS or CSRF standpoint.

@phoenix128 I disagree on the idea of removing single user options because in most cases you can't have a rigid enforcment for 2fa providers. Users should be able to choose whatever method they prefer among the available ones.

Based on what I've seen around on other systems that use 2FA probably a good security comprosmise is to notify the admin or user when global or personal configuration have changed so they can take appropriate action if the change was not operated by themselves.