PDU processing when crc_flag true

[bob-wiegand]

Checklist (Please check before submitting)

• I reviewed the Contributing Guide.
• I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
The issue involves a difference in interpretation of the CFDP specification. In particular, the format and processing of CFDP PDUs when the PDU header indicates CRC present.

For FD PDUs, the code ignores 4 bytes instead of 2 bytes for the CRC at the end of the PDU (see context). That will result in ignoring the last 2 bytes of file data from FD PDUs encoded with a CRC.

If an EOF or FIN PDU is received with CRC present, the CRC at the end will (almost always) cause a decoding error in CF_CFDP_DecodeAllTlv called by CF_CFDP_DecodeEof/CF_CFDP_DecodeFin.

To Reproduce
Unknown.
If helpful, I could provide what I believe to be valid CFDP PDUs and show how their processing differs from my expectation.

Expected behavior
The software could refuse PDUs with crc_flag true,
or, preferably, it could decrement the data_len of (all) PDUs with crc_flag true by 2 (bytes).

Code snips
This code has the per PDU CRC declaration:

CF/fsw/src/cf_logical_pdu.h

Line 365 in 281a941

uint32 content_crc;

The comments on lines 358-363 do not match my understanding.

File data processing excerpt:
https://github.com/nasa/CF/blob/281a94188cd7d885a5aed01ee041f3ece2b0486f/fsw/src/cf_cfdp.c#L726-737

System observed on:
N/A

Additional context

Unfortunately, the CFDP standard 4.1 CRC PROCEDURES has an external reference for the CRC and the reference is wrong:
4.1.3.1 The CRC computation algorithm shall be the standard CCSDS Telecommand CRC
algorithm specified in 4.2.1.3 of the CCSDS Telecommand Recommendation (reference [4]).
and 232.0-B-3 section 4.2.1.3 does not specify a CRC.
However, the only CRC in 232.0-B-3 is defined in 4.1.4 Frame Error Control Word as a 2 octet field.

Reporter Info
Bob Wiegand, NASA/GSFC

[jphickey]

Good observation, it looks like the FD CRC option may not be actually implemented in CFDP ... as I don't see anything on the transmit side computing this value per FD PDU. What's in the function called CF_CRC_Digest is actually the checksum algorithm described by book 727 section 4.2.2, and that is what goes into the EOF PDU.

[jphickey]

My recommendation at this time is that we categorize the FD PDU CRC as an unsupported feature. I concur that book 727 section 4.1 is flawed here; the reference to TC is incorrect. CCSDS should just directly specify the CRC parameters here (polynomial, initial value, reflection) rather than as a reference some other book that is bound to change - I even looked in the older revs of that book, and section 4.2.1.3 never described a CRC, only 4.1.4 - so who knows where that reference came from.

Therefore, in my interpretation, the FD PDU CRC in book 727 section 4.1 is not implementable until CCSDS fixes this and properly specifies the CRC algorithm (while we could assume what they really meant here, I wouldn't propose spending time on it until they release some sort of corrective document confirming that).

[hugormluis]

In fact, the CRC flag is never set to any value other than 0. It is an unsupported feature for every type of PDU.

The standard states that any PDU can have a CRC. If it has, then set the corresponding flag to 1 and append the CRC to the end of the PDU. Currently, the code does not support any of this.

[timkordas]

I've some time chasing the CRCs, As pointed out above, the reference in the 2020 version of the blue book: https://public.ccsds.org/Pubs/727x0b5.pdf is to section 4.2.1.3 in the 2015 version of https://public.ccsds.org/Pubs/232x0b3s.pdf but the only CRC in there is in section 4.1.4.

but in the 2001 version: https://public.ccsds.org/Pubs/202x0b3s.pdf the reference is correct (and corresponds to the 4.1.4 in the newer document).

Further, there is a 2006 document clarifying the CRC here: https://cwe.ccsds.org/sls/docs/SLS-CandS/Draft%20Documents/Meeting%20Public%20Materials/2006/200606.Rome.Meetings/CRC%20Discussion/crc_clarify2.pdf

and the CRC16 described is a standard CCITT crc16.