Removed "OurFileProvider"

[L-Henke]

This seems to be a relict which isn't really needed anymore.

@Doplgangr maybe OurFileProvider did something else that I missed, but it seems to me that the changed FileViewer should do the same as before.

[Doplgangr]

-------------Outdated------------
(see comment below)

Somehow along the way I ditched using it without removing.it is now merely a placeholder to contain the class name.I'll check the individual commits when I am back on a pc.

[Doplgangr]

A. per. the comments above, the fileprovider actually provides a file obfuscation problem. What really needs to be changed might be editing out the uneeded functions and renaming the fucntion to a more useful name.

[L-Henke]

Now I see what the intention of the FileProvider was, but I still think that it is a very big solution for a very small problem. I don't know if the temp file really has to be hidden from the user. Further this leads to problems with some apps. e.g. I can open a video from Secrecy with the default Android video player. But if I use VLC as my default video player, I can't open a video. This is no problem without the FileProvider.

If a user really cares where the temp files are, it would be no problem to find out where they are, with or without the FileProvider. Also deletion of the temp files also works without the FileProvider, so I'm not really sure what advantages that brings..

[Doplgangr]

Is there a good way to handle purging of the temp file after being read by the opening app?

The very basic reason why a fileprovider is used is that it can provide a ParcelFileDescriptor, which acts more or like a file location pointer. We can have better control over the file when the other app is opening it.

The problem (I think) using the very simple system file provider is that you can never know whether the app has opened the file or not, and react according to the actions. Some apps are quite messy in their implementation of file I/O, with repeated file open/close and non-specific pattern where we are sure the app has already read all of data it needed, and it can be messy in the sense of security implementation.

In the current implementation, when the file is opened,
public ParcelFileDescriptor openFile(Uri uri, String mode)
(line 319 of OURFILEPROVIDER.java)
is called, signalling the data to be sent to the opening app. There I have injected the code to handle purging of the file. As we know that the opening app should already have a handle or something to read the file, we can safely delete the temp file and spend time purging it. This approach is better than waiting for the close action since we are not sure that the apps will close the files gracefully after they are done with it.

Note line 327 and 328 is custom implementation:
//startFileOb

•        CustomApp.jobManager.addJobInBackground(new ObserveFileJob(file));
  

But if there is a better way of doing this with fileprovider and ensuring some time where we can purge the file I think I am fine with it.

Ref1: on the behaviour of ParcelFileDescriptors
http://blog.tomgibara.com/post/779006479/sharing-dynamic-resources

Ref2: on a similar problem, which might be a pointer to a better implementation but I am still puzzling over this.
http://stackoverflow.com/questions/15897208/when-to-delete-file-shared-with-external-apps-by-content-provider

[L-Henke]

I understood that reason and I have tested it, but the ParcelFileDescriptor seems to be broken in the master, because the files were not deleted as long as secrecy was running. There deleted when I left secrecy by the Storage.deleteTemp() method.

I'll give it another try, but I thing that was the behavior back when I tested that.

[Doplgangr]

The behavior is quite fluctuant I guess. It is not really the best security implementation I could think of.

[L-Henke]

Maybe we can improve the behavior. The Linux kernel provides the inotify methods to watch files for changes. It has been implemented into the FileObserver in Android http://developer.android.com/reference/android/os/FileObserver.html. Maybe we can use them for an improved file deletion.

Edit: I see you are also using the FileObserver. I'll check that code.

[Doplgangr]

Yes. I have tried one implementation here:
https://github.com/SecrecySupportTeam/secrecy/blob/master/app/src/main/java/com/doplgangr/secrecy/Jobs/ObserveFileJob.java

I have experimented it with a number of ways, coming to the following conclusions:

1. You never know if the app would call the close function. If then, how could one find a suitable moment to believe the file is really closed.
2. Even if the app calls the close function, it might not actually mean that it is finished with the file. It is actually very common for an app to repeatedly open and close a file for repeated readings especially for large files.

These are the biggest obstacles. If only we can get over them.

[L-Henke]

Its pretty challenging to adapt to other bad implementations..

We always clean the cache directory when secrecy is properly closed. So we always have that as a fall back. And maybe that is already enough security if other apps are really using files that bad.