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. 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. 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. 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. 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. 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. 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." 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. 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. 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. 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!! 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. 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. |
_______________________________________________ Ietf mailing list Ietf@xxxxxxxx https://www.ietf.org/mailman/listinfo/ietf