[tram] Benjamin Kaduk's No Record on draft-ietf-tram-stunbis-16: (with COMMENT)

[Benjamin Kaduk <[e-mail]>]

Benjamin Kaduk has entered the following ballot position for
draft-ietf-tram-stunbis-16: No Record

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-tram-stunbis/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

I'm balloting No Objection and not DISCUSS because no individual
item rises quite to that level of criticality, but there still are
quite a number of things that ought to be addressed prior to
publication.

In addition to the excellent points raised in the secdir review
(which are mostly addressed but only in the editor's copy?), I
would call out a few more key observations:

If I understand correctly, the server's PASSWORD-ALGORITHMS array
from the response are echoed back by the client in the subsequent
request in order to provide downgrade protection -- the initial
response is at least sometimes not authenticated, and by having the
client repeat it back under an authenticated scheme, the server can
detect if the list was tampered to remove any new, stronger,
algorithms.  This is probably an important enough concept to be
called out explicitly, either where the behavior is described or in
Section 9.2.1 ("Bid Down Attack Prevention").

Section 8.1 has some text that could be read as requiring a STUN
client to implement dual-stack IPv4/IPv6, which presumably is not
the intention:
   In addition, instead of querying either the A or the AAAA resource
   records for a domain name, the client MUST query both and try the
   requests with all the IP addresses received, as specified in
   [RFC8305].

There are still some places where the text talks about
MESSAGE-INTEGRITY when it really means "message integrity
protection" (i.e., either the SHA1 or SHA-256 variants, for now);
I've attempted to note them in the notes below.

There is text in both Sections 9.1 and 9.2 that talks of replay
protection being provided, by the time-limited nature of the
short-term credentials or the nonce for long-term credentials.  This
seems to only, strictly speaking, be true if any given
password/nonce can only be used once, but for the short-term
credentials the password is used for the duration of the
"session-equivalent", and for the long-term credentials a nonce can
be reused for some amount of time as well.  So while these are both
replay countermeasures that can limit replay attacks, it seems a
stretch to claim that they prevent replay attacks (entirely).

The Stale Nonce behavior seems potentially worrisome, in that it
opens up a side channel for a distinguishing attack,
between a 401 and 438 response.  (That is, "password correct" vs.
"password incorrect".)  The impact seems rather muted, though, since
the gain to the attacker is to be able to precompute a bunch of
requests using a nonce of the attacker's choosing and blindly replay
the precomputed object against (possibly multiple) servers looking
for a guess.  The realm and username are still in play, so the scope
for the attacker to gain from the precomputation seems limited.
(The same level of brute-force guessing can be obtained "live" just
by computing the trial responses against a live server, using a
valid nonce.)  So, while it might be nice to give guidance that 438
should only be used when the server can validate that it did
generate the nonce and the nonce was valid "recently", and to treat
other cases as authentication failures, it's not clear to me that
there's enough of a benefit from the change to make it worth doing.


Section 6.2.3

Maybe just cite BCP 195/RFC 7525 instead of inlining requirements?

Also, the "client SHOULD verify ..." and "client MUST verify ..." are
similar enough that some clarification of certificate vs. identity
verification would be in order, if both statements are believed
accurate.


Section 6.3

   [...] Known-but-unexpected attributes SHOULD be ignored by
   the agent. [...]

Why is this not an error?


Section 6.3.1, 6.3.2, etc.

Should the second paragraph start with "Otherwise, "?  Presumably
once an error response is sent or processing ceases, processing stops...


Section 8.1

   [...] A "stuns" URI
   containing an IP address MUST be rejected, unless the domain name is
   provided by the same mechanism that provided the STUN URI, and that
   domain name can be passed to the verification code.

"the verifcation code" is probably unnecessarily vague; we're just
talking about the (D)TLS SNI and certifciate verification code,
right?



Section 9.2

   [...[ The nonce provides the replay protection.  It is a cookie,
   selected by the server, and encoded in such a way as to indicate a
   duration of validity or client identity from which it is valid.

Is this like a TLS cookie, where only the server needs to know about
the internal structure (viz. "encoded" in the above)?  If so, the
text could probably be more clear.

When mentioning just "a message-integrity", maybe forward-ref both
versions of it to be more clear that we don't mean just the old one?

It's unclear why indications can't use a nonce, if one is known
from a previous request/response on the same connection.

When encoding the value of 0 for the security feature, it may be
worth clarifying that it is still encoded as a 24-bit integer and
base64-encoded, though the "13 character string" text ought to make
this clear to the reader.


Section 9.2.4

      *  If the request contains neither PASSWORD-ALGORITHMS nor
         PASSWORD- ALGORITHM, then the request is processed as though
         PASSWORD- ALGORITHM were MD5 (Note that if the original
         PASSWORD-ALGORITHMS attribute did not contain MD5, this will
         result in a 400 Bad Request in a later step below).

This Note seems a bit odd, since we're predicated on
PASSWORD-ALGORITHMS being absent.

      *  Otherwise, unless (1) PASSWORD-ALGORITHM and PASSWORD-
         ALGORITHMS are both present, (2) PASSWORD-ALGORITHMS matches
         the value sent in the response that sent this NONCE, and (3)
         PASSWORD-ALGORITHM matches one of the entries in PASSWORD-
         ALGORITHMS, the server MUST generate an error response with an
         error code of 400 (Bad Request).

Maybe we could be more clear about "the value of the
PASSWORD-ALGORITHMS attribute in the message matches the value of
the PASSWORD-ALGORITHMS attribute sent in the response that sent the
NONCE used with this message"?


Section 9.2.5

   If the response contains a PASSWORD-ALGORITHMS attribute, the
   subsequent request MUST be authenticated using MESSAGE-INTEGRITY-
   SHA256 only.

This means "all subsequent requests to this server"?  Maybe the
wording could be more clear.  (It also means that if we get a new
scheme when SHA256 is broken, the document introducing it may need
to Update this document to remove the MUST.)

Also, I think the Security Feature bit is "Password algorithms"
plural (in the next paragraph and maybe multiple locations in the
document).


Section 10

   [...] If the transaction uses TLS or DTLS and if the
   transaction is authenticated by a MESSAGE-INTEGRITY-SHA256 attribute
   and if the server wants to redirect to a server that uses a different
   certificate, then it MUST include an ALTERNATE-DOMAIN attribute
   containing the subjectAltName of that certificate.

The (definite article), implying the entire subjectAltName?  Or just
one name that is contained as a SAN entry?


Section 14.8

Need to not just say MESSAGE-INTEGRITY since the SHA256 variant
should be fine as well.


Section 14.10

Maybe the nonce cookie should get a shout-out here?


Section 14.12

Do we need to talk about the padding, since this is the the sole
contents of the attribute and the attribute padding suffices?

Also, "derive the long-term password" doesn't seem quite right;
we're deriving the key from the long-term password, per Section
17.15.1.


Section 15.1.2

   There is no amplification of the number of packets with this attack
   (the STUN server sends one packet for each packet sent by the
   client), though there is a small increase in the amount of data,
   since STUN responses are typically larger than requests.

I usually hear amplification in terms of bytes or bandwidth, not
packets.  So maybe just "amplification is modest"?


Section 15.3

   This specification uses both HMAC-SHA1 and HMAC-SHA256 for
   computation of the message integrity.

Could perhaps be reworded, since the number of situations when both
are used on the same message is small, and we try to push SHA256
when we can.

   o  STUN Client and Server using the Short Term Credential Mechanism

Clients and Servers, plural.


Section 17.32.

I'm confused why we have MUST-level requirements about using
PASSWORD-ALGORITHMS when it is in the comprehension-optional range
(e.g., echoing from response to repeated request as in Section
9.2.5).  The server cannot actually enforce this MUST when talking
to an old client if we want to retain the comprehension-optional
semantics, IIUC.


Section 17.6

If we're adding support for DTLS-over-udp (per the next section),
should we be registering stuns/udp?

"password encryption algorithm" is not quite the right statement to
make, since we're hashing, not encrypting.