Merge pull request #2468 from jphickey:fix-2467-msg-integrity

Fix #2467, message integrity API

modules/cfe_testcase/src/sb_sendrecv_test.c

@@ -145,7 +145,7 @@ void TestBasicTransmitRecv(void)
     UtAssert_INT32_EQ(CFE_SB_ReceiveBuffer(&MsgBuf, PipeId1, -100), CFE_SB_BAD_ARGUMENT);
 
     /*
-     * Note, the CFE_SB_TransmitMsg now adheres to the "UpdateHeader" flag.
+     * Note, the CFE_SB_TransmitMsg now adheres to the "IsOrigination" flag.
      * Thus, the sequence numbers should come back with the value from the Route (1-2)
      * rather than the value the message was filled with initially.
      *

modules/core_api/fsw/inc/cfe_msg.h

@@ -60,30 +60,6 @@
  */
 CFE_Status_t CFE_MSG_Init(CFE_MSG_Message_t *MsgPtr, CFE_SB_MsgId_t MsgId, CFE_MSG_Size_t Size);
 
-/*****************************************************************************/
-/**
- * \brief Set/compute all dynamically-updated headers on a message
- *
- * \par Description
- *          This routine updates all dynamic header fields on a message, and is typically
- *          invoked via SB just prior to broadcasting the message.  Dynamic headers include
- *          are values that should be computed/updated per message, including:
- *                  - the sequence number
- *                  - the timestamp, if present
- *                  - any error control or checksum fields, if present
- *
- *          The MSG module implementation determines which header fields meet this criteria
- *          and how they should be computed.
- *
- * \param[inout] MsgPtr  A pointer to the buffer that contains the message @nonnull.
- * \param[in]    SeqCnt  The current sequence number from the message route
- *
- * \return Execution status, see \ref CFEReturnCodes
- * \retval #CFE_SUCCESS             \copybrief CFE_SUCCESS
- * \retval #CFE_MSG_BAD_ARGUMENT    \copybrief CFE_MSG_BAD_ARGUMENT
- */
-CFE_Status_t CFE_MSG_UpdateHeader(CFE_MSG_Message_t *MsgPtr, CFE_MSG_SequenceCount_t SeqCnt);
-
 /**\}*/
 
 /** \defgroup CFEAPIMSGHeaderPri cFE Message Primary Header APIs
@@ -725,12 +701,44 @@ CFE_Status_t CFE_MSG_GetTypeFromMsgId(CFE_SB_MsgId_t MsgId, CFE_MSG_Type_t *Type
 
 /**\}*/
 
-/** \defgroup CFEAPIMSGMsgVerify cFE Message Checking APIs
+/** \defgroup CFEAPIMSGMsgIntegrity cFE Message Integrity APIs
  * \{
  */
+
 /*****************************************************************************/
 /**
- * \brief Checks message headers against expected values
+ * \brief Perform any necessary actions on a newly-created message, prior to sending
+ *
+ * \par Description
+ *          This routine updates and/or appends any necessary fields on a message, is
+ *          invoked via SB just prior to broadcasting the message.  The actions include
+ *          updating any values that should be computed/updated per message, including:
+ *                  - setting the sequence number
+ *                  - updating the timestamp, if present
+ *                  - computing any error control or checksum fields, if present
+ *
+ *          The MSG module implementation determines which header fields meet this criteria
+ *          and how they should be computed.
+ *
+ *          The BufferSize parameter indicates the allocation size message of the buffer that
+ *          holds the message (i.e. the message envelope size).  In some implementations, the
+ *          allocated buffer may include extra space in order to append a CRC or digital signature.
+ *
+ * \sa CFE_MSG_VerificationAction
+ *
+ * \param[inout] MsgPtr       A pointer to the buffer that contains the message @nonnull.
+ * \param[in]    BufferSize   The size of the buffer encapsulating the message
+ * \param[out]   IsAcceptable Output variable to be set, indicates message acceptability @nonnull
+ *
+ * \return Execution status, see \ref CFEReturnCodes
+ * \retval #CFE_SUCCESS             \copybrief CFE_SUCCESS
+ * \retval #CFE_MSG_BAD_ARGUMENT    \copybrief CFE_MSG_BAD_ARGUMENT
+ */
+CFE_Status_t CFE_MSG_OriginationAction(CFE_MSG_Message_t *MsgPtr, size_t BufferSize, bool *IsAcceptable);
+
+/*****************************************************************************/
+/**
+ * \brief Checks message integrity/acceptability
  *
  * \par Description
  *        This routine validates that any error-control field(s) in the message header
@@ -740,14 +748,23 @@ CFE_Status_t CFE_MSG_GetTypeFromMsgId(CFE_SB_MsgId_t MsgId, CFE_MSG_Type_t *Type
  *        and may be a no-op if no error checking is implemented.  In that case, it
  *        will always output "true".
  *
+ * \note  Due to the fact that software bus uses a multicast architecture, this function
+ *        must not modify the message, as the buffer may be shared among multiple receivers.
+ *        This should generally be the inverse of CFE_MSG_OriginationAction(), but on the
+ *        origination side it may update header fields and/or modify the message, on
+ *        the verification/receive side it must only check those fields, not modify them.
+ *
+ * \sa CFE_MSG_OriginationAction
+ *
  * \param[in]  MsgPtr        Message Pointer @nonnull
- * \param[out] VerifyStatus  Output variable to be set to verification result @nonnull
+ * \param[in]  BufferSize    The size of the buffer encapsulating the message
+ * \param[out] IsAcceptable  Output variable to be set, indicates message acceptability @nonnull
  *
  * \return Execution status, see \ref CFEReturnCodes
  * \retval #CFE_SUCCESS            \copybrief CFE_SUCCESS
  * \retval #CFE_MSG_BAD_ARGUMENT   \copybrief CFE_MSG_BAD_ARGUMENT
  */
-CFE_Status_t CFE_MSG_Verify(const CFE_MSG_Message_t *MsgPtr, bool *VerifyStatus);
+CFE_Status_t CFE_MSG_VerificationAction(const CFE_MSG_Message_t *MsgPtr, size_t BufferSize, bool *IsAcceptable);
 
 /**\}*/
 

modules/core_api/fsw/inc/cfe_sb.h

@@ -402,9 +402,15 @@ CFE_Status_t CFE_SB_UnsubscribeLocal(CFE_SB_MsgId_t MsgId, CFE_SB_PipeId_t PipeI
 **          software bus will read the message ID from the message header to
 **          determine which pipes should receive the message.
 **
-**          In general, the "UpdateHeader" parameter should be passed as "true"
-**          if the message was newly constructed by the sender and is being sent
-**          for the first time.  When forwarding a message that originated from
+**          The IsOrigination parameter should be passed as "true" if the message was
+**          newly constructed by the sender and is being sent for the first time.  This
+**          enables the message origination actions as determined by the CFE MSG module,
+**          which may include (but not limited to):
+**            - Updating sequence number
+**            - Updating timestamp
+**            - Calcualating a CRC, checksum, or other message error control field
+**
+**          Conversely, when forwarding a message that originated from
 **          an external entity (e.g. messages passing through CI or SBN), the
 **          parameter should be passed as "false" to not overwrite existing data.
 **
@@ -421,15 +427,15 @@ CFE_Status_t CFE_SB_UnsubscribeLocal(CFE_SB_MsgId_t MsgId, CFE_SB_PipeId_t PipeI
 **
 ** \param[in]  MsgPtr       A pointer to the message to be sent @nonnull.  This must point
 **                          to the first byte of the message header.
-** \param[in] UpdateHeader  Update the headers of the message
+** \param[in] IsOrigination  Update the headers of the message
 **
 ** \return Execution status, see \ref CFEReturnCodes
 ** \retval #CFE_SUCCESS         \copybrief CFE_SUCCESS
 ** \retval #CFE_SB_BAD_ARGUMENT \copybrief CFE_SB_BAD_ARGUMENT
 ** \retval #CFE_SB_MSG_TOO_BIG  \copybrief CFE_SB_MSG_TOO_BIG
 ** \retval #CFE_SB_BUF_ALOC_ERR \covtest \copybrief CFE_SB_BUF_ALOC_ERR
 **/
-CFE_Status_t CFE_SB_TransmitMsg(const CFE_MSG_Message_t *MsgPtr, bool UpdateHeader);
+CFE_Status_t CFE_SB_TransmitMsg(const CFE_MSG_Message_t *MsgPtr, bool IsOrigination);
 
 /*****************************************************************************/
 /**
@@ -471,6 +477,7 @@ CFE_Status_t CFE_SB_TransmitMsg(const CFE_MSG_Message_t *MsgPtr, bool UpdateHead
 ** \retval #CFE_SB_NO_MESSAGE   \copybrief CFE_SB_NO_MESSAGE
 **/
 CFE_Status_t CFE_SB_ReceiveBuffer(CFE_SB_Buffer_t **BufPtr, CFE_SB_PipeId_t PipeId, int32 TimeOut);
+
 /** @} */
 
 /** @defgroup CFEAPISBZeroCopy cFE Zero Copy APIs
@@ -544,9 +551,15 @@ CFE_Status_t CFE_SB_ReleaseMessageBuffer(CFE_SB_Buffer_t *BufPtr);
 **          internal buffer.  The "zero copy" interface can be used to improve
 **          performance in high-rate, high-volume software bus traffic.
 **
-**          In general, the "UpdateHeader" parameter should be passed as "true"
-**          if the message was newly constructed by the sender and is being sent
-**          for the first time.  When forwarding a message that originated from
+**          The IsOrigination parameter should be passed as "true" if the message was
+**          newly constructed by the sender and is being sent for the first time.  This
+**          enables the message origination actions as determined by the CFE MSG module,
+**          which may include (but not limited to):
+**            - Updating sequence number
+**            - Updating timestamp
+**            - Calcualating a CRC, checksum, or other message error control field
+**
+**          Conversely, when forwarding a message that originated from
 **          an external entity (e.g. messages passing through CI or SBN), the
 **          parameter should be passed as "false" to not overwrite existing data.
 **
@@ -567,15 +580,15 @@ CFE_Status_t CFE_SB_ReleaseMessageBuffer(CFE_SB_Buffer_t *BufPtr);
 **          -# This function will increment and apply the internally tracked
 **             sequence counter if set to do so.
 **
-** \param[in] BufPtr        A pointer to the buffer to be sent @nonnull.
-** \param[in] UpdateHeader  Update the headers of the message
+** \param[in] BufPtr         A pointer to the buffer to be sent @nonnull.
+** \param[in] IsOrigination  Update applicable header field(s) of a newly constructed message
 **
 ** \return Execution status, see \ref CFEReturnCodes
 ** \retval #CFE_SUCCESS         \copybrief CFE_SUCCESS
 ** \retval #CFE_SB_BAD_ARGUMENT \copybrief CFE_SB_BAD_ARGUMENT
 ** \retval #CFE_SB_MSG_TOO_BIG  \copybrief CFE_SB_MSG_TOO_BIG
 **/
-CFE_Status_t CFE_SB_TransmitBuffer(CFE_SB_Buffer_t *BufPtr, bool UpdateHeader);
+CFE_Status_t CFE_SB_TransmitBuffer(CFE_SB_Buffer_t *BufPtr, bool IsOrigination);
 
 /** @} */
 

modules/core_api/ut-stubs/src/cfe_msg_handlers.c

@@ -376,3 +376,35 @@ void UT_DefaultHandler_CFE_MSG_GetNextSequenceCount(void *UserObj, UT_EntryKey_t
 
     UT_Stub_SetReturnValue(FuncKey, return_value);
 }
+
+/*------------------------------------------------------------
+ *
+ * Default handler for CFE_MSG_OriginationAction coverage stub function
+ *
+ *------------------------------------------------------------*/
+void UT_DefaultHandler_CFE_MSG_OriginationAction(void *UserObj, UT_EntryKey_t FuncKey, const UT_StubContext_t *Context)
+{
+    bool *IsAcceptable = UT_Hook_GetArgValueByName(Context, "IsAcceptable", bool *);
+
+    /* by default just always return true -- a UT case that needs something else can override this handler */
+    if (IsAcceptable != NULL)
+    {
+        *IsAcceptable = true;
+    }
+}
+
+/*------------------------------------------------------------
+ *
+ * Default handler for CFE_MSG_VerificationAction coverage stub function
+ *
+ *------------------------------------------------------------*/
+void UT_DefaultHandler_CFE_MSG_VerificationAction(void *UserObj, UT_EntryKey_t FuncKey, const UT_StubContext_t *Context)
+{
+    bool *IsAcceptable = UT_Hook_GetArgValueByName(Context, "IsAcceptable", bool *);
+
+    /* by default just always return true -- a UT case that needs something else can override this handler */
+    if (IsAcceptable != NULL)
+    {
+        *IsAcceptable = true;
+    }
+}

modules/core_api/ut-stubs/src/cfe_msg_stubs.c

@@ -42,7 +42,9 @@ void UT_DefaultHandler_CFE_MSG_GetSubsystem(void *, UT_EntryKey_t, const UT_Stub
 void UT_DefaultHandler_CFE_MSG_GetSystem(void *, UT_EntryKey_t, const UT_StubContext_t *);
 void UT_DefaultHandler_CFE_MSG_GetType(void *, UT_EntryKey_t, const UT_StubContext_t *);
 void UT_DefaultHandler_CFE_MSG_GetTypeFromMsgId(void *, UT_EntryKey_t, const UT_StubContext_t *);
+void UT_DefaultHandler_CFE_MSG_OriginationAction(void *, UT_EntryKey_t, const UT_StubContext_t *);
 void UT_DefaultHandler_CFE_MSG_ValidateChecksum(void *, UT_EntryKey_t, const UT_StubContext_t *);
+void UT_DefaultHandler_CFE_MSG_VerificationAction(void *, UT_EntryKey_t, const UT_StubContext_t *);
 
 /*
  * ----------------------------------------------------
@@ -366,6 +368,24 @@ CFE_Status_t CFE_MSG_Init(CFE_MSG_Message_t *MsgPtr, CFE_SB_MsgId_t MsgId, CFE_M
     return UT_GenStub_GetReturnValue(CFE_MSG_Init, CFE_Status_t);
 }
 
+/*
+ * ----------------------------------------------------
+ * Generated stub function for CFE_MSG_OriginationAction()
+ * ----------------------------------------------------
+ */
+CFE_Status_t CFE_MSG_OriginationAction(CFE_MSG_Message_t *MsgPtr, size_t BufferSize, bool *IsAcceptable)
+{
+    UT_GenStub_SetupReturnBuffer(CFE_MSG_OriginationAction, CFE_Status_t);
+
+    UT_GenStub_AddParam(CFE_MSG_OriginationAction, CFE_MSG_Message_t *, MsgPtr);
+    UT_GenStub_AddParam(CFE_MSG_OriginationAction, size_t, BufferSize);
+    UT_GenStub_AddParam(CFE_MSG_OriginationAction, bool *, IsAcceptable);
+
+    UT_GenStub_Execute(CFE_MSG_OriginationAction, Basic, UT_DefaultHandler_CFE_MSG_OriginationAction);
+
+    return UT_GenStub_GetReturnValue(CFE_MSG_OriginationAction, CFE_Status_t);
+}
+
 /*
  * ----------------------------------------------------
  * Generated stub function for CFE_MSG_SetApId()
@@ -623,34 +643,35 @@ CFE_Status_t CFE_MSG_SetType(CFE_MSG_Message_t *MsgPtr, CFE_MSG_Type_t Type)
 
 /*
  * ----------------------------------------------------
- * Generated stub function for CFE_MSG_UpdateHeader()
+ * Generated stub function for CFE_MSG_ValidateChecksum()
  * ----------------------------------------------------
  */
-CFE_Status_t CFE_MSG_UpdateHeader(CFE_MSG_Message_t *MsgPtr, CFE_MSG_SequenceCount_t SeqCnt)
+CFE_Status_t CFE_MSG_ValidateChecksum(const CFE_MSG_Message_t *MsgPtr, bool *IsValid)
 {
-    UT_GenStub_SetupReturnBuffer(CFE_MSG_UpdateHeader, CFE_Status_t);
+    UT_GenStub_SetupReturnBuffer(CFE_MSG_ValidateChecksum, CFE_Status_t);
 
-    UT_GenStub_AddParam(CFE_MSG_UpdateHeader, CFE_MSG_Message_t *, MsgPtr);
-    UT_GenStub_AddParam(CFE_MSG_UpdateHeader, CFE_MSG_SequenceCount_t, SeqCnt);
+    UT_GenStub_AddParam(CFE_MSG_ValidateChecksum, const CFE_MSG_Message_t *, MsgPtr);
+    UT_GenStub_AddParam(CFE_MSG_ValidateChecksum, bool *, IsValid);
 
-    UT_GenStub_Execute(CFE_MSG_UpdateHeader, Basic, NULL);
+    UT_GenStub_Execute(CFE_MSG_ValidateChecksum, Basic, UT_DefaultHandler_CFE_MSG_ValidateChecksum);
 
-    return UT_GenStub_GetReturnValue(CFE_MSG_UpdateHeader, CFE_Status_t);
+    return UT_GenStub_GetReturnValue(CFE_MSG_ValidateChecksum, CFE_Status_t);
 }
 
 /*
  * ----------------------------------------------------
- * Generated stub function for CFE_MSG_ValidateChecksum()
+ * Generated stub function for CFE_MSG_VerificationAction()
  * ----------------------------------------------------
  */
-CFE_Status_t CFE_MSG_ValidateChecksum(const CFE_MSG_Message_t *MsgPtr, bool *IsValid)
+CFE_Status_t CFE_MSG_VerificationAction(const CFE_MSG_Message_t *MsgPtr, size_t BufferSize, bool *IsAcceptable)
 {
-    UT_GenStub_SetupReturnBuffer(CFE_MSG_ValidateChecksum, CFE_Status_t);
+    UT_GenStub_SetupReturnBuffer(CFE_MSG_VerificationAction, CFE_Status_t);
 
-    UT_GenStub_AddParam(CFE_MSG_ValidateChecksum, const CFE_MSG_Message_t *, MsgPtr);
-    UT_GenStub_AddParam(CFE_MSG_ValidateChecksum, bool *, IsValid);
+    UT_GenStub_AddParam(CFE_MSG_VerificationAction, const CFE_MSG_Message_t *, MsgPtr);
+    UT_GenStub_AddParam(CFE_MSG_VerificationAction, size_t, BufferSize);
+    UT_GenStub_AddParam(CFE_MSG_VerificationAction, bool *, IsAcceptable);
 
-    UT_GenStub_Execute(CFE_MSG_ValidateChecksum, Basic, UT_DefaultHandler_CFE_MSG_ValidateChecksum);
+    UT_GenStub_Execute(CFE_MSG_VerificationAction, Basic, UT_DefaultHandler_CFE_MSG_VerificationAction);
 
-    return UT_GenStub_GetReturnValue(CFE_MSG_ValidateChecksum, CFE_Status_t);
+    return UT_GenStub_GetReturnValue(CFE_MSG_VerificationAction, CFE_Status_t);
 }

modules/core_api/ut-stubs/src/cfe_sb_stubs.c

@@ -384,12 +384,12 @@ void CFE_SB_TimeStampMsg(CFE_MSG_Message_t *MsgPtr)
  * Generated stub function for CFE_SB_TransmitBuffer()
  * ----------------------------------------------------
  */
-CFE_Status_t CFE_SB_TransmitBuffer(CFE_SB_Buffer_t *BufPtr, bool UpdateHeader)
+CFE_Status_t CFE_SB_TransmitBuffer(CFE_SB_Buffer_t *BufPtr, bool IsOrigination)
 {
     UT_GenStub_SetupReturnBuffer(CFE_SB_TransmitBuffer, CFE_Status_t);
 
     UT_GenStub_AddParam(CFE_SB_TransmitBuffer, CFE_SB_Buffer_t *, BufPtr);
-    UT_GenStub_AddParam(CFE_SB_TransmitBuffer, bool, UpdateHeader);
+    UT_GenStub_AddParam(CFE_SB_TransmitBuffer, bool, IsOrigination);
 
     UT_GenStub_Execute(CFE_SB_TransmitBuffer, Basic, UT_DefaultHandler_CFE_SB_TransmitBuffer);
 
@@ -401,12 +401,12 @@ CFE_Status_t CFE_SB_TransmitBuffer(CFE_SB_Buffer_t *BufPtr, bool UpdateHeader)
  * Generated stub function for CFE_SB_TransmitMsg()
  * ----------------------------------------------------
  */
-CFE_Status_t CFE_SB_TransmitMsg(const CFE_MSG_Message_t *MsgPtr, bool UpdateHeader)
+CFE_Status_t CFE_SB_TransmitMsg(const CFE_MSG_Message_t *MsgPtr, bool IsOrigination)
 {
     UT_GenStub_SetupReturnBuffer(CFE_SB_TransmitMsg, CFE_Status_t);
 
     UT_GenStub_AddParam(CFE_SB_TransmitMsg, const CFE_MSG_Message_t *, MsgPtr);
-    UT_GenStub_AddParam(CFE_SB_TransmitMsg, bool, UpdateHeader);
+    UT_GenStub_AddParam(CFE_SB_TransmitMsg, bool, IsOrigination);
 
     UT_GenStub_Execute(CFE_SB_TransmitMsg, Basic, UT_DefaultHandler_CFE_SB_TransmitMsg);
 

modules/msg/CMakeLists.txt

@@ -18,7 +18,7 @@
 set(${DEP}_SRC
     fsw/src/cfe_msg_ccsdspri.c
     fsw/src/cfe_msg_init.c
-    fsw/src/cfe_msg_verify.c
+    fsw/src/cfe_msg_integrity.c
     fsw/src/cfe_msg_msgid_shared.c
     fsw/src/cfe_msg_sechdr_checksum.c
     fsw/src/cfe_msg_sechdr_fc.c

modules/msg/fsw/src/cfe_msg_init.c

@@ -53,34 +53,3 @@ CFE_Status_t CFE_MSG_Init(CFE_MSG_Message_t *MsgPtr, CFE_SB_MsgId_t MsgId, CFE_M
 
     return status;
 }
-
-/*----------------------------------------------------------------
- *
- * Implemented per public API
- * See description in header file for argument/return detail
- *
- *-----------------------------------------------------------------*/
-CFE_Status_t CFE_MSG_UpdateHeader(CFE_MSG_Message_t *MsgPtr, CFE_MSG_SequenceCount_t SeqCnt)
-{
-    if (MsgPtr == NULL)
-    {
-        return CFE_MSG_BAD_ARGUMENT;
-    }
-
-    /* Sequence count is in the basic CCSDS Primary Hdr, so all msgs have it */
-    CFE_MSG_SetSequenceCount(MsgPtr, SeqCnt);
-
-    /*
-     * TLM packets have a timestamp in the secondary header.
-     * This may fail if this is not a TLM packet (that is OK)
-     */
-    CFE_MSG_SetMsgTime(MsgPtr, CFE_TIME_GetTime());
-
-    /*
-     * CMD packets have a checksum in the secondary header.
-     * This may fail if this is not a CMD packet (that is OK)
-     */
-    CFE_MSG_GenerateChecksum(MsgPtr);
-
-    return CFE_SUCCESS;
-}