Subject: [radext] (belated) AD review of draft-ietf-radext-coa-proxy-04

From: Benjamin Kaduk <[email-masked]>
Date: Sat, 14 Jul 2018 17:21:49 -0500

Apologies for the long delay on this one; I had written up an AD review a
month or two ago but it got lost in the ether.  I've tried to reconstruct
my comments based on notes I made on a paper printout, so there will be a
few places where I imply that I changed my mind since the initial review.

I have a couple of concerns about this document that I think should be
resolved before I move it into IETF Last Call:

This document lists RFC 5176 and RFC 2866 as informative references,
possibly so as to avoid the awkwardness of a standards-track document
making a normative downreference to informational documents.  I think that
at least RFC 5176 should be a normative reference, since it really must be
understood in order to implement this document -- after all, we even Update
it!  Looking back again (for the third time, actually), it's less clear
that the RFC 2866 reference is actually as problematic as it seemed on
first read; as there is only a single reference and it is just explaining
why this document has a MUST-level requirement.  I don't think there's an
actual issue with making 5176 a normative reference in terms of process; we
just have to call out the "downref" during IETF Last Call and then it goes
on the list of approved downrefs.

I also think there needs to be some more clarity in the Requirements on
Visited Networks (Section 4.2), in several ways:

* First off, it refers to "all the checks discussed above for proxies",
  which presumatly is an artifact of subsection reordering.  Even if it is
  changed to "below", though, I'm still unclear on exactly which checks are
  implied -- is this just the filtering from Section 4.3.2?

* Furthermore, it's unclear what it actually means to have a MUST-level
  requirement to do some checks that are only SHOULD-level in Section
  4.3.2.

* I'm also a little fuzzy on where thse requirements apply, as a "Visited
  Network" is kind of a fuzzy concept, and is going to potentially involve
  things like a network perimeter/firewall, some form of proxies, RADIUS
  servers, etc. -- should we say something more specific about where the
  actions to meet these requirements will be taken?

* I don't think I followed how assuming there's a proxy between the NAS and
  any external proxy implied a requirement for the Visited Network to
  perform "all of the checks".  (Maybe if what the checks were was
  clarified this concern would go away, though.)

* The second paragraph leaves me generally confused, mostly in that it is
  talking about somethinf "phrased more generically in Section 4.3.2",
  which is my best guess for the thing that the first paragraph is trying
  to require.  So is this whole paragraph redundant?

* Why is the cryptographically-strong requirement for Operator-NAS-Identity
  only SHOULD and not MUST?

* Is there a conflict between the "without requiring to track ... in a
  database" in the third paragraph, and the text for the Value field in the
  Operator-NAS-Identifier definition, that says "either track these tokens
  in a database, or [...] create tokens which can be decoded"?

I also think this document should probably switch over to the RFC 8174
boilerplate instead of the RFC 2119 boilerplate, if only to save all of the
IESG and the genart reviewer from having to comment about it.

Somewhat editorial, but with great potential for confusion: in Section 3.3,
please insert text like "The new Operator-NAS-Identifier attribute is
defined as follows" before the Description/Type/etc.

Also in Section 3.3, I wondered if the "in practice" clarification for
using a database or self-encrypted tokens was intentionally excluding the
possibility of always using the same value for the token (e.g., if there is
only one NAS in the Visited Network, which is admittedly unlikely in
nontrivial deployments).

Some editorial comments follow.

As a general comment, the RFC Editor generally likes to distinguish between
"which" (inate attributes that apply to all elements of a class) and "that"
(attributes that only apply to a subset of the class, and thereby
specifying a new restriction on what is being discussed).  I don't really
have a horse in the race, but if you want to avoid getting asked about it
later, you could make a pass over the document looking for them.  There's a
couple spots in Section 4.3.1 that would probably change from "which" to
"that", if you do.

In Section 1, I think the second paragraph should start with "We partially
correct" given the limitation made in the third paragraph.  I might also
reword the third paragraph to something like "This correction is limited to
the use case of CoA proxying Realm-basec proxying [...]", as it's not
exactly the use case that's being limited as much as the applicability of
this document.  (It looks like I had originally suggested reordering the
second and third paragraphs, but that doesn't make sense to me now.)

In Section 2.1, fourth paragraph, it may be useful to clarify whether "The
RADIUS client" applies only to the endpoint (non-proxy) client or to the
client portion of all entities participating in a proxy chain along the
path.

In Section 2.3, "there may be a different CoA server which could accept the
packet", I assume that "accept the packet" is supposed to imply "and do
something useful with it"; perhaps this could be reworded to something like
"successfully process the packet".  Additionally, it doesn't seem accurate
to say that 5176 "is silent on the topic of 'session identification
attributes'", since that phrase appears ten times; it seems like we are
saying that it doesn't actually tell you how to use them or what they are,
but just says that they can exist (kind of like the situation for CA
proxying itself).

The pedant in me takes issue with "impossible" in "impossible to send CoA
packets to the Visited Network", in Section 3.1 -- I assume that there are
other contrived and inefficient potential solutions.

s/one ore more/one or more/ in Section 3.2

In Section 3.3, third paragraph, the references to 2865 and 2866 are both
Section 4.1; we should probably say both "Section 4.1 of [2865], and of
Section 4.1 of [2866].  It also seems that a reference to Section 4.1 (what
a coincidence!) of RFC 5580 might be in order, to note the parallelism
between Operator-NAS-Identifier and Operator-Name.

In Section 4.2, third paragraph, presumably the non-requirement for
tracking in a database "every individual value" applies only to those
values that are actually issued.  (That is, I'm asking to add "issued," in
"every value of Operator-NAS-Identifier issued, in a database".)

In Section 4.3.2, do we want to "specifically note" or "call out" instead
of just "note" that the above requirements apply to future attributes added
by a proxy (as well as the ones defined here)?  (This is something of a
style question, so it's up to you.)

Section 5.1, s/xwill/will/

In the last paragraph of Section 5.1, do we need to say that the recording
is done "if present"?

Section 5.2, s/disconnecte/disconnected/; s/recieve/receive/;
s/fromt he/from the/

Also in Section 5.2, do we want to add "next-hop" when talking about
finding "the CoA server for that realm"?  And, in the last paragraph, we
say "Any response from the CoA server [...]", but isn't the CoA server
required to send such a response?

-Ben

---

From: Alan DeKok <[email-masked]>
Date: Sun, 15 Jul 2018 09:52:56 -0400

On Jul 14, 2018, at 6:21 PM, Benjamin Kaduk <[email-masked]> wrote:
> at least RFC 5176 should be a normative reference,

  Sure.

>  implied -- is this just the filtering from Section 4.3.2?

   Yes.

>  requirement to do some checks that are only SHOULD-level in Section

  4.3.2 has a few MUST requirements.

  The text could say:

A Visited Network which receives a CoA packet which will be proxied MUST perform all
of the operations required for proxies by Section 4.3.2. 

>  Network" is kind of a fuzzy concept,

  It's a common term in RADIUS.    See RFC 7542:

https://tools.ietf.org/html/rfc7542

  In short, you have a "home" network.  This is the network managed by the people who gave you your account.  A "visited" network is where you use your "home" credentials to visit someone else's network.

> and is going to potentially involve

  I don't think so.  Most of these actions are administrative and/or contractual.  i.e. can a AT&T user roam onto Verizon's network?  Answer: ask the lawyers. :)

  The other technical details are necessary, but don't affect RADIUS here.  We just require that (magically) the RADIUS servers can exchange packets.

>  clarified this concern would go away, though.)

  The assumption is that the visited NAS doesn't send packets directly to the home RADIUS server.  Instead, the visited NAS sends packets to it's local RADIUS server: the visited RADIUS server.

  That one goes "oops, not my user.." and proxies the packet.  So it's a proxy, and all of the proxy rules apply.

  The text is there because otherwise people might think that the proxy rules only apply for "proxy to proxy" packets.

>  to require.  So is this whole paragraph redundant?

  It's largely a reminder to that the Operator-Name attribute is for server to server signalling.  The NAS doesn't know anything about it, so you should never send it to the NAS.

  It's redundant, but IMHO a useful reminder.

>  only SHOULD and not MUST?

  It could be a simple incrementing counter, with the visited network tracking counter to NAS packets.

  Yes, implementations are that dumb.

>  in a database, or [...] create tokens which can be decoded"?

  I don't think so.  If the operator tracks the values in a DB, those values don't need to be cryptographically strong.

  If the operator "hides" the real NAS identity in a signed / encrypted value, then that "hiding" method should be cryptographically strong.

  Perhaps it could be worded more clearly.

> IESG and the genart reviewer from having to comment about it.

  Sure.

> defined as follows" before the Description/Type/etc.

  Sure.

> only one NAS in the Visited Network, which is admittedly unlikely in

  No, that's a permitted use-case.  I'll add a comment for "non-trivial use-cases".

> couple spots in Section 4.3.1 that would probably change from "which" to

  Sure.  I'll take a look.

> correct" given the limitation made in the third paragraph.

  Yes.

>  I might also

  OK.

> client portion of all entities participating in a proxy chain along the

  It applies to the client portion of all RADIUS transactions.  That should be clarified by the subsequent sentence in that paragraph.

> "successfully process the packet".

  Yes.

>  Additionally, it doesn't seem accurate

  It's silent on the topic of which attributes constitute "session identification attributes".

Q:  Session identifications attributes can exist?
A:  Yes
Q:  Great!  What are they?
A:  (runs away screaming)

  That was the subject of LONG discussions in RADEXT.   The polite version is that there was no consensus to make *any* recommendations here, no matter how small.

  I'll change the text to:

Finally, [RFC5176] is silent on the topic of which
attributes constitute "session identification attributes", which makes
it impossible for a proxy to determine if a CoA packet matches a
particular user session.

> other contrived and inefficient potential solutions.

  Yes.

> s/one ore more/one or more/ in Section 3.2

  Fixed.

> between Operator-NAS-Identifier and Operator-Name.

  Yes.  I'll update it.

> "every value of Operator-NAS-Identifier issued, in a database".)

  Yes.  I'll update it.

> style question, so it's up to you.)

  I think it's fine as-is.

> Section 5.1, s/xwill/will/

  Fixed.

> In the last paragraph of Section 5.1, do we need to say that the recording

  Yes.

> Section 5.2, s/disconnecte/disconnected/; s/recieve/receive/;

  Yes.

> finding "the CoA server for that realm"?

  Yes.

>  And, in the last paragraph, we

  Yes.  But it may be down. :)

  Alan DeKok.