[art] Re: Artart last call review of draft-ietf-tsvwg-sctp-z ero-checksum-09

Comments below.

On Tue, May 7, 2024 at 7:18 AM Michael Tuexen <
[email protected]> wrote:

> > On 3. May 2024, at 07:23, Bernard Aboba via Datatracker <
> [email protected]> wrote:
> >
> > Reviewer: Bernard Aboba
> > Review result: Ready with Issues
> >
> > Reviewer: Bernard Aboba
> > Document: draft-ietf-tsvwg-sctp-zero-checksum
> > Status: Ready with (minor) Issues
> >
> Hi Bernard,
>
> thank you very much for the review. See my comments in-line.
>
> Best regards
> Michael
> > The document is well written and allowing a zero checksum for SCTP over
> DTLS makes sense.
> > In order to allow for other potential protection mechanisms, the
> document sets up an
> > IANA registry and associated documentation requirements. However, I am
> not sure
> > that the registration criteria are clear enough.
> >
> > At present the document does not talk about implementation status,
> although I believe it
> > has been implemented in Pion, dcSCTP and perhaps other WebRTC data
> channel implementations.
> It is also implemented in the FreeBSD implementation and therefore in
> usrsctp.
> There are also test cases available at:
> https://github.com/tsvwg/zero-checksum/tree/main/packetdrill-tests
> which have been used to test the FreeBSD implementation.
> I think all of this is covered in the shepherds write-up.
> Do you think it would be better to write the information up in a
> "Implementation Status Section"?



> > Have any issues arisen during implementation?
> The implementer of dcSCTP did not report any issues, he explicitly said
> that
> allowing sending packets with correct checksums where using a zero checksum
> would be allowed was simplifying the initial implementation.
>
> The Pion stack implementer implemented a feature negotiation instead of
> feature declaration. Not sure why. I think the document is very clear
> about this. But this has been fixed in the Pion implementation now.
>

[BA] Thanks for the pointers and summary.  I don't think you need to add an
implementation section, but you might want to note that the specification
has been implemented and deployed widely.


> >
> > NITs
> > ----
> >
> > 3.  Alternate Error Detection Methods
> >
> >   SCTP uses a CRC32c checksum to provide some level of data integrity.
> >   The CRC32c checksum is computed based on the SCTP common header and
> >   the chunks contained in the packet.  In particular, the computation
> >   of the CRC32c checksum does not involve a pseudo header for IPv4 or
> >   IPv6 like the computation of the TCP checksum, as specified in
> >   [RFC9293], or the UDP checksum, as specified in [RFC0768].
> >
> > [BA] Would it be appropriate to advise against turning off the UDP
> checksum as well?
> I think using a zero checksum for UDP when using SCTP/DTLS/UDP/IP
> or SCTP/UDP/IP is independent of using the zero checksum for SCTP
> as discussed in this document. So I think there is no need to discuss
> it here.
> In case you are referring to SCTP/UDP/IP, this is discussed in
> RFC 6951 and this document does not change anything.
> The reference to the UDP and TCP checksum is only given, to
> stress the difference between the these and the SCTP checksum.
> The use of the pseudo-header for UDP and TCP.
>

[BA] Makes sense.


> >
> >   Alternate error detection methods have two requirements:
> >
> >   1.  An alternate error detection method MUST provide an equal or
> >       better level of data integrity than the one provided by using the
> >       CRC32c checksum algorithm.  This MAY only apply to packets
> >       satisfying some method specific constraints.
> >
> > [BA] I think you may need to define the meaning of "equal or better" more
> > exactly.  For example, that the alternative provides the same coverage
> > as the CRC32c checksum, with a lower probability of a false negative.
> Well, I intentionally used a vague term. The critical part in your
> definition is the "same coverage".
> The CRC32c covers the from the common header the port numbers and the
> verification tag and then all chunks.
> The authors of
> *
> https://www.ietf.org/archive/id/draft-ietf-tsvwg-dtls-over-sctp-bis-08.html
> *
> https://www.ietf.org/archive/id/draft-westerlund-tsvwg-sctp-crypto-chunk-02.html
> *
> https://www.ietf.org/archive/id/draft-westerlund-tsvwg-sctp-dtls-chunk-01.html
> suggest that you can also use an incorrect checksum of zero for
> all packets which have the AUTH, CRYPTO, or DTLS chunk as its first chunk.
> Then the coverage would only be over the chunks, not the fields contained
> in the common header.
> The protection would also work for these fields, since you would use
> these fields to lookup the association and check the verification
> tag. So if there would be an error in the port numbers, most likely
> the lookup will result in no association or one with different
> key material, such that the AUTH, CRYPTO, or DTLS chunk check would
> fail. If the transmission error is in the verification tag, the
> packet would be dropped anyway.
>
> So if you prefer, we could say
>
> An alternate error detection method MUST provide an equal or
> lower probability of false negatives than the one provided by using the
> CRC32c checksum algorithm.
>
> Would that address your concern?
>

[BA] Yes.


> >
> >   2.  Using an alternate error detection method MUST NOT result in a
> >       path failure for more than two retransmission timeouts (RTO) due
> >       to middleboxes on the path expecting correct CRC32c checksums.
> >
> > [BA] This requirement depends on the behavior of middleboxes, so it's
> > not clear to me how adherence to the MUST NOT can be tested.
> My understanding is that an implementation must not fail a path for longer
> than 2 RTOs if the path starts to drop all packets with an incorrect
> checksum of zero at any time. The next sentence gives a hint how
> this could be done.
> You might want to combine this with some probing when you initially
> confirm the path. But since all of this is not needed for WebRTC,
> we wanted to keep this out of this document and put that into
> any upcoming document, where this is relevant.
>

[BA]  OK, that makes sense.


> >
> >   To fulfill the second requirement, alternate error detection methods
> >   MAY use a heuristic to detect the existence of such middleboxes and
> >   use correct CRC32c checksums on these affected paths.
> >
> > [BA] The "MAY" here seems to be somewhat in conflict with the much
> > stronger MUST NOT, particularly since we are talking about middlebox
> > detection. At present the document only allocates a code point for
> > DTLS, to which this requirement doesn't apply.
> Would it be less confusing to write:
>
> To fulfill the second requirement, alternate error detection methods
> could use a heuristic to detect the existence of such middleboxes and
> use correct CRC32c checksums on these affected paths.
>
> [BA] Yes, that would help.


> >   One example fulfilling the first requirement is using DTLS as the
> >   lower layer of SCTP as specified in [RFC8261].  Another example is
> >   using SCTP Authentication as specified in [RFC4895].  Of course, this
> >   only applies to all SCTP packets having an AUTH chunk as its first
> >   chunk.  However, using SCTP Authentication without any heuristic does
> >   not fulfill the second requirement.  Since using DTLS as the lower
> >   layer of SCTP as specified in [RFC8261] also fulfills the second
> >   requirement, it can be used as an alternate error detection method
> >   (see Section 6).
> >
> > [BA] SCTP Authentication is not allocated a code point. So not sure
> > why this is mentioned as "another example" - is this just to indicate
> > why it is not acceptable (e.g. not meeting the second requirement)?
> Yes.
> Basically, SCTP/DTLS is an example fulfilling both requirements and
> not have any addition method specific constraints.
> The document is just using SCTP AUTH is an example fulfilling the first,
> but not the second requirement. It is also an example, which has a method
> specific constraint: "The AUTH chunk must be the first chunk".
> If you can suggest an improvement to the wording to make this clearer,
> I'm happy to take it.
>

[BA]  The text is clear as it is; was just unsure of why it was there.


> >
> > 5.1.  Declaration of Feature Support
> >
> >   An endpoint willing to accept SCTP packets with an incorrect checksum
> >   of zero MUST include the Zero Checksum Acceptable Chunk Parameter
> >   indicating the alternate error detection method it is willing to use
> >   in the INIT or INIT ACK chunk it sends.
> >
> >   An SCTP implementation MAY also require the upper layer to indicate
> >   that it is fine to use a specific alternate error detection method
> >   before including the corresponding Zero Checksum Acceptable Chunk
> >   Parameter.
> >
> > [BA] What if the alternate error detection method is not consistent
> > with what has been established? For example, SCTP over DTLS/UDP has
> > been established, but some other method (not yet allocated a code point)
> > is negotiated?
> Please note that there is no negotiation. Each side declared what it
> is willing to accept.
> Packets received with an incorrect checksum of zero are accepted if
> they fulfill the additional requirements of the error detection method
> announced. If that is not true they are dropped.
>
> The text in 5.3 contains:
>
> If an endpoint has sent the Zero Checksum Acceptable Chunk Parameter
> indicating the support of an alternate error detection method in an
> INIT or INIT ACK chunk, it MUST accept SCTP packets fulfilling the
> requirements of the announced alternate error detection method using
> an incorrect checksum value of zero in addition to SCTP packets
> containing the correct CRC32c checksum value for this association.
>
> Should it be stated explicitly that if the is not true, packets are
> dropped?
>

[BA] Yes.


> >
> > Section 5.2
> >
> >   4.  Alternate error detection methods might have some additional
> >       conditions requiring that the sender MUST include a correct
> >       CRC32c checksum in the packet.
> >
> > [BA] The combination of "might" and MUST is an odd one. Is this
> normative language needed?
> An alternate error detection method can have additional requirements.
> Like in the SCTP AUTH case "the AUTH chunk must the first chunk". Not
> all packets can fulfill this requirement. In this case, the sender
> MUST use a correct CRC32c checksum.
> Whether an alternate error detection method has such additional
> requirements
> depends on the alternate error detection method.
> Would you prefer something like:
>
> If an alternate error detection method has some method specific
> constraints,
> the sender MUST include a correct CRC32c checksum in all packets not
> fulfilling
> these method specific constraints.
>

[BA] Yes.


> >
> >   An SCTP end point MAY require that the upper layer allowed the use of
> >   the alternate error detection method that was announced by the peer
> >   before sending packets with an incorrect checksum of zero.
> >
> > [BA] MAY? In the case of DTLS, the alternate error detection method
> > was setup prior to initiation of the SCTP association. So why would this
> > be optional?
> If an SCTP stack implements this MAY (as the FreeBSD stack does), it gives
> the upper layer control whether the method is needed or not. See the
> sentence
>
> An implementation might only send packets with an incorrect checksum of
> zero,
> if the alternate error detection method announced by the peer is also
> enabled
> locally via this socket option.
>
> in the Socket API section. For example, if there are any problems with
> using this feature, the upper layer can disable this.
> But it is a MAY. So if an SCTP implementation doesn't want to allow
> this level of control, it is fine.
>

[BA] OK.


> >
> > Section 8 IANA Considerations
> >
> >   2.  A reference to a specification describing:
> >
> >       (a)  the alternate error detection method,
> >
> >       (b)  why the alternate error detection method provides an equal
> >            or better level of data integrity protection than the one
> >            provided by using the CRC32c checksum,
> >
> > [BA] It might help to sharpen the definition of "equal or better".
> This needs to be handled consistently with the above text. See discussion
> there.
> >
> >
> >
> >
> > _______________________________________________
> > art mailing list
> > [email protected]
> > https://www.ietf.org/mailman/listinfo/art
>
>

_______________________________________________
art mailing list -- [email protected]
To unsubscribe send an email to [email protected]