> On Mar 18, 2019, at 2:57 PM, Alan DeKok via Datatracker <noreply@xxxxxxxx> wrote: > > Reviewer: Alan DeKok > Review result: Not Ready > > I think that the document is fairly good, but could use additional > text. Alan, thank you for your comments. We will review the RADIUS documents and try to integrate these comments into the next revision of draft-cel-nfsv4-rpc-tls. One question below: > It would a good idea for the authors to review RFC 6614 (RADIUS over > TLS) and RFC 7360 (RADIUS over DTLS). Those documents both "patch" > RADIUS to allow for TLS / DTLS transport. The RADIUS bits are perhaps > uninteresting, but it is useful to learn from previous approaches to > patching legacy protocols. > > e.g. Section 1 of RFC 7360 says: > > The DTLS protocol does not add reliable > or in-order transport to RADIUS. DTLS also does not support > fragmentation of application-layer messages, or of the DTLS messages > themselves. > > It may be worth using similar text in this document. Also, Section > 2.1 of RFC 7360 clarifies that the standad does not change anything > existing in the legacy protocol, but adding a DTLS layer may affect PMTU: > > We note that the DTLS encapsulation of RADIUS means that RADIUS > packets have an additional overhead due to DTLS. Implementations > MUST support sending and receiving encapsulated RADIUS packets of > 4096 octets in length, with a corresponding increase in the maximum > size of the encapsulated DTLS packets. This larger packet size may > cause the packet to be larger than the Path MTU (PMTU), where a > RADIUS/UDP packet may be smaller. See Section 5.2, below, for more > discussion. > > RFC 7360 also mandates support for particular TLS cipher suites, which is > lacking from this document. I suggest adding text to address this issue. > > There may be other TLS / DTLS issues in the RADIUS documents which apply here. > > For this document: > > 4.3.2 > > ... However, once encryption of the > transport connection is established, the server MUST NOT utilize TLS > identity for the purpose of authorizing RPC requests. > > It may be worth reiterating that the protocols are independent. > i.e. This document does not define the *combination* of TLS and RPC, > so much as RPC carried over TLS. The underlying RPC protocol is > largely unaware of the encapsulating TLS information. It is true that the RPC protocol is unchanged (except for the addition of AUTH_TLS). However, I'm not clear what triggered your comment. Can you expand a bit? > Section 5: > > ... In circumstances where > the users on that NFS client belong to multiple distinct security > domains, it may be necessary to establish separate TLS-protected > connections that do not share the same encryption parameters. > > IMHO that's not a "may be necessary", but a hard requirement. And the > last bit of that sentence should be clearer. The users will each have > their own encryption credentials and profiles, I suspect. > > Section 5.1: > > Therefore, the RECOMMENDED deployment mode is that both servers and > clients have certificate material available > > Perhaps "configured and used" is better than "available". An > certificate which is "available" may, in fact, be unused. > > -- Chuck Lever
