Re: [tram] More issues with RFC 5389

[Marc Petit-Huguenin <[e-mail]>]

Thanks for the review.  Note that the latest version of stunbis has all the proposals below integrated.  With that done, we can now work on the remaning issues.

See below for my answers.

On 12/20/2016 07:52 AM, Simon Perreault wrote:
> Sorry for taking so long to reply...
> 
> Le 2016-11-27 à 08:12, Marc Petit-Huguenin a écrit :
>> 1. In both section 10.1.3 and section 10.2.3, the text says "If the
>> value does not match, or if MESSAGE-INTEGRITY was absent, the
>> response MUST be discarded, as if it was never received.  This means
>> that retransmits, if applicable, will continue."  The issue there is
>> that when the response is discarded but the transaction layer is for
>> a reliable transport as described in section 7.2.2, then the
>> transaction has no way to ever succeed, as there is no
>> retransmissions in that case, and the transaction will simply timeout
>> after 39.5 seconds.  That seems wrong to wait nearly 40 secs to
>> return an answer that we already know about.
>>
>> Our proposal is to send immediately an "attack error" back to the
>> upper layers on receiving an invalid or missing M-I on reliable
>> transactions.  This is equivalent of what is happening today, but
>> without having to wait 40 seconds for that.  The attack error is a
>> signal similar to the timeout error, in that it does not carry any
>> other information.  It is internal to the implementation, so it does
>> not require a Error Code, in the same way that the timeout signal
>> does not have an Error Code.
>>
>> Unreliable transactions could also use a similar signal in case these
>> transactions fail not because no packet was ever received but because
>> all of them where discarded.  In that case, the "attack error" signal
>> is sent after the normal timeout.
> 
> This makes sense to me.
> 
>> 2. In section 10.2.3 there is no provision for processing a 400
>> response that does not have a valid MESSAGE-INTEGRITY attribute.
>> That means that if the text is implemented as it is written (which I
>> am sure is not), receiving an unauthenticated 400 (as triggered by
>> bullet 2 of 10.2.2) will trigger the discarding packet rule on the
>> last paragraph of 10.2.3 and will take 40 seconds to timeout.
>>
>> Our proposal is to just add text between the 2nd and 3rd paragraph of
>> 12.2.3 to explain that receiving a unauthenticated 400 will cause a
>> timeout.
> 
> Does this mean that an attacker could just send unauthenticated 400
> responses, much like a TCP RST attack?

Yes, but that's the case today with RFC 5389.  The idea is to make that explicit by saying that waiting until the timeout was and is the right thing to do, because dropping all the 400 until the last one gives a chance that the real response reaches that layer, at least with an unreliable transport.

> 
>> 3. Section 10.2.1.1 states that a first request SHOULD omit the
>> USERNAME, MESSAGE-INTEGRITY, REAL and NONCE, but inserting these
>> attribute will in fact make the first request a subsequent request.
>> Conversely Section 10.2.1.2 states a subsequent request SHOULD
>> include a USERNAME, MESSAGE-INTEGRITY, REAL and NONCE, but omitting
>> them will in fact make the subsequent request a first request.
>>
>> Our proposal is to say MUST omit in 10.2.1.1 and MUST include in
>> 10.2.1.2, and add new text to explain the logic.
> 
> Makes sense. This is only about making text clearer, right? No protocol
> changes?

No protocol change, just making clear what is a first request and what is a subsequent request.

Thanks.

> 
>> 4. The (simplified) order of processing in Section 10.2.2 is to
>> process 438 (Nonce stale) first (bullet 3) and then password invalid
>> (bullet 5).  If a server changes the password and the nonce at the
>> same time, then 3 transactions are needed to recover.
>>
>> Our proposal is to change bullet 3 to say that if the nonce is
>> invalid and the Message-Integrity is wrong (showing that the password
>> also changed), then a 401 must be returned instead of a 438.  The new
>> nonce is sent in the 401, so the session recovers in one additional
>> transaction instead of 2.  The additional message-integrity
>> validation needed for this is minimal (with TURN, the NONCE should be
>> expired at least once each hour.  Allocations have a default lifetime
>> of 10 minutes, 5 minutes for permissions/channel bindings.  So if we
>> have 3 permissions - which seems the minimum with ICE - that means a
>> maximum of 2.4% of the transactions will have an expired nonce and
>> will require an additional verification of the M-I).
> 
> Makes sense.
> 
>> 5. When a 438 is returned, it says that no MESSAGE-INTEGRITY is
>> returned.  But in fact the server could use the previous nonce to
>> generate a valid MESSAGE-INTEGRITY and protect the response.  Note
>> that there is not really a lot of advantages for doing this, excepted
>> that in STUNbis it will permit to not have to return the
>> PASSWORD-ALGORIHMS attribute in that case in the response to protect
>> against MiTM attacks (if we go with the idea that the password
>> algorithm list can be changed during a session, which is a separate
>> issue).
>>
>> Our proposal is to say that 438 response MAY contain a
>> MESSAGE-INTEGRITY attribute that reuses the previous nonce.
> 
> No objection.
> 
>> 6. Same issue than before, but when the password changes.  Instead of
>> not signing the 401 in bullet 5, it can be signed using the previous
>> password.
>>
>> Our proposal is to say that the 401 response in that case MAY contain
>> a MESSAGE-INTEGRITY attribute that reuses the previous password, i.e.
>> if the server remembers it.
> 
> No objection.
> 
>> 7. In section 15.6, it says "401 Unauthorized".  It should really be
>> "401 Unauthenticated".  The concern here is that some higher level
>> protocol overloads the real meaning of 401 and returns a 401 when
>> something is not authorized at this level and so messes with the Long
>> Term Credential Mechanism.  Note that TURN got that right by using a
>> 403 to mean Unauthorized, but other STUN Usages can get this wrong.
>>
>> Our proposal is to change the text from "Unauthorized" to
>> "Unauthenticated”.
> 
> Cool.
> 
>> 8. Bullets 2 and 4 of Section 10.2.2 says that the response SHOULD
>> NOT contain a MESSAGE-INTEGRITY attribute, but even if the
>> application wanted to, there is no possible way to generate a
>> MESSAGE-INTEGRITY in these cases (for bullet 2 either USERNAME, REALM
>> or NONCE are missing, and for bullet 4 the USERNAME is invalid).
>>
>> Our proposal is to remove MESSAGE-INTEGRITY in these two paragraphs,
>> and to add some informative text saying that the message cannot
>> contain MESSAGE-INTEGRITY, because necessary fields are missing.
> 
> Makes sense.
> 


-- 
Marc Petit-Huguenin
Email: [e-mail]
Blog: https://marc.petit-huguenin.org
Profile: https://www.linkedin.com/in/petithug