[Captive-portals] AD review of draft-ietf-capport-rfc7710bis-03

[Barry Leiba <[e-mail]>]

Good work on this, folks: a nice improvement on 7710, and let’s hope for
wider deployment.  Some comments below; I think we should have a quick
revised I-D before we go to last call, mostly because of the comment about
the diagrams in Sections 2.1 and 2.2, though most of the comments are
editorial.

— Section 1.1 —
Please use the new BCP 14 boilerplate from RFC 8174.

— Section 2 —

   negotiation ([RFC7231] Section 3.4), captive portals implementors

Make it “captive portal implementors” (or “implementors of captive
portals”).

It looks like the two consecutive paragraphs that both begin with “A
captive portal” have some duplication with regard to the absence of
“Accept” headers; please check that and adjust if you agree that some
information can be merged or removed.

   The URI SHOULD NOT contain an IP address literal.

Why not, and how does it maintain interoperability to have this as SHOULD
NOT, rather than MUST NOT?

   Networks with no captive portals MAY explicitly indicate this
   condition by using this option with the IANA-assigned URI for this
   purpose (see Section 4.1.1).  Clients observing the URI value
   "urn:ietf:params:capport-unrestricted" MAY forego time-consuming
   forms of captive portal detection.

Two things here:  I don’t see that the forward reference to 4.1.1 adds
anything, as the only thing that section does that’s useful here is to
repeat the URI that you’re already including.  And why
“capport-unrestricted” rather than “capport:unrestricted”?  The latter
would seem more in line with how “ietf:param” URNs are generally done.

— Sections 2.1 and 2.2 —
Why aren’t the diagrams for the two in the same format?  And neither
explicitly says what the length of the “len” field is.  In the v4 version,
one has to guess that it’s the same size as the “code” field, 1 byte.  In
the v6 version, one can look at the diagram.  I suggest making the diagrams
the same format (calibrated with bit/byte markings) and explicitly
specifying in the text the size of the “len” field.

— Section 3 —

   Specifically,
   URIs learned via any of the options in Section 2 should take
   precedence over any URI learned via some other mechanism

Given the MUST that comes before this, I think the word “should” ought to
be removed, to avoid any confusion.

— Section 5 —
Purely a judgment thing here, so do what you hink is best: it seems to me
that the Security Considerations in 7710 rather buried the lede.  The
bottom line is that by removing the MitM interception, this mechanism
improves security by making the portal and its actions visible, rather than
hidden, and that vulnerabilities that exist here also existed before and
are now lessened.  I would say something like that up front, in a new first
paragraph, and then continue with the existing text as is.

But, again, as you judge best; this is just a comment.

—
Barry