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

IvType - Revisit Reasoning #218

Closed jlucas9 closed 8 months ago

dccutrig commented 8 months ago

Main crypto.h header file... there is an IVType enum... revisit why this is here. (In dev, not in main)

dccutrig commented 8 months ago

General notes:

Initially this appears to only affect TC Apply functionality, which makes sense as it seems to be KMC related (which only makes use of ground side command encryption at this time). There are several references within crypto_tc_validate_sa which attempt to provide some sanity before any reference to null_iv magic happens.

from crypto_config.c:

int32_t Crypto_Config_CryptoLib(uint8_t key_type, uint8_t mc_type, uint8_t sa_type, uint8_t cryptography_type, 
                                uint8_t iv_type, uint8_t crypto_create_fecf, uint8_t process_sdls_pdus,
                                uint8_t has_pus_hdr, uint8_t ignore_sa_state, uint8_t ignore_anti_replay,
                                uint8_t unique_sa_per_mapid, uint8_t crypto_check_fecf, uint8_t vcid_bitmask, uint8_t crypto_increment_nontransmitted_iv)

IvType enum from crypto_config_structs.h:

IV_INTERNAL

IV_CRYPTO_MODULE

From crypto_tc Crypto_TC_ApplySecurity:

        if (crypto_config.iv_type == IV_INTERNAL)
        {
            // Start index from the transmitted portion
            for (i = sa_ptr->iv_len - sa_ptr->shivf_len; i < sa_ptr->iv_len; i++)
            {
                *(p_new_enc_frame + index) = *(sa_ptr->iv + i);
                index++;
            }
        }
        // IV is NULL / IV_CRYPTO_MODULE
        else
        {
            // Transmitted length > 0, AND using KMC_CRYPTO
            if ((sa_ptr->shivf_len > 0) && (crypto_config.cryptography_type == CRYPTOGRAPHY_TYPE_KMCCRYPTO))
            {
                index += sa_ptr->iv_len - (sa_ptr->iv_len - sa_ptr->shivf_len);
            }
            else if (sa_ptr->shivf_len == 0)
            {
                // IV isn't being used, so don't care if it's Null
            }
            else
            {
                status = CRYPTO_LIB_ERR_NULL_IV;
                mc_if->mc_log(status);
                return status;
            }
        }
dccutrig commented 8 months ago

Relevant commit in dev appears here:

https://github.com/nasa/CryptoLib/commit/98fcb56704c331457f26f184d10ceb81b0a1540d

jlucas9 commented 8 months ago

Current unit tests are functional. As they expand to cover these cases we should open a new issue and reassess.