w3c / webrtc-pc

WebRTC 1.0 API
https://w3c.github.io/webrtc-pc/
Other
438 stars 115 forks source link

STUN/TURN OAuth token auth parameter passing #714

Closed misi closed 7 years ago

misi commented 8 years ago

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

     HTTP/1.1 200 OK
        Content-Type: application/json
        Cache-Control: no-store

        {
          "access_token":
   "U2FsdGVkX18qJK/kkWmRcnfHglrVTJSpS6yU32kmHmOrfGyI3m1gQj1jRPsr0uBb
   HctuycAgsfRX7nJW2BdukGyKMXSiNGNnBzigkAofP6+Z3vkJ1Q5pWbfSRroOkWBn",
          "token_type":"pop",
          "expires_in":1800,
          "kid":"22BIjxU93h/IgwEb",
          "key":"v51N62OM65kyMvfTI08O"
          "alg":HMAC-SHA-256-128
        }
                        Figure 8: Response

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

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!

aboba commented 8 years ago

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).

misi commented 8 years ago

Please see and comment this branch with my proposed changes: https://github.com/misi/webrtc-pc/tree/issue-714-patch

alvestrand commented 8 years ago

Adding more attributes doesn't seem like the ideal solution. People who understand OAuth might want to weigh in. @pthatcherg ?

misi commented 8 years ago

Sure. I will.

2016-08-11 16:28 keltezéssel, aboba írta:

@misi https://github.com/misi Can you send a PR?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/w3c/webrtc-pc/issues/714#issuecomment-239177596, or mute the thread https://github.com/notifications/unsubscribe-auth/AATBTS2m79Kf_SxTXFwmTwfQvtjmkx_8ks5qezGngaJpZM4I94P9.

misi commented 8 years ago

@alvestrand
We had some private a discussion with @pthatcherg about it, but without conclusion.

I see at least two ways, and I have no preference:

1. 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";
};

2. 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...

aboba commented 8 years ago

@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.

juberti commented 8 years ago

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.

juberti commented 8 years ago

As such, I believe no changes are needed here, save for some explanatory text similar to my comment above.

juberti commented 8 years ago

The MAC key is extracted from the access token as indicated in RFC 7635, S 6.2.

misi commented 8 years ago

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 As I wrote on the list recognize that in access-token the mac_key is encrypted! And the key is not known on client side!!

opaque { uint16_t key_length; opaque mac_key[key_length]; uint64_t timestamp; uint32_t lifetime; } encrypted_block;

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.

misi commented 8 years ago

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 the WebRTC app that is requesting the token knows that information..

But it is just my opinion 2 cent,

Misi

2016-08-23 18:44 keltezéssel, aboba írta:

@misi https://github.com/misi Don't we also need token_type in the TokenCredential dictionary?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/w3c/webrtc-pc/issues/714#issuecomment-241796381, or mute the thread https://github.com/notifications/unsubscribe-auth/AATBTWHO4YlVRHhIIZvroSGWJfOf19Dtks5qiyNTgaJpZM4I94P9.

misi commented 8 years ago

@Justin

See RFC 7635 S7.

This speaks about server side but from it you can understand the client side too.

o The STUN server selects the keying material based on kid signaled in the USERNAME attribute.

Get from the client in username the kid Using kid you get the AS-RS key

o The AEAD decryption operation has four inputs: K, N, A, and C, as defined in Section 2.2 of [RFC5116] https://tools.ietf.org/html/rfc5116#section-2.2. The AEAD decryption algorithm has only a single output, either a plaintext or a special symbol FAIL that indicates that the inputs are not authentic. If the authenticated decrypt operation returns FAIL, then the STUN server rejects the request with an error response 401 (Unauthorized). K= AS-RS key N=nonce from access_token from the not encypted part. A=domain/server name C=access_token encrypted part.

o The STUN server obtains the mac_key by retrieving the content of the access token (which requires decryption of the self-contained token using the AS-RS key).

.......... Decrypts and validates the token and gets the "key" ("session key"/"mac_key")

o The STUN server uses the mac_key to compute the message integrity over the request, and if the resulting value does not match the contents of the MESSAGE-INTEGRITY attribute, then it rejects the request with an error response 401 (Unauthorized).

The client needs password/mac_key to calculate Message Integrity that is checked in this point.

_To create the Message Integrity on client side, it needs the "key" ("session key"/"mackey") in clear text.

STUN use the "key" normally as it use the "password". => in webrtc api we should use "credential"="key", and define new attribute as stun did for the access token.

On client side AS-RS key is not known, and so it is not possible to extract from access token because the part of the token is encrypted that contains the key ("session key"/"mac_key").

2016-08-23 22:44 keltezéssel, Justin Uberti írta:

The MAC key is extracted from the access token as indicated in RFC 7635, S 6.2.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/w3c/webrtc-pc/issues/714#issuecomment-241872331, or mute the thread https://github.com/notifications/unsubscribe-auth/AATBTcodOd7D4PgsekMUs4uqM-mDkPuhks5qi1uYgaJpZM4I94P9.

misi commented 8 years ago

What happens if the token close to its expiry or expires?

  1. Should the WebRTC app renew and take care about the access_token validity, handle expiry? And set the renewed token through pc setConfiguration method? (Then the token expiry information is not really needed to pass to the browser.)
  2. Or is it better if the ICE agent know the access_token expiry timestamp and could correlate the expiry with the 401 unauthorized STUN error (RFC 7635 S7), and intelligently fire a new access_token expired event to the app?
misi commented 8 years ago

I watched the last interim recording.. Everybody was confused. So I started to work on AFAIU doc to present my understanding about it. May it help others to clarify, or for me if you correct me if I am wrong somewhere.

http://goo.gl/BS2ZcT

(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. The only change that I may would may make, to cut back and add only one field. (Not add the expiry timestamp, but only add the access_token.)

@juberti So junk dropped. One plus field. Sounds good/Ok? (Or at least make it any sense?)

I have double-checked that these three informations are enough. The kid=username,mac_key=password, access_token are enough for STUN/TURN client to Auth with the TURN server.

  1. I am sure that the mac_key could NOT be extracted from the access_key on client side.
  2. The mac_key must anyhow be used as password, so put it on credential is the right and straight forward way. If we in the right variable this value on the interface, then it is used as the password is used, and does not require any change in the browsers internal workflow of credential/password handling. Same code/workflow as Long Term Credential.
  3. So if we move access_token to a new field it provides a much clear and better interface.
taylor-b commented 8 years ago

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 https://tools.ietf.org/html/draft-ietf-oauth-pop-architecture-08).

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?

misi commented 8 years ago

Some updates from me: I asked Roland Hedberg (https://events.nordu.net/display/NDN2016/Roland+Hedberg) to help me double check this issue. He is one of our NREN community best AAI expert. During NORDUnet 2016 conference we met. I asked earlier his advice about RFC7635, so make sure my proposal is not wrong. ( and check that I am not totally crazy :-). We agreed that my proposal is good in this form from OAuth point of view. The thing we also agreed that STUN client do not need to receive the OAuth related params. The Web Application OAuth client modul should use these parameters only, and the Application OAuth modul should take care about acces_token/credential lifetime and renew access-token before the expiry. Only the three parameters (kid,mac_key,access_token) that STUN needs should be passed through WebRTC interface, and according this some correction needed in RFC 7635. So I have to go to TRAM WG and raise the issue that despite RFC 7635 is not clear but intending to say we should pass all OAuth params to STUN client, but my and Rlaond view is tha we don't see any reason to do it in that way. Roland was open to help us and consult with us in any OAuth related questions if the workgroup chairs will need such help.

misi commented 8 years ago

One thing that have not been yet discussed. But it also needs some change in WebRTC interface. Somehow the WebRTC PC interface should discover and contain as read only parameter information about what kind of message integrity algorithm supported by the STUN/TURN client. It is used to determine mac_key length, and create key with appropriate length, depending on the HMAC algorithm.

so in RFC7635 Appendix B. the alg

  The client signals the algorithm supported by it
   to the authorization server in the 'alg' parameter defined in
   [POP-KEY-DIST].  The authorization server determines the length of
   the mac_key based on the HMAC algorithm conveyed by the client.  If
   the client supports both HMAC-SHA-1 and HMAC-SHA-256-128, then it
   signals HMAC-SHA-256-128 to the authorization server, gets a 256-bit
   key from the authorization server, and calculates a 160-bit key for
   HMAC-SHA-1 using SHA1 and taking the 256-bit key as input.
        Host: server.example.com
        Content-Type: application/x-www-form-urlencoded
        aud=stun1@example.com
        timestamp=1361471629
        grant_type=implicit
        token_type=pop
        alg=HMAC-SHA-256-128
juberti commented 8 years ago

@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. Currently, the plan is that username = kid and credential = token, but clearly that is insufficient.

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.

dictionary RTCTokenCredential {
    DOMString mac_key;
    DOMString token;
};

dictionary RTCIceServer {
    required (DOMString or sequence<DOMString>) urls;
    DOMString                          username;
    (DOMString or RTCTokenCredential)  credential;
    RTCIceCredentialType               credentialType = "password";
};
alvestrand commented 8 years ago

@juberti thanks! This looks much like one of the proposals misi was proposing earlier. Would we assume that any new value of "credentialType" would specify whcih fields are required in RTCTokenCredential? Would it make sense to add a "credentialType" of "oauth"?

(I'm worried more about setting a consistent pattern for later extensions than exactly what this extension looks like....)

juberti commented 8 years ago

Do you mean replacing "token" with "oauth"?

alvestrand commented 8 years ago

Yes. There are many tokens, but only a few oauths. (can we forget about oauth1 now? I think the current one is oauth2)

misi commented 8 years ago

@juberti many thanks for your answer! Highly appreciated! I wrote an email to IETF TRAM to ask for their advice in few things about the RFC. I think there is no need to pass so many oauth related attributes to stun. IMHO only that info should be passed down through WebRTC to ICE agent that the STUN/TURN really needs, and are mandatory..

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, then I think we should keep the terminology of the RFC, and not introduce a different new terminology. Keep it what it is in the RFC: "access_token".. So my proposal is to rename "token" to "accesstoken" in dictionary RTCIceServer

@juberti I still think that we could avoid a bigger change, and with a simple modification add only one "accesstoken" attribute.

See below my overview figure about the operation:

IMHO


                                                       +---------------+
                                                       |               =*******+
                                        +------------->+ Authorization +       *
                                        |              | server        |       *
                                        |   +----------+(WebRTC server)|       *  AS+RS,
                                        |   |          |               |       *  AUTH keys
                               (1)      |   |          +---------------+       *   (0)
                               Access   |   |  (2)                             *
                               Token    |   | Access Token                     *
                               request  |   |    +                             *
                                        |   | Session Key                      *
                                        |   +                                  *
                                        |   V                                  *
+---------------Web-Browser---------+---+-------+                              *
|                                   |           |                              *
|                                   |           |                              *
|                                   | OAuth     |                              *
|                          +--------> client    |                              *
|     Web Application      |        | modul     |                              *
|                          |        |           |                              *
|                          |        +------+----+                              *
|                          |               |    |                              *
|                 STUN/TURN|               |    |                              *
|                 Supported|               |    |                              *
|                 HMAC Algs|            kid|    |                              *
|                          |        mac_key|    |                              *
|                          |   access_token|    |                              *
|                          |               |    |                              *
|                          |               |    |                              *
+-------------WebRTC API------------------------+                              V
|                          |               |    |                       +-----+=+----+
|                          |               |    |         (3)           |            |
|                          | +-------------v----+ TURN request + Access |            |
|    WebRTC Stack          +-+                  | Token                 | TURN       |
|                            |    STUN/TURN     +----------------------^+ server     |
|                            |                  | Allocate response (4) |            |
|                            +-------------------<----------------------+            |
+-----------------------------------------------+                       +------------+
misi commented 8 years ago

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 RF7635 implementation should work.

IETF RFCs gives big freedom to implementers. It is focusing on externally observable behaviours. A more detailed picture may could help to avoid any confusion.

So, I think there is need for some more explanation about how WebRTC 1.0 API implementing RFC7635. What are the assumptions, reasons why the WebRTC API designed that way, that I tried explained on figure above.

A document about how the W3C WebRTC API understand and propose implementation should work. Explain WebRTC 1.0 understanding:

This document could go one step further and specify and detail more how to implement the RFC7635 in case of WebRTC. Give more detailed examples. In my view it worth to document these design concepts, and background/reasons behind them. Concepts that WebRTC API (STUN+OAuth related part) is designed in mind..

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? How the WebRTC implementers should implement/understand RFC7635? IMHO such doc would help a lot to fill this gap.

Does it make any sense to you?

Let me know..

If yes I will try to draft something about it. (I already asked one of the IETF TRAM chairs: Simon Perreault opinion, and he offered his help in reviewing, etc.)

aboba commented 8 years ago

@misi Do we have an updated PR for discussion at the Virtual Interim on November 9? Slides are here: https://docs.google.com/presentation/d/1SSokCdnEEg8SO8laBWCWK0emU-W-4uuuqdOIIJG4bOY/edit#slide=id.p

misi commented 8 years ago

I try to make an updated PR.

Thanks,

Misi

On 2016-11-08 04:33, aboba wrote:

@misi https://github.com/misi Do we have an updated PR for discussion at the Virtual Interim on November 9? Slides are here: https://docs.google.com/presentation/d/1SSokCdnEEg8SO8laBWCWK0emU-W-4uuuqdOIIJG4bOY/edit#slide=id.p

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/w3c/webrtc-pc/issues/714#issuecomment-259036352, or mute the thread https://github.com/notifications/unsubscribe-auth/AATBTeE47F3jlKEVYxnYS5Z56-YFaOCLks5q7-1_gaJpZM4I94P9.

juberti commented 7 years ago

Responding to your summary: a) WebRTC PC should be changed and add a new read-only field, about the supported STUN/TURN HMAC alg. (!?!) b) _Only 3 parameters: the kid, the mac_key and the accesstoken must passed to the (ICE agent) STUN/TURN client. c) _The application should take care about token expiry and renew it. And after it pass the new kid, mac_key and accesstoken credentials thorough WebRTC PC interface to the stun client.

juberti commented 7 years ago

So the final question here is regarding syntax. We have 3 options:

1. Add another attribute.

dictionary RTCIceServer {
    required (DOMString or sequence<DOMString>) urls;
             DOMString                          username;
             DOMString                          credential;
             DOMString                          accesstoken;
             RTCIceCredentialType               credentialType = "password";
};

2. Fully separate password vs token auth.

dictionary TokenCredential {
    required DOMString kid;
    required DOMString key;
   required DOMString access_token;
};

dictionary PasswordCredential {
             DOMString                                  username;
             DOMString                                  password;
};

dictionary RTCIceServer {
    required (DOMString or sequence<DOMString>) urls;
             (PasswordCredential or TokenCredential)    credential;
             RTCIceCredentialType                       credentialType = "password";
};

3. Hybrid.

dictionary RTCTokenCredential {
    DOMString mac_key;
    DOMString access_token;
};

dictionary RTCIceServer {
    required (DOMString or sequence<DOMString>) urls;
    DOMString                          username;
    (DOMString or RTCTokenCredential)  credential;
    RTCIceCredentialType               credentialType = "password";
};

My preference is #3, then #1, then #2.

misi commented 7 years ago

I am ambivalent between option 2. and 1.

But prefer little bit more Option 2.

Option 1.

For me the hybrid (the Option 3.) is the least preferred option

fluffy commented 7 years ago

I don't want option #1

misi commented 7 years ago

Why I prefer more option #2 and less #3

misi commented 7 years ago

@alvestrand @juberti as I promised I have just added a new pull request with my new version. "Separated auth dictionaries for STUN/TURN (issue 714)" Please review it, and let me know your thoughts!

alvestrand commented 7 years ago

misi's PR got #1000