Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix #2467, message integrity API #2468

Merged
merged 1 commit into from
Dec 5, 2023
Merged

Fix #2467, message integrity API #2468

merged 1 commit into from
Dec 5, 2023

Conversation

jphickey
Copy link
Contributor

Checklist (Please check before submitting)

Describe the contribution
Adds two new APIs to the MSG module related to integrity:

CFE_MSG_OriginationAction
CFE_MSG_VerificationAction

These should perform all calculations/updates on origination and all verifcation/checks at termination of the message path, respectively.

The specific set of actions depends on how the user has implemented their particular MSG module. By default, the traditional CFE encapsulation sets a checksum on commands and a timestamp on TLM, and no verification at the receiver.

Fixes #2467

Testing performed
Execute SB bulk message transfer performance testing.

Expected behavior changes
Consistent MSG origination and verification actions.

System(s) tested on
Debian

Additional context
Note that if apps are not changed, this means that the TLM timestamp may be set twice - once by the app itself, and once by MSG when OriginationAction() runs. This creates a very minor performance impact, about 0.7% slower message rates in the bulk test. This could be regained by fixing the apps eventually.

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Nov 17, 2023
github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 17, 2023
View reviewed changes
modules/core_api/ut-stubs/src/cfe_msg_stubs.c
* ----------------------------------------------------
*/
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)

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
modules/core_api/ut-stubs/src/cfe_sb_stubs.c
@@ -401,12 +401,12 @@
* 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)

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
modules/core_api/ut-stubs/src/cfe_sb_stubs.c
@@ -384,12 +384,12 @@
* 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)

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
modules/core_api/ut-stubs/src/cfe_msg_stubs.c
* Generated stub function for CFE_MSG_OriginationAction()
* ----------------------------------------------------
*/
CFE_Status_t CFE_MSG_OriginationAction(CFE_MSG_Message_t *MsgPtr, size_t BufferSize, bool *IsAcceptable)

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
modules/msg/fsw/src/cfe_msg_integrity.c
* See description in header file for argument/return detail
*
*-----------------------------------------------------------------*/
CFE_Status_t CFE_MSG_OriginationAction(CFE_MSG_Message_t *MsgPtr, size_t BufferSize, bool *IsAcceptable)

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
modules/msg/ut-coverage/test_cfe_msg_integrity.c
/*
* Test MSG integrity origination action(s)
*/
void Test_MSG_OriginationAction(void)

Check notice

Code scanning / CodeQL

Long function without assertion Note test

All functions of more than 10 lines should have at least one assertion.
modules/msg/ut-coverage/test_cfe_msg_integrity.c
/*
* Test MSG integrity verification action(s)
*/
void Test_MSG_VerificationAction(void)

Check notice

Code scanning / CodeQL

Long function without assertion Note test

All functions of more than 10 lines should have at least one assertion.
modules/core_api/ut-stubs/src/cfe_sb_stubs.c
@@ -401,12 +401,12 @@
* 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)

Check notice

Code scanning / CodeQL

Function too long Note

CFE_SB_TransmitMsg has too many lines (98, while 60 are allowed).
@jphickey jphickey force-pushed the fix-2467-msg-integrity branch 3 times, most recently from 464d965 to a3b7477 Compare November 17, 2023 21:38
@jphickey
Fix nasa#2467, message integrity API
4a4ada4 
Adds two new APIs to the MSG module related to integrity:

CFE_MSG_OriginationAction
CFE_MSG_VerificationAction

These should perform all calculations/updates on origination and
all verifcation/checks at termination of the message path, respectively.

The specific set of actions depends on how the user has implemented
their particular MSG module.  By default, the traditional CFE
encapsulation sets a checksum on commands and a timestamp on TLM,
and no verification at the receiver.
github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 18, 2023
View reviewed changes
modules/sb/fsw/src/cfe_sb_api.c
@@ -1287,7 +1288,7 @@
* See description in header file for argument/return detail
*
*-----------------------------------------------------------------*/
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)

Check notice

Code scanning / CodeQL

Function too long Note

CFE_SB_TransmitMsg has too many lines (98, while 60 are allowed).
modules/sb/fsw/src/cfe_sb_api.c
@@ -1287,7 +1288,7 @@
* See description in header file for argument/return detail
*
*-----------------------------------------------------------------*/
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)

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
modules/sb/fsw/src/cfe_sb_api.c
@@ -2118,7 +2123,7 @@
* See description in header file for argument/return detail
*
*-----------------------------------------------------------------*/
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)

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
@dzbaker dzbaker added CCB:Approved Indicates code review and approval by community CCB and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Nov 30, 2023
dzbaker added a commit that referenced this pull request Dec 5, 2023
@dzbaker
Merge pull request #2468 from jphickey:fix-2467-msg-integrity
9ea5125 
Fix #2467, message integrity API
@dzbaker dzbaker mentioned this pull request Dec 5, 2023
Merged
2 tasks
dzbaker added a commit to nasa/cFS that referenced this pull request Dec 5, 2023
@dzbaker
IC: Caelum-rc4+dev65
6a330c5 
*Combines:*

to_lab v2.5.0-rc4+dev71
ci_lab v2.5.0-rc4+dev77
cFE v7.0.0-rc4+dev427
PSP v1.6.0-rc4+dev102
osal v6.0.0-rc4+dev243

**Includes:**

*to_lab*
- nasa/to_lab#173

*ci_lab*
- nasa/ci_lab#157
- nasa/ci_lab#159
- nasa/ci_lab#161

*cFE*
- nasa/cFE#2411
- nasa/cFE#2409
- nasa/cFE#2373
- nasa/cFE#2466
- nasa/cFE#2468
- nasa/cFE#2470

*PSP*
- nasa/PSP#421

*osal*
- nasa/osal#1413

Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
Co-authored by: Isaac Rowe <irowebbn@users.noreply.github.com>
Co-authored by: Jacob Hageman <skliper@users.noreply.github.com>
Co-authored by: Frank Kühndel <frank-kue@users.noreply.github.com>
@dzbaker dzbaker mentioned this pull request Dec 5, 2023
Merged
2 tasks
dzbaker added a commit to nasa/cFS that referenced this pull request Dec 5, 2023
@dzbaker
IC: Caelum-rc4+dev65
eb4c641 
*Combines:*

to_lab v2.5.0-rc4+dev71
ci_lab v2.5.0-rc4+dev77
cFE v7.0.0-rc4+dev424
PSP v1.6.0-rc4+dev102
osal v6.0.0-rc4+dev243

**Includes:**

*to_lab*
- nasa/to_lab#173

*ci_lab*
- nasa/ci_lab#157
- nasa/ci_lab#159
- nasa/ci_lab#161

*cFE*
- nasa/cFE#2409
- nasa/cFE#2373
- nasa/cFE#2466
- nasa/cFE#2468
- nasa/cFE#2470

*PSP*
- nasa/PSP#421

*osal*
- nasa/osal#1413

Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
Co-authored by: Isaac Rowe <irowebbn@users.noreply.github.com>
Co-authored by: Jacob Hageman <skliper@users.noreply.github.com>
Co-authored by: Frank Kühndel <frank-kue@users.noreply.github.com>
@dzbaker dzbaker merged commit 10d4c34 into nasa:main Dec 5, 2023
22 checks passed
dzbaker added a commit to nasa/cFS that referenced this pull request Dec 5, 2023
@dzbaker
IC: Caelum-rc4+dev65
5e4df8a 
*Combines:*

to_lab v2.5.0-rc4+dev71
ci_lab v2.5.0-rc4+dev77
cFE v7.0.0-rc4+dev424
PSP v1.6.0-rc4+dev102
osal v6.0.0-rc4+dev243

**Includes:**

*to_lab*
- nasa/to_lab#173

*ci_lab*
- nasa/ci_lab#157
- nasa/ci_lab#159
- nasa/ci_lab#161

*cFE*
- nasa/cFE#2409
- nasa/cFE#2373
- nasa/cFE#2466
- nasa/cFE#2468
- nasa/cFE#2470

*PSP*
- nasa/PSP#421

*osal*
- nasa/osal#1413

Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
Co-authored by: Isaac Rowe <irowebbn@users.noreply.github.com>
Co-authored by: Jacob Hageman <skliper@users.noreply.github.com>
Co-authored by: Frank Kühndel <frank-kue@users.noreply.github.com>
@jphickey jphickey mentioned this pull request Dec 12, 2023
Closed
@jphickey jphickey deleted the fix-2467-msg-integrity branch December 13, 2023 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CCB:Approved Indicates code review and approval by community CCB
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Generalize/formalize message integrity checking options in MSG API
2 participants
@jphickey @dzbaker