search

nasa / CryptoLib

Provide a software-only solution using the CCSDS Space Data Link Security Protocol - Extended Procedures (SDLS-EP) to secure communications between a spacecraft running the core Flight System (cFS) and a ground station.
Other
70 stars 28 forks source link

Don't skip Handle_Increment if Do_Encrypt_NONPLAINTEXT succeeds #280

Closed Niautanor closed 1 month ago

Niautanor commented 1 month ago

Currently, Crypto_TM_Do_Encrypt_Handle_Increment only gets called if Crypto_TM_Do_Encrypt_NONPLAINTEXT (/ _AEAD_Logic) fails which leads to the IV and ARSN being stuck at the same value if everything goes right.

codecov-commenter commented 1 month ago

Codecov Report

Attention: Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.

Project coverage is 83.50%. Comparing base (ae02d55) to head (5c905e6).

Files Patch % Lines
src/core/crypto_tm.c 0.00% 0 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #280 +/- ## ========================================== + Coverage 83.29% 83.50% +0.21% ========================================== Files 39 39 Lines 11049 11054 +5 Branches 832 832 ========================================== + Hits 9203 9231 +28 + Misses 1544 1518 -26 - Partials 302 305 +3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Niautanor commented 1 month ago

I think this probably also deserves a unit test to make sure this kind of thing doesn't happen again. I think that should probably go in test/unit/ut_tm_apply.c but I'm not sure if I should add a new test for that or just add it to one of the existing ones (e.g. AEAD_AES_GCM_BITMASK_1)

rjbrown2 commented 1 month ago

I believe you are correct with the assumption of needing a test to catch this. If you would like to add this to AEAD_AES_GCM_BITMASK_1, within test/unit/ut_tm_apply.c, that would be great.

rjbrown2 commented 1 month ago

Thank you for adding the test. Additional changes will be coming in another issue to address my question. Thanks again!