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

[Barry Leiba <[e-mail]>]

> Thanks for the prompt review.

You're welcome, and thanks for the prompt reply!

I've eliminated the resolved items below, and leave the ones for which
issues are open, for my own quick reference.  I think they should be
easy to resolve, and then we can get a revised I-D and start last
call.

-- 
Barry


>> 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.
>
> Filed issue #17.


>>    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?
>
> Filed issue #18.


>>    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.
>
> Filed issue: #19.


>> — 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.
>
> Filed issue: #20.


>> — 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.
>
> Filed issue: #21.


>> — 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.
>
> Filed issue: #22.