Re: [Captive-portals] Benjamin Kaduk's No Objection on draft-ietf-capport-api-07: (with COMMENT)

Benjamin Kaduk <kaduk@mit.edu> Mon, 15 June 2020 21:31 UTC

Return-Path: <kaduk@mit.edu>
X-Original-To: captive-portals@ietfa.amsl.com
Delivered-To: captive-portals@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id CFF893A12B5; Mon, 15 Jun 2020 14:31:11 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.897
X-Spam-Level:
X-Spam-Status: No, score=-1.897 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id oFPKBJyw5OZb; Mon, 15 Jun 2020 14:31:08 -0700 (PDT)
Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 6FCE23A12A8; Mon, 15 Jun 2020 14:31:03 -0700 (PDT)
Received: from kduck.mit.edu ([24.16.140.251]) (authenticated bits=56) (User authenticated as kaduk@ATHENA.MIT.EDU) by outgoing.mit.edu (8.14.7/8.12.4) with ESMTP id 05FLUsFT011110 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 15 Jun 2020 17:30:56 -0400
Date: Mon, 15 Jun 2020 14:30:53 -0700
From: Benjamin Kaduk <kaduk@mit.edu>
To: Tommy Pauly <tpauly@apple.com>
Cc: draft-ietf-capport-api@ietf.org, capport-chairs@ietf.org, captive-portals@ietf.org, The IESG <iesg@ietf.org>, mt@lowentropy.net
Message-ID: <20200615213053.GN11992@kduck.mit.edu>
References: <159173018778.27821.5944050976754720340@ietfa.amsl.com> <7CFF83D3-84E7-42C1-A478-7326599DD6C4@apple.com> <20200612231701.GT11992@kduck.mit.edu> <78F65A81-2AB8-48AF-9524-A179CCAA2A82@apple.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <78F65A81-2AB8-48AF-9524-A179CCAA2A82@apple.com>
User-Agent: Mutt/1.12.1 (2019-06-15)
Archived-At: <https://mailarchive.ietf.org/arch/msg/captive-portals/cTIi5lC_T7zARp4WaU4Li5OUzWA>
Subject: Re: [Captive-portals] Benjamin Kaduk's No Objection on draft-ietf-capport-api-07: (with COMMENT)
X-BeenThere: captive-portals@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Discussion of issues related to captive portals <captive-portals.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/captive-portals>, <mailto:captive-portals-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/captive-portals/>
List-Post: <mailto:captive-portals@ietf.org>
List-Help: <mailto:captive-portals-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/captive-portals>, <mailto:captive-portals-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 15 Jun 2020 21:31:17 -0000

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 <kaduk@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 <noreply@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
>