Re: [Captive-portals] WGLC on draft-ietf-capport-architecture and ...-api

[Kyle Larose <[e-mail]>]

Hello,

I support publication of capport-api. Thanks for all the hard work!

I have a bunch of minor comments:

Regarding Section 4.1.1:


>   If the certificates do
>   require the use of AIA, the captive network will need to allow client
>   access to the host specified in the URI.

Should this be phrased as "... the captive network MUST allow client
access..." Without this the solution won't work in this situation, so
isn't a strong requirement necessary?

Regarding this section as a whole, we are placing some requirements on
the enforcement device here... Do you think
those should be repeated in the architecture document so those
implementing only enforcement devices can consider them?

Regarding Section 4.2:


>     "bytes-remaining" (optional, integer): indicates the number of
>      bytes remaining, after which the client will be in placed into a
>      captive state.  The byte count represents the total number of IP
>      packet (layer 3) bytes sent and received by the client.  Captive
>      portal systems might not count traffic to whitelisted servers,
>      such as the API server, but clients cannot rely on such behavior.

We say that the "seconds-remaining" field should be omitted if
`captive=false`. We do
not for "bytes-remaining". That feels inconsistent. I think we should
add the following
statement to the end of this field's definition:

Added text:
"
The API server SHOULD include this value if the client is not captive
(i.e. captive=false)
and SHOULD omit this value for captive clients.
"

That brings me to another point. What should the API reply with if
there are no time or byte constraints
on the session? Should the fields be omitted in that case? The
document does not discuss that. If we do
decide to add that, do we need discussion about what the client
can/cannot infer from omission of these fields (i.e.
if bytes-remaining is not set, then there is no byte quota for the connection)?

Regarding the byte-count: we say it represents the total number of
tx/rx packets. I think it's obvious that this means
the sum of the total of each, but maybe it'd be better to make that
explicit. New text:

"The byte count represents the sum of the total number of IP packet
(layer 3) bytes sent and received by the client."

On that note, do most captive portal vendors count this way, or is
there often a distinction between tx/rx that we may
want to account for? Or is that complicating things too much, since
this is more a hint than anything else?

Finally, do we need to version the API (i.e. add a version field)? I'm
not sure what the best practice for IETF-defined HTTP APIs is.

Regarding section 6.2:

> Type:  The type of the JSON value to be stored, as one of the value
>    types defined in [RFC8259].

I don't think the types given in section 4.2 (the existing key
registry) exactly follow RFC8259.
In particular, whether or not a field is required/optional is not part
of its type in JSON.
Should we mention what the type field in the registry should include that?

E.g.:

> Type:  The type of the JSON value to be stored, as one of the value
>    types defined in [RFC8259], along with whether it is optional or required.

Or, add a new field:

> Required:  Whether or not the key is required in the document.

I guess it's unlikely that we'd add a new required field since the API
isn't versioned right now, but we should probably be explicit and
consistent
about it.

Thanks,
Kyle

On Thu, 5 Mar 2020 at 01:56, Martin Thomson <[e-mail]> wrote:
>
> Hi folks,
>
> Our fine editor teams have contributed updates to these drafts.
>
> https://tools.ietf.org/html/draft-ietf-capport-architecture-06
> https://tools.ietf.org/html/draft-ietf-capport-api-05
>
> This starts a joint working group last call on these documents. Please respond this mail with your views regarding the suitability of these documents for publication (as Informational RFC and Proposed Standard RFC respectively) before 2020-03-23.
>
> There are a few minor issues, but I consider those to be minor enough to require only trivial fixes. Some appear to be already addressed. If you think that major changes are needed, or have proposed resolutions to issues, adding those to your email would be helpful.
>
> Issues are tracked here:
> https://github.com/capport-wg/architecture/issues
> https://github.com/capport-wg/api/issues
>
> I encourage people to add issues to the tracker as they review these documents. Directly raising minor editorial issues on GitHub will help us focus attention on substantive issues here.
>
> Cheers,
> Martin (and Erik)
>
> _______________________________________________
> Captive-portals mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/captive-portals