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.

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

Inmemory Clear Mode SA Encryption is Off By 2 Bytes (used in unit tests) #139

Closed
IbraheemYSaleh opened this issue Jan 6, 2023 · 3 comments · Fixed by #173
Closed

Inmemory Clear Mode SA Encryption is Off By 2 Bytes (used in unit tests) #139

IbraheemYSaleh opened this issue Jan 6, 2023 · 3 comments · Fixed by #173
Assignees
Labels
bug Something isn't working kmc NASA JPL KMC

Comments

@IbraheemYSaleh
Copy link
Contributor

IbraheemYSaleh commented Jan 6, 2023

I'm trying to use the inmemory/libgcrypt configuration for some down stream unit tests for a JPL application and it is wrongly dropping 2 bytes (likely an FECF length miscalculation or something related).

INPROGRESS DIAGNOSTIC "TcWrap is in progress."
Managed Parameter:
         tfvn: 0         scid: 3         vcid: 0         has_fecf: 1     has_segmentation_headers: 0
         max_tc_frame_size: 1024
Managed Parameter:
         tfvn: 0         scid: 3         vcid: 1         has_fecf: 1     has_segmentation_headers: 0
         max_tc_frame_size: 1024
Managed Parameter:
         tfvn: 0         scid: 3         vcid: 2         has_fecf: 1     has_segmentation_headers: 0
         max_tc_frame_size: 1024
Managed Parameter:
         tfvn: 0         scid: 3         vcid: 3         has_fecf: 1     has_segmentation_headers: 1
         max_tc_frame_size: 1024
Gcrypt Version: 1.9.4   ERROR: gcrypt version mismatch!
Crypto Lib Intialized.  Version 1.0.3.0
INPROGRESS DIAGNOSTIC "Wrapping records from CMD_PKT file (cdh_nop.cmdpkt)."
INPROGRESS DIAGNOSTIC "No. of bytes in packet not equal to LENGTH arg (2), line 48."

----- Crypto_TC_ApplySecurity START -----
32 TF Bytes received
DEBUG - 2003001F00000100011880D2C9000E197F0B001B0004000400003040D95E0000
Printed 32 bytes
Valid operational SA found at index 1.
DEBUG - Printing SA Entry for current frame.
SA status:
         spi   = 1
         sa_state   = 0x3
         est        = 0x0
         ast        = 0x0
         shivf_len  = 0
         shsnf_len  = 2
         shplf_len  = 0
         stmacf_len = 0
         ecs_len    = 0
         ekid       = 1
         ek_ref     = (null)
         akid       = 1
         ak_ref     = (null)
         iv_len     = 0
         acs_len    = 0
         abm_len    = 0
         arsn_len    = 2
         arsn        = 0000
         arsnw_len   = 1
         arsnw       = 5
Creating a TC - CLEAR!
DEBUG - Total TC Buffer to be malloced is: 34 bytes
        len of TF        = 31
        segment hdr len  = 0
        spi len          = 2
        shivf_len        = 0
        iv_len   = 0
        shsnf_len        = 2
        shplf len        = 0
        arsn_len         = 2
        stmacf_len       = 0
Printing updated TF Header:
        2003002100
        Length set to 0x21
Calcing FECF over 32 bytes
Printing new TC Frame:
        200300210000010000000100011880D2C9000E197F0B001B00040004000030400000
        The returned length is: 33
----- Crypto_TC_ApplySecurity END -----

Notice how, according to the debug statement, the end of the frame I pass in is 40D95E0000 (0000 are placeholder bytes for the FECF)
but the resulting Clear Mode frame ends with 400000 ... Some length miscalculation has resulted in D95E being zero'd out (I am not asking CryptoLib to do my FECF calculation, the downstream application handles that). That shouldn't happen, D95E should be part of the original frame (I should get 40D95E0000 just like I passed in) since the managed parameter for this says has_fecf: 1.

@IbraheemYSaleh IbraheemYSaleh added the bug Something isn't working label Jan 6, 2023
@jlucas9 jlucas9 added the kmc NASA JPL KMC label Jun 21, 2023
@dccutrig
Copy link
Contributor

Taking initial look at this, The finished packet is indeed 2 bytes longer (as it should be - it adds an SPI). But, it looks like the SPI is being improperly added which is shifting things rightwards:

2003001F00 0001 0001 1880D2C9000E197F0B001B0004000400003040D95E0000

2003002100 0001 0000 0001 0001 1880D2C9000E197F0B001B00040004000030400000

@dccutrig
Copy link
Contributor

@jlucas9 I have completed a fix for this. The ARSN field length was not checked / added to plaintext overall length.

Needs a little more discussion:

Should Clearmode support the presence of unused fields? e.g. a MAC field that exists, but is zero'ed out? I lean towards not necessarily, because we typically used the header field length as key logic in other areas. However - we typically allow permissive use of fields and try not to restrict users for any reason.

@dccutrig dccutrig self-assigned this Jun 22, 2023
@dccutrig
Copy link
Contributor

Per Docs, clear mode should be supportable. Security header and trailer field lengths are kept constant in this configuration - so presumably they can exist. Updated logic for simplicity and to support this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working kmc NASA JPL KMC
Projects
Archived in project
3 participants