Skip to main content

Last Call Review of draft-ietf-wish-whip-09
review-ietf-wish-whip-09-secdir-lc-housley-2023-08-01-00

Request Review of draft-ietf-wish-whip
Requested revision No specific revision (document currently at 16)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2023-08-08
Requested 2023-07-25
Authors Sergio Garcia Murillo , Dr. Alex Gouaillard
I-D last updated 2023-08-01
Completed reviews Secdir Last Call review of -09 by Russ Housley (diff)
Artart Last Call review of -09 by Barry Leiba (diff)
Genart Last Call review of -09 by Dale R. Worley (diff)
Secdir Telechat review of -14 by Russ Housley (diff)
Tsvart Last Call review of -09 by Dr. Bernard D. Aboba (diff)
Httpdir Last Call review of -09 by Darrel Miller (diff)
Artart Early review of -08 by Barry Leiba (diff)
Secdir Last Call review of -13 by Russ Housley (diff)
Assignment Reviewer Russ Housley
State Completed
Request Last Call review on draft-ietf-wish-whip by Security Area Directorate Assigned
Posted at https://mailarchive.ietf.org/arch/msg/secdir/t4JsVL9wkeV1wHbiGbfa_LJ945g
Reviewed revision 09 (document currently at 16)
Result Has issues
Completed 2023-08-01
review-ietf-wish-whip-09-secdir-lc-housley-2023-08-01-00
I 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 authors, document editors, and WG chairs should
treat these comments just like any other IETF Last Call comments.

Document: draft-ietf-wish-whip-09
Reviewer: Russ Housley
Review Date: 2023-07-29
IETF LC End Date: 2023-08-08
IESG Telechat date: Unknown

Summary: Has Issues


Major Concerns:

Section 4 says:

   The HTTP POST request will have a content type of "application/sdp"

It seems to me that this ought to be a MUST statement.  Also, what
will happen if the media type is something else?  Or, what happens if
the attempt to parse the content as an SDP fails?

Section 4.1 says:

   The initial offer by the WHIP client MAY be sent after the full ICE
   gathering is complete with the full list of ICE candidates, or it MAY
   only contain local candidates (or even an empty list of candidates)
   as per [RFC8863].

I do not understand this paragraph.  The client MAY do X OR MAY do Y.
There is no context to tell why a client might want to do either X or Y.

Section 4.2 says:

   In order to reduce the complexity of implementing WHIP in both
   clients and Media Servers, WHIP imposes the following restrictions
   regarding WebRTC usage:

However, there is no clear formatting to determine where the list of
restrictions ends.  Maybe a list of bullets would be more clear.

Section 5 says:

   *  HTTP security: Section 11 of [RFC9112], Section 17 of [RFC9110],
      etc.

The use of "etc" is not going to help an implementer.  Please be complete.

Section 5: Please reference RFC 4086 for guidance on generation of random
numbers.


Minor Concerns:

Please merge the definition in Section 2 and the overview in Section 3.
Figure 1 really is needed to understand the definitions, but the
definitions come first.

The figures are not referenced from body of the document. It is best to
include a reference in the body that offers some description of what the
reader is expected to learn from the figure. When I as a Security AD, the
other Security AD was blind. The text-to-audio system that he used was
surprisingly good, but it could not handle ASCII art. The discussion of
the figures was vital to him being able to understand a document.

The following paragraph appears twice in Section 4:

   The WHIP endpoints MUST return an "405 Method Not Allowed" response
   for any HTTP GET, HEAD or PUT requests on the endpoint URL in order
   to reserve its usage for future versions of this protocol
   specification.

Section 4.2 says:

   ... sections as per [RFC8858] i.

I do not understand this reference.

Section 4.2 says:

   This version of the specification only supports, at most, a single
   audio and video MediaStreamTrack in a single MediaStream as defined
   in [[!RFC8830]] ...

Does it ever make sense for there to be zero audio and video tracks?

I do not understand this reference.  I suspect it is malformed markdown.


Nits:

Section 4: s/non graceful/non-graceful/

Section 4: s/mime type/media type/

Section 5: s/[RFC8446], [RFC8446],/[RFC8446]/

Section 5: s/enought/enough/

Section 5: s/legit/legitimate/

Section 5: s/abalanche/avalanche/

Section 5: s/currentlyrunning/currently running/