[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
- [radext] draft-ietf-radext-radiusv11-00 minor fix… Matthew Newton
- Re: [radext] draft-ietf-radext-radiusv11-00 minor… Alan DeKok
- Re: [radext] draft-ietf-radext-radiusv11-00 minor… Matthew Newton
- Re: [radext] draft-ietf-radext-radiusv11-00 minor… Alan DeKok
- Re: [radext] draft-ietf-radext-radiusv11-00 minor… Matthew Newton