Re: [tsvwg] Ambiguities in RFC 4895: Auth chunks for SCTP

[Nagesh shamnur <[e-mail]>]

Hi Micheal,
	Thanks for your reply. Please find my opinion inline.

Regards,
Nagesh S

-----Original Message-----
From: Michael Tuexen [mailto:[e-mail]] 
Sent: 08 June 2019 13:15
To: Nagesh shamnur <[e-mail]>
Cc: [e-mail]; Ashutosh prakash <[e-mail]>
Subject: Re: [tsvwg] Ambiguities in RFC 4895: Auth chunks for SCTP



> On 28. May 2019, at 16:09, Nagesh shamnur <[e-mail]> wrote:
> 
> Hi Group,
>                 Greetings. While going the RFC 4895: Auth chunks for SCTP, I found there are some discrepancies which might have been encountered by everyone who have implemented this RFC. So, for the benefit of all, I have listed all the issues which I found has problem and how to address these issues. Request all to check and provide your feedback on the same.
>                 Considering the count, I feel, it calls for a bis document.
Hi Nagesh,

thanks for providing feedback and sorry for the late response.
See my comments in-line.

Best regards
Michael

>  
> Issue-1:
> --------
> Type: Technical
>  
> 3.1. Random Parameter (RANDOM)
>  
> Issue text:
>  0                   1                   2                   3
>  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | Parameter Type = 0x8002       | Parameter Length              |
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> |                                                               |
> \                         Random Number                         /
> /                               +-------------------------------\
> |                               | Padding                       |
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>  
>                 Random Number: n bytes (unsigned integer)
>                 This value represents an arbitrary Random Number in network byte
>                 order.
>  
> Modified to:
>  
>  0                   1                   2                   3
>  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | Parameter Type = 0x8002       | Parameter Length              |
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> |                                                               |
> \                   Random Number (32 bytes)                    /
> /                                                               \
> |                                                               |
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>                 Random Number: 32 bytes (unsigned integer)
>                 This value represents a 32 bytes of Random Number in network byte
>                 order.
>  
> Notes:
>                 The parameter random in the section 3.1 mentions that the length of
>                 the random parameter is of variable length and so padding bytes are
>                 required, whereas in section 6, it clearly says that the length of
>                 the random parameter needs to be exactly 32 bytes. Both the sections
>                 are contradicting each other and hence needs to be synchronized.
I don't consider them contradicting. The RANDOM parameter is defined in a way to handle random numbers of arbitrary size. The current usage of this parameter in this document is for 32 byte random numbers. So this is a special case.

This allows us to use this parameter in the future also for random numbers of other sizes.
[Nagesh]: Agreed, but adding a statement to clarify the same in the document would be good

>  
> Issue-2:
> --------
> Type: Technical
>  
> Handling of invalid or unsupported chunks Issue text:
>                 RFC 4895 doesn't explain the handling of invalid or unsupported chunks.
>                 which can be received in the Chunk List Parameter (CHUNKS) (sec 3.2).
>                 
> Modified to:
>                 While handling the parameter Chunk List Parameter (CHUNKS), if a
>                 chunk type is unknown or unrecognized chunk, then the receiver
>                 MUST ignore this chunk and don't need to add it to AUTH chunk list.
>                 
> Notes:  
>                 Since these chunks are unrecognized chunks, the local node would
>                 not send these chunks to peer and hence situation doesn't arise
>                 for sending these chunks in an authenticated way to peer node.
>                 It is better to be present in the RFC explicitly stating the action
>                 to be taken, when such a case arises..
The receiver of a CHUNKS parameter has to ensure that it sends listed chunks only in an authenticated way. If the does not know a listed chunk type, it will not send it and therefore no action is really required.
For a clarification, one could add such a sentence. But it is an editorial thing in my view.

[Nagesh]: I beg to differ here. Let's assume client supports RFC 4895 and RFC 6525, but server supports only RFC 4895 (and doesn't support RFC 6525). So, when client sends INIT chunk, the order of parameter placement in INIT chunk is in such a way that, client adds Chunk List Parameter (0x8003) before supported extensions parameter (0x8008) which in-turn contains the parameter RE-CONFIG (value 130) which needs to be negotiated between client and server. The parameter Chunk List parameter contains RE-CONFIG as one of the chunks in the list. So, when server parses the INIT chunk, it would thus be unable to recognize the chunk type RE-CONFIG which is present as a  part of Chunk List Parameter. So, in such a case, RFC doesn't explains what needs to be done and hence needs to add a section in 4895 bis explaining the handling for the same.

>  
> Issue-3:
> --------
> Type: Technical
>  
> 4.1. Unsupported HMAC Identifier Error Cause
>  
> Issue text:
>  
>  0                   1                   2                   3
>  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | Cause Code = 0x0105           | Cause Length = 6              |
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | HMAC Identifier               | Padding                       |
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>  
> Modified to:
>  
>  0                   1                   2                   3
>  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | Cause Code = 0x0105           | Cause Length = 6              |
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | HMAC Identifier 1             | HMAC Identifier 2             |
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> /                                                               /
> \                              ...                              \
> /                                                               /
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | HMAC Identifier n             | Padding                       |
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>  Notes:
>                 Though currently only 2 HMAC algorithms are supported SHA-1 and SHA-256,
>                 but for extensibility, in future new HMAC algorithms might be registered.
>                 In such case, the current packet format, doesn't support such an
>                 extension. 
>                 
>                 Also, even when only 2 HMAC algorithms (SHA-1 and SHA-256) is currently
>                 supported, sec 3.3 (HMAC-ALGO) still supports more than 2 identifiers,
>                 the same format can be even extended to unsupported HMAC Identifier Error
>                 cause.
You use this error cause when receiving an AUTH chunk using an HMAC Identifier you don't support. This is only one value you need to support.
[Nagesh]: Yes, as per the RFC 4895, now only SHA-1 and SHA-256 is supported and SHA-1 needs to be supported mandatorily. Going by this statement, the error can be generated if SHA-256 is not supported and error chunk is in place. But however the HMAC-ALGO parameter (Sec 3.3) supports to send more than 2 different types HMAC types in the request.
   This discrepancy has 2 problems, in future, if any new HMAC needs to be added to the list (SHA-1 and SHA-256), Unsupported HMAC Identifier Error Cause cannot support this. The packet format in 3.3 and 4.1 is not consistent.

> Issue-4:
> --------
> Type: Technical
>  AUTH negotiation Procedure
> Issue text:
>                 Authenticated chunks negotiation sub-section is not present, due to
>                 which negotiation procedure is not well explained and documented thus
>                 leading to ambiguities in association states between local and remote
>                 nodes.
>                 
> Modified to:
>                 Need to add a sub-section "Authenticated chunks negotiation" as a part
>                 of section 6 which explains the AUTH negotiation procedure.
>   
> Notes:
>                 If the local node supports AUTH and peer doesn't support AUTH, then
>                 the AUTH related parameters sent in INIT by local node will be ignored
>                 by peer node since the upper 2 bits for AUTH related parameters is 10
>                 which as per RFC 4960, needs to ignore and continue processing with other
>                 parameters in the INIT/INIT-ACK chunk.
>                 
>                 Eventually association would get established but meaningful data transfer
>                 wouldn't happen since one endpoint would be expecting data in an
>                 authenticated way but the other endpoint would be sending the in un-AUTH
>                 mode, resulting in association getting closed eventually.
Whether user data transfer can happen or not depends on whether the DATA and or SACK chunks are requested to the authenticated. You might only request the authentication of control chunks like ASCONF/ASCONF-ACK.

The API defined in 6458 allows the application to figure out
* if SCTP AUTH support has been negotiated successfully
* which chunks are required to be authenticated.

So it is easy for the application to ensure that the required configuration has been made and can continue or abort the communication.

[Nagesh]: The scenario what I am explaining is different. Let's say, client support AUTH and server doesn't support AUTH and client has requested both DATA+SACK chunk in an authenticated way. With 4 way handshake, connection will get established successfully, since, server ignores the AUTH related parameter and considers the association has a normal association (non-AUTH). So in this, everytime server sends DATA/SACK to client, it would be encoded as normal chunk, but client continues to drop this DATA/SACK since it is not received in an Authenticated way. So eventually all the T3 timers expires and association gets aborted without any meaningful data transfer.

>                 
>                 Presence of this sub-section is required, absence of this sub-section
>                 is leading to ambiguity with respect to association capability at local
>                 and remote node (one side will be AUTH enabled and other side isn't). Also
>                 both the endpoints of the association should know this by the time association
>                 establishment procedure is completed and thus both sides needs to switch
>                 to non-AUTH mode for compatibility.
> Issue-5:
> --------
> Type: Editorial
>  
> Order of concatenation of parameters to form byte vectors
>  
> Issue text:
>                 The RANDOM parameter, the CHUNKS parameter, and the HMAC-ALGO
>                 parameter sent by each endpoint are concatenated as byte vectors.
>                 
> Modified to:
>                 The RANDOM parameter, the CHUNKS parameter, and the HMAC-ALGO
>                 parameter sent by each endpoint are concatenated as byte vectors in
>                 the same order as given.
Not sure what you mean by "as given".

The concatenation has to happen in the sequence:
* The RANDOM parameter first,
* followed by the CHUNKS parameter, if present,
* and finally the HMAC-ALGO
[Nagesh]: This is the implicit assumption based on the text and most implementation follows it but not stated explicitly in the document.

It does not matter in which sequence the parameters occur in the packet.

I think this is covered by the text in the RFC, but it could be clearer. If you prefer we can discuss an improved text and file an editorial errata.
[Nagesh]: Text only says that, these parameters are concatenated to form byte vectors, but It doesn't say the order of concatenation of these parameters. It would be lot clearer, if stated explicitly in the document.

> Notes:
>                 Section 6.1: Establishment of an Association Shared Key though explains
>                 that the creation of key vectors by appending RANDOM, chunks parameter
>                 and HMAC-ALGO parameter which is sent by each endpoint. But it doesn't
>                 explicitly states the order in which the same needs to be encoded. Though
>                 all the implantations are encoding the same in the order of the text in the
>                 RFC, it is thus better to state it explicitly to avoid different
>                 interpretations by the implementers
It should not matter in which sequence the parameters are in the packet. So no need to specify that.
[Nagesh]: I am not referring to the order of the occurrence of these parameters in INIT/INIT-ACK chunk, but the order of concatenation of these parameters to form byte vector.
>                 
>                 Subsequent paragraph mentions the order of concatenation of shared key,
>                 shorter key vector and larger key vector, but it doesn't clearly mandates
>                 the encoding order for parameter to generate key vector thus making room
>                 for discrepancies.
The order in which the parameters are in the INIT/INIT-ACK is not relevant.
[Nagesh]: I am not referring to the order of the occurrence of these parameters in INIT/INIT-ACK chunk, but the order of concatenation of these parameters to form byte vector.
>                 
> Issue-6:
> --------
> Type: Technical
>  
> Inclusion and handling of EXTENSION parameter as defined in RFC 5061
>  
> Issue text:
>                 The current RFC 4895 doesn't mandates the sending and processing of
>                 EXTENSION parameter (RFC 5061 sec 4.2.7).
>                 
> Modified to:
>                 Add the requirement to send and receive EXTENSION parameter in
>                 INIT/INIT-ACK chunk, if endpoints requires AUTH to be supported.
When RFC 4895 was written, RFC 5061 didn't exist.

The negotiation of SCTP AUTH support is based on the inclusion of the RANDOM and HMAC-ALGO parameter in the INIT/INIT-ACK.

If you support RFC 5061, you can add SCTP-AUTH to the list of supported chunks, but that is not necessary. FreeBSD does this, since I think it is more consistent, but that is not required.
[Nagesh]: Interoperability with lksctp fails, since it expects peer to add supported extension parameter even when peer doesn't support RFC 5061. When this problem was reported in lksctp in the past, it was suggested to add supported extension parameter to make it work with lksctp even when there was no support for RFC 5061 in peer implementations.  Link to discussion as stated below:

https://sourceforge.net/p/lksctp/mailman/message/20743069/

	This suggestion is made, just to make RFC 4895 consistent with all the new RFC which are written for SCTP post RFC 5061. Since in all the newer RFCs, any new chunk that are added are explicitly negotiated using supported extension parameter.
 
Best regards
Michael
>                 
> Notes:
>                 The endpoint supporting both RFC 4895 and RFC 5061 send EXTENSION
>                 parameter when it sends INIT/INIT-ACK and also expects peer to send
>                 the same. lksctp has mandated such a behavior. So, to ensure
>                 inter-operability, it was discussed in mailing list in the past to
>                 include this parameter, when supporting only AUTH to ensure
>                 inter-operability. 
>                 
> https://sourceforge.net/p/lksctp/mailman/message/20743069/
>  
> Regards,
> Nagesh S