Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add information to "Extending the Permission API" section #265

Merged
merged 2 commits into from
Sep 20, 2017

Conversation

alexshalamov
Copy link

@alexshalamov alexshalamov commented Sep 7, 2017

Fixes: #132
Fixes: #22


Preview | Diff

index.bs Outdated
<h3 id="permission-api">Extending the Permission API</h3>

Provide guidance on how to extend the Permission API [[PERMISSIONS]]
for each [=sensor types=].
{{Sensor}} interface for concrete [=sensor=] must protect it's [=sensor reading|reading=]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An implementation of the {{Sensor}} interface for each [=sensor types=] must protect its

index.bs Outdated
for each [=sensor types=].
{{Sensor}} interface for concrete [=sensor=] must protect it's [=sensor reading|reading=]
by associated {{PermissionName}} or more complex {{PermissionDescriptor}}.
[=Low-level=] {{Sensor|sensor}} may use it's interface name as a {{PermissionName}}, for instance

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A [=Low-level=] {{Sensor|sensor}} may use its

index.bs Outdated
by associated {{PermissionName}} or more complex {{PermissionDescriptor}}.
[=Low-level=] {{Sensor|sensor}} may use it's interface name as a {{PermissionName}}, for instance
"gyroscope" or "accelerometer". [=sensor fusion|Fusion sensors=] must
[=request permission to use|request permission to use=] sensors used as a source of fusion.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

request permissions for each of the fused sensors ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might sound that source sensors are also fusion sensors.

index.bs Outdated
"gyroscope" or "accelerometer". [=sensor fusion|Fusion sensors=] must
[=request permission to use|request permission to use=] sensors used as a source of fusion.

Even though, it might be difficult to reconstruct [=low-level=] [=sensor readings=] from

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this comma needed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so.

index.bs Outdated
therefore, those sensors must [=request permission to use|request permission to use=]
"magnetometer" as it provides information about orientation of device in relation to Earth's
magnetic field. In contrast, relative orientation sensor does not expose such information, thus,
does not need to [=request permission to use|request permission to use=] "magnetometer".

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it does

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brainstorming some examples that I don't think (subjectively) require permission:

  • ambient light
  • ambient temperature
  • barometric pressure

I'm interested in reasonable scenarios that counter my initial intuition

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pozdnyakov you mean relative orientation sensor readings fused from gyro and accel need magnetometer, or comment was for different line? 😄

@rwaldron ALS needs permission, there are pin skimming research papers. Ambient temperature and pressure, I'm not sure, maybe you are right. Do you want me to add exception for such sensors?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assume someone lives alone at sea level (1 Bar) on the east coast of America and that room temperature inside their home is 20°C. If an ambient temperature sensor says it's 5°C this person is more like to be outdoor in winter than in their home. If a barometric pressure sensor says it's 0.5 Bar this person is more likely to be at the top of a 5000 meter mountain than in their home. In either case it's possible to deduce that the person is not at home and to safely burgle their home without being disturbed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fivdi Thanks, those are possible attack vectors for information leaks.

@rwaldron I thought about possible scenarios where permission might not be required and here are few things to consider:

  • UA can decide to autogrant permission to access 'safe' sensors.
  • Attack vectors are unknown, would be better to have permission token, so that UA might patch and remove autogranting or apply other mitigation strategy for previously 'safe' sensors.
  • GS API spec and all concrete sensor derivatives rely on Permission API integration and need permission tokens. Also we don't have sensors that fall into 'no permission token needed' category at this moment.

My proposal is to keep Permission API extendability for new sensors as is.

I can create issue for Level2 to investigate possibility of supporting sensors without dedicated permission token / descriptor.

Copy link

@pozdnyakov pozdnyakov Sep 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just meant that it would be better to add "it" => "It does not", but I see now how it actually looks :/ sorry for the hassle..

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pozdnyakov fixed. Thanks.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rwaldron Rick, could you please take a look again? Thanks!

index.bs Outdated
does not need to [=request permission to use|request permission to use=] "magnetometer".

In order to provide fine grained control over the sensor's data,
{{PermissionDescriptor|permission descriptors}} may be used. For example, descriptor can contain

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{{PermissionDescriptor|permission descriptors}} can be also used to control accuracy or sample rate of the exposed [=sensor readings=] ?

index.bs Outdated
{{PermissionDescriptor|permission descriptors}} may be used. For example, descriptor can contain
settings for accuracy or [=sampling frequency=].

Here is an example of {{PermissionDescriptor}} for a possible extension of the Permission API for

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Here/Below ?

@alexshalamov alexshalamov force-pushed the permission_api_extensions branch 3 times, most recently from f7ef32b to bb82b3e Compare September 8, 2017 08:25
it does not need to [=request permission to use|request permission to use=] magnetometer.

{{PermissionDescriptor|Permission descriptors}} can also be used to set maximum allowed limits
for accuracy or [=sampling frequency=]. An example for a possible extension of the Permission API

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

of the PermissionDescriptor interface ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PermissionDescriptor is a dictionary. All API specific dictionaries are defined in Permission API. Actually it would be nice to have something like partial dictionary or namespace in Permission API, so that other specs would not need to modify it whenever new token is required.

@rwaldron
Copy link
Contributor

I attempted to resolve this conflict, because it looks like index.html just needs to be regenerated, but bikeshed did something bizarre:

@alexshalamov alexshalamov force-pushed the permission_api_extensions branch from bb82b3e to 26ff704 Compare September 20, 2017 07:26
@alexshalamov alexshalamov merged commit 298c81d into w3c:master Sep 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants