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
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Ietf mailing list Ietf@xxxxxxxx https://www.ietf.org/mailman/listinfo/ietf