Improve message sanity checks in camessage.c

[jesusvasquez333]

This PR improves the sanity checks perform to incoming messages. This improvements will resolve #128 .

The following improvements are implemented:

• One issue is that the test to detect deprecated client versions fails when we haven't receive a client version yet (and therefore client->minor_version_number is 0). So, I added an extra condition to that test to check is client->minor_version_number!=0. (This situation happens with clients < CA_V49, but the proposed change was not enough to make this test backward compatible, so I removed it from the PR).
• I also added 2 extra sanity checks to incoming messages, before they are processed, for the 2 fields of the headers that are fixed:
  • A check to detect message with invalid command ID right after the message is received. This will caught messages like the one described in Improve the CAS error message generated when an IOC gets port-scanned #128, and
  • A check to detect invalid payload sizes, considering both standard and extended forms.

I couldn't figure out a way to detect deprecated clients on the first message. When the first message arrives, we still do not know the client version; clients starting at CA_V49 will send a CA_PROTO_VERSION message which does includes the version number. However, the deprecated clients we are trying to detect ( < CA_V44) do not implement the CA_PROTO_VERSION messages, but instead use the old CA_PROTO_NOOP message which doesn't include the version number. So, I just added a comment in the code describing this. (this topic is left to be revisit in the future).

I also added a few more fixes:

• According to the documentation, the extended message form header should have both fields payload set to 0xffff and data count set to 0. The code however, was only checking the payload field. So, I added the check for the data count filed as well.
• The CA_PROTO_EVENT_CANCEL message contains the data type filed. Although the type is not actually important for this command, I added a check for that field as a sanity check of the received message.
• I replaced one data type check, which was using directly LAST_BUFFER_TYPE , for the macro INVALID_DB_REQ() instead.
• Some other minor formatting issues, and added some more comments.

[jesusvasquez333]

With this changes, when I reproduce the issue as described in #128 (i.e. sending a HTTP GET request to the IOC), I see this error instead in the IOC shell:

epics> CAS: request from 172.16.125.3:50770 => CAS: Invalid command rejected
CAS: Request from 172.16.125.3:50770 => cmmd=18245 cid=0x54502f31 type=12064 count=18516 postsize=21536
CAS: Request from 172.16.125.3:50770 =>   available=0x2e310d0a 	N=0 paddr=(nil)
CAS: forcing disconnect from 172.16.125.3:50770

Note: After some discussions, this log message is now controlled using CASDEBUG. If the variable is 0, the message won't be printed.

[AppVeyorBot]

❌ Build EPICS Base 7 base-7.0-236 failed (commit a6d18b09b7 by @jesusvasquez333)

[AppVeyorBot]

❌ Build EPICS Base 7 base-7.0-262 failed (commit b04ba3c0c9 by @jesusvasquez333)

[AppVeyorBot]

❌ Build EPICS Base 7 base-7.0-283 failed (commit 1df6da7b7d by @jesusvasquez333)

[anjohnson]

Core Group 7/28: AI on MAD.

[marciodo]

I've created the function validate_camessage to check for valid commands (current last command number + 16 for making space for newer clients with more commands) and a few combinations of expected value for a field based on the command type.

When trying an HTTP GET, the IOC prints the message below and close the connection:

CAS: request from 134.79.228.91:52850 => CAS: Invalid channel access message rejected
CAS: Request from 134.79.228.91:52850 => cmmd=18245 cid=0x54502f31 type=12064 count=18516 postsize=21536
CAS: Request from 134.79.228.91:52850 =>   available=0x2e310d0a         N=0 paddr=(nil)
CAS: forcing disconnect from 134.79.228.91:52850

I've tested with caput, caget, and camonitor, using scalars and waveform with elements below the standard protocol size and above it, to have tests covering both standard and extended sizes. Versions of caput, caget, and camonitor used: 3.14.12, 3.15.5, and the ones from this pull request.

Let me know if you'd like to print the message only if CASDEBUG is set.

I've merged the branch of this pull request with the most recent 7.0 to make sure the previous changes in 7.0 would work with the changes made by this PR and check for conflicts.

[AppVeyorBot]

✅ Build EPICS Base 7 base-7.0-577 completed (commit 6685bbe52b by @marciodo)

[anjohnson]

Ping @mdavidsaver please.

[mdavidsaver]

This change looks ok to me.

However, on reflection I'm not comfortable taking sole responsibility for review and testing of this PR. Changes to message handling code carry a risk of breaking wire compatibility. After having been responsible for one such accidental break, I hesitate to risk another.

wrt. #128 my now less than long term answer for confusing RSRV log messages is to disable RSRV in favor of PVA (with its own confusing messages ;) ).