[radext] Nits and discussion items for draft-cullen-radextra-status-realm-01

Jan-Frederik Rieckers <rieckers@dfn.de> Thu, 23 November 2023 20:14 UTC

Return-Path: <rieckers@dfn.de>
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 B3B68C18771E for <radext@ietfa.amsl.com>; Thu, 23 Nov 2023 12:14:40 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.107
X-Spam-Level:
X-Spam-Status: No, score=-2.107 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_ZEN_BLOCKED_OPENDNS=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01, 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=dfn.de
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 fT0I32qcTLnQ for <radext@ietfa.amsl.com>; Thu, 23 Nov 2023 12:14:36 -0800 (PST)
Received: from a1004.mx.srv.dfn.de (a1004.mx.srv.dfn.de [194.95.233.6]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 0632BC17DBF1 for <radext@ietf.org>; Thu, 23 Nov 2023 12:14:34 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=dfn.de; h= content-type:content-type:organization:content-language:subject :subject:from:from:user-agent:mime-version:date:date:message-id :received; s=s1; t=1700770470; x=1702584871; bh=yvAFdb7c5LzG8fzv VFy5kM2+8ckqfs3y/uX3TH/4IAk=; b=sVzl/a6ldX5VqTwTs2o7KKIunQK4EC9g f3fPIOMSVZ3+LY7wWHUMiF0xXKytkVjmnVVJX0nCEZ4mHjdYJQDQ15s6wqa4yiLU dnGrbKkV8AGnyGKuCIjACLFyasxm2fzAggmOu72Ra9PYIHyXXk42aq1GLPyUM03j BTpmm9aAu5M=
Received: from mail.dfn.de (mail.dfn.de [194.95.245.150]) by a1004.mx.srv.dfn.de (Postfix) with ESMTPS id 565DC2000E4 for <radext@ietf.org>; Thu, 23 Nov 2023 21:14:30 +0100 (CET)
Received: from [IPV6:2001:638:d:2016::1000] (unknown [IPv6:2001:638:d:2016::1000]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mspool2.in.dfn.de (Postfix) with ESMTPSA id CA8D341C for <radext@ietf.org>; Thu, 23 Nov 2023 21:14:29 +0100 (CET)
Message-ID: <88b85255-8bd9-49c4-8613-77601b3374fc@dfn.de>
Date: Thu, 23 Nov 2023 21:14:28 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
From: Jan-Frederik Rieckers <rieckers@dfn.de>
To: radext@ietf.org
Content-Language: en-US
Organization: DFN e.V.
Content-Type: multipart/signed; protocol="application/pkcs7-signature"; micalg="sha-512"; boundary="------------ms050909010704050506010105"
Archived-At: <https://mailarchive.ietf.org/arch/msg/radext/LhNBRBsT-FqSDXo2nm0ZX6f0kNU>
Subject: [radext] Nits and discussion items for draft-cullen-radextra-status-realm-01
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: Thu, 23 Nov 2023 20:14:40 -0000

Hi,

here the nits I found while reading the draft.
The nits are mostly typos/editorial.
However, I have two bigger discussion items:


I would love for the Security Considerations section to address the 
security and privacy concerns of being able to detect the proxy path 
with server identifiers, routes, etc.
This could be something like "if you don't want your server names out 
there, use non-resolvable alias names for them" or something like this.
I can imagine that some administrators would not use this, because "how 
my local RADIUS server works is my trade secret", so adding a few 
paragraphs about the actual security and privacy concerns about this 
would help convince these administrators to activate it.



Secondly, I think it would be useful if the Status-Realm-Request could 
have some other method of identifying the Requestor than just 
NAS-Identifier or NAS-IP(v6)-Address. Could we allow (and encourage) the 
"Operator-Name" Attribute in that too?

This way, proxy servers could block/rate-limit Status-Realm-Requests 
that originate from one specific operator, because their monitoring 
system has gone bananas (and i.e. tries to build markov-chains to build 
a list of all possible realms and if found bombards them with requests), 
but allow all other Status-Realm-Request, that don't originate from there.
I don't think that NAS-Identifier or NAS-IP(v6)-Address can be used for 
that, esp. if NAS-IP(v6)-Address is a RFC1918 (or v6 equivalent) address 
or the NAS-Identifier has a very <sarcasm>unique</sarcasm> value like 
"wlan-controller".

Additionally, this allows the RADIUS proxy to also account for policy 
configuration based on the Operator Name when answering the requests.
I can imagine that, for example in the OpenRoaming world, some IdPs 
would deny access from specific SPs/ANPs that are found out to not 
comply with the federation policy, or there may be a case where a RADIUS 
Server only allows access to the IdP from two specific locations (i.e. 
guest access issued at one university is valid at both universities and 
three other research facilities in one town, but not outside of the town 
and that distinction is made based on the OperatorName attribute)

Currently, the RADIUS Server would signal reachability, but all 
Access-Request packets would get denied (or in the worst case even 
discarded).



Now the nits and some questions on requirement language:
(please check them again before applying, I'm not a native speaker)


**Section 2 (Reqiurements Notation):**

Is there a reason this is only the bcp14 and not bcp14+ boilerplate?
(https://datatracker.ietf.org/doc/html/rfc8174#page-3)
(keywords valid if and only if in all-caps)


**Section 3 (Terminology):**

Authentication Server:

Typo
- sends Access-Access, Access-Challenge, Access-Reject
+ sends Access-Accept, Access-Challenge, Access-Reject

Inconsistent use of RFC2119 keyword capitalization:
For Authentication server it says "... may serve more then ..."
For Authentication realm it says "... MAY be served by ..."


RADIUS Proxy: Mix-up Server/Client
- sending it to the RADIUS Server from which the corresponding Request
+ sending it to the RADIUS Client from which the corresponding Request

RADIUS Proxy Fabric:
It is described as group of RADIUS servers, but IMHO the clients are 
also part of this fabric, so in order to align with the previous 
terminology, this should be RADIUS Instances.

Additionally, this is the only term that does not have a full sentence 
but only a partial one (starting with "A multi-hop group [...]" instead 
of "A RADIUS Proxy Fabric is a multi-hop group [...]", like the other 
explanations.)

RADIUS Proxy Path:
-The RADIUS Server Path is a set
+The RADIUS Proxy Path is a set

Proxy Loop:
- a RADIUS Request follow a circular
+ a RADIUS Request follows a circular
or
+ RADIUS Requests follow a circular

First-Hop Server:
- In other case, the First-Hop
+ In other cases, the First-Hop

Last-Hop Proxy:
- may or may not know that is the Last-Hop Proxy
+ may or may not know that it is the Last-Hop Proxy


- for a single RADIUS instance to server in multiple roles
+ for a single RADIUS instance to serve in multiple roles


Over the whole document, it is not consistent when words like 
"Instance", "Client", "Server", "Proxy" or "Proxy Fabric" are 
capitalized after "RADIUS".
I would suggest to adjust that so that these terms are consistent 
throughout the document.


**Section 4 (Overview):**

- mitigating, detecting and preventing loops in a RADIUS Proxy 
forwarding loops
+ detecting, mitigating and preventing loops in a RADIUS Proxy Fabric

First we should be able to detect before we mitigate. And I think you 
mean RADIUS Proxy Fabric here and not RADIUS Proxy forwarding loops.


**Section 4.1 (Status-Realm Overview):**

RFC2119 keywords in the 2nd paragraph are not capitalized, I think that 
at least the "should"s should be "SHOULD"s.

- determine the reachability and status of a Authentication Realm
+ determine the reachability and status of an Authentication Realm


**Section 5.1 (Status-Realm-Request Packet):**

- A Status-Realm-Request packets MUST include a User-Name Attribute
+ A Status-Realm-Request packet MUST include a User-Name Attribute


- MUST also include a Max-Hop-Count attribute, as defined above
+ MUST also include a Max-Hop-Count attribute, as defined in section ...
(or "as defined below". Or should this be a reference to the 
loop-prevention sections?)


**Section 5.2 (Status-Realm-Response Packet):**

I don't fully understand the last paragraph. What exactly should not 
happen? Is this meant to forbid retransmitted answers in case of a 
retransmitted request? Maybe add a sentence explaining exactly what the 
problem is that this requirement solves.

**Section 7 (Status-Realm-Response-Code Attribute):**

- sub-attribute MUST be included in in every Status-Realm-Response Code
- sub-attribute MUST be included in every Status-Realm-Response Code

**Section 9.1 (RADIUS Client Requirements):**

- Client sends a Status-Realm-Request packets from a source port also
+ Client sends a Status-Realm-Request packet from a source port also


**Section 9.2 (Server Requirements):**

Inconsistent Naming of sections (9.1 "RADIUS Client Requirements" vs 9.2 
"Server Requirements" and 9.3 "Proxy Server Requirements")
I'd suggest renaming 9.3 to "RADIUS Proxy Requirements" and 9.2 to 
"RADIUS Server Requirements"

The first paragraph has the sentence "The default SHOULD be that 
acceptance is enabled" twice. Either merge them to "The default for both 
globally and per-Client SHOULD be that acceptance is enabled" or remove 
one sentence (probably the first one in the middle of the paragraph)

- MUST silently discard any Status-Realm-Requests messages that it
+ MUST silently discard any Status-Realm-Request messages that it
(or remove the "messages", would align with the previous part of that 
sentence)

"then the server should check if the Target Realm"
This is not clear to me. Should this be an RFC2119 SHOULD or is this 
just a suggestion for implementations? (if it is the latter then I would 
suggest to replace the "should" with a different word or rephrase the 
whole sentence, to not get people confused.

"If it does match, the server should send a Status-Realm-Response"
Again, I feel this should be a SHOULD (or even a conditional MUST? I 
mean, what other option does the server have, if there is no error 
condition?)


".. when the accepted load on the server is lower than the current load".
I tripped over this sentence. After reading it twice I read it as "if 
the client is generating more traffic than the server's rate limits allow."
I'd suggest rephrasing this sentence, especially because "load" is a 
term that is not defined or used before in the spec.


In the last paragraph, there is a reference to RFC2866 and the updated 
list of acceptable RADIUS Message Type Codes. The text is similar to 
RFC5997 (Status-Server), but RFC5997 has an "Updates: 2866". I'd suggest 
that this draft does the same.


**Section 9.3 (Proxy Server Requirements):**

- the Status-Realm Request packet should be proxied toward the Target
+ the Status-Realm Request packet SHOULD be proxied toward the Target


**Section 11.1 (Server Requirements):**

- that implements Proxy Loop Prevention add its own Server-Information
+ that implements Proxy Loop Prevention adds its own Server-Information


**Section 11.2 (Proxy Requirements):**

- if so, the Proxy should discard the message
+ if so, the Proxy SHOULD discard the message

- attribute to any RADIUS Request that they forward toward the Target
+ attribute to any RADIUS Request that it forwards toward the Target


**Section 12 (Proxy Loop Detection Implementation Status):**

- similar to RADIUS Vendor-Specific attribute used today to detect
+ similar to RADIUS Vendor-Specific attributes used today to detect

- single, globally-defrined attribute
+ single, globally-defined attribute

- requiring that a unique vendor identifiers be allocated
+ requiring that unique vendor identifiers be allocated


**Section 16 (IANA Considerations):**

- and "Valies for RADIUS Attribute
+ and "Values for RADIUS Attribute



In general, I would love if the document could link the sections of the 
referenced RFCs too. (At least for kramdown-rfc this is relatively easy, 
maybe a task for later, just wanted to say this now if it is easy to fix 
for you)

I'm not sure if I caught and/or marked all discrepancies (at least as 
far as I see them) with capitalization of RFC2119 keywords, so it would 
be good to look for all occurrences of "should" and "may" in the draft 
and determine if they are a requirement or just a description.



Cheers,

Janfred

-- 
Herr Jan-Frederik Rieckers
Security, Trust & Identity Services

E-Mail: rieckers@dfn.de | Fon: +49 30884299-339 | Fax: +49 30884299-370
Pronomen: er/sein | Pronouns: he/him
__________________________________________________________________________________

DFN - Deutsches Forschungsnetz | German National Research and Education 
Network
Verein zur Förderung eines Deutschen Forschungsnetzes e.V.
Alexanderplatz 1 | 10178 Berlin
www.dfn.de

Vorstand: Prof. Dr. Odej Kao (Vorsitzender) | Dr. Rainer Bockholt | 
Christian Zens
Geschäftsführung: Dr. Christian Grimm | Jochem Pattloch
VR AG Charlottenburg 7729B | USt.-ID. DE 1366/23822