Hi Tommy,
On Mon, Jun 15, 2020 at 09:49:43AM -0700, Tommy Pauly wrote:
> Hi Ben,
>
> Responses inline. I’ve updated the working copy here:
>
> https://capport-wg.github.io/api/draft-ietf-capport-api.html
>
> > On Jun 12, 2020, at 4:17 PM, Benjamin Kaduk <ka...@mit.edu> wrote:
> >
> > Hi Tommy,
> >
> > Only a few things left to comment on, inline.
> >
> > On Wed, Jun 10, 2020 at 11:08:38AM -0700, Tommy Pauly wrote:
> >> Hi Ben,
> >>
> >> Thanks as always for your detailed comments. I’ve updated our working copy
> >> with this commit:
> >>
> >> https://github.com/capport-wg/api/commit/ef6f9afe84f2e49827560b5e2e8f292966107896
> >>
> >> <https://github.com/capport-wg/api/commit/ef6f9afe84f2e49827560b5e2e8f292966107896>
> >>
> >> The full editor’s copy can be viewed here:
> >>
> >> https://capport-wg.github.io/api/draft-ietf-capport-api.html
> >> <https://capport-wg.github.io/api/draft-ietf-capport-api.html>
> >>
> >>> On Jun 9, 2020, at 12:16 PM, Benjamin Kaduk via Datatracker
> >>> <nore...@ietf.org> wrote:
> >>>
> >>> Benjamin Kaduk has entered the following ballot position for
> >>> draft-ietf-capport-api-07: No Objection
> >>>
> >>> When responding, please keep the subject line intact and reply to all
> >>> email addresses included in the To and CC lines. (Feel free to cut this
> >>> introductory paragraph, however.)
> >>>
> >>>
> >>> Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
> >>> for more information about IESG DISCUSS and COMMENT positions.
> >>>
> >>>
> >>> The document, along with other ballot positions, can be found here:
> >>> https://datatracker.ietf.org/doc/draft-ietf-capport-api/
> >>>
> >>>
> >>>
> >>> ----------------------------------------------------------------------
> >>> COMMENT:
> >>> ----------------------------------------------------------------------
> >>>
> >>> I'll start off with a handful of high-level comments, some of which
> >>> might qualify for Discuss-level points, but which have obvious
> >>> resolution and for which I trust the authors and AD to do the right
> >>> thing.
> >>>
> >>> JSON only has Numbers, not integers; Section 5 should not describe
> >>> fields as integers without additional clarification (e.g., "integer,
> >>> represented as a JSON Number").
> >>
> >> Fixed to mark this as a number, and mention that it is an integer in the
> >> description.
> >>>
> >>> Also, the GET example in Section 6 seems to be malformed, not specifying
> >>> an HTTP-version.
> >>
> >> Fixed!
> >>
> >>>
> >>> I also think we should go into more detail on the TLS usage, pulling in
> >>> the relevant preexisting IETF BCP and proposed standards to take
> >>> advantage of the current state of the art (details in the
> >>> section-by-section comments).
> >>
> >> Indeed! Replied in individual comments.
> >>>
> >>> Additionally, I note some places in the section-by-section comments
> >>> where we can go into more detail on the "chain of custody" of
> >>> information presented to the user, making sure that we keep a trusted
> >>> path from initial provisioning through to API server access and captive
> >>> portal server access. There are some places where we don't seem to have
> >>> a 100% airtight solution, and those gaps can be called out more clearly.
> >>
> >> I’ve tried to tighten this text throughout. Comments inline.
> >>>
> >>> Abstract
> >>>
> >>> with a Captive Portal system. With this API, clients can discover
> >>> how to get out of captivity and fetch state about their Captive
> >>> Portal sessions.
> >>>
> >>> The architecture document only implicitly talks about "Captive Portal
> >>> sessions" (one mention in passing of when the "end of a session is
> >>> imminent"). Should it have more text introducing the term/topic?
> >>
> >> Opened an issue on the architecture:
> >> https://github.com/capport-wg/architecture/issues/92
> >>
> >>>
> >>> Also, the architecture doc talks about learning the "state of
> >>> captivity", but this formulation implies that there might be a much
> >>> richer state to learn about.
> >>
> >> Opened an issue on the architecture:
> >> https://github.com/capport-wg/architecture/issues/93
> >>
> >>>
> >>> Section 1
> >>>
> >>> * A URI that a client browser can present to a user to get out of
> >>> captivity
> >>>
> >>> nit: this feels worded oddly to me; merely presenting (displaying?) a
> >>> URI to the user does not help to do anything to get out of captivity.
> >>> Presenting the dereferenced content referred to by that URI might be
> >>> more effective at it, but would still require user action...
> >>
> >> Reworded to: "A URI of a user-facing web portal that can be used to get
> >> out of captivity"
> >>>
> >>> * An encrypted connection (using TLS for connections to both the API
> >>> and user portal)
> >>>
> >>> I think "authenticated" is equally as important as "encrypted" and
> >>> should be mentioned alongside it.
> >>
> >> Reworded to: "Authenticated and encrypted connections, using TLS for
> >> connections to both the API and user-facing web portal"
> >>>
> >>> Section 3
> >>>
> >>> 2. API Server interaction, in which a client queries the state of
> >>> the captive portal and retrieves the necessary information to get
> >>> out of captivity.
> >>>
> >>> I may be overly tied to this "state of captivity" phrase, but is there a
> >>> need to use "state" in a different formulation here?
> >>
> >> Made consistent as “state of captivity"
> >>>
> >>> Section 4
> >>>
> >>> For example, if the Captive Portal API server is hosted at
> >>> "example.org", the URI of the API could be "https://example.org/
> >>> captive-portal/api"
> >>>
> >>> The architectures says that "the URIs provided in the API SHOULD be
> >>> unique to the UE and not dependent on contextual information to function
> >>> correctly." I guess this is referring to the contents of the resource
> >>> retrieved by dereferencing the URI of the API and not the URI of the API
> >>> itself? So this example is not in conflict with the SHOULD.
> >>
> >> Indeed, this is not in conflict, as this is about the URI of the API
> >> server itself, not the URIs contained with the API JSON.
> >>>
> >>> If the API server needs information about the client identity that is
> >>> not otherwise visible to it, the URI provided to the client during
> >>> provisioning can be distinct per client. Thus, depending on how the
> >>> Captive Portal system is configured, the URI might be unique for each
> >>> client host and between sessions for the same client host.
> >>>
> >>> I appreciate having this explanation for why "[t]he client SHOULD NOT
> >>> assume that the URI for a given network attachment will stay the same"
> >>> as the first paragraph of the section tells us. The explanation is a
> >>> little further from the requirement than I would like, but I don't have
> >>> a suggestion for bringing them closer together :-/
> >>
> >> I’ve re-ordered this slightly, to move the "SHOULD NOT assume” to after
> >> the example.
> >>>
> >>> For example, a Captive Portal system that uses per-client session
> >>> URIs could use "https://example.org/captive-portal/api/X54PD"; as its
> >>> API URI.
> >>>
> >>> Hmm, assuming a base64 alphabet, that has like 30-ish bits of entropy
> >>> available in the final path component. Is that representative of a
> >>> large-enough identifier space for real deployments?
> >>
> >> Expanded to "https://example.org/captive-portal/api/X54PD39JV
> >> <https://example.org/captive-portal/api/X54PD39JV>”.
> >>>
> >>> Section 4.1
> >>>
> >>> I mentioned this on the architecture document already, though perhaps it
> >>> makes more sense to be done in the procotol document (this one): RFC
> >>> 6125 has guidelines for TLS server authentication and is a good
> >>> reference to cite. However (and regardless of whether we reference 6125
> >>> or not), we still will need to say what name type the client looks for
> >>> in the certificate and how the client obtains the reference name to
> >>> compare against the certificate contents. For us the latter is probably
> >>> simple: just use what we got from the capport provisioning stage (and
> >>> leave to 7710bis the question of how we authenticate *that*
> >>> information), but it should still be said.
> >>
> >> Definitely a good point. See updated paragraph below.
> >>
> >>>
> >>> example above). The hostname of the API SHOULD be displayed to the
> >>> user in order to indicate the entity which is providing the API
> >>> service.
> >>>
> >>> Should this hostname only be presented to the user (as, presumably, the
> >>> identity of the captive network?) when the certificate validation
> >>> succeeds? Presenting a name that has not been authenticated leaves the
> >>> user open to spoofing attacks.
> >>
> >> There’s been some discussion about this entire display issue on the secdir
> >> review. I think that the clients are unlikely to actually use the API
> >> hostname instead of the portal hostname. As such, I’ve re-worded this
> >> authentication explanation to not be about user display, but be about
> >> validating that the API server is the one the network intended. This also
> >> details the RFC6125 reference.
> >>
> >> The purpose of accessing the Captive Portal API over an HTTPS connection
> >> is twofold: first, the encrypted connection protects the integrity and
> >> confidentiality of the API exchange from other parties on the local
> >> network; and second, it provides the client of the API an opportunity to
> >> authenticate the server that is hosting the API. This authentication
> >> allows the client to ensure that the entity providing the Captive Portal
> >> API has a valid certificate for the hostname provisioned by the network
> >> using the mechanisms defined in {{?I-D.ietf-capport-rfc7710bis}}, by
> >> following the rules for DNS domain name validation in {{!RFC6125}}.
> >
> > This helps a lot, though usually we couple the RFC 6125 procedures with a
> > note about which name type(s) to use. E.g., I'd expect DNS-ID to be
> > nearly-universal for the capport usage.
>
> That’s a good clarification. Changed this to:
>
> "...by validating that a DNS-ID {{!RFC6125}} on the certificate is equal to
> the provisioned hostname."
Sounds good.
> >
> >>>
> >>> Clients performing revocation checking will need some means of
> >>> accessing revocation information for certificates presented by the
> >>> API server. Online Certificate Status Protocol [RFC6960] (OCSP)
> >>> stapling, using the TLS Certificate Status Request extension
> >>> [RFC6066] SHOULD be used. OCSP stapling allows a client to perform
> >>>
> >>> This is consistent though not fully alligned with the recommendations in
> >>> BCP 195 (RFC 7525). Should we reference the BCP and discuss our
> >>> deviations from its recommendations?
> >>
> >> I’ve added a reference as: "For more discussion on certificate revocation
> >> checks, see Section 6.5 of BCP 195 {{?RFC7525}}.”
> >>
> >> I’m not sure there’s much to say here regarding deviations, as mainly this
> >> is advice to the deployment about allowing traffic through the portal.
> >> Even if CRLs are not encouraged, etc, we are noting that they must be
> >> allowed if that’s what’s needed to validate the certificates.
> >>>
> >>> revocation checks without initiating new connections. To allow for
> >>> other forms of revocation checking, a captive network could permit
> >>> connections to OCSP responders or Certificate Revocation Lists (CRLs)
> >>> that are referenced by certificates provided by the API server. In
> >>> addition to connections to OCSP responders and CRLs, a captive
> >>>
> >>> This leaves it to the reader to know that there may be clients that
> >>> don't support OCSP stapling and would need access to some other
> >>> mechanism for revocation checking. It might be worth making that more
> >>> explicit, since the type of client on the network is not always under
> >>> the control of the network operator.
> >>
> >> Updated to:
> >>
> >> To allow for other forms of revocation checking, especially for clients
> >> that do not support OCSP stapling, a captive network SHOULD permit
> >> connections to OCSP responders...
> >>>
> >>> network SHOULD also permit connections to Network Time Protocol (NTP)
> >>> [RFC5905] servers or other time-sync mechnisms to allow clients to
> >>> accurately validate certificates.
> >>>
> >>> Is there a way to reliably identify "NTP servers or other time-sync
> >>> mechanisms"? My understanding is that the network generally doesn't
> >>> have a way to indicate to a client what NTP (or other time protocol)
> >>> server to use, so it would be fairly easy to end up in a situation where
> >>> client and enforcement device disagree on what time-synchronization
> >>> mechanism to use and the client gets locked out.
> >>
> >> My assumption here is that the portal allows traffic on the port for NTP,
> >> with potentially a whitelist/blacklist to prevent abuse.
> >
> > That's my assumption, too, though it's not fully reliable (and having a
> > port completely open allows for clever people with a cooperating entity on
> > the other side of the portal to tunnel arbitrary traffic).
>
> Agreed. I think in this case it’s best to give leave the specific firewalling
> details to the deployment.
> >
> >>>
> >>> be used by the Captive Portal API server. If the certificates do
> >>> require the use of AIA, the captive network MUST allow client access
> >>> to the host specified in the URI.
> >>>
> >>> [This implicitly assumes that the DNS resolution of that host is the
> >>> same at both client and enforcement device, which hopefully goes without
> >>> saying.]
> >>
> >> Indeed.
> >>>
> >>> Section 6
> >>>
> >>> Upon receiving this information the client will use this information
> >>> direct the user to the the web portal (as specified by the user-
> >>>
> >>> nit: "to direct"
> >>
> >> Fixed!
> >>>
> >>> Captive Portal JSON content can contain per-client data that is not
> >>> appropriate to store in an intermediary cache. Captive Portal API
> >>> servers SHOULD set the Cache-Control header in any responses to
> >>> "private", or a more restrictive value [RFC7234].
> >>>
> >>> Is there a well-defined ordering on Cache-Control header [field]
> >>> restrictiveness such that "more restrictive value" is clearly defined?
> >>
> >> There isn’t anything that’s a strict ordering, no. I’ve added the
> >> clarification “…such as "no-store”."
> >>
> >>> (nit: s/header/header field/.)
> >>
> >> Fixed
> >>>
> >>> Also, it's easy to miss a normative requirement like this when it's in
> >>> an "Example Interaction" section; I suggest moving it elsewhere.
> >>
> >> Fair point. Moved up to section 5.
> >>>
> >>> Section 7
> >>>
> >>> which the client can perform revocation checks. This allows the
> >>> client to ensure that the API server has authority for a hostname
> >>> that can be presented to a user.
> >>>
> >>> We should say something about how a client does (or does not) determine
> >>> that the authenticated hostname is authorized to act for the network
> >>> being joined. The last paragraph's discussion of confusables and
> >>> "trustworthy name"s suggests that there isn't much of a direct chain
> >>> here, just whether the certified domain name is "trustworthy" or not.
> >>> Even if so (which I could understand being the case; it's not an easy
> >>> problem), we should be clear about the gap in the system and the
> >>> potential risks it entails.
> >>>
> >>> It is important to note that while the server authentication checks
> >>> can validate a specific hostname, it is certainly possible for the
> >>> API server to present a valid certificate for a hostname that uses
> >>> non-standard characters or is otherwise designed to trick the user
> >>> into believing that its hostname is some other, more trustworthy,
> >>> name. This is a danger of any scenario in which a hostname is not
> >>> typed in by a user.
> >>>
> >>> Do we want to reference any of the PRECIS stuff (RFC 7564/etc.)?
> >>
> >> For this section, I think it’s clearer to focus (as you point out) on the
> >> API’s ability to act on behalf of the network that did the provisioning,
> >> and not on the user relationship to the network. Reworded to:
> >>
> >> This allows the client to ensure that the API server has authority for the
> >> hostname that was provisioned by the network using
> >> {{?I-D.ietf-capport-rfc7710bis}}. Note that this validation only confirms
> >> that the API server matches what the network's provisioning mechanism
> >> (such as DHCP or IPv6 Router Advertisements) provided, and not validating
> >> the security of those provisioning mechanisms or the user's trust
> >> relationship to the network.
> >
> > This is good and correct, thanks. I think it's still worth keeping some
> > discussion of confusables in URLs, though (even though it would be most
> > relevant for the non-API server's site), since all the elements of the
> > portal might be considered hostile in some situations, and that includes
> > the Captive Portal Server (in the architecture doc's definition).
>
> That’s fair. I’ve added an extra paragraph to describe this for the web
> portal server:
>
> It is important to note that although communication to the user-facing web
> portal requires using TLS, the authentication only validates that the web
> portal server matches the name in the URI provided by the API server. Since
> this is not a name that a user typed in, the hostname of the web site that
> would be presented to the user may include "confusable characters" that can
> mislead the user. See Section 12.5 of {{?RFC8264}} for a discussion of
> confusable characters.
I think that covers the important bits. We ... don't exactly have great
answers in this space, yet, so pointing out the risks will have to suffice.
(Please do double-check whether this also addresses the comments of the
other reviewer, that initiall got a response of "per Ben's review we
removed this text. Unfortunately I forget which reviewer that was...)
> >>>
> >>> Section 7.1
> >>>
> >>> Information passed between a client and a Captive Portal system may
> >>> include a user's personal information, such as a full name and credit
> >>> card details. Therefore, it is important that Captive Portal API
> >>> Servers do not allow access to the Captive Portal API over
> >>> unencrypted sessions.
> >>>
> >>> While I support this requirement, it seems like the personal information
> >>> exchange is more likely to occur between client and Captive Portal
> >>> Server than between client and Captive Portal API Server. Protecting
> >>> the exchange with the API server is still important, though, to make
> >>> sure that the client talks to the right Captive Portal Server!
> >>
> >> Reworded to:
> >>
> >> Information passed between a client and the user-facing web portal may
> >> include a user's personal information, such as a full name and credit card
> >> details. Therefore, it is important that both the user-facing web portal
> >> and the API server that points a client to the web portal are only
> >> accessed over encrypted connections.
> >>
> >>>
> >>> Section 8.2
> >>>
> >>> New assignments for Captive Portal API Keys Registry will be
> >>> administered by IANA using the Specification Required policy
> >>> [RFC8126]. The Designated Expert is expected to validate the
> >>> existence of documentation describing new keys in a permanent
> >>> publicly available specification. The expert is expected to validate
> >>>
> >>> Does an I-D that is never published as an RFC meet this bar?
> >>
> >> Indeed, as that is a stable reference.
> >
> > I find that this is not an opinion universally held, and being explicit
> > about it might help. (I actually somewhat expect IANA to ask about it,
> > whether now or at some later point.)
>
> Sure, I’ve made this explicit as:
>
> The Designated Expert is expected to validate the existence of documentation
> describing new keys in a permanent
> publicly available specification, such as an Internet Draft or RFC.
Thanks!
-Ben
> >>>
> >>> that new keys have a clear meaning and do not create unnecessary
> >>> confusion or overlap with existing keys. Keys that are specific to
> >>>
> >>> I trust that this includes "matches an existing key name using a
> >>> case-insensitive comparison".
> >>
> >> Yes, that should be within the scope of what experts would consider
> >> confusing or overlapping (along with many other variations of confusing
> >> allocations).
> >
> > Sounds good.
> >
> > Thanks for the updates!
>
> >
> > -Ben
>
> Thank you!
> Tommy
> >
> > _______________________________________________
> > Captive-portals mailing list
> > Captive-portals@ietf.org
> > https://www.ietf.org/mailman/listinfo/captive-portals
>
_______________________________________________
Captive-portals mailing list
Captive-portals@ietf.org
https://www.ietf.org/mailman/listinfo/captive-portals
