FileFirewall: Handle Reply Codes Gracefully

[dragotin]

If the client connects to a server with a file firewall enabled, it should handle the reply codes sent by the firewall gracefully, ie. display proper error messages and not block the entire sync.

@MTRichards is it that?

@nickvergessen can you provide us with a list of the possible reply codes?

[dragotin]

@nickvergessen maybe you want take this issue, even though it's client? We do not expect that there is anything to code, just more testing.

[nickvergessen]

I have no idea of the firewall so far, but will put it on my todo for tomorrow.

[MTRichards]

Yep - This is correct - so adding more detail here.

There are really three conditions we need to check on the desktop client.
Assume that the file firewall is in operation on the server and that a rule is set.
Outcome 1) The file firewall rejects all requests because of the rule, for example the user is not on the corporate IP address range. Result: desktop client says this is the case and the sync run fails due to server file firewall rule settings. This is a complete rejection of the sync request.

Outcome 2) The file firewall rejects an upload or download for a specific folder or file, for example an mp3 upload, or a file / folder that is prohibited access from the desktop outside of the corporate network. This is a partial rejection - but the rest of the sync run and all of the other files and folders are synced properly. The ones that fail get errors on the file or folder.

More specifically, say a folder "financials" is prevented by the file firewall from being seen or synced by our clients. The client should red x that folder and everything in it, they don't get synced, but everything else not in the folder is still synced.

Outcome 3) The rule is passed and everything is allowed to be synced. This already works.

[nickvergessen]

Okay, looks like currently it just replies with 403. Will have a look for improvements

[dragotin]

@nickvergessen have had time to look into this? I think in case 1) the file firewall should reply with 503 to make the client silently disconnect (kindof).

[MTRichards]

Pushed to the next maintenance release for timing and availability.

[nickvergessen]

Sorry @dragotin I was away until sunday...

I had a quick look at the firewall code and it looks like all the rules just return a boolean. These booleans are then combined and the result is "block" or "pass". So I guess we need to change the logic of the firewall quite heavily, so we can get the reason of the fail in order to send it back as a status code.

As for the "blocking only the sync of a specific folder" point, I didn't find that feature/rule yet. All persons that ever worked on FW are not here anymore, so it will take a while until I find if that rule exists and how we can change the outcome of that.

[DeepDiver1975]

As for the "blocking only the sync of a specific folder" point, I didn't find that feature/rule yet. All persons that ever worked on FW are not here anymore, so it will take a while until I find if that rule exists and how we can change the outcome of that.

We do this the other way round:

We did request a representative rule set from PM and GCX - is the rule set in there?
No -> ignore

[DeepDiver1975]

We did request a representative rule set from PM and GCX - is the rule set in there?
No -> ignore

I'm sorry to sound rude - but this firewall nightmare is way to expensive when it comes down to time being eaten by core devs

[MTRichards]

@DeepDiver1975 Please separate this into two different requests, as they are different. Related, of course, but different:

1. A list of file firewall rules
2. The ability to have responses understood by the client

I agree that 1) is still open, got it. 2), though, is a here and now problem. The client tends to fatally fail as it assumes rules pass or fail, and fail stops the sync.

Outcome 2) The file firewall rejects an upload or download for a specific folder or file, for example an mp3 upload, or a file / folder that is prohibited access from the desktop outside of the corporate network. This is a partial rejection - but the rest of the sync run and all of the other files and folders are synced properly. The ones that fail get errors on the file or folder.

This rule can be done today, using regex, and the client doesn't handle it gracefully. All we need is a way for the client to understand "oh, this particular request failed non fatally" and to move on. That is the basis for this request.

[MTRichards]

So basically a response that handles three types of rule problems:

1. Failed, fatal.
2. Failed, non-fatal (in other words, this specific request is denied)
3. Success

The rules that throw those errors, and more lists of file firewall rules - this is still owed.

[MTRichards]

this may already exist. If it does, we just need to know what the error codes are

[danimo]

@nickvergessen Now that you are back, can we revisit this issue? Or do you think the FF needs more work before we can address this issue in the client?

[nickvergessen]

The FF needs a lot more work, https://github.com/owncloud/firewall/issues/133
I will try to fix this asap, afterwards we can talk about which "rule" should be handled by the client and what you need for that.

[danimo]

@dragotin @MTRichards Let's move this to 2.1 then, so it doesn't block 2.0.2.

[nickvergessen]

Okay, here I am again. I'm currently working on the part that returns the Error.
Atm it is always a HTML page, which of course is bad.

Now the question is, what do you guys want as returned status codes:
I'd say 403 (Forbidden) makes sense and is quite obvious.

We will throw an Sabre Exception with that status code, similar to maintenance mode:

<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>Sabre\DAV\Exception\ServiceUnavailable</s:exception>
  <s:message>System in maintenance mode.</s:message>
</d:error>

Would you like to have custom exceptions, so you know something that needs to change before you can retry it, or would a simple clean exception also be fine?

The things that can currently be combined are:

• Invalid Time (changes timebased)
• Invalid IP / Subnet (changes on network change)
• Invalid UploadSize (only this file/request)
• Invalid RequestUrl (only this request)
• Invalid Device / UserAgent (does not change)
• Invalid GroupMembership (does not change)
• Invalid Others (unknown)

Starting from 8.2 the rules are defined as a set of "Ands".
So once either the time, ip or item changes, it's worth trying again.

[MTRichards]

Ultimately the best thing is to have an error code that is either fatal or non fatal to a sync, so at least those two separate error codes. I we can have that, it allows the clients to decide "stop the sync run" or "stop syncing one folder or file". User agent could be stop sync, whereas file size would be stop syncing that specific file.

After that, error codes or strings could be used for more specifics, but not the expert on how to do that best.

[felixboehm]

Use case: Block syncing of a specific folder for a group of users
File Firewall does its job, but Sync Client stops syncing for all folders, as access is forbidden for single folder.
@dragotin Please confirm, I assume this is the the way it behaves right now?

[dragotin]

Ok, I tested that again, with #3490 merged: The file firewall of the test server forbids mp3 files. When I add an mp3 file to the local sync folder, the server comes back with an error, and the client shows a red-X for this particular file. Other files upload properly. After a short while, the red X turns to a information icon in the activity-issues-view, telling that the file was not uploaded because of firewall violation.

In the screenshot you see the previously uploaded file die_drei_bösen_buben.mp3 that is now blacklisted, while adele.mp3 was uploaded before and the red-X entry is gone.

I think this is the exact behaviour how we want it.

[dragotin]

@Dianafg76 you might want to verify.

[MTRichards]

I think so to.
@jancborchardt ?

[Dianafg76]

I verify the issue and is working OK
Desktop v ownCloud-2.1.0.2907-nightly20151125.pkg
Server v {"installed":true,"maintenance":false,"version":"8.2.1.4","versionstring":"8.2.1","edition":"Enterprise"}

Closed