text app can apparently bypass files_accesscontrol workflows

[michielbdejong]

As reported in https://git.radicallyopensecurity.com/ros/pen-sunet-mfa-zones/-/issues/1
Investigating

[michielbdejong]

Trying this out with:

• usr1 creates a folder 'asdf' and shares it with Admin
• Admin can have a browser session at the same time, in the second Firefox container, to accept the share
• Then usr1 makes it an MFA Zone
• See what happens

[michielbdejong]

[updated] this does not happen for folders, the non-MFA Admin user sees a 'Forbidden' error when you try to access a share that was made into an MFA Zone by usr1.

[michielbdejong]

I had another look specifically at the PROPFIND result, which is what https://git.radicallyopensecurity.com/ros/pen-sunet-mfa-zones/-/issues/1 reports, and once usr1 toggles the MFA Zone switch, the PROPFIND result for Admin changes from 207 to 403 the next time they reload the page:

[michielbdejong]

There is a problem, it just only affects text files and not folders:

Will have to look into the text file sync functionality and how that interacts with the file access workflow.

[michielbdejong]

Renaming this to be more specific about the text app, and splitting out ... to be about the related issue I'm investigating for remote access of files that should be blocked according to a files_accesscontrol workflow.

[michielbdejong]

It might be that when the text app looks at the filesystem, it does not go through the workflow engine.
Investigating.

[michielbdejong]

This might require some changes to the https://github.com/nextcloud/text

[michielbdejong]

Trying to dig deeper into the code with https://github.com/pondersource/nextcloud-mfa-awareness/tree/text-app

[michielbdejong]

Oh dear, there is a text_sessions db table, that is probably where the leak is.

[michielbdejong]

docker exec -it sunet-mdb2 mariadb -u nextcloud -puserp@ssword -h sunet-mdb1 nextcloud 

[michielbdejong]

It seems to be doing some access checks in https://github.com/nextcloud/text/blob/d6947c8e36cd4e37f9efd96c57e218f3a480e4f8/lib/Service/ApiService.php#L111 and differentiating between shared and local files.

[michielbdejong]

I'm trying to force a bug into apps/workflowengine/lib/Service/RuleMatcher.php to see how the workflow engine is engaged during normal filesystem access.

[michielbdejong]

Ah, good! The text app create function does break on that. So maybe I'm just doing something wrong here:

{"reqId":"aEw2z7kzfSkKFBf235wo","level":3,"time":"2023-11-29T15:49:43+00:00",
"remoteAddr":"192.168.16.7","user":"usr1","app":"index",
"method":"PUT","url":"/index.php/apps/text/session/create","message":
"Call to undefined function OCA\\WorkflowEngine\\Service\\breakMe() in file 
'/var/www/html/apps/workflowengine/lib/Service/RuleMatcher.php' line 224",
"userAgent":"Mozilla/5.0 (X11; Linux x86_64; rv:84.0) Gecko/20100101 Firefox/84.0","version":"26.0.7.1","
exception":{"Exception":"Exception","Message":"Call to undefined function OCA\\WorkflowEngine\\Service\\breakMe() in
 file '/var/www/html/apps/workflowengine/lib/Service/RuleMatcher.php' line 224","Code":0,"Trace":
[{"file":"/var/www/html/lib/private/AppFramework/App.php","line":183,"function":"dispatch","class":"OC\\AppFramework\\Http\\Dispatcher","type":"->","args":[["OCA\\Text\\Controller\\SessionController"],"create"]},
{"file":"/var/www/html/lib/private/Route/Router.php","line":315,"function":"main","class":"OC\\AppFramework\\App","type":"::","args":["OCA\\Text\\Controller\\SessionController","create",["OC\\AppFramework\\DependencyInjection\\DIContainer"],["text.Session.create"]]},
{"file":"/var/www/html/lib/base.php","line":1065,"function":"match","class":"OC\\Route\\Router","type":"->","args":["/apps/text/session/create"]},
{"file":"/var/www/html/index.php","line":36,"function":"handleRequest","class":"OC","type":"::","args":[]}
],
"File":"/var/www/html/lib/private/AppFramework/Http/Dispatcher.php","Line":169,"Previous":
{"Exception":"Error","Message":"Call to undefined function OCA\\WorkflowEngine\\Service\\breakMe()","Code":0,"Trace":
[
{"file":"/var/www/html/apps/workflowengine/lib/Service/RuleMatcher.php","line":180,"function":"check","class":"OCA\\WorkflowEngine\\Service\\RuleMatcher","type":"->","args":[[1,"OCA\\WorkflowEngine\\Check\\MfaVerified","!is","","defae2f365bcd4ddd78219d1d5e12b53"]]},
{"file":"/var/www/html/apps/workflowengine/lib/Service/RuleMatcher.php","line":128,"function":"getMatchingOperations","class":"OCA\\WorkflowEngine\\Service\\RuleMatcher","type":"->","args":["OCA\\FilesAccessControl\\Operation",true]},
{"file":"/var/www/html/apps/files_accesscontrol/lib/Operation.php","line":98,"function":"getFlows","class":"OCA\\WorkflowEngine\\Service\\RuleMatcher","type":"->","args":[]},
{"file":"/var/www/html/apps/files_accesscontrol/lib/CacheWrapper.php","line":60,"function":"checkFileAccess","class":"OCA\\FilesAccessControl\\Operation","type":"->",
    "args":[["OCA\\FilesAccessControl\\StorageWrapper","*** sensitive parameters replaced ***","*** sensitive parameters replaced ***","*** sensitive parameters replaced ***","*** sensitive parameters replaced ***","*** sensitive parameters replaced ***","/usr1/"],
    "files/Readme.md",false,["OC\\Files\\Cache\\CacheEntry"]]},{"file":"/var/www/html/lib/private/Files/Cache/Wrapper/CacheWrapper.php","line":87,"function":"formatCacheEntry","class":"OCA\\FilesAccessControl\\CacheWrapper","type":"->","args":[["OC\\Files\\Cache\\CacheEntry"]]},
    {"file":"/var/www/html/lib/private/Files/Node/Root.php","line":464,"function":"get","class":"OC\\Files\\Cache\\Wrapper\\CacheWrapper","type":"->","args":["*** sensitive parameters replaced ***"]},
    {"function":"OC\\Files\\Node\\{closure}","class":"OC\\Files\\Node\\Root","type":"->","args":["*** sensitive parameters replaced ***"]},{"file":"/var/www/html/lib/private/Files/Node/Root.php","line":462,"function":"array_map","args":[["Closure"],["*** sensitive parameters replaced ***"]]},
    {"file":"/var/www/html/lib/private/Files/Node/LazyUserFolder.php","line":68,"function":"getByIdInPath","class":"OC\\Files\\Node\\Root","type":"->","args":["*** sensitive parameters replaced ***","/usr1/files"]},
    {"file":"/var/www/html/apps/text/lib/Service/DocumentService.php","line":486,"function":"getById","class":"OC\\Files\\Node\\LazyUserFolder","type":"->","args":["*** sensitive parameters replaced ***"]},
    {"file":"/var/www/html/apps/text/lib/Service/ApiService.php","line":93,"function":"getFileById","class":"OCA\\Text\\Service\\DocumentService","type":"->","args":["*** sensitive parameters replaced ***"]},
    {"file":"/var/www/html/apps/text/lib/Controller/SessionController.php","line":61,"function":"create","class":"OCA\\Text\\Service\\ApiService","type":"->","args":["*** sensitive parameters replaced ***"]},
    {"file":"/var/www/html/lib/private/AppFramework/Http/Dispatcher.php","line":230,"function":"create","class":"OCA\\Text\\Controller\\SessionController","type":"->","args":["*** sensitive parameters replaced ***"]},
    {"file":"/var/www/html/lib/private/AppFramework/Http/Dispatcher.php","line":137,"function":"executeController","class":"OC\\AppFramework\\Http\\Dispatcher","type":"->","args":[["OCA\\Text\\Controller\\SessionController"],"create"]},
    {"file":"/var/www/html/lib/private/AppFramework/App.php","line":183,"function":"dispatch","class":"OC\\AppFramework\\Http\\Dispatcher","type":"->","args":[["OCA\\Text\\Controller\\SessionController"],"create"]},
    {"file":"/var/www/html/lib/private/Route/Router.php","line":315,"function":"main","class":"OC\\AppFramework\\App","type":"::","args":["OCA\\Text\\Controller\\SessionController","create",["OC\\AppFramework\\DependencyInjection\\DIContainer"],["text.Session.create"]]},
    {"file":"/var/www/html/lib/base.php","line":1065,"function":"match","class":"OC\\Route\\Router","type":"->","args":["/apps/text/session/create"]},{"file":"/var/www/html/index.php","line":36,"function":"handleRequest","class":"OC","type":"::","args":[]}],
    "File":"/var/www/html/apps/workflowengine/lib/Service/RuleMatcher.php","Line":224},"CustomMessage":"--"}}

Will continue debugging this tomorrow!

[michielbdejong]

So that's weird, the code seems to pass through the StorageWrapper but still in the browser you see updates arrive at the other end that should not be arriving. Will see what happens if you close and reopen the doc at the receiving end, that should then definitely give a "forbidden" error.

[michielbdejong]

To simplify the testing I'm going to put the text doc into a folder that is an MFA zone

[michielbdejong]

OK, opening a file inside an MFA zone for the first time is correctly blocked:

• recipient browses to the folder
• sender makes it an MFA zone
• recipient clicks on the text file inside the folder
• 'this file no longer exists'

Similarly for the recipient creating a file inside the folder while it's a zone: 'Unable to create new file from template'
If the sender untoggles the zone and the recipient retries, the file is created as expected.

[michielbdejong]

And now it gets interesting:

• the recipient has the file open
• the sender toggles MFA zone on the containing folder
• both type

Expected: sender can edit like they are the only person there, and recipient gets a sync error

Actual: sender's edits appear in the recipient side! Also, sender sees the cursor (but not the edits) of the recipient.

It's clear that there are some checks missing here.

I'll test the same thing across two servers.

[michielbdejong]

Other tests:

• recipient log out and go back to the text file URL
• MFA requirement just on the file (so that the filename is still discoverable for the recipient)
• use files_accesscontrol app directly

[michielbdejong]

Still not got sunet-nc3 working as an additional server in the test setup in the text-app branch of this repo

Error creating the share: Sharing file.md failed, could not find admin@sunet-nc3, maybe the server is currently 
unreachable or uses a self-signed certificate

I'll do that for the basic file-access check though, then I can just do it in dev-stock where the TLS certs are trusted.

[michielbdejong]

Even the recipient logging out and back in doesn't stop them from receiving new updates if they browse back to a file that became MFA-protected after they had already opened it in the text app.

[michielbdejong]

OK so for text files inside an MFA zone it only happens until the recipient refreshes their screen.
But for text files that are themselves carrying an MFA requirement, only writing to the file is blocked, reading is not.

[michielbdejong]

If I set a flow to block access to files names test.md then it does block access with the text app as soon as I refresh the screen.

[michielbdejong]

maybe the tags are only per user, so for a first user the file looks tagged but for a second user it does not? Testing that hypothesis.

[michielbdejong]

I'll add a rule where access to a file with mfazone tag is blocked for everybody, regardless of their MFA state. That would rule out that last hypothesis.

[michielbdejong]

Ah in that case it's correctly blocking! So that supports the hypothesis that this has something to do with the tags that the sender sees and the tags that the recipient sees. I had assumed that would be the same system tags, but looks like maybe it's not.

Will continue tomorrow.

Should also test this cross-server in dev-stock.