On 08/29/2013 05:36 PM, Robert Craig wrote:
On Thu, Aug 29, 2013 at 3:20 PM, Ken Black <[email protected]
<mailto:[email protected]>> wrote:
On 08/29/2013 02:46 PM, rpcraig wrote:
>
> c) "Combine" the handling of the availability of a
feature [e.g.
>PackageManager.hasSystemFeature(PackageManager.FEATURE_CAMERA)]
and a
>"pre-defined", named SE Boolean, so that:
> hasSytemFeature() returns "true", iff
"device supports
>camera" AND "SE Boolean allows its use";
> Reflect this also in the method
"getSystemAvailableFeatures()";
>
I certainly get that having a boolean to toggle the state of
the camera
would be confusing if the device doesn't even have a camera.
With your
proposed changes though, what if the camera use is toggled
back on via
the boolean but the hasSytemFeature() has already executed and
reported
back that no camera exists. Just a thought.
Not sure what the above paragraph means. But to clarify what we
are suggesting:
<snip>
Both values must be true for the menu to appear. There are (at
least) two problems with this style:
1) This code must be sprinkled all over settings, hence the
suggestion of hiding the checking of the SE Boolean in the
hasSystemFeature() call.
2) This code (or at least the boolean name: SECURE_WIFI_ALLOWED)
is now tightly coupled with the names in the .te file where the
booleans are defined.
This second problem is the uglier of the two. Esp since we've
added several booleans related to many poss H/W
accessories/devices (e.g. wifi, bluetooth, NFC, etc); and as we
rev over time, we see more booleans of the same ilk being added to
the "standard" sepolicy files, with different names.
So, we're trying to minimize code changes throughout the AOSP
tree, (especially in settings and framework/base areas); and
trying to eliminate (or constrain to a single place) the
dependency on specific SEboolean names. Finally, trying to deal
with the overlap in functionality with the
hasSystemFeature(.<SOME_FEATURE_NAME>).
I guess I needed to be a bit more verbose with my thoughts. If the
proposed changes are just to things
like the Settings app and other menu UI things, then I can see the
real utility in this. I'm more worried
about pushing the changes inside the hasSystemFeature() function. I
would think we need to be mindful
of cases like the following. You start with a policy that has the
"bluetooth" boolean set to disabled (or even
your init.rc sets the boolean to disable). When the system server
starts it does a routine check on
hasSystemFeature(FEATURE_BLUETOOTH). Because the policy is blocking
use of bluetooth your new
logic would have hasSystemFeature() return false. Because of this the
bluetooth manager service is never
registered with the Service Manager. Then, when the phone has booted I
decide to toggle the bluetooth
boolean back to enabled or maybe a new policy is pushed to my phone
that enables bluetooth. How would
you now get access to bluetooth given that it's not registered with
the Service Manager?
Maybe we could always start with bluetooth enabled or just reboot the
phone.... But these are the types of
cases I think we should be taken a closer look at. FYI, there are
other similar cases like this,
i.e FEATURE_USB_HOST, FEATURE_WIFI_DIRECT, ...
Yes. Totally valid comments. It may not be a good thing to overload the
hasSystemFeature() function for the reason you gave.
Or said differently, we are changing the semantics of what it means from
"Can it do X" to "Can it do X, and is it allowed to do X".
And this may be a bad change for other parts of the system. Maybe an
addition of isSystemFeatureAllowed() could be added that
checks the boolean.
We are just throwing it out, because we've already implemented the
changes mentioned above to Settings and to other portions
of AOSP for our project, but immediately thought they were not ideal for
the reasons mentioned, and hoped for a better
solution that *may* be rollable into an upstream GIT.; but at the least
would be more in-line with the current SE Android direction.
One big question that occurred to us immediately, was: Is calling the
SELinux API to get the SEBoolean value from within the
general Android code base abusing the purpose of those SEBooleans? It is
needed (or something similar) if you want to dynamically and
gracefully change some core Android functionality, but calling it
directly causes your Java code to become tightly coupled w/ .te files. :-(
W.r.t.. your bluetooth example above. Yes, we have had to deal w/ some
enable/disabling of features is timing dependent, and
may require a reboot if you enable a feature after the Android world has
been brought up. All of your examples in that last sentences are
good ones, and you can certainly add GPS to it. Of course there's the
tradeoff of starting up daemons, and then trusting that you turn
them off "enough" versus not starting them, but maybe requiring a
reboot, or more explicit code to start them later...
-KB
