Re: [MMUSIC] Benjamin Kaduk's No Objection on draft-ietf-mmusic-rfc4566bis-35: (with COMMENT)

[Benjamin Kaduk <[e-mail]>]

On Mon, Jun 03, 2019 at 02:44:18PM -0400, Paul Kyzivat wrote:
> Benjamin,
> 
> Thank you for your comments. I've included followup inline below.
> 
> On 5/30/19 10:34 AM, Benjamin Kaduk via Datatracker wrote:
> > Benjamin Kaduk has entered the following ballot position for
> > draft-ietf-mmusic-rfc4566bis-35: No Objection
> 
> > ----------------------------------------------------------------------
> > COMMENT:
> > ----------------------------------------------------------------------
> > 
> > I reviewed the diff from RFC 4566 but didn't make it quite all the way
> > through the full document itself.  What I did find in that partial read
> > suggests that a full read-through by the authors may find some lingering
> > stale language or minor internal inconsistencies.
> 
> There really hasn't been an attempt to thoroughly remove stale (dated)
> language. Fundamentally this is a 21 year old document that has been tidied
> up a bit. Anyone that started over today to define something new for the
> same purpose would surely define something very different. We've only been
> trying to fix areas where there has been confusion or ambiguity has been
> found, or where new concerns (e.g., security) have needed to be addressed.

I think the RFC Editor is still going to be reviewing the whole thing for
consistency with the current style/practices, so getting a headstart now
would probably be more effective and save some time overall.

> > Another broad topic (with more comments throughout) is that an early and
> > clear discussion of time representation may be helpful, and arguably
> > does not need to refer to NTP time at all.  (This is especially so when
> > we refer to "NTP time format" and RFC 5905, which lists three
> > formats, none of which have a straightforward translation to numeric
> > strings without fraction part.)
> 
> Good point. It appears that the intent is for the value to be seconds since
> the beginning of NTP Era 0 ("0 h 1 January 1900 UTC"). We can either
> reference that directly, or, since that starting point isn't likely to be
> changed in NTP, simply state it. Which of those would you prefer?

The latter seems fine (and IIRC we even have seen RFCs published that just
refer to "seconds since the Unix epoch (excluding leap seconds)" with no
further reference.

> > Section 4
> > 
> >                                     SDP is primarily intended for use in
> >     an internetwork, although it is sufficiently general that it can
> >     describe multimedia conferences in other network environments.  [...]
> > 
> > nit(?): this verbiage ("internetwork") feels quite dated.
> 
> Does "... primarily intended for use with Internet protocols, although ..."
> work for you?

Sure.

> > Section 4.1
> > 
> >                                                      Some media types may
> >     redefine this behavior, but this is NOT RECOMMENDED since it
> >     complicates implementations (including middleboxes that must parse
> >     the addresses to open Network Address Translation (NAT) or firewall
> >     pinholes).
> > 
> > (Similarly for "firewall pinholes".)
> 
> Do you have a suggestion. This still seems ok to me.

If it's okay to you, then no-change is the proper course of action.
(I don't remember encountering "pinholes" previously, and would have
worded it as "to open holes in NATs and/or firewalls".)

> > Section 4.3
> > 
> > The usual security considerations about (potentially) referencing remote
> > content would seem to apply.  Perhaps a RFC 3986 reference (or more)
> > would be appropriate.
> 
> Will adding a reference to [RFC3986] be sufficient?

I would prefer a note about "Use of URIs to indicate remote resources
is subject to the security considerations from [RFC3986]", but just the
reference would be okay.

> > Section 4.6
> > 
> > Perhaps we should mention here that this categorization mechanism is
> > deprecated/obsolete?
> 
> Section 4.6??? Did you mean 4.4?

I clearly did; I'm not sure what I was looking at (especially since
this was 4.5 (not 4.6) in RFC 4566 itself).

> Since a=cat has been declared obsolete, I'm inclined to simply remove
> section 4.4.

That works for me.

> > Section 5
> > 
> >     An SDP description is entirely textual.  SDP field names and
> >     attribute names use only the US-ASCII subset of UTF-8 [RFC3629], but
> >     textual fields and attribute values MAY use the full ISO 10646
> >     character set in UTF-8 encoding, or some other character set defined
> >     by the "a=charset:" attribute.  [...]
> > 
> > nit: here (and in Section 4.4 just above) may be the only times we
> > include the colon when discussing an "a=" attribute; consistency would
> > seem to suggest removing the colons.
> 
> OK.
> 
> >                                    However, since SDP may be used in
> >     environments where the maximum permissible size of a session
> >     description is limited, the encoding is deliberately compact.  Also,
> >     since descriptions may be transported via very unreliable means or
> >     damaged by an intermediate caching server, the encoding was designed
> >     with strict order and formatting rules so that most errors would
> >     result in malformed session descriptions that could be detected
> >     easily and discarded.  This also allows rapid discarding of encrypted
> >     session descriptions for which a receiver does not have the correct
> >     key.
> > 
> > I don't really see why the "rapid discarding" property follows from the
> > compactness of the encoding, when the correct key for the encrypted
> > description is not known.
> 
> I'm inclined to simply remove that last sentence. I'd remove the whole
> paragraph, but keeping it may provide insight into why the protocol is as
> weird and crufty as it is.

OK

> > Section 5.x
> > 
> > It's interesting to note that while the SDP attributes (Sections 6.x)
> > got ABNF constructions for their values, the type descriptions here
> > are still presented in a somewhat informal syntax (that, e.g., relies on
> > implicitly stating that fields are whitespace-separated).
> 
> It is an interesting point. The best rationale I can give is that *not*
> having ABNF got to be a serious problem for attributes, because there are a
> lot of them, and new ones get defined regularly (with complex syntax) and
> the syntax was often recognized as being ambiguous. So we wanted to demand
> ABNF for future extensions, and felt we should make the ones defined in this
> document comply with that.
> 
> OTOH, the other fields are revised/extended much more rarely, so it hasn't
> been a problem. And so there is much less motivation to "rock the boat" by
> changing the manner of defining them.

Indeed.  Changing this seems like more effort than it's worth, at this point
in the process.  (But can I have whitespace in a comma-separated list?  I
assume "no", but have been wrong on many occasions...)

> > Section 5.2
> > 
> >     <sess-id>  is a numeric string such that the tuple of <username>,
> >        <sess-id>, <nettype>, <addrtype>, and <unicast-address> forms a
> >        globally unique identifier for the session.  The method of <sess-
> >        id> allocation is up to the creating tool, but it has been
> >        suggested that a Network Time Protocol (NTP) format timestamp be
> >        used to ensure uniqueness [RFC5905].
> > 
> > (et seq) There is not a single "NTP format timestamp"; RFC 5905 provides
> > three different formats.  If any of the three is fine, that should be
> > stated, and if a single distinguished one is preferred, that should also
> > be stated.
> > Furthermore, the three formats all include multiple fields and not a
> > rule for presenting them as a "numeric string" as described here, which
> > on the face of it seems to exclude fractions.  ("numeric string" does
> > not seem to be specifically defined by this document.)
> 
> I'm inclined to change this to suggest the same format as used for t=, as
> discussed above.
> 
> Because the values of these fields is only loosely specified there is no
> *guarantee* that the combination provides the intended uniqueness. But there
> are so many implementations deployed that it would be impossible to redefine
> to make such a guarantee. In practice it isn't a problem - things are unique
> *enough*.
> 
> > Section 5.3
> > 
> >     The "s=" line MUST NOT be empty and SHOULD contain ISO 10646
> >     characters (but see also the "a=charset" attribute).  [...]
> > 
> > Is this perhaps intended to be "SHOULD only contain"?
> > (Similarly in following subsections.)
> 
> I received other comments on this and will work on it.

Sure, and sorry for not deduplicating my comments -- I simply ran out
of time and sent what I currently had.

-Ben

> > Section 5.9
> > 
> >     The first and second sub-fields of the time-field give the start and
> >     stop times, respectively, for the session.  These values are the
> >     decimal representation of Network Time Protocol (NTP) time values in
> >     seconds since 1900 [RFC5905].  To convert these values to UNIX time
> >     (UTC), subtract decimal 2208988800.
> > 
> > Looking at the time formats listed in RFC 5905, one perhaps wonders if
> > the values used by SDP are more appropriately described informally as
> > just "seconds since the Unix epoch" without specific reference to NTP
> > (both here and elsewhere in the document).
> > 
> >     NTP timestamps are elsewhere represented by 64-bit values, which wrap
> >     sometime in the year 2036.  [...]
> > 
> > I don't understand what this is referring to.  Is this perhaps the "32
> > bits of seconds and 32 bits of fraction" format, which suffers from the
> > y2038 (note '8', not '6') problem?
> 
> See my earlier comments.
> 
> > Section 6.2
> > 
> > Perhaps "this was to assist" would be more fitting, given the obsolete
> > nature of a=keywds.
> 
> OK.
> 
> > Section 6.9
> > 
> >     This specifies the type of the multimedia conference.  Suggested
> >     values are "broadcast", "meeting", "moderated", "test", and "H332".
> > 
> > nit: I don't think these are "suggested" values; they are the only ones
> > allowed by the ABNF.
> 
> Good point. Will fix.
> 
> >     "recvonly" should be the default for "type:broadcast" sessions,
> >     "type:meeting" should imply "sendrecv", and "type:moderated" should
> >     indicate the use of a floor control tool and that the media tools are
> >     started so as to mute new sites joining the multimedia conference.
> > 
> > There seems to be redundancy here with the "SHOULD" in Section 6.7 about
> > "sendrecv" being the default for non-broadcast non-H322 sessions.
> 
> Thanks for pointing this out. This needs work, since IMO it is wrong to use
> "default" here. At best these are recommendations for what the creator of
> and SDP body should do.
> 
> > Section 6.11
> > 
> > Is it intentional to lose the language about "order of importance" of
> > multiple languages?
> 
> Yes.
> 
> This is describing what is used in the text of the SDP body. It isn't saying
> anything about future use. RFC8373 defines 'hlang-send' and 'hlang-recv' for
> indicating preferences of languages to use in media.
> 
> 	Thanks,
> 	Paul