A security review; some nits

[richsalz]

@samuelweiler asked me to review the document. I am not an expert in this area. I have a few questions/nits and it seems reasonable to merge them into one issue.

Sec 2.1; why is "SHOULD NOT" not a "MUST NOT" ? What is the use-case for exposing an API that is not supported? Is there some interoperability issue that I don't understand (quite likely).

Sec 2.1.2, I like the highly-detailed processing steps; made it easy to follow.

Sec 3, the note about "fire and forget" should be mentioned in 2.1.2 with a pointer here if needed.

Sec 4. Is there a registry of permission strings (i.e., where "web-share" would appear?) If not, I strongly recommend W3C maintain one.

Sec 6. I could not think of other security concerns, but note again that I am not an expert in this field. The last bullet leads me to think that the share target should fetch the content and use that for any downstream processing rather than handing off URL's. Is that the intent? Is that a best practice? Should that be made more explicit?

[marcoscaceres]

Sec 2.1; why is "SHOULD NOT" not a "MUST NOT" ? What is the use-case for exposing an API that is not supported? Is there some interoperability issue that I don't understand (quite likely).

I agree. This wording needs to go. No one is exposing .share() without supporting share.

Sec 2.1.2, I like the highly-detailed processing steps; made it easy to follow.

👍

Sec 3, the note about "fire and forget" should be mentioned in 2.1.2 with a pointer here if needed.

Moving the "fire and forget" to 2.1.2. makes sense.

Sec 4. Is there a registry of permission strings (i.e., where "web-share" would appear?) If not, I strongly recommend W3C maintain one.

Good call. It now lives here:
https://w3c.github.io/permissions-registry/#registry-table-of-standardized-permissions

Sec 6. I could not think of other security concerns, but note again that I am not an expert in this field. The last bullet leads me to think that the share target should fetch the content and use that for any downstream processing rather than handing off URL's. Is that the intent? Is that a best practice? Should that be made more explicit?

It's application dependent. I'm of the opinion that the guidance as is clear enough. However, I'd be open to specific suggestions.

[marcoscaceres]

Hi @richsalz (and @samuelweiler for sec-review tracking purposes), I've sent a pull request addressing your feedback in #243 ... To allow us to close this issue (and per the W3C process for wide review), could you let me know if you are satisfied with this response and the proposed changes in #243?

[ericwilligers]

Yes, I am satisfied. Thank you for the changes.

…

On Tue, Jun 7, 2022 at 3:47 PM Marcos Cáceres ***@***.***> wrote: Hi @richsalz <https://github.com/richsalz> (and @samuelweiler <https://github.com/samuelweiler> for sec-review tracking purposes), I've sent a pull request addressing your feedback in #243 <#243> ... To allow us to close this issue (and per the W3C process for wide review), could you let me know if you are satisfied with this response and the proposed changes in #243 <#243>? — Reply to this email directly, view it on GitHub <#223 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA5JIYOZE5L7OXZMS2KS53LVN3PA3ANCNFSM5ICD66AQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[richsalz]

The PR looks fine to me.

[marcoscaceres]

Thanks for confirming!

[marcoscaceres]

@samuelweiler, given @richsalz approval, could you close the related issue? w3c/security-review#122