Last Call Review of draft-ietf-radext-dynamic-discovery-13
review-ietf-radext-dynamic-discovery-13-genart-lc-thomson-2015-03-21-00

Request Review of draft-ietf-radext-dynamic-discovery
Requested rev. no specific revision (document currently at 15)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2015-04-07
Requested 2015-03-09
Draft last updated 2015-03-21
Completed reviews Genart Last Call review of -13 by Martin Thomson (diff)
Secdir Early review of -09 by Brian Weis (diff)
Secdir Telechat review of -13 by Brian Weis (diff)
Assignment Reviewer Martin Thomson
State Completed
Review review-ietf-radext-dynamic-discovery-13-genart-lc-thomson-2015-03-21
Reviewed rev. 13 (document currently at 15)
Review result Ready with Issues
Review completed: 2015-03-21

Review
review-ietf-radext-dynamic-discovery-13-genart-lc-thomson-2015-03-21

I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at

<

http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Please resolve these comments along with any other Last Call comments
you may receive.

Document: draft-ietf-radext-dynamic-discovery-13
Reviewer: Martin Thomson
Review Date: 2015-03-21
IETF LC End Date: 2015-03-20
IESG Telechat date: 2015-04-19

Summary: This document is of a quality I rarely see even for proposed
standard.  It is certainly fit for publication as an experimental RFC.
(In case it isn't clear, I agree with the decision to choose
experimental here, this is in a tricky area and deployment experience
will validate choices.)

Major issues: None

Minor issues:

S-NAPTR tags for RADIUS are defined, but none for Diameter.  I'm sure
this was considered, but it seems odd on first reading.

S2.1.1.2 uses SHOULD to recommend that clients retry with their other
certificates.  I'd recommend use of MAY here, unless you would like to
expand on why this a SHOULD is valuable.

S2.1.1.3.3. I know that this is experimental and all, but this seems a
little too hand-wavy even for that.  I think that this is the right
design, but the section could be a little more confident (and precise)
in the description of how this works.  It took me a couple of goes to
understand how this was intended to work.  One important realization
was that a common root for the realm needs to be configured.  I'd like
to see this section expanded slightly (my initial inclination would
have been toward removal, but the content is in line with the
experiment, so it's probably valuable).

S2.2 says:
   Since the Domain Name System is not
   necessarily trustworthy (e.g. if DNSSEC is not deployed for the
   queried domain name), it is important to verify that the server which
   was contacted is authorized to service requests for the user which
   triggered the discovery process.

This implies that DNS integrity is the only reason for authentication.
To paraphrase Steve Bellovin: you hand your packets to the attacker to
deliver.

Also:
   It MAY substitute labels on the leftmost dot-
   separated part of the NAI with the single character "*" to indicate a
   wildcard match for "all labels in this part".
Use the singular form here to avoid confusion:
  It MAY substitute the leftmost dot-
   separated label of the NAI with the single character "*" to indicate a
   wildcard match for "all labels in this part".

S3.4.1 is the first mention of UTF-8 realms, which are not permitted
by RFC 4282.  This was a point of confusion for me.  I note that the
new nai draft permits UTF-8 realms.  Please just cite that and remove
the reference to 4282.

S 3.4.3 doesn't cover how to handle NAPTR records with flags set to
"a", though other parts of the document describe that.  I think there
is some refactoring of the dependency on the S-NAPTR algorithm, and an
allowance for a default port.

Nits/editorial comments:

S1.3 Capital 'P' on first bullet
S2.1.1.1 Missing period on first note.
S2.1.1.2 Please provide a reference for "Effective TTL".
S2.1.1.3 s/PKS/PSK/ and "used for in the subsequent"
(S2.1.3 Please be consistent with the trailing period in DNS record entries)
2.1.3.c. "x-" ugh...RFC 6648
S.3.4.4 "If these
   TTLs are very low, thrashing of connections becomes possible; the
   Effective TTL mitigates that risk." - isn't this the MIN_EFF_TTL instead?
S3.4.4 the last MAY here can be a SHOULD, I think
S5 "the result O can not be trusted" -> "the result of the discovery
process can not be trusted"