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

Web Share API #179

Closed
3 of 5 tasks
owencm opened this issue May 31, 2017 · 13 comments
Closed
3 of 5 tasks

Web Share API #179

owencm opened this issue May 31, 2017 · 13 comments

Comments

@owencm
Copy link

@owencm owencm commented May 31, 2017

Hello TAG!

I'm requesting a TAG review of:

Further details (optional):

You should also know that we've run an origin trial for this feature and the results are summarized here: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/kSEGnTX2dsA

We'd prefer the TAG provide feedback as (please select one):

  • open issues in our Github repo for each point of feedback
  • open a single issue in our Github repo for the entire review
  • leave review feedback as a comment in this issue and @-notify [github usernames]
@dbaron
Copy link
Member

@dbaron dbaron commented May 31, 2017

I'm curious what considerations led to the decisions about how much to expose about whether the sharing succeeded. In particular:

  • what's the value of exposing whether the user chose to share somewhere or cancelled, relative to the violation of any privacy expectation the user might have (that is, the user might not expect that whether they've chosen something at the choice of applications gets exposed back to the Web page)
  • are there possible share targets where success or failure starting the target or transmitting the data might leak other information about the user's configuration? (For example, if there were a common share target that failed if the user's username in that system plus the length of the share target were greater than 140 characters, then it would be possible to extract the length of the user's username.) Or is that intended to be avoided by the provisions about how declining to receive the data must happen immediately?

@mgiuca
Copy link

@mgiuca mgiuca commented Jun 1, 2017

Hi David, thanks for taking a look!

what's the value of exposing whether the user chose to share somewhere or cancelled, relative to the violation of any privacy expectation the user might have (that is, the user might not expect that whether they've chosen something at the choice of applications gets exposed back to the Web page)

Honestly this wasn't something we really considered. I think the value is probably pretty small but so is the privacy risk, and it just seemed like a logical thing to return (success or failure). Uses for this information are:

  1. If you are showing some modal UI with a share button, share success could dismiss the UI while failure would keep it up. For example, at the end of a game, "You got a high score! " The user isn't forced to share, but if they do successfully share, the dialog dismisses itself. If they cancel the share dialog, we keep the dialog up in case the cancellation was by accident.
  2. Just for collecting metrics on share numbers. You would not want to count cancelled shares.

Are there real privacy risks in exposing whether the user cancelled?

Are you similarly concerned with the provision that the user agent may differentiate the three failure cases (no targets available, cancellation, or starting the target)? If there's concern about that, we could mandate that all the errors look the same. The only concern I can think of is that differentiating "no apps available" from "cancellation" could indicate what apps the user has installed in a world where there are only a very small number of share targets (just by whether it gets past the "are any targets available" step). Or in a future world where the site can filter by MIME type or something, the set of targets that can accept a particular MIME type. So it might make sense to combine "no targets available" with "cancellation".

If those two error cases are indistinguishable, I can't really see how an app could take failure as a definitive or even probable signal that a particular app is installed or not installed.

are there possible share targets where success or failure starting the target or transmitting the data might leak other information about the user's configuration? (For example, if there were a common share target that failed if the user's username in that system plus the length of the share target were greater than 140 characters, then it would be possible to extract the length of the user's username.) Or is that intended to be avoided by the provisions about how declining to receive the data must happen immediately?

Spooky. No, that provision was not intended to prevent such a "scanning" attack, because it would still be possible for a target to "immediately" decide to accept or reject based on some stateful (private data) condition.

I don't think this would be a privacy problem if the error cases were indistinguishable, because it would be impossible to distinguish failure (app not installed, user cancelled, or revealing a bit-value-0 from private data) from success (revealing a bit-value-1 from private data, or a successful share to any other application). In light of this discussion, I'm happy to change "The user agent MAY differentiate between the three different causes of AbortError failure" to "The user agent MUST NOT differentiate between the three different causes of AbortError failure". But I would prefer to still allow the site to distinguish between success and failure.

@torgo
Copy link
Member

@torgo torgo commented Jun 6, 2017

Discussed on call 06-06-2017

@dbaron
Copy link
Member

@dbaron dbaron commented Jun 6, 2017

Yeah, the privacy thing isn't something I have major concerns about -- it's just something I wanted to make sure you'd thought about. Thanks for looking into it.

@cynthia
Copy link
Member

@cynthia cynthia commented Jun 6, 2017

One small issue I noticed in the spec was this bit:

On every call to navigator.share, the user must be presented with a dialog asking them to select a target application (even if there is only one possible target). This surface serves as a security confirmation, ensuring that websites cannot silently send data to native applications.

Seems to be a pretty significant detail, but is in a non-normative section. I think it would be best to make that normative. Additionally, I think for how it should behave on privacy mode could be easier for implementors if the spec strongly recommends one option over another rather than propose implementations doing either this or that.

@torgo
Copy link
Member

@torgo torgo commented Jun 6, 2017

Consensus on tag call 06-06 is that we like it and we think it should move forward. We'd like to know what the plan for the spec is going forward. Will the spec itself go into a rec track document in web platform for example? We would encourage this to happen sooner rather than later. Never the less we are closing this issue.

@torgo torgo closed this Jun 6, 2017
@mgiuca
Copy link

@mgiuca mgiuca commented Jun 7, 2017

@cynthia

Seems to be a pretty significant detail, but is in a non-normative section. I think it would be best to make that normative.

It's also in normative text, in §2.1.1.3.2: "Present the user with a choice of one or more share targets, selected at the user agent's discretion." This unambiguously requires the UA to present a dialog. It's re-stated in the security considerations section in order to highlight it as a strict requirement and explain that there is a security reason for this (so an implementor won't just go "oh hey, if (num_targets == 1) { /* skip dialog */ }".

Additionally, I think for how it should behave on privacy mode could be easier for implementors if the spec strongly recommends one option over another rather than propose implementations doing either this or that.

Do you refer to the security consideration "Implementors should carefully consider what information is revealed in the error message when navigator.share is rejected."? I deliberated this a lot after David suggested this as a privacy issue. I don't think it makes sense for the spec to mandate this one way or the other. On some platforms there will be no privacy risk whatsoever, for example, on Chrome for Android we will just send an intent so if there are no apps we won't even know, and on other platforms we may always guarantee there is at least one option ("copy to clipboard"). It's up to the user agent to decide whether there is a privacy risk here, and also to provide the most useful error messages which could also be platform-specific.

Will the spec itself go into a rec track document in web platform for example?

I hope so, but we haven't reached out to a standards group yet.

@cynthia
Copy link
Member

@cynthia cynthia commented Jun 7, 2017

It's also in normative text, in §2.1.1.3.2: "Present the user with a choice of one or more share targets, selected at the user agent's discretion." This unambiguously requires the UA to present a dialog. It's re-stated in the security considerations section in order to highlight it as a strict requirement and explain that there is a security reason for this (so an implementor won't just go "oh hey, if (num_targets == 1) { /* skip dialog */ }".

Great. (I totally missed that bit, I was reading the spec during yesterday's call so I sort of skimmed through it, missing a lot of details.)

As for the "privacy mode" remark, I was noting this blurb: (should have written private browsing mode, in retrospect)

Use of navigator.share from a private browsing mode may leak private data to a third-party application that does not respect the user's privacy setting. User agents should consider presenting additional warnings or disabling the feature entirely when in a private browsing mode.

I'm not sure what the implications would be (I'm guessing this would be quite platform dependent) by having this feature in private browsing mode - so I can't really suggest which would be better (e.g. I have no idea how this works on Windows Phone), but the remark was about a stronger proposing a stronger preference on one option over another. As a completely unrelated task, we have been looking into implementation consistency with regards to private browsing (and it's surprisingly inconsistent across different implementations), and it seemed like it would be easier for implementors to decide what to do if the spec had a stronger recommendation on what implementations should do.

This isn't a blocker, but more of a suggestion.

@mgiuca
Copy link

@mgiuca mgiuca commented Jun 8, 2017

the remark was about a stronger proposing a stronger preference on one option over another

I see. I think this is so weak because we had trouble making up our minds on Chrome (and it is ultimately at the user agent's discretion what to do around UX in private browsing). Our security team initially flagged it as a problem so we added a warning dialog when sharing from Incognito. Then our UX team said they didn't think it was necessary, and security agreed on the grounds that there would always be a picker shown, and that would be enough for a user to realise they are sharing data, so we removed it.

I left the paragraph in the spec because other user agents may feel differently about explicitly warning the user. I guess if you want the spec to reflect the opinion of Google, we would either take this paragraph out, or change it to a SHOULD NOT. But it seems like something for an implementor to consider, and we don't have a particularly strong reason not to show an additional warning in this case. What would you recommend?

@mgiuca
Copy link

@mgiuca mgiuca commented Jun 20, 2017

Hi again,

Just a note that we've added some new logic to the draft spec (this functionality is in Chrome's implementation but wasn't previously in the spec draft due to an oversight):

https://wicg.github.io/web-share/#share-method

Three new steps have been added to the share algorithm (Steps 2, 3 and 4). What this amounts to is that the url field is processed as a URL relative to the current document, with three practical effects:

  • If url is invalid, share rejects with a TypeError.
  • If url is relative, it is resolved with respect to the current document URL.
  • url is normalized into ASCII with UTF-8 percent encoding.

Discussion here.

Please let me know if you think this change is bad (if the url field should actually just be treated as a string) or whether this needs a new TAG review. Thanks.

@domenic
Copy link
Member

@domenic domenic commented Jun 20, 2017

I raised a few relevant issues (as well as some editorial ones):

@cynthia
Copy link
Member

@cynthia cynthia commented Jun 20, 2017

Apologies for the delayed reply. Last week's call was cancelled due to low participation, so I didn't have time to discuss this further with the group.

I left the paragraph in the spec because other user agents may feel differently about explicitly warning the user. I guess if you want the spec to reflect the opinion of Google, we would either take this paragraph out, or change it to a SHOULD NOT. But it seems like something for an implementor to consider, and we don't have a particularly strong reason not to show an additional warning in this case. What would you recommend?

Since the share() logic in the normative text suggests that the user must be presented with a chooser dialog, I gave it some extra thought and I don't think the additional warning isn't needed. (If any OS comes up with a OS level private mode, we might need to add some extra notes there - but we can deal with that when there is a OS that does this. I have my doubts. :-))

The changes against the URL field look good to me, but I'll discuss this in this week's call so we can move forward on this. If you have any large changes that may need a second round of review at any point in the development cycle we'd be more than glad to sit down and review the changes.

@mgiuca
Copy link

@mgiuca mgiuca commented Jun 20, 2017

Great, thanks Domenic and Cynthia. I'm processing those new bugs now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants