pondersource / nextcloud-mfa-awareness

Make Nextcloud aware of whether the current user is logged in with Multi-Factor Authentication
MIT License
0 stars 2 forks source link

See if we can increase security by invert the logic of our tag from ‘mfa required’ to ‘no mfa required’ #34

Closed michielbdejong closed 1 year ago

michielbdejong commented 1 year ago

If we get a very confident conclusion from #33 we might decide to skip this.

mrvahedi68 commented 1 year ago

I think this change will have very side effects or perhaps don't understand it completely. Can you describe it with some example?

shokri-navid commented 1 year ago

as I understand your suggestion. we should ban all file access except those with the no_mfa_required tag. so it seems that we should change the core logic of the following cloud because the current logic in the nextcloud uses the tag to ban access. so the best approach may be that we should create a tag that bans all access for each folder or file is creating on the server. and after that, we should unban accessing them using another tag.

mrvahedi68 commented 1 year ago

After that Navid Describes the issue to me, and I found we are on the same page regarding the solution. We need some jobs that when Nextclud starts search for all existing files without a tag and tag them to MFA-Required and then admin changes the tag for files that does not needs this check anymore. For newly created files also we should tag them automatically. This solution has one side effect that I explain it: Assume that User1 is a user without the MFAVerified check and creates a file for himself/herself, Because we tag this file to Mfa-Required user will not have access to owned file and must ask from Admin to remove the tag.

Turn on a torch in our minds if we go the wrong way. @michielbdejong

michielbdejong commented 1 year ago

Hm, that was not how I had envisioned it;

  1. I had hoped we could use an automated/virtual ("restricted") tag that is present by default on each file and folder. For this we would not use a cron job. We would use https://docs.nextcloud.com/server/latest/admin_manual/file_workflows/automated_tagging.html#assigning-restricted-and-invisible-tags which is actually not applying anything to files one-by-one, it is setting up a rule that is applied in code only and just-in-time when someone tries to access a file.
  2. The default would be to add a tag MFA NOT required, and so by default MFA would NOT be required
  3. You would have an access rule: Access to files is always denied, unless the MFA NOT required tag is missing, OR the user has MFA enabled in the current session.
michielbdejong commented 1 year ago

In the current situation (we add a MFA required tag, and use an allow-then-deny rule), if there is a bug in the tags system, the failure mode is to give too much access.

In the situation I'm proposing here (we add a MFA NOT required tag, and use a deny-then-allow rule), if there is a bug in the tags system, the failure mode would be to give too little access.

Then, a bug in the tags system would still be annoying to users, but it would not be catastrophic.

This principle is also sometimes called "Implicit Deny, Explicit Allow" and it's often used in firewall systems.

michielbdejong commented 1 year ago

We decided to skip this