> On 7. Jun 2024, at 18:46, Charles Eckel (eckelcu) <eckelcu@xxxxxxxxx> wrote: > > One late comment (sorry), please see inline. > >> On May 16, 2024, at 7:33 AM, Michael Tuexen <Michael.Tuexen@xxxxxxxxxxxxxxxxx> wrote: >> >>> On 14. May 2024, at 02:54, Bernard Aboba <bernard.aboba@xxxxxxxxx> wrote: >>> >>> Comments below. >> Thanks a lot. Answers inline. >> >> Best regards >> Michael >>> >>> On Tue, May 7, 2024 at 7:18 AM Michael Tuexen <Michael.Tuexen@xxxxxxxxxxxxxxxxx> wrote: >>>> On 3. May 2024, at 07:23, Bernard Aboba via Datatracker <noreply@xxxxxxxx> 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. >> I wouldn't say it is widely deployed yet, since, as far as I know, it >> is not enabled in the Google stack yet, since they want to wait for the >> IANA assignment. >> I would prefer to keep the implementation status in the shepherd writeup >> and not as part of the specification. >> In the SCTP case, we usually have some implementation in an open source >> stack. > > You can use the Datatracker “Additional Resource” mechanism to point to the known implementations. I guess, "one" can use it, but it is not "me". I, as an author/editor, can not change any Additional Resources. Since I provided all the links to the document shepherd, I leave it up to him to chose the place he prefers to make the information available. Right now it is in the shepherd's writeup... Best regards Michael > > Cheers, > Charles > >>>> >>>> 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. >> Done. >>>> >>>> 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. >> Done. >>>> 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. >> OK. >>>> >>>> 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. >> I added: >> Otherwise, the endpoint <bcp14>MUST</bcp14> drop all SCTP packets with an >> incorrect CRC32c checksum.</t> >>>> >>>> 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. >> Done. >>>> >>>> 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. >> I changed the text to >> <li><t>why the alternate error detection method provides an equal or >> lower probability of false negatives than the one provided by using the CRC32c >> checksum,</t></li> >>>> >>>> >>>> >>>> >>>> _______________________________________________ >>>> art mailing list >>>> art@xxxxxxxx >>>> https://www.ietf.org/mailman/listinfo/art >>> >>> _______________________________________________ >>> art mailing list -- art@xxxxxxxx >>> To unsubscribe send an email to art-leave@xxxxxxxx >> >> _______________________________________________ >> art mailing list -- art@xxxxxxxx >> To unsubscribe send an email to art-leave@xxxxxxxx > -- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx