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

Security considerations section - second PR #194

Merged
merged 3 commits into from
Mar 5, 2015
Merged

Conversation

alvestrand
Copy link
Contributor

Adding security considerations. This replaces PR #15 .

@stefhak
Copy link
Contributor

stefhak commented Feb 27, 2015

LGTM!

@juberti
Copy link
Contributor

juberti commented Feb 27, 2015

lgtm

<ul>
<li>Always requesting permission to communicate using ICE. This
ensures that the browser can only send to partners who you have
shared credentials with.</li>
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that prompting the user is one of the mitigations we are OK with - needs more discution with WG

Copy link
Contributor

Choose a reason for hiding this comment

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

This confused me the first time as well, but I think the intent here is that "ICE is always used, and so you can only communicate with endpoints that are expecting inbound traffic", not that an infobar shows up.

I agree it should be reworded to make it clear it's the remote endpoint's permission that we are talking about.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The word "consent" was used here because "consent (to communicate)" is the term used in the IETF base spec. The fact that people act as if "consent" means "user prompt" no matter what the context is is unfortunate, but probably will pass. The text is, after all, rather explicit that this section does not do user prompting.
Still, edits can be suggested.

@burnburn burnburn merged commit a429ea4 into master Mar 5, 2015
@martinthomson
Copy link
Member

I think that I would have liked to have seen @fluffy's comments addressed before merging this. @burnburn, is there a plan to address those?

adam-be added a commit that referenced this pull request Mar 6, 2015
adam-be added a commit that referenced this pull request Apr 8, 2015
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.

7 participants