Re: [secdir] SecDir review of draft-ietf-tram-turn-third-party-authz-07

["Tirumaleswar Reddy (tireddy)" <[e-mail]>]

Hi Yaron,

Thanks for the detailed review. Please see inline

> -----Original Message-----
> From: Yaron Sheffer [mailto:[e-mail]]
> Sent: Sunday, February 01, 2015 11:26 PM
> To: IETF Security Directorate; The IESG; draft-ietf-tram-turn-third-party-
> [e-mail]
> Subject: SecDir review of draft-ietf-tram-turn-third-party-authz-07
> 
> I have reviewed this document as part of the security directorate's ongoing
> effort to review all IETF documents being processed by the IESG.
> These comments were written primarily for the benefit of the security area
> directors.  Document editors and WG chairs should treat these comments
> just like any other last call comments.
> 
> This document defines a mechanism where a STUN client can present an
> OAUTH token and get authorized to use a particular STUN server.
> 
> Summary
> 
> IMHO, the document is not yet ready to be published.
> 
> Details
> 
> * What is the motivation for this authorization? Is it intended to conserve
> server resources?

Yes, to ensure that only authorized clients can use the STUN server. For example allow the client to use the TURN server only when making a call using the WebRTC server.

> 
> * What is HMAC-SHA-256-128? Is the output 128 or 256 bits long? 

The output is truncated to 128 bit.

> Please
> include a reference.

Added reference to RFC 4868.

> 
> * Maybe I'm confused, but it seems to me that the discussion immediately
> following Fig. 2 mixes the hash algorithm that signs the token itself with the
> algorithm that integrity-protects STUN messages. If they must be the same
> for some reason, please state it clearly.

No, they do not need to be same. RFC 5389 mandates HMAC-SHA1 for integrity protection of STUN message and in future the WG plans to remove this limitation, hence we added this paragraph after Fig.2.

> 
> * 4.1: there are obvious benefits to using an asymmetric key here, and in fact
> this can be done efficiently using elliptic curves (ECDSA). So I think the "MUST
> establish a symmetric key" is misguided.

Agreed. 
NEW:
Elliptic curve cryptography (ECC) or symmetric-key algorithm MUST be chosen to ensure that the size of encrypted token is not large because usage of RSA will result in large encrypted tokens which may not fit into a single STUN message.

> 
> * 4.1.1: the derivation of both keys seems to be the same, so are they
> identical?!

The derivation of the keys is required if the encryption and HMAC algorithm require keys of different length.

NEW:
For example
   if the input symmetric key (K) is 32 octets length, encryption
   algorithm is AES_256_CBC and HMAC algorithm is HMAC-SHA1 [RFC2104]
   then the secondary keys AS-RS, AUTH are generated from the input key
   K as follows

   1.  HKDF-Extract(zero, K) -> PRK

   2.  HKDF-Expand(PRK, zero, 16) -> AUTH key 

   3.  HKDF-Expand(PRK, zero, 32) -> AS-RS key

> 
> * 4.1.2: this interaction (a simple REST query to get a long-term key) is by
> default insecure, so I'm surprised to see it here with no comments about the
> need to encrypt it an authenticate the client.

Good point, updated draft.
NEW:
The two servers could choose to use REST API over HTTPS to establish a symmetric key. HTTPS MUST be used for mutual authentication and confidentiality.

> 
> * 4.1.3: please provide a reference for AES_256_CBC_HMAC_SHA_512.

Added reference.

> 
> * 6.1, last sentence: so an attacker who can disrupt communication to the AS
> can force the client to switch to 1st party authentication, which is easily
> susceptible to dictionary attacks ?

The possible way to mitigate the attack is use (D)TLS for communication b/w the client and server. 

NEW text (added to Security considerations):
   An attacker may remove the THIRD-PARTY-AUTHORIZATION STUN attribute
   from the error message forcing the client to pick first party
   authentication, this attack may be mitigated by opting for Transport
   Layer Security (TLS) [RFC5246] or Datagram Transport Layer Security
   (DTLS) [RFC6347] as a transport protocol for Session Traversal
   Utilities for NAT (STUN), as defined in [RFC5389]and [RFC7350].

> 
> * 6.2: the timestamp value is strange, specifically the fractional part.
> Why are fractions needed, do we think this improves security?

We used the description from http://tools.ietf.org/html/rfc3971#section-5.3.1  
Do you see a problem ?

> 
> * 6.2 example: when using AES-256-CBC something must be said about
> padding and about the IV.

Yes, updated encrypted_block to include padding, padding_length;

         struct {
             opaque {
                 uint16_t key_length;
                 opaque mac_key[key_length];
                 uint64_t timestamp;
                 uint32_t lifetime;
                 uint8_t  padding_length;
                 uint8_t padding[padding_length];
             } encrypted_block;
             opaque mac[mac_length];
             uint8_t mac_length;
         } token;

   padding_length:  The padding length MUST be such that the total size
      of the encrypted_block structure is a multiple of the cipher's
      block length.

   padding:  Padding that is added to force the length of the plaintext
      to be an integral multiple of the block cipher's block length.

And the below text to address the comment on IV

   o  C = AES_256_CBC(AS-RS, encrypted_block)

      *  Initialization vector can be set to zero because the
         encrypted_block in each access token will not be identical and
         hence will not result in generation of identical ciphertext.

> 
> * 6.2, last sentence: the length of the nonce surely depends on the selected
> algorithm, and cannot be a constant 12.

Yes, removed the last sentence and added the following line:
The length of nonce for AEAD algorithms is explained in [RFC5116].

> 
> * 8: Is the MESAGE INTEGRITY attribute itself mandatory ? 

Yes.

> The client should
> reject messages if this attribute is not included.

Added following text for clarity:

   o  The client looks for the MESSAGE-INTEGRITY attribute in the
      response.  If MESSAGE-INTEGRITY is absent or the value computed
      for message integrity using mac_key does not match the contents of
      the MESSAGE-INTEGRITY attribute then the response MUST be
      discarded.

> 
> * 9: if the server is allowed to cache access tokens, there must be a way for
> the client to refer to a token by name. Otherwise there may be confusion
> when tokens are expired and replaced by new ones, especially in the
> presence of dropped messages.

That line is only for server implementation optimization. 
NEW:
Since the access token is only valid for a specific period of time, the TURN server can cache it so that it can check if the access token in a new allocation request matches one of the cached tokens and avoids the need not decrypt the access token.

> 
> * 10: is the server required to cache old transaction IDs to avoid replay
> attacks? If this is the case, you need to mandate it explicitly.

Fixed.
NEW:
An attacker trying to replay message with ACCESS-TOKEN attribute can be mitigated by
frequent changes of nonce value as discussed in section 10.2 of
[RFC5389].

> 
> * App. A: the mac_key as included in the token should be binary IMO, and
> not as shown here, encoded in base64.

Assuming mac_key is negotiated using DSKPP base64 encoding is shown, will add a note that mac_key is base64 encoded and 
must be converted to binary for HMAC computation.


> 
> * I suggest to rename "long term password" to "long term key" 

Fixed.

> (and again, it
> should be binary).

Added note that long_term_key is base64 encoded and must be converted to binary for encrypting and decrypting of token.

> 
> * Similarly, the nonce should be binary.

Updated.

Best Regards,
-Tiru

> 
> Thanks,
> 	Yaron