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

[Barry Leiba]

You have, indeed, and I’ve just requested last call.

Thanks!

Barry

On Tue, Apr 28, 2020 at 5:52 PM Warren Kumari  wrote:

> Thank you -- I think that we have addressed all of the comments (thank
> you!).
>
> See below.
>
> On Sun, Apr 26, 2020 at 12:37 AM Erik Kline  wrote:
> >
> > Barry,
> >
> > Thanks for the prompt review.
> >
> > On Thu, Apr 23, 2020 at 9:18 PM Barry Leiba 
> wrote:
> >>
> >> 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.
> >
> >
> > Fixed in the repo (commit).
> >
> >> — Section 2 —
> >>
> >>negotiation ([RFC7231] Section 3.4), captive portals implementors
> >>
> >> Make it “captive portal implementors” (or “implementors of captive
> portals”).
> >
> >
> > Fixed in the repo (commit).
> >
> >> 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.
>
> ... so, we had incorrectly thought that the colon character (:) was
> similar in operation to the . in a DNS name, and implied a hierarchy
> -- looking at RFC8141 it turns out that we were wrong, and that colons
> can be part of the NSS:
>
>Because the colon character (":") is used to separate "urn" from the
>NID and the NID from the NSS, it's tempting to think of the entire
>URN as being structured by colon characters and to assume that colons
>create a structure or hierarchy within the NSS portion of the URN.
>Such structure could be specified by a particular NID specification,
>but there is no implicit structure.  In a URN such as
>
>   urn:example:apple:pear:plum:cherry
>
>the NSS string is "apple:pear:plum:cherry" as a whole, and there is
>no specific meaning to the colon characters within that NSS string
>unless such meaning is described in the specification of the
>"example" namespace.
>
> So, we are changing to capport:unrestricted  (Commit:
>
> https://github.com/capport-wg/7710bis/commit/aae1c3cbd08d3822fa22056f7c2a7ec4a12e0a9a
> ) -- Thank you!
>
> >
> >
> > 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.
>
> These were represented differently because of hysterical raisins /
> tradition -- the format we had used for IPv4 was how many of the other
> IPv4 documents represented their DHCP option (and the same for
> IPv6)... however, seeing them on the same page does look silly, and so
> we have changed the IPv4 one to look like the IPv6 one (
> https://github.com/capport-wg/7710bis/pull/28 )
>
> We also fixed the missing length for length...
> (
> https://github.com/capport-wg/7710bis/commit/d229279dfba7f2a7d3e1f50be2ecf9244b4bc9ba
> )
>
> In addition, we clarify that the URI should not exceed 255 bytes if
> you are using IPv4 and IPv6 (because they have to match) - this closes
> Issues #20 and PR #25 (https://github.com/capport-wg/7710bis/issues/20
> , https://github.com/capport-wg/7710bis/pull/25 ). Thanks to Subash
> Tirupachur Comerica for pointing this out.
>
>
> >
> >
> > 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 t

[Captive-portals] I-D Action: draft-ietf-capport-rfc7710bis-04.txt

[internet-drafts]

A New Internet-Draft is available from the on-line Internet-Drafts directories.
This draft is a work item of the Captive Portal Interaction WG of the IETF.

Title   : Captive-Portal Identification in DHCP / RA
Authors : Warren Kumari
  Erik Kline
Filename: draft-ietf-capport-rfc7710bis-04.txt
Pages   : 12
Date: 2020-04-28

Abstract:
   In many environments offering short-term or temporary Internet access
   (such as coffee shops), it is common to start new connections in a
   captive portal mode.  This highly restricts what the customer can do
   until the customer has authenticated.

   This document describes a DHCP option (and a Router Advertisement
   (RA) extension) to inform clients that they are behind some sort of
   captive-portal enforcement device, and that they will need to
   authenticate to get Internet access.  It is not a full solution to
   address all of the issues that clients may have with captive portals;
   it is designed to be used in larger solutions.  The method of
   authenticating to, and interacting with the captive portal is out of
   scope of this document.

   This document replaces RFC 7710.  RFC 7710 used DHCP code point 160.
   Due to a conflict, this document specifies 114.

   [ This document is being collaborated on in Github at:
   https://github.com/capport-wg/7710bis.  The most recent version of
   the document, open issues, etc should all be available here.  The
   authors (gratefully) accept pull requests.  Text in square brackets
   will be removed before publication. ]


The IETF datatracker status page for this draft is:
https://datatracker.ietf.org/doc/draft-ietf-capport-rfc7710bis/

There are also htmlized versions available at:
https://tools.ietf.org/html/draft-ietf-capport-rfc7710bis-04
https://datatracker.ietf.org/doc/html/draft-ietf-capport-rfc7710bis-04

A diff from the previous version is available at:
https://www.ietf.org/rfcdiff?url2=draft-ietf-capport-rfc7710bis-04


Please note that it may take a couple of minutes from the time of submission
until the htmlized version and diff are available at tools.ietf.org.

Internet-Drafts are also available by anonymous FTP at:
ftp://ftp.ietf.org/internet-drafts/


___
Captive-portals mailing list
Captive-portals@ietf.org
https://www.ietf.org/mailman/listinfo/captive-portals

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

[Warren Kumari]

Thank you -- I think that we have addressed all of the comments (thank you!).

See below.

On Sun, Apr 26, 2020 at 12:37 AM Erik Kline  wrote:
>
> Barry,
>
> Thanks for the prompt review.
>
> On Thu, Apr 23, 2020 at 9:18 PM Barry Leiba  wrote:
>>
>> 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.
>
>
> Fixed in the repo (commit).
>
>> — Section 2 —
>>
>>negotiation ([RFC7231] Section 3.4), captive portals implementors
>>
>> Make it “captive portal implementors” (or “implementors of captive portals”).
>
>
> Fixed in the repo (commit).
>
>> 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.

... so, we had incorrectly thought that the colon character (:) was
similar in operation to the . in a DNS name, and implied a hierarchy
-- looking at RFC8141 it turns out that we were wrong, and that colons
can be part of the NSS:

   Because the colon character (":") is used to separate "urn" from the
   NID and the NID from the NSS, it's tempting to think of the entire
   URN as being structured by colon characters and to assume that colons
   create a structure or hierarchy within the NSS portion of the URN.
   Such structure could be specified by a particular NID specification,
   but there is no implicit structure.  In a URN such as

  urn:example:apple:pear:plum:cherry

   the NSS string is "apple:pear:plum:cherry" as a whole, and there is
   no specific meaning to the colon characters within that NSS string
   unless such meaning is described in the specification of the
   "example" namespace.

So, we are changing to capport:unrestricted  (Commit:
https://github.com/capport-wg/7710bis/commit/aae1c3cbd08d3822fa22056f7c2a7ec4a12e0a9a
) -- Thank you!

>
>
> 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.

These were represented differently because of hysterical raisins /
tradition -- the format we had used for IPv4 was how many of the other
IPv4 documents represented their DHCP option (and the same for
IPv6)... however, seeing them on the same page does look silly, and so
we have changed the IPv4 one to look like the IPv6 one (
https://github.com/capport-wg/7710bis/pull/28 )

We also fixed the missing length for length...
(https://github.com/capport-wg/7710bis/commit/d229279dfba7f2a7d3e1f50be2ecf9244b4bc9ba)

In addition, we clarify that the URI should not exceed 255 bytes if
you are using IPv4 and IPv6 (because they have to match) - this closes
Issues #20 and PR #25 (https://github.com/capport-wg/7710bis/issues/20
, https://github.com/capport-wg/7710bis/pull/25 ). Thanks to Subash
Tirupachur Comerica for pointing this out.


>
>
> 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 remov

Re: [Captive-portals] Captive-Portal Identification in DHCP / RA draft-ietf-capport-rfc7710bis-03

[Tirupachur Comerica, Subash]

Thank you Erik.

Thanks,
Subash

From: Erik Kline 
Reply-To: "e...@loon.com" 
Date: Monday, April 27, 2020 at 4:21 PM
To: "Tirupachur Comerica, Subash" 
Cc: Erik Kline , Martin Thomson , 
captive-portals 
Subject: Re: [Captive-portals] Captive-Portal Identification in DHCP / RA 
draft-ietf-capport-rfc7710bis-03

That seems reasonable to me. I've added a comment to 
https://github.com/capport-wg/7710bis/issues/20 to remind myself.  ‌  ‌  ‌  ‌  
‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌ 
 ‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌  ‌
External (e...@loon.com)
Report This 
Email
  FAQ  Protection by 
INKY

That seems reasonable to me.  I've added a comment to 
https://github.com/capport-wg/7710bis/issues/20
 to remind myself.

On Mon, 27 Apr 2020 at 16:08, Tirupachur Comerica, Subash 
mailto:subash.tirupachurcomer...@commscope.com>>
 wrote:
Hi Martin,
Thanks for your quick response.
Good to know it is already taken care of.

Hi Erik,
Thanks, but it would be very handy next to the TLV formats (-:

Thanks,
Subash


From: Captive-portals 
mailto:captive-portals-boun...@ietf.org>> on 
behalf of Erik Kline mailto:ek.i...@gmail.com>>
Date: Monday, April 27, 2020 at 4:01 PM
To: Martin Thomson mailto:m...@lowentropy.net>>
Cc: captive-portals mailto:captive-portals@ietf.org>>
Subject: Re: [Captive-portals] Captive-Portal Identification in DHCP / RA 
draft-ietf-capport-rfc7710bis-03

And the 255 byte URI limit is mentioned in section 2 (~3rd paragraph).

I guess if someone wants longer URIs they have to move to an IPv6-only network. 
 ;-)

On Mon, Apr 27, 2020 at 3:54 PM Martin Thomson 
mailto:m...@lowentropy.net>> wrote:
Thanks for the input.  Apparently great minds think alike as another reviewer 
found the exact same shortcoming just days ago.  The next revision should have 
these fixed.

On Tue, Apr 28, 2020, at 05:07, Tirupachur Comerica, Subash wrote:
>
> Hi,
>
> I was reviewing this draft and found a few missing text(sometimes
> obvious) enumerated below(missing text in *_bold underline_*)
>
> Section 2.1 IPv4 DHCP Option
>
>  o Code: The Captive-Portal DHCPv4 option (TBD) (one octet).
>
>  o Len: The length, in octets of the URI.*_(one octet)_*
>
> Section 2.2: IPv6 DHCP Option
>
>  o option-code: The Captive-Portal DHCPv6 option (103) (two octets).
>
>  o option-len: The length, in octets of the URI.*_(two octets) --?
> Please see question below_*
>
>  o URI: The contact URI for the captive portal that the user should
>
>  connect to (encoded following the rules in [RFC3986
> >]).
>
>
> - *Question on the above option-len: If this is two octets in IPv6 DHCP
> option, then the URI can be longer then 255. Option-len-value <=255,
> correct?*
>
>
> Section 2.3: The Captive-Portal IPv6 RA Option
>
>  o Type: 37*_(one octet)_*
>
>  o Length: 8-bit unsigned integer. The length of the option
>
>  (including the Type and Length fields) in units of 8 bytes.(*_one octet_*)
>
>  o URI: The contact URI for the captive portal that the user should
>
>  connect to. For the reasons described above, the implementer
>
>  might want to use an IP address literal instead of a DNS name.
>
>  This should be padded with NULL (0x0) to make the total option
>
>  length (including the Type and Length fields) a multiple of 8
>
>  bytes.
>
>
> Thanks,
>
> Subash
>
>
> ___
> Captive-portals mailing list
> Captive-portals@ietf.org
> https://www.ietf.org/mailman/listinfo/captive-portals