[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Fwd: draft-ietf-radext-status-server-02 editorials (fwd)
> From A.Hoenes@TR-Sys.de Thu Dec 11 16:15:59 MEZ 2008
...
> (1) Section 1, last para -- typo/grammar
>
> The remaining text contains ... and discussed security ...
> --- ^
> The remaining text contains ... and discusses security ...
Fixed.
^
>
> (2) Section 2.1, last para -- typo
>
> s/qeuries/queries/
> ^^ ^^
Fixed.
> (3) Section 2.3.2
>
> (3a) headline missing (already noted)
>
> (3b) requirements language
>
> Section 2.3.1 starts:
>
> Status-Server SHOULD be used instead ...
> !!!!!!
>
> But 2.3.2, which apparently is intended to catch the alternative
> case left open by that 'SHOULD', starts with:
>
> | Status-Server may be used instead of ...
> ^^^
> For logical consistency, this counterpoint also should use uppercase:
>
> | Status-Server MAY be used instead of ...
OK, thanks.
^^^
>
> (4) Section 2.3.3
>
> Following up from above, 2.3.3 should also be changed:
>
> Status-Server may be pro-actively sent ...
> ---
> Status-Server MAY be pro-actively sent ...
> ^^^
That section has been deleted, at the suggestion of the RADEXT chairs
&& Area Director.
> (5) Section 3 -- clarifications
>
> (5a) 1st para below indented part 'Request Authenticator'
>
> The draft says:
>
> Similarly, the Response Authenticator field of an Access-Accept
> packet sent in response to Status-Server queries MUST be generated
> using the same method as used for for calculating the Response
> | Authenticator of the Access-Accept, with the Status-Server Request
> Authenticator taking the place of the Access-Request Request
> Authenticator.
>
> For clarity, I suggest to insert a phrase better qualifying the
> Access-Accept taken as a reference:
>
> Similarly, the Response Authenticator field of an Access-Accept
> packet sent in response to Status-Server queries MUST be generated
> using the same method as used for for calculating the Response
> | Authenticator of the Access-Accept sent in response to an Access-
> vvvvvvvv ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> | Request, with the Status-Server Request Authenticator taking the
> place of the Access-Request Request Authenticator.
Good suggestion, thanks.
> (5b) 2nd para below indented part 'Request Authenticator'
>
> Similarly, I suggest to improve the wording in the next paragraph:
>
> The Response Authenticator field of an Accounting-Response packet
> sent in response to Status-Server queries MUST be generated using the
> same method as used for for calculating the Response Authenticator of
> | the Accounting-Response, with the Status-Server Request Authenticator
> taking the place of the Accounting-Request Request Authenticator.
> ---
> The Response Authenticator field of an Accounting-Response packet
> sent in response to Status-Server queries MUST be generated using the
> same method as used for for calculating the Response Authenticator of
> | the Accounting-Response sent in response to and Accounting-Request,
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> with the Status-Server Request Authenticator taking the place of the
> Accounting-Request Request Authenticator.
Agreed.
>
> (6) Section 3.1
>
> (6a) headline
>
> To better indicate the simplification for implementations, which seems
> to be the spirit of this detail, I suggest to change the headline
> (this will be even more become clear by the subsequent modification
> I'll propose below):
>
> |3.1. Consistent definition for Status-Server
> ---
> |3.1. Single Definition for Status-Server
> ^^^^^^^^
>
> (6b) 2nd paragraph -- clarification / textual improvement
>
> I suggest to replace "one" by "a single":
>
> vvv
> | Having one definition for Status-Server packets is simpler than
> having different definitions for different destination ports. [...]
> --- vvvvvvvvv
> | Having a single definition for Status-Server packets is simpler than
> having different definitions for different destination ports. [...]
That is reasonable.
>
> (7) Section 4 -- consistent spelling of abbreviation
>
> For uniformity and consistency, I suggest to s/coa/CoA/
> (2 instances in Section 4).
That text has been deleted at the suggestion of the RADEXT chairs &&
the Area Director.
>
> (8) Section 4.2
>
> (8a) need to update the metadata (front matter)
>
> The third para says:
>
> We note that [RFC2865] Section 3 defines a number of RADIUS Codes,
> but does not make statements about which Codes are valid for port
> | 1812. In contrast, [RFC2866] Section 3 specifies that only RADIUS
> | Accounting packets are to be sent to port 1813. This specification
> is compatible with [RFC2865], as it uses a known Code for packets to
> port 1812. This specification is not compatible with [RFC2866], as
> it adds a new code (Status-Server) that is valid for port 1812.
> | However, as the category of [RFC2866] is Informational, this conflict
> | is acceptable.
>
> This statement clearly indicates that this specification
> *** Updates RFC 2866 ***.
I'll have to ask the RADEXT chairs for help on this one. Can we do this?
> This relationship really should be noted in the front matter of the
> draft.
>
> (8b) 7th para -- word omission
>
> Please correct:
> v
> | The server MAY prioritize the handling Status-Server queries over the
> handling of other requests, subject to the rate limiting described
> above.
> --- vvvv
> | The server MAY prioritize the handling of Status-Server queries over
> the handling of other requests, subject to the rate limiting
> described above.
Fixed as per earlier comments.
>
> (9) Section 4.3
>
> (9a) 1st para -- improvement #1
...
> (9b) 1st para -- improvement #2 (missing word)
...
> (9c) 4th para -- inconsistency with 1st para
...
> (9d) last para -- punctuation
All of this text has been deleted as per comments from the chairs &&
the AD.
> (10) Section 4.4, 2nd para -- improvement
>
> I suggest to slightly change the wording:
>
> [...]. If the server is still unresponsive, however, the result
> may be user login failures. The Status-Server solution is an ideal
> | one to solve this problem.
> ---^^^
> [...]. If the server is still unresponsive, however, the result
> may be user login failures. The Status-Server solution is an ideal
> | way to solve this problem.
> ^^^
Fixed, thanks.
>
> (11) Section 4.6
>
> (11a) 4th para below first figure -- improvement/grammar
>
> I suggest to change:
>
> [...]. If the implementation fails to respond, then the NAS
> cannot distinguish between the Proxy Server being down, or the next
> | server along along the proxy chain is unreachable.
> -- ^^
> [...]. If the implementation fails to respond, then the NAS
> cannot distinguish between the Proxy Server being down, or the next
> | server along along the proxy chain being unreachable.
> ^^^^^
Agreed.
> (11b) 5th para below first figure -- missing word
>
> I suggest to insert "in" as follows:
>
> In the worst case, failures in routing for Realm A may affect users
> | Realm B. [...]
> ---^
> In the worst case, failures in routing for Realm A may affect users
> | in Realm B. [...]
> ^^^^
Changed to "of" Realm B, as per earlier comments.
> (11c) denomination of entities
>
>>From the second figure ...
>
> /-> Proxy Server P -----> Home Server P
> / \ /
> NAS X
> \ / \
> \-> Proxy Server S -----> Home Server S
>
> ... it becomes apparent that the denominations of the entities are
> not chosen carefully; in this case, 'P' and 'S' are both overloaded.
>
> To disambiguate the whole stuff, here's my proposal:
>
> Use 'P1' and 'P2' (in place of 'P' and 'S') for the two Proxy Servers
> in both scenarios (figures and text); use 'H1' and 'H2' for the Home
> Servers in the second scenario.
The terms "Primary" and "Secondary" are commonly used in RADIUS. I'll
see if there's some compromise that uses historical terminology, but
also clarifies the diagram.
> (12) Section 4.7 -- terminology
>
> The draft uses the colloquial, but incorrect short term "MIB" whenever
> in wants to indicate a "MIB Module". Please see the BCP 111, RFC 4181
> for the detailed rationale of the precise terminology.
>
> Therefore, please change MIB[s] --> MIB module[s]
> throughout the text.
Fixed.
> (13) Section 4.7.1
>
> (13a) need to update the metadata (front matter)
>
> The 1st para clearly indicates that this draft
> *** Updates RFC 4669 and RFC 4671 ***
>
> To make it easier for developers and users to find this update,
> please amend the draft front matter accordingly.
I'm not sure it *updates* those documents. Comments from the Area
Director would help here.
> (13b) last para
>
> The draft text is confusingly complicated, yet inprecise.
> The draft text could be misunderstood as recommending to
> define alternate MIB modules where it better should recommend
> using extensions for standard MIB tables.
>
> I suggest to modify this paragraph as follows:
>
> If a server supports Status-Server and the [RFC4669] or [RFC4671]
> | MIBs, then it SHOULD also support vendor-specific MIBs containing
> | similar information as the standard MIBs, but which are instead
> dedicated solely to tracking Status-Server requests and responses.
> | Any definition of the server MIBs for Status-Server is outside of the
> scope of this document.
> ---
> | If a server supports Status-Server and the [RFC4669] or [RFC4671] MIB
> | modules, then it SHOULD also support vendor-specific MIB extensions
> ^^^^^^^ ^^^^^^^^^^^^^^^^
> dedicated solely to tracking Status-Server requests and responses.
> | Any definition of the server MIB module extensions for Status-Server
> ^^^^^^^^^^^^^^^^^^^^^
> is outside of the scope of this document.
OK.
> (Please note the deletion of the full 3rd line!)
Noted.
> (14) Section 4.7.2
>
> (14a)
> Similar as noted in (13a) above, the first para of 4.7.2 clearly
> indicates that this specification *** updates RFCs 4668 and 4670 ***.
Maybe clarifies? I'm unsure here.
> (14b)
> Similarly as in (13b) above, the last para of 4.7.2 should be modified
> as follows:
>
> If an implementation supports Status-Server and the [RFC4668] or
> | [RFC4670] MIBs, then it SHOULD also support vendor-specific MIBs
> | containing similar information as those MIBs, but which are instead
> dedicated solely to tracking Status-Server requests and responses.
> | Any definition of the client MIBs for Status-Server is outside of the
> scope of this document.
> ---
> If an implementation supports Status-Server and the [RFC4668] or
> | [RFC4670] MIB modules, then it SHOULD also support vendor-specific
> ^^^^^^^^^^^
> | MIB extensions dedicated solely to tracking Status-Server requests
> ^^^^^^^^^^^^^^^
> | and responses. Any definition of the client MIB module extensions
> ^^^^^^^^^^^^^^^^^^^^^
> for Status-Server is outside of the scope of this document.
OK.
> (15) Section 5.1
>
> (15a) 2nd para, last sentence -- typo/grammar
>
> Please s/server/servers/ in ...
>
> [...]. These queries are most useful in
> | deployments where an administrator has internal RADIUS servers that
Fixed, thanks.
> (15b) 3rd para -- improvement
>
> To improve the readability, I suggest varying the multiple uses of
> "use", e.g.:
> vvvv vvvv vvvvv
> If used, the names used for these test users SHOULD be difficult to
> guess by an attacker. [...]
> --- vvvvvvvv
> If used, the names utilized for these test users SHOULD be difficult
> to guess by an attacker. [...]
Ok.
> (16) Section 5.2 -- outdated text
>
> The first paragraph seems to be outdated now by the RADIUS-over-TCP
> draft!
Yes. I'll see if I can coordinate the text. The "RADIUS over TCP"
will be published after this one. I don't know that we want to have the
two drafts depend on each other...
> (17) Section 5.3, 1st para
>
> The draft says:
> [...]. It may be
> tempting to increase the utility of Status-Server by having the
> | responses carry additional information, implementors are warned that
> such uses have not been analyzed for potential security issues or
> network problems.
>
> To better reflect the relationship between the two (half-)sentences
> connected with a comma, I propose to either use a semicolon instead
> of the comma, or insert " but" after the comma:
>
> [...]. It may be
> tempting to increase the utility of Status-Server by having the
> | responses carry additional information, but implementors are warned
> that such uses have not been analyzed for potential security issues
> or network problems.
I would suggest using "but".
Thanks for the feedback.
Alan DeKok.
--
to unsubscribe send a message to radiusext-request@ops.ietf.org with
the word 'unsubscribe' in a single line as the message text body.
archive: <http://psg.com/lists/radiusext/>