Working together on one framework.

[CollegeDev]

Hey mateo, FFU5y, wbedard and Lekensteyn,

I've made a bunch of changings to PDroid since the latest Update. It contains a lot of Bug-Fixes and some new features which are (in my Opinion) very useful. I would like to see that we melt together to create ONE Powerful framework where each of us can review the code and discuss new features or changings inside the framework. It makes no sense to split the whole work at the framework and work against. And really, I'm new to GitHub itself but I like the way we can discuss and provide code here. You can find the patches and a partial description of fixed bugs and new features here. I will push the patches to GitHub asap. What I would like to see in the framework:

• Privacy Debugging Bridge (PDB)
• Complete PrivacyCaching (it has so many advantages, please read the post in my thread for more information)
• service connection bug in the central way -> just have a look the patches
• hardcoded default deny mode -> it is ready now, why should we code it again? (it also gives notification to use, like wsot handled it in his preversion. I've updated the whole code to this so you just have to diff your source)
• disabling apps feature -> it works pretty well at all and is very powerful
• task killing -> I use it every day and it is better than going to every app and force-close it
• FailSafeMode -> I think it is a nice feature, but not mellowed and optional to include. If you want to improve it, feel free to provide Code

Please let me know what you think about the new features and the idea of one PDroid framework*. If you have any questions to the new features of bugfixes, please get in touch with me :-)

[mateor]

I think that is absolutely the way to go. FF doesn't have internet at the moment, but we have talked about it before. I am looking at some options for a code review system, something where we can formalize some sort of review process for various merges.

I am very excited to have all the PDroid related minds working on the same project,

[wbedard]

Just a few quick thoughts having finally read all the current issues in this repo. First, I am ecstatic that OpenPDroid finally has several knowlegable, talented devs reviewing the code and making notes here in a public forum. The value of your time and effort cannot be overstated! Second, I'm not sure that we have enough developers that we can simultaneously add sweeping new features and address the numerous low-level security "soft spots" that you guys are uncovering. Honestly, while I understand CollegeDev's motivations and appreciate his innovative approach to the feature, I really feel that Task Killing and Application Force-Closing/Freezing is a feature for a plugin or an altogether separate patch+app. Including it in OPD really feels like "feature creep".

[mateor]

I was thinking of a plugin as well. There is all sorts of value to giving users different options for the apps and so forth. CollegeDev, you might have to work with me a little bit, but if we try and have portability as part of the goal, then we could have 'pd2.0' in the auto-patcher be OpenPdroid+ a tasks plugin. Something to think about.

[CollegeDev]

I've changed the code in a way, that a new system app (called PDroidAgent) does the job. But it needs 4 (only!) additional methods in the Service to work and verify that only the manager application is able to run this operations. The only thing that OPD needs is some more static final variables and 4 methods, nothing more to do here. And if the user prefer this kind of feature, he/she can install the system application manually or downloading an additional patch for this "plugin". Is that okay for you, guys? That means that OPD is "ready" for the new features, but it depends on the user if he/she would like to use it or not.

I know that there are some philosophical difference between the devs working on PDroid, but I think a good goal is to provide an "flexible" OPD that everybody can decide what he wants to use or not (as far as the implementation is not "overheading" the core).

[Lekensteyn]

I am worried about feature creep too. The "P" in PDroid stands for "privacy" right? Can we keep a core patch that adds sanitizing/filtering capabilities for permissions, and make things like task killing and disabling apps optional? That would make it easier to perform review.

[wbedard]

Let me continue to play "Devil's Advocate" a bit longer before I let this one go...:>)

OpenPDroid (OPD) is an architecture consisting of "low level" patches to the Android OS and a "high level" application. The application's function is to provide a UI for managing the operation of the underlying OS patches. If you look at CollegeDev's app, there's no question that he has many more features than FFU5y's PDroid Manager and I have absolutely no issue with that. Many of his users really like that feature-set and, in fact, want more. However, the underlying patches to the Android OS is and always have been about permission and "privacy data" management.

As far as the aforementioned Task Management features being added as a "plugin", what exactly would they be plugging into? Obviously, OPD could certainly extend the core patches to include support for such features at a very low level. I think it then begs the question though, where do you draw the line? Working at the OS level, OPD could also extend itself into Anti-Virus/Malware defense, Firewall management and other "security-related" areas that, on the surface, sound like a nice fit. However, OPD was never "scoped" to be a general-purpose security app, just as it was never scoped to be a Task Management architecture. The discussions about the interface to "iptables" were right on point. That's already being handled much more effectively by other projects. To CollegeDev, Task Management may not be handled to his satisfaction by any other project but why does OPD have to be the vehicle that takes that feature to a new level? Unless there's some natural tie-in or interface within OPD that I'm missing, I think you run the risk of allowing the project to grow to the point that it either fails under the weight of it's own feature-set or simply become unmanageable due to it's complexity or simple lack of developer man-hours.

I'm pretty sure that's all I have to say about that...:>)

R/
wbedard

[CollegeDev]

Yes, wbedard. I like your statement and I can understand you.
I'm going to remove it from framework and implement it full in my application. Furthermore I will give the user the possibility to enable/disable this features (also in GUI). I think that's a good solution.

What I would like to know is what you're thinking about the leftover implementations:

• Privacy Debugging Bridge (PDB)
• Complete PrivacyCaching
• service connection bug
• hardcoded default deny mode

I would also like to hear your opinion about the implementation itself.
Many thanks :-)

[wbedard]

Hey CollegeDev,

While we could certainly discuss the add'l features here, it would be better it we did it in the context of a "Pull Request" from you against your fork of OPD. Do you have a specific timeframe when you think you will be setup on GitHub to include your fork of OPD and pull requests highlighting the individual features and their associated code? There's no rush but, for the sake of posterity, it would be better if this discussion was held in a more traditional context that will help additional/new contributing developers understand what is going on.

In the meantime though, I am re-reading your announcement of the new features to refresh my understanding of what they do. However, I am not prepared to dig through your patches to find the code that relates to those features without your assistance. A formal GitHub pull-request would probably be the best way you could provide that assistance to your fellow OPD developers.

R/
wbedard

[Lekensteyn]

@CollegeDev

• PDB: Sounds good, I haven't looked at the implementation yet, can you try to separate the changes and do a pull request?
• Caching: Sounds good, I haven't looked at ....
• Service connection bug: I ....
• Hardcoded default deny: I like this, a whitelist approach. I haven't... alright you get the idea ;)

Currently I am shorter on time than last week, so I won't be able to do a full review on every line I am afraid. Splitting it up would make it easier to review.

[wsot]

Right: so I finally have internet again (actually, got it a couple of days ago but I've been playing catch-up).

Just to summarise what I've done in the 'development' branch of OpenPDroid, the key things are:

• Added pdroid-exceptions which can be thrown by the PrivacyServiceManager: the point of these is to make it possible to differentiate 'failures' from 'no result'. Basically, the previous code returned 'null' if either: there were no settings for the app, or an error occurred. The problem there was that if an error occurred, then the action would be allowed. In order to be able to handle errors differently, it was necessary to add exceptions. Rather than using generic exceptions, I added specific PDroid exceptions.
• I added a bunch of code to handle those exceptions (as well as service connection failures) by 'deny'ing.
• I added cacheing of settings loaded from the database
• I added service reconnection attempts in the PrivacySettingsManager class (which is used to interface with the service). Basically, if the service connection is null, then it attempts to reconnect to the service.
• Vaguely intelligent checking of the service: reflection is used to make sure that the 'service' object is of an acceptable class, rather than being a subclass. The goal of this was to avoid reflection attacks, but I'm pretty sure there are still weaknesses there
• Other things I've forgotten and I'll need to check

I'm all for one PDroid framework - the upside is that there can more eyes on one codeset. The downside is if different people feel that different things should be included. I haven't really seen a problem with having both OpenPDroid and PDroid 2.0 co-existing, but ideally with mostly shared code - e.g. all of the main privacy stuff shared code, and maybe some 'extras' (e.g. task killing) only added to one. Mind you, it looks (if I read it right) like task killing is going to be moved out the PDroid 2.0 app, which I think is great BTW. I wasn't really sure it belonged in the core.

I'll look over the code in the patches, and I imagine there will be some duplication with what has been done. It sounds like we've both implemented default-deny, both implemented some kind of debug bridge, both implemented cacheing, and indeed both implemented some form of service reconnection.

There is also some stuff I have implemented in an older release but not brought up to the most recent version: actually, two things:

• PDroid Application Chooser: the app/mod which allows multiple simultaneously installed manager apps to be used, and identifies them by signature
• Batch processing: this is basically just code so that a whole lot of PrivacySettings can be retrieved, updated, or deleted in a single function call. It is simply a performance booster. However, it was built with the old database locking (which didn't work properly) so I'd need to rejig it to work with the new database locking.

I'm not wedded to either of these, but it wouldn't hurt.

In terms of the manager app: I'd love to be able to spend more time on PDroid Manager (even just to fix bugs) but I think it is more important to get OpenPDroid kicking along better first. PDroid Manager has bugs, but it does do the job reasonably well. Plus, there is now a helpful contributor to it, which is awesome.

[manad777]

Pooling the resources of similar-minded developers is a great idea, and I'm glad you guys are considering it. Remember what happened to Linux when a few developers all went their separate ways duplicate efforts on different distros and frameworks. Teaming up is the best thing you can do.

I'm new to Android and I don't know Java, but I would like to contribute to this project in any way I can. I'm too busy at work in the next couple of months to join the coding effort, but I can write docs, test stuff, etc. Are any of you on IRC, and if so, which server/chan/nicks?

[CollegeDev]

Added pdroid-exceptions which can be thrown by the PrivacyServiceManager: the point of these is to make it possible to differentiate 'failures' from 'no result'. Basically, the previous code returned 'null' if either: there were no settings for the app, or an error occurred. The problem there was that if an error occurred, then the action would be allowed. In order to be able to handle errors differently, it was necessary to add exceptions. Rather than using generic exceptions, I added specific PDroid exceptions.

I like that, but where do you catch the exceptions? I would prefer to catch them in the pSetMan itself, to prevent overheading inside the framework methods!

I added a bunch of code to handle those exceptions (as well as service connection failures) by 'deny'ing.

And that is what I mean, it is horrible to maintain such bunch of code. I would prefer a central implementation like mine, what do you think about?

I added cacheing of settings loaded from the database

Can we get together that you cache the whole settings at startup? Also, for better debugging purpose it is recommended to spend a final class for the cache itself and writing some Logs for better overview. What do oyu think about?

I added service reconnection attempts in the PrivacySettingsManager class (which is used to interface with the service). Basically, if the service connection is null, then it attempts to reconnect to the service.

Here too, where and how did you implemented it? It is also recommended to handle it in a central way with some reconnect parameters, so that we can change and maintain the code very easy and prevent overhead in the frameworks.

Vaguely intelligent checking of the service: reflection is used to make sure that the 'service' object is of an acceptable class, rather than being a subclass. The goal of this was to avoid reflection attacks, but I'm pretty sure there are still weaknesses there

Sounds good. Can you explain it a little bit more detailed with some line datas?

PDroid Application Chooser: the app/mod which allows multiple simultaneously installed manager apps to be used, and identifies them by signature

Currently I think it is not recommended to use this, because we should base on a stable security system from android. Developing this to get it work is easy, developing this kind of code in a very secure way is most likely impossible with 4 guys only.

Batch processing: this is basically just code so that a whole lot of PrivacySettings can be retrieved, updated, or deleted in a single function call. It is simply a performance booster. However, it was built with the old database locking (which didn't work properly) so I'd need to rejig it to work with the new database locking.

Can you point to a peace of a code?

[wsot]

MachinTrucChose: I'm not on IRC much, but I could be on freenode if that is where you hang out.

Stefan/CollegeDev:
(quick note: all of this refers to the openpdroid-devel code)
With the exceptions, I've been letting them go all the way to the code which calls the PrivacySettingsManager and adding exception handling where that is called. I agree with you that the best place to catch and handle those errors is actually inside the PrivacySettingsManager. When the outcome of an error in the PrivacySettingsManager was always to 'deny', then having exception handling at that next step up made sense, but giving the user the ability to choose whether the action will be to 'deny' or 'allow' if an error occurs means the handling should be in the PrivacySettingsManager, as you say.

With the caching: I'm currently caching on demand, and keeping the cache fairly small. The main goal of the cache was to hold things like the phone, settings for something using the camera or compass (and so rapidly and repeatedly requiring access checks), etc. The LRUCache is good for that, but it isn't necessarily the best caching approach. The advantage of caching everything is that if something modifies the privacy database directly when it shouldn't, then the content can be restored from the cache. Mind you, if someone did want to modify the database directly they could just kill the privacy service first and then replace the database while the service was offline.
It is basically a tradeoff between memory and database lookups. I don't think that the LRUCache or the complete hash table cache is substantially better than the other, really. It probably comes down to how much memory we really want to hand over to a cache.

" Also, for better debugging purpose it is recommended to spend a final class for the cache itself and writing some Logs for better overview. What do oyu think about?" I'm not sure what you mean by this, sorry.

With the reconnection to the service: I put that in the PrivacySettingsManager (same as you). I don't do multiple reconnection attempts, though, and I do type-checking on the service object. I also added a static function to the PrivacySettingsManager so that the service is always obtained using that static function. The reason for that is that in doing so, I can grab and hold onto a weak reference to the context provided when the service is obtained. That can then be used to reconnect to the service again in the same way (getSystemService or stub.proxy).

The class-checking code is:

public boolean isServiceValid() {
if (!isServiceAvailable()) return false;

    String serviceClass = this.service.getClass().getCanonicalName();
    if (serviceClass.equals("android.privacy.IPrivacySettingsManager.Stub.Proxy") || serviceClass.equals("android.privacy.PrivacySettingsManagerService")) {
        return true;
    } else {
        Log.e(TAG, "PrivacySettingsManager:isServiceValid:PrivacySettingsManagerService is of an incorrect class (" + service.getClass().getCanonicalName() +")");
        (new Throwable()).printStackTrace();
        return false;
    }
}
    }</code>

Basically, without this it is possible to create a subclass of IPrivacySettingsManager, implement functions like getSettings which returns 'allow all' settings, and inject it into the PrivacySettingsManager service field.

With the 'choosing among multiple security managers': For the moment, I'm ok with only allowing one manager app at a time to be installed. If you want to look at the current state of the core code for the 'chooser' though, you can look at the 'jb-mr1-release-pdroid-PAC' branch of the 'platform_frameworks_base' in OpenPDroid. In terms of security - it is OK unless the PDroid database is modified, but if the PDroid database is modified then something can add/remove manager apps easily. It is implemented to use one of two identifiers for the calling app - either the complete signature (which is specific to that version of the app), or the public key of the signature (which would then work for subsequent versions of the app). Basically, it still relies completely on the cryptographic signatures that are used in Android for permissions generally.

For the batch processing, you can similarly look at the 'jb-mr1-release-pdroid-batch' branch.

[mateor]

I am not going to close this issue until complete, but this is moving forward. Gerrit is still not up, due to my inexperience with any number of things. I have it running on the server, but it doesn't expose to the world as an app, only a directory listing. Hopefully, the repo/gerrit list can set me straight, it seems like I am missing something fairly basic.

CollegeDev, if you want to work on merging this code as well, let me know. I am going to do my best to keep the best additions of OpenPdroid while still realizing that your 1.54 patches are probably the latest thinking.