Re: [Wish] Secdir last call review of draft-ietf-wish-whip-09

[Sean Turner <[e-mail]>]

Russ,

Just a heads up that Sergio is not ignoring you; he’s just on PTO until early September.

spt

> On Aug 1, 2023, at 18:07, Russ Housley via Datatracker <[e-mail]> wrote:
> 
> Reviewer: Russ Housley
> Review result: Has Issues
> 
> 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/
> 
> 
>