Subject: [radext] [Technical Errata Reported] RFC7585 (4991)

From: RFC Errata System <[email-masked]>
Date: Fri, 7 Apr 2017 08:36:16 -0700 (PDT)

The following errata report has been submitted for RFC7585,
"Dynamic Peer Discovery for RADIUS/TLS and RADIUS/DTLS Based on the Network Access Identifier (NAI)".

--------------------------------------
You may review the report below and at:
http://www.rfc-editor.org/errata_search.php?rfc=7585&eid=4991

--------------------------------------
Type: Technical
Reported by: Alan DeKok <[email-masked]>

Section: GLOBAL

Original Text
-------------

Corrected Text
--------------

Notes
-----
The document describes how to look up realms, but doesn't describe what to do after that.  i.e. are the lookups cached? Are the lookups done for every request that comes in?

This gives little guidance for an implementor.

I suggest leveraging the "logical AAA routing table" described in RFC 7542 Section 3.  In short form:

* a client MUST maintain a table of known dynamic realms.

* the table MUST be keyed by the realm name, and the contents MUST be server-specific information as described in RFC 7542 Section 3

* when a realm is being looked up, the realm SHOULD be inserted into the table with a "pending" flag, so subsequent requests during the lookup process do not cause additional lookups to be made

* if server validation fails, the realm SHOULD be updated with a "failed" state and a TTL, so that subsequent requests do not cause additional lookups for a period of time.  No server information should be stored for this entry. i.e. the realm is marked as "failed", the corresponding server MUST NOT be marked as "failed".  This prevents attacks where a malicious actor could point their realm to an unknowing third-party server, and cause poorly implemented proxies to mark it "failed".

* if server validation succeeds, the realm MUST be updated with a "live" state (and a TTL), so that subsequent requests go to the same server without additional lookups

*  if server validation succeeds, all realms in the TLS "subject alternative names" presented by the server SHOULD be inserted into the realm table, all pointing to the same server definition, so that subsequent requests for those other realms do not cause additional lookups to be made.

* servers which have been dynamically looked up MUST NOT be added to any global pool of servers.  i.e. the lookup is always "realm -> server", and not "There's a known server at this IP address"

I thought we had a short discussion of some of these topics on RADEXT, but I can't find it in the archives.

I think these comments are substantial enough to warrant "wait for document update".  I just wanted to ensure they weren't lost.

Instructions:
-------------
This erratum is currently posted as "Reported". If necessary, please
use "Reply All" to discuss whether it should be verified or
rejected. When a decision is reached, the verifying party  
can log in to change the status and edit the report, if necessary. 

--------------------------------------
RFC7585 (draft-ietf-radext-dynamic-discovery-15)
--------------------------------------
Title               : Dynamic Peer Discovery for RADIUS/TLS and RADIUS/DTLS Based on the Network Access Identifier (NAI)
Publication Date    : October 2015
Author(s)           : S. Winter, M. McCauley
Category            : EXPERIMENTAL
Source              : RADIUS EXTensions
Area                : Operations and Management
Stream              : IETF
Verifying Party     : IESG

---

From: Alan DeKok <[email-masked]>
Date: Fri, 7 Apr 2017 11:38:22 -0400

Me causing trouble again. :(

  I think this all makes sense, and I didn't see anywhere in the RFC were these issues were discussed.

> On Apr 7, 2017, at 11:36 AM, RFC Errata System <[email-masked]> wrote:
> 
> _______________________________________________
> radext mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/radext

---

From: Stefan Winter <[email-masked]>
Date: Fri, 7 Apr 2017 19:58:55 +0200

Hello,
> "Dynamic Peer Discovery for RADIUS/TLS and RADIUS/DTLS Based on the Network Access Identifier (NAI)".

Well, there's section 3.4.4 "Validity of results" which has some amount
of text on that topic:

"After executing the above algorithm, the RADIUS server establishes a
   connection to a home server from the result set. 
[...]
When a connection is open and the
   smallest of the Effective TTL value that was learned during
   discovering the server has not expired, subsequent new user sessions
   for the realm that corresponds to that open connection SHOULD reuse
   the existing connection and SHOULD NOT re-execute the discovery
   algorithm nor open a new connection.  To allow for a change of
   configuration, a RADIUS server SHOULD re-execute the discovery
   algorithm after the Effective TTL that is associated with this
   connection has expired.  The server SHOULD keep the session open
   during this reassessment to avoid closure and immediate reopening of
   the connection should the result not have changed."

That's a probably slightly convoluted form of saying: 

* keep up sessions until DNS validity TTLs tell you to stop.
* Then, re-check if DNS changed. 
* Do no drop connections on the floor until you're sure you've got to.

> * when a realm is being looked up, the realm SHOULD be inserted into the table with a "pending" flag, so subsequent requests during the lookup process do not cause additional lookups to be made

With the text above, is this needed? I would think that internal
housekeeping like deciding to store the data in a table, and that it has
keys, and what this key has to be is *too* much implementation specific
and does not need to be in an RFC?

> * if server validation fails, the realm SHOULD be updated with a "failed" state and a TTL, so that subsequent requests do not cause additional lookups for a period of time.  No server information s…

The same section 3.4.4 goes on to state:

"Should the algorithm above terminate with O-1 = { empty set }, the
   RADIUS server SHOULD NOT attempt another execution of this algorithm
   for the same target realm before the timeout O-2 has passed."

Is that not enough? I you want to include a failed TLS connection setup
phase result in the negative result state then that's probably more than
an errata can do: it's not in scope of the RFC, it only defines the
algorithm to find a set of servers to talk to. It hands off to RFC6614
once the server to talk to is known.

A tighter integration between those two phases is certainly desirable
(and a useful thing to consider when thinking about the update & move of
the specs from Experimental to Standards Track).

> * if server validation succeeds, the realm MUST be updated with a "live" state (and a TTL), so that subsequent requests go to the same server without additional lookups

See above.

> *  if server validation succeeds, all realms in the TLS "subject alternative names" presented by the server SHOULD be inserted into the realm table, all pointing to the same server definition, so t…

That is in principle a good suggestion. However the discovery process
also yielded some meta data about the server: there's a priority order
of servers. For a different realm, there might be different discovery
results with a different top-priority target; but the same server can
still be configured to serve the realm, but with a lower priority.

The mechanism above would mean that a client which has learned the
target for one realm will also talk to that same target about another
realm without validating that it is the one to talk to /in priority/.
That may lead to results not expected by the owner of that realm.

> * servers which have been dynamically looked up MUST NOT be added to any global pool of servers.  i.e. the lookup is always "realm -> server", and not "There's a known server at this IP address"

Well, that's how the algorithm is set up. The input is a realm, the
output is a set of servers. It is implicit that there's a link between
the input and the output of the algorithm. Any other interpretation of
the algorithm's results appear counter-intuitive to me.

> I think these comments are substantial enough to warrant "wait for document update".  I just wanted to ensure they weren't lost.

There certainly was discussion at some meetings. Many of those led to
text updates; which is why section 3.4.4 has corresponding text.

Can you re-check the document with section 3.4.4 in mind and provide a
more narrow description on what text exactly you think needs to be
corrected/added, and where?

Greetings,

Stefan Winter

---

From: Alan DeKok <[email-masked]>
Date: Fri, 7 Apr 2017 19:01:50 -0400

> Well, there's section 3.4.4 "Validity of results" which has some amount

  Essentially "subsequent packets for an open connection should re-use that connection".

> and does not need to be in an RFC?

  It would be good to recommend that only one connection discover is ongoing at a time.  This is an issue of network overload, not implementation IMHO.

> Is that not enough?

  Yes, it's largely the same thing.  It also doesn't say what to do with requests for that realm.  Discard them?  Reject them?

  RADIUS is bad that way. :(

> I you want to include a failed TLS connection setup

  The dynamic discovery RFC should also say what to do when thing go wrong.  This isn't an issue for RFC 6614, I think.

> the specs from Experimental to Standards Track)

  I agree.

> still be configured to serve the realm, but with a lower priority.

  Sure.

> That may lead to results not expected by the owner of that realm.

  Sure.  It just would be nice to avoid the discovery phase in some cases.

> the algorithm's results appear counter-intuitive to me.

  It's described, but isn't as explicit as the text in RFC 7542.

  As an implementor, it's nice to know what information I need to track, and what state changes I need to make in order to implement the protocol.

  The existing document describes the lookup algorithm in detail, but is a little more vague about what information is tracked, and what state changes are made.

> corrected/added, and where?

  I'd like more text on what to do as an implementor, and what to do when things go wrong.

  I think we can leave the errata as "hold for document update", so that the next rev can add some more text based on my comments.

  Alan DeKok.