[radext] draft-ietf-radext-radiusv11-00 minor fixes

Matthew Newton <matthew-ietf@newtoncomputing.co.uk> Tue, 23 May 2023 17:16 UTC

Return-Path: <matthew-ietf@newtoncomputing.co.uk>
X-Original-To: radext@ietfa.amsl.com
Delivered-To: radext@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 88B0CC151B0C for <radext@ietfa.amsl.com>; Tue, 23 May 2023 10:16:58 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.095
X-Spam-Level:
X-Spam-Status: No, score=-2.095 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_BLOCKED=0.001, RCVD_IN_ZEN_BLOCKED_OPENDNS=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001, URIBL_DBL_BLOCKED_OPENDNS=0.001, URIBL_ZEN_BLOCKED_OPENDNS=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=newtoncomputing.co.uk
Received: from mail.ietf.org ([50.223.129.194]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Ky43phj4gdxR for <radext@ietfa.amsl.com>; Tue, 23 May 2023 10:16:53 -0700 (PDT)
Received: from mail.newtoncomputing.co.uk (mail.newtoncomputing.co.uk [IPv6:2001:8b0:b2:1::140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id C5B82C151B0A for <radext@ietf.org>; Tue, 23 May 2023 10:16:53 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=newtoncomputing.co.uk; s=feb2010; h=Content-Transfer-Encoding:Content-Type: Subject:To:From:MIME-Version:Date:Message-ID:Sender:Reply-To:Cc:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:In-Reply-To:References:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=K7UfwioMmezgZa6YBVeZgZTvjyb1Te+8tQlotbr0ATo=; b=if1Py9K5v6Nr9VM2kzDFRsWnWQ 4Dji+gamcJKluXT50VhjsfY6q+vhXFeUbcHo1J6GLpy9UD2/iK3uSyR+3+1XFhUjsrpDHP/DxECJX hT6QQsQRxpzu9H9J8Uu1ogZAO7cBjKWAWW2Qxwj6rSUmr0pWTJCE89XSxElQMBXth9Ag=;
Received: from [2001:8b0:b2:1:e884:d1b7:7807:a9b8] by mail.newtoncomputing.co.uk with esmtpsa (Exim 4.92 #3 (Debian)) id 1q1Vct-0008W0-QE for <radext@ietf.org>; Tue, 23 May 2023 18:16:48 +0100
Message-ID: <eb2cb3c9-10f3-49f9-4b4f-65a283811a90@newtoncomputing.co.uk>
Date: Tue, 23 May 2023 18:16:45 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0
Content-Language: en-GB
From: Matthew Newton <matthew-ietf@newtoncomputing.co.uk>
To: radext@ietf.org
Content-Type: text/plain; charset="UTF-8"; format="flowed"
Content-Transfer-Encoding: 7bit
X-NC-Fw-Sig: d28eb7783fcaa8a96965cdcb17b1e1fe matthew-ietf@newtoncomputing.co.uk
Archived-At: <https://mailarchive.ietf.org/arch/msg/radext/Gv_z9gALm4ML3orJTAGALq_kPpM>
Subject: [radext] draft-ietf-radext-radiusv11-00 minor fixes
X-BeenThere: radext@ietf.org
X-Mailman-Version: 2.1.39
Precedence: list
List-Id: RADIUS EXTensions working group discussion list <radext.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/radext>, <mailto:radext-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/radext/>
List-Post: <mailto:radext@ietf.org>
List-Help: <mailto:radext-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/radext>, <mailto:radext-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 23 May 2023 17:16:58 -0000

I've gone through the latest draft. Page numbers below are from 
draft-ietf-radext-radiusv11-00.pdf.

Mostly minor corrections and word smithing, the document's looking good 
to me.

I've checked all references (and pointed to sections) up to section 3, 
any errors noted below; will report back again if I find any in the 
latter half.



-

Page 3 Introduction

para8: "all uses of the RADIUS shared secret have
been removed" should be "All" for consistency


-

Page 4 Introduction

para6: "The following items are left unchanged..."
"the RADIUS packet header is the same size" lower-case "t" -> "T"


-

para8: "As this extension is a transport profile for one "hop"".... "The
only systems which are aware that this transport profile is in use
are the client and server which have"

   Suggest changing that second "which" to "that" to avoid same
word twice.


-

para10: "A major benefit of this extensions is that a home server which
implements it can also choose to also implement full FIPS-140
compliance."

   Suggest instead: (fix "extension(s)", don't duplicate
"implement")

"A major benefit of this extension is that a home server
implementing it can also choose to be fully FIPS-140 compliant."


-

para11: "As such, it is possible" change to "It is therefore
possible" (so not two sentences in same para starting with "As")


-

Page 5 Terminology

Last line lists RFC2865 twice, I assume that should be RFC2865 and
RFC2866 (for Accounting).


-

Page 6 Terminology


"RADIUS/TLS" has an indent in the PDF, but not in any other
definition.


-

"TLS" has "the Transport Layer Security protocol" - lowercase "the" -> "The"


-

Last sentence "Where ALPN is not configured or is not received in a TLS 
connection, systems supporting ALPN MUST not use RADIUS/1.1." -> "MUST NOT"


-

Page 7 Operation of ALPN

para7 (point 2): "If the server finds no acceptable common protocols, it
closes the connection" -> "If the server finds no acceptable
common protocols (ALPN or otherwise), it closes the connection"
(clarify that acceptable protocols may not be ALPN, rather than
ALPN negotiated protocols only)


-

para9 (point 3): "If the client does not signal ALPN, or server
does not accept the ALPN proposal, the server does not reply with
any ALPN name."

   "client did", not "does"
   missing "the" before "server"


-

para10 (point 4) missing full stop.


-

para13 - last sentence "This flag controls how the implementations
signal use of this protocol via ALPN."

   "implementations" -> "implementation"
   "signal" -> "signals"


-

Page 8 config of ALPN

para7 starting "A server with this configuration"...

Suggest replace with "Instead, it simply replies without ALPN"
otherwise it looks like it may not reply at all.


-

Page 9

para1: "which signals ^^ the other end" missing "to"

-

para3: "its'" -> "its"

s/reponses/responses/

-

para4: "its'" -> "its"


-

Page 10

"Close": missing full stop on both Note 1 and Note 2

"OK" second sentence s/repies/replies/


-

"Implementations of this specification MUST support TLS-PSK." ->
suggest removing this (on the basis it's not really relevant to
this spec), or replace (as it's a good idea) with "Implementations
of this specification SHOULD support TLS-PSK."


-

para starting "In order to prevent down-bidding attacks"...

"That is, even if the server configuration is "allow" for new
connections, it MUST signal "radius/1.1" when resuming a session
which had previously negotiated "radius/1.1"."

suggested replacement text to be clear that this upgrades the
config to "required" in this case:

-> That is, even if the server configuration is "allow" for new
connections, it MUST signal "radius/1.1" and treat the server
configuration as "require" when resuming a session
which had previously negotiated "radius/1.1".


-

para "If a server sees"....

"The server SHOULD send an appropate TLS error, such as
no_application_protocol (120), or insufficient_security (71)."

This is vague, how about:

"The server SHOULD (MUST??) send an appropriate TLS error,
no_application_protocol (120) if RADIUS/1.1 is disabled in the
configuration, or insufficient_security (71) if RADIUS/1.1 was
previously negotiated for this session."


-

Page 11 para 7 "Reserved-1"
"It is now unused, as the Token field is now used to identify
requests and responses."... add "It MUST be ignored when receiving
a packet."


-

Page 12 4.2.1 sending packets

"A client which sends packets uses the Token field to increase the
number of RADIUS packets which can be sent over one connection."

Increase over what? Maybe:

"A client which sends packets uses the Token field to increase the
number of RADIUS packets which can be sent over one connection,
compared to traditional RADIUS."


-

"Systems generating the Token can do so via any method they
choose, but for simplicity," move comma: "they choose but, for
simplicity,"


-

"The counter can then be incremented for every new packet which is
sent." remove "which is" (try to cut down the number of times
"which" is used :) )


-

Page 13 para 2 "which send packets to a server, each subsystem MAY
open a new port which is" -> "that is" (less whiches)


-

Page 14 para 3
"While passwords are encoded in packets as strings, the packets
(and thus passwords) are protected by TLS."

maybe clarify it's RADIUS/1.1 that are protected?

"While passwords are encoded in packets as strings, the RADIUS/1.1
packets (and thus passwords) are protected by TLS."


-

para 5

"These others ways" -> "There are other ways"


-

Page 15 para 1

"The contents of the User-Password attribute do not have to
printable" -> "be printable"


-

para 3 5.1.2 CHAP-Challenge

last line clarify incoming?: "using the contents from the Request
Authenticator" -> "using the contents from the incoming Request
Authenticator"


-

5.1.3 Tunnel-Password

"MUST be encoded the same as any other attribute of data type
'text' which contains a tag" this should be type 'string'


-

second para:

"The contents of the Tunnel-Password attribute do not have to
printable text, or UTF-8 data as per the"

-> add "be" and ","

"The contents of the Tunnel-Password attribute do not have to
be printable text, or UTF-8 data, as per the"


-

Page 16 para 1

"We note that as the RADIUS" -> add "," "We note that, as the RADIUS"


-

5.3 Message-Authentication-Code

"the same was as" -> "the same way as"


-

5.5 second para

"[RFC6929], Section 2.8" remove "," to be consistent with other
refs.


-

Page 17 6.1 Status-Server

second para: s/packers/packets/
s/Packet/packet/


-

Page 18 6.3 Crypto-Agility

para 1 add comma "The use of ALPN, and the removal of MD5_,_ has"


-

para 2 should "eduroam" and "OpenRoaming" have references (e.g
https://eduroam.org/)?


-

para 3 "It is RECOMMENDED that all implementations of RADIUS over TLS be
updated to support this specification. The effort to implement this
specification is minimal. Once implementations support this
specification, administrators can gain the benefit of it with little
or no configuration changes."

suggest small reword to not duplicate "this specification" as much

"It is RECOMMENDED that all implementations of RADIUS over TLS be
updated to support this specification. The effort to implement
this specification is minimal and, once implementations support it,
administrators can gain the benefit of it with little or no
configuration changes."


-

s/downbidding/down-bidding/ (consistency with other use)


-

para 4

"As discussed in the next section below, this specification" ->
"As discussed in the following section, this specification"


-

para 6

"type, ("text" ([RFC8044] Section 3.4)" remove first "("


-

para 6

I think two paragraphs have been joined together here by mistake?

"Section 3.5). a New RADIUS" -> "Section 3.5).<p>New RADIUS"


-

para 6

lowercase for consistency with other uses
"Invalid Attributes" -> "invalid attributes"


-

Page 19 para 2

[RFC2865] at al -> et al


-

10 IANA considerations

"Reference: This document" PDF has double space after ":", maybe
just PDF formatting though.


-

11 Acknowledgements

spelling

Richardons -> Richardson
Netwon -> Newton





-- 
Matthew