Early Review of draft-ietf-radext-dtls-07
review-ietf-radext-dtls-07-genart-early-campbell-2013-11-19-00

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>.

This is an early review at WGLC last call.

Document:   draft-ietf-radext-dtls-07
Reviewer: Ben Campbell
Review Date: 2013-11-18

Summary: This draft is on the right track, but there are significant open
issues that should be resolved before it progresses.

[Note: This review is different from the usual Gen-ART review due to the fact
that it's an early (as in prior to pubreq) review, and that the draft is
intended to become an experimental RFC. The gen-ART review process is tuned for
drafts at IETF last call or later. It's a little hard to figure out what
criteria should be used for drafts at an earlier point in the life-cycle. That
being said, I reviewed this as if it were near-final, and apologize if that
turns out to be too strict for an early review.

I used similar review criteria as I would for a standards-track draft, since
this draft still specifies protocol with the hope of interoperable
implementations. The only difference comes from a few comments explicitly about
the experimental status of the draft.]

*** Major issues:

-- General:

The draft needs an overhaul of it's use of normative language. There appears to
be redundant (and possibly contradictory) language for the same requirements
sprinkled throughout. There are also cases of normative language being used for
internal implementation guidance, which is not appropriate as described by RFC
2119. (See the minor issues section for details--most of the instances would
not qualify as major issues by themselves, but I think they constitute a major
issue in the aggregate.)

-- section 3, 2nd paragraph:  "... long-term use of RADIUS/UDP is NOT
RECOMMENDED."

While I agree with the sentiment, that's not an appropriate assertion for an
experimental RFC. It would either need to go into an standards-track update to
the RADIUS spec, or a BCP.

(Also applies to the reiteration in 10.1)

*** Minor issues:

-- General:  It might be worth some text on why this is experimental rather
than informational. Is there any plan to evaluate the implementation results?
Is there an expectation that a future RADIUS/DTLS spec could become
standards-track? Is there any plan to evaluate the outcome of this document?

-- Section 1, 2nd paragraph:

Isn't this true for almost any use of IPSec? Is there some specific reason this
is worse for RADIUS than for other protocols?

-- Section 1, 4th paragraph:

The second sentence mentions that firewall rules do not need to be changed. The
following sentence recommends a change to firewall rules.

The firewall rule recommendations in the 3rd sentence seems odd, since it seems
like any protocol over DTLS would pass. Also, does this imply a recommendation
that firewalls with DPI be used in the first place, since the absence of such a
firewall has the same effect as having one that doesn't enforce the protocol?
(Is there a reason a protocol spec should recommend firewall rules in the first
place, other than to mention places where certain firewall rules might prevent
interfere with operation?)

-- section 2.1, 5th paragraph (3rd paragraph after bullet list) : 
"Implementations MUST support encapsulated RADIUS packets of 4096 in length..."

Please be more precise than "MUST support". Specifically, does "MUST support"
indicate you must support both sending and receiving of that size? (since 4096
would generally exceed PMTU even for RADIUS/UDP)

-- 2.2 and children:

I find this section confusing as to where to find the authoritative text. Some,
but not all, of the material from 6614 is repeated as normative text in later
sections.  The description of this draft as applying 'semantic "patches"' to
6614 seems to imply you mean the 6114 text, with these patches applied, to be
normative, which creates some potential conflicts. If, on the other hand, 2.2
(.x) is intended more as a informative description of the differences, please
say so.

-- 3, 1st paragraph:

Can you elaborate on the resulting "timeouts, lost traffic, and network
instabilities"?

-- 3.1:

The section describes UDP/2083 as the "default port". But later sections
describe port related behavior in a way that makes it it look like the
impementations must always use 2083, which makes it more than just a default.
Is the administrator allowed to choose some other port for any reason? If so,
it might make more sense to refer to the port by role rather than number when
discussing port related behavior. (I note that 6614 only mentions the port
number in the initial assignment, the IANA considerations, and the appendix.)

-- 3.2, last paragraph:

Can you elaborate on the bid down attack? Given that the use of dtls is
configured, rather than negotiated, how would an attacker bid it down?

-- 4, 2nd paragraph:

Seems like an (or maybe even the) important point would be that a client should
not fall back to cleartext if a server appears to not support it.

-4, 3rd paragraph:

Is the recommendation to use a local proxy for all clients, or just those
implemented as multiple processes?

-- 5.1.1, third paragraph: "In those cases, the implementation MAY delete the
underlying session"

Can you elaborate on why that's a MAY and not a SHOULD or MUST?

-- 5.1.1, 4th paragraph:

This paragraph rephrases 2119 normative language with more normative language.
That creates confusion about which text is authoritative. I suggest either
keeping the second paragraph and deleting the first, or make the second
non-normative.

-- 5.1.1, 6th paragraph: "The timestamp SHOULD be updated ..."

Why not MUST?

-- 5.1.1, 7th paragraph:

Is the "idle time" configurable setting on a per peer or global basis?

-- 5.1.1, 8th paragraph:

What does it mean to "track" sessions?  Also, it seems like the "SHOULD stop
creating" guidance contradicts later guidance that least recently used sessions
might get dropped instead?

-- 5.1.1, 10th paragraph:

Should the second sentence contain a normative MUST?

Also, the 2nd and 3rd sentences taken together seem say "the server must keep
the session open until it decides not to"

How would an unauthenticated client close an active DTLS session?

-- 5.2, 1st paragraph:

The normative requirement for a client to use heartbeats _or_ the application
level watchdog algorithm seems to contradict the normative guidance that a
server SHOULD use both.

-- 5.2, 3rd paragraph, 1st sentence:

I would hesitate to phrase this that a client may violate the previous
normative SHOULDs. It would be better to phrase this as describing th
econsequences should the client ignore the SHOULD.

-- 5.2, 2nd to last paragraph:

Does this have actual interop or security issues beyond saying, "it's harder to
implement and you might screw it up"? If not, it seems counter to the 2119
guidance on when normative language is appropriate. It would be more properly
non-normative  implementation guidance.

-- 6, 1st paragraph:

You mention that these are implementation guidelines, and not part of the
protocol. But the section contains 2119 style normative requirements. in
general, that's not appropriate unless non-compliance impact interoperability
or create some other Bad Thing, such as insecure behavior, excessive bandwidth
usage, congestion, etc. Implementation guidance for behavior that is not
externally observable should use non-normative.

-- 6, 2nd and 3rd paragraph:

The RECOMMENDation to allow administrative entry of keys and to derive keys
from a PRNG seem contradictory.

-- 6.1, 1st paragraph:

Does the guidance to use connected sockets remove previous normative
requirements about session management and tracking? If not, please indicate the
difference.

-- 6.1, 3rd paragraph:

This seems to assume that all radius clients are implementd as multiple
processes. Is that the intent?

Also,  it's better not to use normative requirements for internal
implementation choices. Describe the externally visible behavior normatively.
You can give implementation advice in the form of example strategies to fulfill
the black-box normative requirements.

-- 9

Why not choose a new port? Is there absolute certainty this won't conflict with
the previous usage? I do note that 6414 made the same choice, so I guess
consistency with radius/TLS has some value. But that draft talks about
compatibility between radsec and radius/tls, which is not mentioned here.

-- 10, last paragraph:

You describe the use of null crypto as an implementation or configuration
error. Was it intended to be forbidden from intentional use? Is there a need to
remove the fixed secret requirement for null crypto, or to disallow null crypto
entirely?

-- 10.1, 3rd paragraph:

It would be helpful to have guidance on how to match a certificate to a client
or server identity, how to configure trust for a self-signed cert, etc.

-- 10.1, 4th paragraph:

-- 10.1, last paragraph:

Why does the historic use of shared secrets matter, since this document
requires all implementations to use a fixed value? Or do you mean the use of
poor secrets as PSKs?

-- 10.2, 2nd paragraph:

This is redundant with previous normative requirements. (Also contradictory,
since the previous text said "SHOULD", and contradictory guidance on what to do
when the limit is exceeded.)

-- 10.3, 4th paragraph:

Does this need to be as strong as SHOULD? Is this likely to conflict with
dynamic discovery, should it ever exist?

-- 10.3, 5th and 6th paragraphs:

Paragraph 5 says credentials should be statically configured. To me,
"credentials" means "the certificate" in this case. That seems to disallow
things like statically configuring a name that must match a certificate
(perhaps signed by a CA.)

Paragraph 6 seems to entirely contradict paragraph 5 by recommending private
CAs, and accepting any peer that presents a cert signed by that CA. That's
pretty much the opposite of statically configured credentials. (Paragraph 8
also seems to contradict the static configuration part.)

The last sentence refers to "the invalid server". What invalid server is that?
None have been mentioned so far.

-- 10.4:

The guidance in the last paragraph does not make sense. The section seems to
say that NATs will break radius/dtls, so you should use dtls when behind a NAT.

-- 10.6, last paragraph:

Can you elaborate on this? Why would an unauthorized 3rd party be able to get
packets past the DTLS layer? OTOH, If an authorized client is sending invalid
radius packets, wouldn't  you want to terminate the session?

-- 10.7

Redundant normative requirements (This is at least the third time the separate
process issue and local proxy guidance has been described.)

*** Nits/editorial comments:

-- IDNits reports some issues; please check.

-- Abstract:
Please avoid citations in the abstract. Please consider removing the "scare
quotes".

-- Section 1, 5th paragraph:

it seems like the RADIUS problems that this does not solve are in generally
solved by RADIUS/TLS. If so, it would probably be worth a mention.

-- section 2.1, 4th paragraph from end:

Seems like there are other changes as well. For example, you don't include "Use
DTLS" as one of the changes in RADIUS/DTLS from RADIUS/USP.

-- 2.2 and children:

In my strictly personal opinion, the approach of patching 6614 transfers a lot
of effort from the author to the reader. A reader will basically need to keep
both docs open side by side to understand this.  I understand the desire to
avoid duplicating normative text, but I think that the balance here has swung
too far away from readability.  (OTOH, see my related comment under "minor
issues).

-- 2.2.1, paragraph 5:

should TLS parameters be DTLS parameters?

-- 2.2.2:

Why a separate section to say sections 4 and 6 also apply? (The re-iteration
seems unneeded.)

-- 3.1, last sentence:

Really, 2.2.1 doesn't describe that. 6114 describes that. Better to cite the
source than to cite a citation to the source, especially when that citation
doesn't mention the issues at all.

-- 4, 3rd paragraph:

Do you really mean multiple independent _implementations_? Or multiple
independent instances or processes, perhaps running the same implementation?

-- section 5 and children:

In consistent use of "connection" vs "session".

-- 5.1.1, 2nd paragraph:

What is an "entry" session?

-- 6.1, 2nd paragraph:

Source port? Source address? Both?

-- 7:

Only the first paragraph seems to be about implementation experience.

-- 10, 2nd paragraph: "All of the security considerations for RADIUS apply to
the RADIUS portion of the specification."

The next paragraph seems to contradict this.

-- 10.3, 2nd paragraph: "... a client has a fixed IP address for a server ..."

This is ambiguous. Do you mean the server knows the client address, or the
client knows the server address? It sounds like the latter, but later text
makes me wonder if you meant the former.

-- 10.3, 7th paragraph: "... pre-configured with a list of known public CAs."

By pre-configured, you mean by the manufacturer or vendor, right?

-- 10.5, 2nd paragraph:

Does this contradict guidance about using a private CA to identify multiple
peers? I don't think you intend it that way; I assume you mean the cert
presented by a client must be unique, but as written it's easy to assume that
you mean the server has to have unique certs configured for each client, which
it would not if it were to allow any arbitrary client that has a cert signed by
a private CA.