Re: [radext] RADIUS/1.1 ALPN minor suggestions

Alan DeKok <aland@deployingradius.com> Sun, 16 April 2023 15:35 UTC

Return-Path: <aland@deployingradius.com>
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 1C4F6C14CE2C for <radext@ietfa.amsl.com>; Sun, 16 Apr 2023 08:35:59 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.896
X-Spam-Level:
X-Spam-Status: No, score=-1.896 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, 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
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 f15b74O_seVu for <radext@ietfa.amsl.com>; Sun, 16 Apr 2023 08:35:54 -0700 (PDT)
Received: from mail.networkradius.com (mail.networkradius.com [62.210.147.122]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 711A5C14CF09 for <radext@ietf.org>; Sun, 16 Apr 2023 08:35:54 -0700 (PDT)
Received: from smtpclient.apple (135-23-95-173.cpe.pppoe.ca [135.23.95.173]) by mail.networkradius.com (Postfix) with ESMTPSA id 8A37E391; Sun, 16 Apr 2023 15:35:51 +0000 (UTC)
Authentication-Results: NetworkRADIUS; dmarc=none (p=none dis=none) header.from=deployingradius.com
Content-Type: text/plain; charset="us-ascii"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.1\))
From: Alan DeKok <aland@deployingradius.com>
In-Reply-To: <3c6cb8bf-8bfe-6008-868a-4022c596c3ad@newtoncomputing.co.uk>
Date: Sun, 16 Apr 2023 11:35:49 -0400
Cc: radext@ietf.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <E6C65074-CA68-47A8-92F4-34DD35FD5CD1@deployingradius.com>
References: <3c6cb8bf-8bfe-6008-868a-4022c596c3ad@newtoncomputing.co.uk>
To: Matthew Newton <matthew-ietf@newtoncomputing.co.uk>
X-Mailer: Apple Mail (2.3696.120.41.1.1)
Archived-At: <https://mailarchive.ietf.org/arch/msg/radext/q4ajbrZfdg4ShM-Shnl1G6SPSJY>
Subject: Re: [radext] RADIUS/1.1 ALPN minor suggestions
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: Sun, 16 Apr 2023 15:35:59 -0000

  Thanks.  I've pulled most of those over.

  Some of the comma changes are stylistic, so I've mainly added the ones which correct or clarify he meaning of the sentence.

> On Apr 15, 2023, at 12:19 PM, Matthew Newton <matthew-ietf@newtoncomputing.co.uk> wrote:
> 
> A first pass at trivial and (hopefully) non-contentious suggestions and fixes for draft-dekok-radext-radiusv11-04.txt. Mostly typos or minor English issues (some may be just personal preference so take as you like.) Maybe too early in the game for fine detail fixes but I've noticed them so may as well feed back.
> 
> I've got other more detailed comments which I'm working through and will send separately.
> 
> 
> Section 1 para 3
> 
> has/have
> 
> -   The ensuing years has shown that it is important for RADIUS to remove
> +   The ensuing years have shown that it is important for RADIUS to remove
> 
> 
> List of changes (para 5) - inconsistencies. Suggest either 1) each
> bullet point is a sentence with capital letter and full stop, or
> 2) they are one continuous sentence, each starting with lowercase
> and ending in a semicolon, except the last. The former probably
> works better here.
> 
> e.g.
> 
> comma to full stop
> 
> -   *  ALPN is used for negotiation of this extension,
> +   *  ALPN is used for negotiation of this extension.
> 
> above plus capital
> 
> -   *  all uses of the RADIUS shared secret have been removed,
> +   *  All uses of the RADIUS shared secret have been removed.
> 
> Suggest removing comma before and in most cases, e.g.
> 
> -   *  The Identifier field is no longer used, and has been replaced by
> -      the Token field,
> +   *  The Identifier field is no longer used as it has been replaced by
> +      the Token field.
> 
> and
> 
>    *  The Message-Authenticator attribute ([RFC3579] Section 3.2) is not
> -      sent in any packet, and if received is ignored,
> +      sent in any packet. If received it is ignored.
> 
> 
> Change to RFC crazy "string" terminology.
> 
> -      "octets" ([RFC8044] Section 3.5), without the previous MD5-based
> +      "string" ([RFC8044] Section 3.5), without the previous MD5-based
> 
> 
> No comma needed.
> 
> -      obfuscation.  This obfuscation is no longer necessary, as the data
> +      obfuscation.  This obfuscation is no longer necessary as the data
>       is secured and kept private through the use of TLS.
> 
> 
> Similar comments in the second list - sentences probably make more
> sense than ending in commas (or semicolons), especially as one
> point is multiple sentences anyway.
> 
> e.g.
> 
> -   *  the RADIUS packet header is the same size, and the Code and Length
> +   *  The RADIUS packet header is the same size, and the Code and Length
> 
> -      as before,
> +      as before.
> 
> 
> Connection"s"
> 
> -      server connection), it does not impact any other connection used
> +      server connection), it does not impact any other connections used
> 
> 
> First paragraph after second list
> 
> Fix comma / full stop
> 
>    A major benefit of this extensions is that a home server which
>    implements it can also choose to also implement full FIPS-140
> -   compliance, That is, a home server can remove all uses of MD4 and
> +   compliance. That is, a home server can remove all uses of MD4 and
> 
> 
> Comma-separated list, rather than A or B or C.
> 
> -   MD5.  In that case, however, the home server will not support CHAP or
> -   MS-CHAP, or any authentication method which uses MD4 or MD5.  We note
> +   MD5.  In that case, however, the home server will not support CHAP,
> +   MS-CHAP, or any other authentication method which uses MD4 or MD5. We note
> 
> 
> Two paragraphs further down, suggest slight rewrite to avoid doing
> "this specification. This specification"
> 
>    We reiterate that the decision to support (or not) any authentication
>    method is entirely site local, and is not a requirement of this
> -   specification.  This specification does not not modify the content or
> -   meaning of any RADIUS attribute other than Message-Authenticator (and
> -   similar attributes), and the only change to the Message-Authenticator
> +   specification. The contents or meaning of any RADIUS attribute, other
> +   than Message-Authenticator (and similar attributes, see below), are not
> +   modified. The only change to the Message-Authenticator
>    attribute is that is no longer used.
> 
> 
> Section 2
> 
> Consistency - full stops on some not others:
> 
>    *  ALPN
> 
> -      Application-Layer Protocol Negotiation, as defined in [RFC7301]
> +      Application-Layer Protocol Negotiation, as defined in [RFC7301].
> 
>    *  RADIUS/TCP
> 
> -      RADIUS over the Transmission Control Protocol [RFC6613]
> +      RADIUS over the Transmission Control Protocol [RFC6613].
> 
>    *  RADIUS/TLS
> 
> -      RADIUS over the Transport Layer Security protocol [RFC6614]
> +      RADIUS over the Transport Layer Security protocol [RFC6614].
> 
>    *  RADIUS/DTLS
> 
>       RADIUS over the Datagram Transport Layer Security protocol
> -         [RFC7360]
> +         [RFC7360].
> 
>    *  TLS
> 
> -      the Transport Layer Security protocol.  Generally when we refer
> +      The Transport Layer Security protocol.  Generally, when we refer
>          to TLS in this document, we are referring interchangeably to
>          TLS or DTLS transport.
> 
> Section 4
> 
>       "radius/1.1"
> 
> -         The protocol defined by specification.
> +         The protocol defined by this specification.
> 
> Section 4.2
> 
> 
> -      The value "RADIUS-1.1" flag is used by the client to determine
> +      The value of the "RADIUS-1.1" configuration flag is used by the client to determine
>       what to send:
> 
> -         forbid - no ALPN
> +         forbid - ALPN is not used
> 
> 
> Section 4.4
> 
> Use [link] as elsewhere
> 
> -      When session resumption or session tickets (RFC5077) are used, the
> +      When session resumption or session tickets [RFC5077] are used, the
> 
> 
> Section 5.1
> 
> Comma list not A and B and C
> 
> -   different meaning.  The Identifier and the Request Authenticator and
> +   different meaning.  The Identifier, Request Authenticator and
>    Response Authenticator fields are no longer used.  Any operations
> 
> 
> In the list of fields:
> 
> Not "definition in RADIUS" - maybe "unchanged from the previous
> RADIUS specification", or just "of". Or (see other e-mail) "is unchanged from standard RADIUS" if "standard RADIUS" is defined in the terminology section.
> 
>       The meaning of the Code field is unchanged from the previous
> -      definition in RADIUS.
> +      definition of RADIUS.
> 
>       The meaning of the Length field is unchanged from the previous
> -      definition in RADIUS.
> +      definition of RADIUS.
> 
> 
> Section 5.2.1 para 4
> 
> Comma
> 
>    from a 32-bit counter which is unique to each connection.  Such a
>    counter SHOULD be initialized to a random value, taken from a random
> -   number generator whenever a new connection is opened.  The counter
> +   number generator, whenever a new connection is opened.  The counter
> 
> 
> Section 5.2.2 para 2
> 
> Move comma
> 
>    Accounting-Request.  This overlap requires that RADIUS clients and
> -   servers, track the Identifier field not only on a per-connection
> +   servers track the Identifier field, not only on a per-connection
>    basis, but also on a per-packet type basis.  This behavior adds
>    complexity to implementations.
> 
> 
> Section 6.1 para 1
> 
> Missing full stop
> 
> -   observer
> +   observer.
> 
> 
> Para 2
> 
> If they are obfuscated then they have already been obfuscated so
> they can't not be obfuscated.
> 
> -   Attributes which are obfuscated with MD5 no longer have the
> +   Attributes defined as being obfuscated with MD5 no longer have the
>    obfuscation step applied when RADIUSv11 is used.  Instead, those
> 
> 
> Para 3 - Make it a bit more concrete
> 
> -   We understand that there is often concern in RADIUS that passwords
> +   There are often concerns where RADIUS is used that passwords
>    are sent "in cleartext" across the network.  This allegation was
> 
> 
> Para 5
> 
> Remove extraneous space
> 
> -   These others ways to mitigate these risks .  One is by ensuring that
> +   These others ways to mitigate these risks.  One is by ensuring that
> 
> 
> Section 6.1.1
> 
> Para 1 - Incorrect ref
> 
>    The User-Password attribute ([RFC2865] Section 5.2) MUST be encoded
>    the same as any other attribute of data type 'string' ([RFC8044]
> -   Section 3.4).
> +   Section 3.5).
> 
> 
> Para 3 - Incorrect ref
> 
>    The contents of the User-Password attribute do not have to printable
>    text, or UTF-8 data as per the definition of the 'text' data type in
> -   [RFC8044] Section 3.5.
> +   [RFC8044] Section 3.4.
> 
> 
> Section 6.1.2
> 
> Possible modification just to make the text flow better?
> 
>    use a Request Authenticator field, proxies may receive an Access-
>    Request containing a CHAP-Password attribute ([RFC2865] Section 5.2)
>    but without a CHAP-Challenge attribute ([RFC2865] Section 5.40).
> -   Proxies which forward that CHAP-Password attribute over a RADIUSv11
> +   In this case, proxies which forward that CHAP-Password attribute over a RADIUSv11
>    connection MUST create a CHAP-Challenge attribute in the proxied
>    packet using the contents from the Request Authenticator.
> 
> 
> Section 6.1.3 para 1
> 
> Comma to full stop
> 
> -   field or Data-Length fields as described in [RFC2868] Section 3,5,
> +   field or Data-Length fields as described in [RFC2868] Section 3.5,
> 
> 
> Para 2 - Incorrect ref
> 
>    data type here, even though the attribute is no longer obfuscated.
>    The contents of the Tunnel-Password attribute do not have to
>    printable text, or UTF-8 data as per the definition of the 'text'
> -   data type in [RFC8044] Section 3.5.
> +   data type in [RFC8044] Section 3.4.
> 
> 
> Section 6.2 para 2
> 
> Remove comma for consistency with the rest of the document
> 
>    If the Message-Authenticator attribute is received over a RADIUSv11
>    connection, the attribute MUST be silently discarded, or treated an
> -   as "invalid attribute", as defined in [RFC6929], Section 2.8.  That
> +   as "invalid attribute", as defined in [RFC6929] Section 2.8.  That
> 
> 
> Section 7.1 para 2
> 
> Fix typos and full stop
> 
>    The rational for reserving one value of the Identifier field was the
>    limited number of Identifiers available (256), and the overlap in
>    Identifiers between Access-Request packets and Status-Server
> -   packers..  If all 256 Identifier values had been used to send Access-
> +   packets.  If all 256 Identifier values had been used to send Access-
>    Request packets, there would be no Identifier value available for
> -   sending a Status-Sercer Packet.
> +   sending a Status-Server Packet.
> 
> 
> Para 3
> 
> Avoid using 'which' twice in one sentence
> 
>    special meaning.  The edge condition is that there are 2^32
>    outstanding packets on one connection with no new Token value
> -   available for Status-Server.  In which case there are other issues
> -   which are more serious, such allowing billions of packets to be
> +   available for Status-Server.  In this case there are other more serious
> +   issues, such allowing billions of packets to be
>    oustanding.  The safest way forward is likely to just close the
>    connection.
> 
> 
> Section 7.3 para 4
> 
> Typo
> 
> -   All crypto-agility needed or use by this specification is implemented
> +   All crypto-agility needed or used by this specification is implemented
> 
> 
> Section 7.4 para 2
> 
> Fix brackets by adding comma
> Change "octets" to "string"
> 
>    how those new attributes are transported in RADIUSv11.  Since
>    RADIUSv11 does not use MD5, any obfuscated attributes will by
> -   definition be transported as their underlying data type ("text"
> +   definition be transported as their underlying data type, "text"
> -   ([RFC8044] Section 3.4) or "octets" ([RFC8044] Section 3.5).
> +   ([RFC8044] Section 3.4) or "string" ([RFC8044] Section 3.5).
> 
> 
> Section 11 - typo
> 
> -   IANA is request to update the "TLS Application-Layer Protocol
> +   IANA is requested to update the "TLS Application-Layer Protocol
>    Negotiation (ALPN) Protocol IDs" registry with one new entry:
> 
> 
> -- 
> Matthew
> 
> _______________________________________________
> radext mailing list
> radext@ietf.org
> https://www.ietf.org/mailman/listinfo/radext