Hi Harshit,

On 08.01.21, 14:37, "Harshit Jain" <hjain@arista.com> wrote:

    Hi Fabian,


    Thanks for the review. It seems there was some duplicate code in the new function, specifically relating to rewriteIn/rewriteOut and ttl checks. I have moved CoA processing to the radsrv function itself.
    Here is the updated proposed design and a couple of questions -

    1. Add an attribute dynAuthServer to the
    realm config block. This attribute specifies the next-hop server to forward the CoA request to corresponding to a realm, just like server and accountingServer specify
     the next-hop server to use for authentication and accounting requests respectively.
    2. Any device which sends a CoA/Disconnect request must be configured as a client in the config file. Similarly, any device to which
     CoA requests are to be proxied to must be configured as a server in the config file.
    3. Add an attribute
    useDynAuthPort of type Boolean
    to the server config block. If this is set to ON and no port is specified for the server, then default CoA port is used. This is 3799 for UDP/TCP and 2083 for TLS/DTLS. If this is set to OFF, then default authentication
     port is used i.e 1812 for UDP/TCP and 2083 for TLS/DTLS.
    4. Update
    radsrv function to enable it to handle CoA/Disconnect requests as well.
    * If no Operator-Name attribute is present in the packet, then the request is ignored. (Should we send a NAK containing Error-Cause attribute having value 402 "Missing Attribute"
     instead?).

    * If the value of Operator-Name attribute is not valid i.e the first character is not an ascii "1", then the request is ignored.
    (Should we send a NAK containing Error-Cause attribute having value 407 "Invalid Attribute Value" instead?).
    * If the realm is unknown by the proxy, then a NAK packet containing Error-Cause attribute 502 "Request not Routable" is sent back.

This sounds good as a description, but It's hard to do a detailed review without actually seeing the code.


    5. Add an attribute DynAuthResponse
    of type Boolean to the realm config block. If this is set to ON and no dynAuthServer is configured for the realm, then a NAK will be sent for requests matching the realm. If this is set to OFF, then the request will
     be silently ignored. This is similar to AccountingResponse
    attribute.

This makes me wonder how CoA Servers will behave if they receive ACK/NAK for some realms they send requests to, but not others (having DynAuthResponse On for some realms, but not others). Haven't looked at the details if the RFC has considered this.

BR,
Fabian

--
SWITCH
Fabian Mauchle, Network Engineer
Werdstrasse 2, P.O. Box, 8021 Zurich, Switzerland
Phone +41 44 268 15 30, direct +41 44 268 15 39