Subject: [Gen-art] Gen-ART review of draft-ietf-radext-dynamic-discovery-13

From: Martin Thomson <[email-masked]>
Date: Sat, 21 Mar 2015 10:52:01 -0700

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"

---

From: Stefan Winter <[email-masked]>
Date: Mon, 23 Mar 2015 09:16:58 +0100

Hello,

thanks for those kind words :-)

> this was considered, but it seems odd on first reading.

Diameter defines its own S-NAPTR tags in its own RFCs - see the IANA
registry at
https://www.iana.org/assignments/s-naptr-parameters/s-naptr-parameters.xhtml#s-naptr-parameters-2
for a list.

The authors if the current RADIUS draft were aware of that and have
defined the radius tags exactly analogous to the Diameter ones (e.g.
diameter.tls.tcp matches radius.tls.tcp).

> expand on why this a SHOULD is valuable.

Well... realm discovery is triggered typically by an end user trying to
authenticate. If a client has a certificate but doesn't want to show it,
this becomes an avoidable DoS for the user. Also, if the client doesn't
always try the certificates in the same order, the connection setup also
becomes indeterministic - sometimes it works, sometimes not.

The clause makes a case for "don't give up prematurely" and makes the
behaviour deterministic.

So, I believe a SHOULD is the right thing to ask for here - would you
prefer an updated draft with this reasoning included, or are the
explanations in this mail sufficient?

> experiment, so it's probably valuable).

Oh, it's totally handwavy, agreed :-) The mechanism described here is
not implemented anywhere, and DANE in general only deals with the
server-side auth, leaving a gaping hole on the question how to validate
client certs. So, it really is just a sketch - that's why I used that
word in the text.

Actually I find that approach rather elegant, but it would take people
with a lot of energy to pick it up, complete the DANE specs for
client-side auth, implement TLS libraries that can do this, and to
actually deploy it.

I'm not sure I know how to handle your comment text-wise... when it
comes to the need of a common root, I would have hoped that there is
already text making it clear, i.e.:

"if
   DANE/TLSA records of all authorized servers are put into a DNSSEC
   zone with a common, consortium-specific branch of the DNS tree, then
   the entity executing the algorithm can retrieve TLSA Resource Records
   (TLSA RR) for the label "realm.commonroot" [...]"

If you can suggest specific text that helps readers understand this
easier, I'm all ears...

> To paraphrase Steve Bellovin: you hand your packets to the attacker to

Hmm... that text indeed is wrong / comes across badly.

Actually, even if DNS is trustworthy, the authorisation needs to be
checked. It's more like:

(new)
"Regardless whether the results from DNS discovery are trustworthy or
not (e.g. DNSSEC in use), it is always important to verify that the
server which was contacted is authorized to service requests for the
user which triggered the discovery process."

I've updated my working copy with that change; waiting for AD go-ahead
to publish a -14 eventually.

(What I planned to say there was that if DNS without SEC is used, not
only the authorisation is in question and needs to be checked, but also
the discovered IP/port; but that's not totally important in itself
because the authorisation check comes unconditionally and covers both
anyway)

>    wildcard match for "all labels in this part".

Thanks, changed in my working copy.

> new nai draft permits UTF-8 realms.  Please just cite that and remove

Right, currently the draft mentions both (see Terminology), but
meanwhile the publication of the new NAI document is imminent, so 4282
can go away. I've purged it from my working copy.

> allowance for a default port.

That's already done in step 9, which basically hands off further
resolution to S-NAPTR by stating: "For the extracted NAPTRs, perform
successive resolution as defined in [RFC3958], section 2.2."

If anything, poiting to section 2.2 is maybe too specific, because many
parts of RFC3958 are about "s" and "a" NAPTR entries; do you think it
makes the document more readable if I remove the "section 2.2" ?

> S1.3 Capital 'P' on first bullet

Changed to lower case.

> S2.1.1.1 Missing period on first note.

Fixed, thanks.

> S2.1.1.2 Please provide a reference for "Effective TTL".

Effective TTL is defined in section 3.3 ("Terms"); I've added a
forward-reference.

> S2.1.1.3 s/PKS/PSK/ and "used for in the subsequent"

Fixed and changed to just "in" (deleted "for")

> (S2.1.3 Please be consistent with the trailing period in DNS record entries)

Added plenty of dots.

> 2.1.3.c. "x-" ugh...RFC 6648

I believe eduroam's decision to make use of x-eduroam significantly
predates RFC6648. In any case, that's how they use it; the draft just
documents reality, however frowned-upon that reality may be :-)

>    Effective TTL mitigates that risk." - isn't this the MIN_EFF_TTL instead?

Well, both - Effective TTL has MIN_EFF_TTL as one of its parameters. In
a way, having MIN_EFF_TTL somewhere in the calculation prevents the
thrashing; if looked at from a different angle, only Effective TTLs wise
inclusion of that MIN_EFF_TTL in its calculations is the neuralgic spot
that actually prevents it.

So, I don't care. Do you insist on a text change?

> S3.4.4 the last MAY here can be a SHOULD, I think

Sounds reasonable. Changed.

> process can not be trusted"

Changed.

Thanks much for the review!

Greetings,

Stefan Winter

-- 
Stefan WINTER
Ingenieur de Recherche
Fondation RESTENA - Réseau Téléinformatique de l'Education Nationale et
de la Recherche
6, rue Richard Coudenhove-Kalergi
L-1359 Luxembourg

Tel: +352 424409 1
Fax: +352 422473

PGP key updated to 4096 Bit RSA - I will encrypt all mails if the
recipient's key is known to me

http://pgp.mit.edu:11371/pks/lookup?op=get&search=0xC0DE6A358A39DC66

---

From: Martin Thomson <[email-masked]>
Date: Mon, 23 Mar 2015 07:06:37 -0700

Thanks.  I think that you have everything in hand.  I'm happy with the
promised changes.

On 23 March 2015 at 01:16, Stefan Winter <[email-masked]> wrote:
> Diameter defines its own S-NAPTR tags in its own RFCs

Ack.

> explanations in this mail sufficient?

I think that mentioning the consequences would be wise.  The
incentives are certainly properly aligned.  (I do worry about the
inordinate number of protocol steps this is going to introduce, but I
guess that is the point of the experiment.)

> If you can suggest specific text that helps readers understand this

Much of the rest of the document is more precise about the steps
involved.  I got the sense that the DANE text was rushed or cursory.
I'd rather see a more precise description of the steps that a client
would use; or the text could be removed; or the text could be reduced
to something even more vague ("You could use TLSA records for this.")

> user which triggered the discovery process."

WFM

> because the authorisation check comes unconditionally and covers both

Right.

> makes the document more readable if I remove the "section 2.2" ?

No, that's fine.  I was just concerned about the port part; I see that
it is there, but it's fairly obtuse.  (My reaction is partly in error:
I was relying on my memory of 3958 a little too much.)

[...]