Hi,
comments inline.
This is a review of "TLS Encryption for RADIUS" draft-ietf-radext-radsec.
Overall, this draft was a pleasant to read, and it is clear that a lot
of thought as well as implementation experience has gone into it.
Kudos to the authors.
Thanks :-)
General Issues
There is a considerable amount of text relating to dynamic discovery in
this document, yet
there is only an informational reference to it.
That's true. As I went through your comments below, I realised that
large parts of the texts you quoted should be moved to the
dynamic-discovery draft altogether as they are are very specific to that
draft.
I'm thinking of taking out all the snippets you mentioned below,
transfer them to dynamic-discovery and leaving nothing but a small
"pointer" paragraph in the RADIUS/TLS document.
This actually coincides with what we've experienced in deployment. While
in earlier times I considered TLS and dynamic discovery rather
intertwined, operations people tell me that these are two very distinct
things; most have no problem changing from UDP to TLS, while leaving
their servers behind a firewall and opening it only to the same known
peers as before; merely changing the transport. They do tend to have
more of a problem with opening the port to the world at large and move
away from hard-wired configurations of known peer addresses though.
So it's indeed best to keep both aspects nicely separated.
Since inserting a normative reference to dynamic discovery could delay
the publication of
this document unnecessarily, my recommendation is to consolidate some of
the dynamic
discovery material into a single section in which you can discuss the
implications, while
clearly indicating the status of the dynamic discovery work (e.g. still
under development, optional to
implement along with RADSEC, etc.).
For example, you might consider consolidating the following text from
Sections 3.1 and 6
and placing it prior to the current Section 3.1:
Section 3.X: Implications of Dynamic Peer Discovery
One mechanism to discover RADIUS over TLS peers dynamically via DNS
is specified in [I-D.ietf-radext-dynamic-discovery]. While this
mechanism
is still under development and therefore is not a normative dependency of
RADIUS/TLS, the use of dynamic discovery has potential future
implications that are
important to understand.
I'll add that to the text. Since the text about dynamic discovery would
be externalised to the other draft, how about if I add a sentence:
"Readers of this document who are considering the deployment of
DNS-based dynamic discovery are thus encouraged to read
[I-D.ietf-radext-dynamic-discovery] and follow its future development." ?
Then, those who care can read up on it, while the others just read how
to do TLS instead of UDP.
If dynamic peer discovery as per
[I-D.ietf-radext-dynamic-discovery] is used, peer authentication
alone is not sufficient; the peer must also be authorised to perform
user authentications. In these cases, the trust fabric cannot depend
on peer authentication methods like DNSSEC to identify RADIUS/TLS
nodes. The nodes also need to be properly authorised. Typically,
this can be achieved by adding appropriate authorisation fields into
a X.509 certificate. Such fields include SRV authority [RFC4985],
subjectAltNames, or a defined list of certificate fingerprints.
Operators of a RADIUS/TLS infrastructure should define their own
authorisation trust model and apply this model to the certificates.
The checks enumerated in Section 2.3 provide sufficient flexibility
for the implementation of authorisation trust models.
[BA] I think you need to be more prescriptive here. Are there specific
fields that a RADSEC TLS certificate should contain? Having individual
implementations/deployments defining their own authorization schemes seems
like a bad idea.
That text is best moved out to dynamic-discovery anyway. But to address
your point: this was discussed numerous times in radext wg meetings. The
conclusion in the end was that it is hard to foresee how trust fabrics
are created by people in the wild, so it seemed like a bad idea to be
too prescriptive. E.g. in eduroam we've already been experimenting with
two (
1) subjectAltName:URI to contain a specific value
2) policyOID to distinguish "authorized eduroam" servers from others
Chances are that we might experiment with more, e.g. DNSSEC subtrees
with DANE records.
This goes together with the current discussion of making dynamic
discovery Experimental - then observe how trust models work, and be more
concrete if the document makes it to standards track.
In the case of dynamic peer discovery as per
[I-D.ietf-radext-dynamic-discovery], a RADIUS/TLS node needs to be
able to accept connections from a large, not previously known, group
of hosts, possibly the whole internet. In this case, the server's
RADIUS/TLS port can not be protected from unauthorised connection
attempts with measures on the network layer, i.e. access lists and
firewalls. This opens more attack vectors for Distributed Denial of
Service attacks, just like any other service that is supposed to
serve arbitrary clients (like for example web servers).
In the case of dynamic peer discovery as per
[I-D.ietf-radext-dynamic-discovery], X.509 certificates are the only
proof of authorisation for a connecting RADIUS/TLS nodes. Special
care needs to be taken that certificates get verified properly
according to the chosen trust model (particularly: consulting CRLs,
checking critical extensions, checking subjectAltNames etc.) to
prevent unauthorised connections.
Both of these security considerations apply only to deployers of
dynamic-discovery; so they should be moved to dynamic-discovery.
Other comments
Section 1
One mechanism to discover RADIUS over TLS peers with DNS is specified in
[I-D.ietf-radext-dynamic-discovery].
[BA] Recommend moving this to a section devoted to dynamic discovery.
Done in my working copy, by using your text above.
Section 2.1
See
section Section 3.3 (4) and (5) for considerations regarding
separation of authentication, accounting and dynauth traffic.
[BA] Recommend changing to:
"See Section 3.3 for considerations regarding separation of
authentication, accounting and dynamic authorisation traffic."
Done in my working copy.
Section 2.3
4. start exchanging RADIUS datagrams. Note Section 3.3 (1) ). The
shared secret to compute the (obsolete) MD5 integrity checks and
attribute encryption MUST be "radsec" (see Section 3.3 (2) ).
Section 3.1
(3) If dynamic peer discovery as per
[I-D.ietf-radext-dynamic-discovery] is used, peer authentication
alone is not sufficient; the peer must also be authorised to perform
user authentications. In these cases, the trust fabric cannot depend
on peer authentication methods like DNSSEC to identify RADIUS/TLS
nodes. The nodes also need to be properly authorised. Typically,
this can be achieved by adding appropriate authorisation fields into
a X.509 certificate. Such fields include SRV authority [RFC4985],
subjectAltNames, or a defined list of certificate fingerprints.
Operators of a RADIUS/TLS infrastructure should define their own
authorisation trust model and apply this model to the certificates.
The checks enumerated in Section 2.3 provide sufficient flexibility
for the implementation of authorisation trust models.
As noted above, I'd suggest removing this material from Section 3.1 and
consolidating it with other dynamic-discovery material.
Moved out to dynamic-discovery.
Section 3.3
Note well: it is not required for an implementation
to actually process these packet types. It is sufficient that upon
receiving such a packet, an unconditional NAK is sent back to
indicate that the action is not supported.
[BA] What Error-Cause attribute value should be included within the NAK
to make it
clear that the action is not supported? Error 406 "Unsupported Extension"?
That is what RFC 5176 Section 3.5 seems to indicate.
Indeed. I'll update the text to that end. Note though that adding
Error-Cause is a MAY only in 5176, so it may very well be that an
implementation which doesn't support dynauth would still send only a
"naked" NAK, ignoring the MAY.
I've put the following text into my working copy:
(4) RADIUS/UDP [RFC2865] uses negative ICMP responses to a newly
allocated UDP port to signal that a peer RADIUS server does not
support reception and processing of the packet types in [RFC5176].
These packet types are listed as to be received in RADIUS/TLS
implementations. Note well: it is not required for an implementation
to actually process these packet types. It is sufficient that upon
receiving such a packet, an unconditional NAK is sent back to
indicate that the action is not supported. The NAK MAY contain an
attribute Error-Cause with the value 406 ("Unsupported Extension");
see [RFC5176] for details.
There
is no RADIUS datagram to signal an Accounting NAK. Clients may be
misconfigured to send Accounting packets to a RADIUS/TLS server which
does not wish to process their Accounting packet. The server will
need to silently drop the packet. The client will need to deduce
from the absence of replies that it is misconfigured; no negative
ICMP response will reveal this.
[BA] This seems like a bad idea. How about requiring implementations not
supporting Accounting to respond with an Accounting-Response containing
Error-Cause attribute value 406? Implementations receiving an
Accounting-Response
with this Error-Cause can be required to treat this like an ICMP response.
I'm slightly confused here. To my best knowledge, Error-cause is only
specified in the context of DynAuth (RFC5176). That attribute is listed
as only allowed in a NAK as per the attribute occurence table in 5176.
I would be hesitant to use Error-Cause in RADIUS Accounting packets -
unless the corresponding RFCs get updated to specify that this attribute
is now also to be used in Accounting semantics. And to be honest, I
would even be rather against such a change, and introduce a proper
Accounting-NAK packet type instead; but that's a different story.
The non-ability to indicate "No accounting please" has been discussed in
a radext wg meetint. The conclusion was that auth and acct are two
separate, unrelated items. RADIUS Accounting needs to be configured
differently and explicitly; so there is little risk that accounting
packets are sent "by chance" anyway. If they are sent to the wrong
place, that is an administrative error: misconfigured on the sender-side.
Section 4
As a consequence, the selection of transports to communicate from a
client to a server is a manual administrative action. An automatic
fallback to RADIUS/UDP is NOT RECOMMENDED, as it may lead to down-
bidding attacks on the peer communication.
[BA] If a fixed shared secret "radsec" is used alongside fallback to
RADIUS/UDP,
that seems more like a MUST NOT!!
That paragraph was also brought up in the AD review. It was not meant in
the way you perceived it; please see the thread of the AD review of this
document for an extensive explanation how it was really meant.
In any case, I take the point that the text is confusing for readers.
While resolving the AD comments, I agreed with Dan Romascanu to
reformulate this whole paragraph and move it to Security Considerations
instead. I'll follow up with the new text later today.
Section 6
In the case of dynamic peer discovery as per
[I-D.ietf-radext-dynamic-discovery], a RADIUS/TLS node needs to be
able to accept connections from a large, not previously known, group
of hosts, possibly the whole internet. In this case, the server's
RADIUS/TLS port can not be protected from unauthorised connection
attempts with measures on the network layer, i.e. access lists and
firewalls. This opens more attack vectors for Distributed Denial of
Service attacks, just like any other service that is supposed to
serve arbitrary clients (like for example web servers).
In the case of dynamic peer discovery as per
[I-D.ietf-radext-dynamic-discovery], X.509 certificates are the only
proof of authorisation for a connecting RADIUS/TLS nodes. Special
care needs to be taken that certificates get verified properly
according to the chosen trust model (particularly: consulting CRLs,
checking critical extensions, checking subjectAltNames etc.) to
prevent unauthorised connections.
[BA] I'd recommend collecting this and other dynamic-discovery related
material
into a separate section prior to 3.1.
Moved out of the document, to go into dynamic-discovery.
Appendix C. Assessment of Crypto-Agility Requirements
The RADIUS Crypto-Agility Requirements (link to RFC once issued here)
defines numerous classification criteria for protocols that strive to
enhance the security of RADIUS. It contains mandatory (M) and
recommended (R) criteria which crypto-agile protocols have to
fulfill. The authors believe that the following assessment about the
crypto-agility properties of RADIUS/TLS are true.
[BA] The Crypto-Agility RFC is now published so you should reference that.
Done now in my working copy.
Thanks for this extensive review, much appreciated!
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
signature.asc
Description: OpenPGP digital signature
_______________________________________________
Ietf mailing list
Ietf(_at_)ietf(_dot_)org
https://www.ietf.org/mailman/listinfo/ietf