-
Notifications
You must be signed in to change notification settings - Fork 115
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
STUN/TURN OAuth token auth parameter passing #714
Comments
Today we only have an RTCIceCredentialType of "token". Are we expecting to only have a single token type? Or are we expecting the browser to be able to distinguish between different token types based on what is passed in the credential? As an example, how do we expect implementations to support MOBILITY-TICKET? (see: https://tools.ietf.org/html/draft-ietf-tram-turn-mobility). |
…ess Token based auth (fixes: w3c#714).
Please see and comment this branch with my proposed changes: https://github.com/misi/webrtc-pc/tree/issue-714-patch |
Adding more attributes doesn't seem like the ideal solution. People who understand OAuth might want to weigh in. @pthatcherg ? |
Sure. I will. 2016-08-11 16:28 keltezéssel, aboba írta:
|
@alvestrand I see at least two ways, and I have no preference: extend RTCIceServer with the plus two attributes as I proposed. dictionary RTCIceServer {
required (DOMString or sequence<DOMString>) urls;
DOMString username;
DOMString credential;
DOMString accesstoken;
DOMTimeStamp expiry;
RTCIceCredentialType credentialType = "password";
};
Define a new dictionaries and change credential type to PasswordCredential or TokenCredential dictionary TokenCredential {
required DOMString kid;
required DOMString key;
required DOMString alg;
required DOMString access_token;
DOMTimeStamp expiry;
};
dictionary PasswordCredential {
DOMString username;
DOMString password;
};
dictionary RTCIceServer {
required (DOMString or sequence<DOMString>) urls;
(PasswordCredential or TokenCredential) credential;
RTCIceCredentialType credentialType = "password";
};
I am looking forward hearing your thoughts and comments... |
@misi Don't we also need token_type in the TokenCredential dictionary? Do we need to remove RTCIceServer.username? Seems like this creates a backward compatibility issue. |
My intent here was to pass 'kid' in RTCIceServer.username, 'access_token' in RTCIceServer.credential, and set RTCIceServer.credentialType appropriately. 'key' is not needed by the browser. |
As such, I believe no changes are needed here, save for some explanatory text similar to my comment above. |
The MAC key is extracted from the access token as indicated in RFC 7635, S 6.2. |
Justin I am not sure. I believe you are wrong. The key/mac_key is in the encrypted block. Please double-check RFC7635 S 6.2 opaque { And AS-RS key that is used to encrypt this block is not known by the stun client side/WebRTC app side.. => WebRTC client could not extract the mac_key from the token. I hope you see now that the 'key' is needed by the browser. |
Hi, I am not sure that it is needed for WebRTC/ICE agent operation. I think ICE agent do not need this information, and it is good enough if But it is just my opinion 2 cent, Misi 2016-08-23 18:44 keltezéssel, aboba írta:
|
See RFC 7635 S7. This speaks about server side but from it you can understand the client
Get from the client in username the kid
The client needs password/mac_key to calculate Message Integrity that is To create the Message Integrity on client side, it needs the "key" STUN use the "key" normally as it use the "password". => in webrtc api On client side AS-RS key is not known, and so it is not possible to 2016-08-23 22:44 keltezéssel, Justin Uberti írta:
|
What happens if the token close to its expiry or expires?
|
I watched the last interim recording.. Everybody was confused. (I pointed out earlier that the cleanest solution is, to move out into separate dictionaries the credential mechanism according the type and not bind so closely the URI-s and credentials in the RTCIceServer.) I still believe that the dropped PR was a appropriate solution, if we speak backward compatibility, minor change, etc. I still believe adding two or one field does not make so much confusion, then making a bigger change. @juberti So junk dropped. One plus field. I have double-checked that these three informations are enough.
|
…ess Token based auth (fixes: w3c#714).
Section 5 of RFC7635 seems to make it pretty clear that the WebRTC ICE agent must know the kid, access token and mac_key. The access token may be sent in the clear to the STUN/TURN server, so the client needs to demonstrate possession of mac_key to prevent an eavesdropper from reusing the access token (as I understand it, from And a secondary problem we haven't really touched on is that the WebRTC app making the OAuth request must know the hash function(s) supported by the ICE agent so it can put that in the OAuth request and obtain a key of the right length. Or we could just mandate support of HMAC-SHA-256-128? @juberti, thoughts? |
Some updates from me: |
One thing that have not been yet discussed. so in RFC7635 Appendix B. the alg
|
@misi, thanks for your comments. As you indicated above, I was wrong; the client cannot (and should not) extract the key from the encrypted token. Management regrets the error. So, we need to find a way to pass kid, token, and mac_key in the RTCIceServer interface. My suggestion would be that we go all the way and make the credential able to be an object, which could contain token, mac_key, and perhaps hash algorithms or whatever else we need in the future. e.g.
|
@juberti thanks! (I'm worried more about setting a consistent pattern for later extensions than exactly what this extension looks like....) |
Do you mean replacing "token" with "oauth"? |
Yes. There are many tokens, but only a few oauths. (can we forget about oauth1 now? I think the current one is oauth2) |
@juberti many thanks for your answer! Highly appreciated! See below my short recap: If the short token name is not enough so "token" name is too short, and this way it is confusing, @juberti See below my overview figure about the operation: IMHO
|
I still feel a gap between W3C WebRTC API and IETF TRAM RFC7635. Something like: RFC7635(STUN&OAuth) in webrtc context. I still miss a document, an explanation about how WebRTC understand IETF RFCs gives big freedom to implementers. It is focusing on externally observable So, I think there is need for some more explanation about how WebRTC 1.0 API implementing RFC7635. A document about how the W3C WebRTC API understand and propose implementation should work.
This document could go one step further and specify and detail more how AFAIU IETF Standards only focus on externally observable behaviours, but W3C is not the place to standardize such deep protocol connected, very low level things, but if I am correct, it is much more IETF's business. What do you think, would it worth to create an Informational RFC about it? Does it make any sense to you?Let me know.. If yes I will try to draft something about it. |
@misi Do we have an updated PR for discussion at the Virtual Interim on November 9? |
…ess Token based auth (fixes: w3c#714).
I try to make an updated PR. Thanks, Misi On 2016-11-08 04:33, aboba wrote:
|
Responding to your summary:
|
So the final question here is regarding syntax. We have 3 options: 1. Add another attribute.
2. Fully separate password vs token auth.
3. Hybrid.
|
I am ambivalent between option 2. and 1. But prefer little bit more
Option 1.
For me the hybrid (the Option 3.) is the least preferred option
|
…ess Token based auth (fixes: w3c#714).
I don't want option #1 |
Why I prefer more option #2 and less #3
|
@alvestrand @juberti as I promised I have just added a new pull request with my new version. |
misi's PR got #1000 |
Fixes w3c#714. The working group decided on this solution in the Jan 25 2017 virtual interim. The "username" field will be used for the kid, and the other two pieces of credential information will go in "credential".
Fixes w3c#714. The working group decided on this solution in the Jan 25 2017 virtual interim. The "username" field will be used for the kid, and the other two pieces of credential information will go in "credential".
Fixes w3c#714. The working group decided on this solution in the Jan 25 2017 virtual interim. The "username" field will be used for the kid, and the other two pieces of credential information will go in "credential".
I think there is a confusion between the current PeerConnection W3C API and RFC7635
In STUN/TURN auth crendtials/parameters handover...
https://tools.ietf.org/html/rfc7635#appendix-B
Here below I have highlighted the three mandatory parameters that needed to pass to the ICE Agent
So we need to pass these 3 value at least to ICE Agent in the browser through PeerConnection iceServers configuration interface.
So according RFC 7635
and has two other "credential" information pieces, that are needed to auth on remote TURN server.
See: https://w3c.github.io/webrtc-pc/#idl-def-rtciceserver
So in WebIDL I could find only one DOMString for Credential.
dictionary RTCIceServer {
required (DOMString or sequence) urls;
DOMString username;
DOMString credential;
RTCIceCredentialType credentialType = "password";
};
And furthermore this credential field normally in case of "password" auth (Long Term Credential) contains the Session Key(Message Integrity, HMAC key).
I am wondering what is the right way to pass the access token, the third value?
How to pass the 3 information in 2 fields username/credential?
I propose to add a third field for the access_token, or add clarification in W3C PeerConnection.
The actual W3C webrtc-pc saying that the access_token need to be passed as credential
https://w3c.github.io/webrtc-pc/#rtcicecredentialtype-enum
"The credential is an access token"
Any comment highly appreciated!
The text was updated successfully, but these errors were encountered: