🐛 [BUG] - SA REKEY, START, and EXPIRE Return Inconsistent Success Codes

[VissaMoutafis]

Description

Bug Report: SA REKEY, START, and EXPIRE Return Inconsistent Success Codes

Summary

Field	Value
Product	NASA CryptoLib (SDLS Protocol Implementation)
Version	1.4.2
Component	src/sa/internal/sa_interface_inmemory.template.c — sa_start, sa_rekey, sa_expire
Issue	SA management functions return CRYPTO_LIB_SUCCESS even when the operation fails
Impact	Callers cannot distinguish successful SA state transitions from failed ones; silent failures mask protocol violations

---

Description

The sa_start, sa_rekey, and sa_expire functions unconditionally return CRYPTO_LIB_SUCCESS regardless of whether the requested operation actually succeeded. When an SA-level precondition fails (e.g., the SA is not in the expected state, or the SPI does not exist), the code prints a debug message and falls through to the else branch, but does not set an error return code.

This means that a caller issuing a REKEY on an SA that is in OPERATIONAL state (instead of the required UNKEYED state) will receive a success response, even though no state change occurred. This violates the SDLS-EP specification (CCSDS 355.1-B-1), which mandates specific precondition states for each SA management command.

Affected Code

sa_start (line 1023)

The function checks if sa[spi].sa_state == SA_KEYED (line 1050), but the else branch (line 1149) only prints a debug message. The function always returns CRYPTO_LIB_SUCCESS at line 1167, even when:

• The SPI does not exist (spi >= NUM_SA)
• The SA is not in KEYED state

static int32_t sa_start(TC_t *tc_frame)
{
    // ...
    if (spi < NUM_SA)
    {
        if (sa[spi].sa_state == SA_KEYED)
        {
            // ... transition to OPERATIONAL ...
        }
        else
        {
            // Only prints debug, does NOT set error status
        }
    }
    else
    {
        // Only prints debug, does NOT set error status
    }

    return CRYPTO_LIB_SUCCESS;  // Always returns success
}

sa_rekey (line 1261)

Same pattern. Checks for SA_UNKEYED state (line 1296), but the else branches (lines 1335, 1343) do not set an error code. Returns CRYPTO_LIB_SUCCESS at line 1354 unconditionally.

sa_expire (line 1361)

Same pattern. Checks for SA_KEYED state (line 1393), but the else branches (lines 1400, 1407) do not set an error code. Returns CRYPTO_LIB_SUCCESS at line 1414 unconditionally.

Contrast with sa_stop (line 1174)

For comparison, sa_stop correctly returns CRYPTO_LIB_ERR_SDLS_EP_WRONG_SPI when the control SPI matches (line 1196), demonstrating that the pattern of returning error codes on precondition failure is used elsewhere and was likely intended for these functions as well.

Example Mismatch Scenarios Observed

The following SA operations returned CRYPTO_LIB_SUCCESS when they should have returned an error:

Operation	SA State	Expected State
SA START	OPERATIONAL	KEYED
SA START	UNKEYED	KEYED
SA START	(non-existent SPI)	N/A
SA REKEY	OPERATIONAL	UNKEYED
SA REKEY	KEYED	UNKEYED
SA REKEY	(non-existent SPI)	N/A
SA EXPIRE	OPERATIONAL	KEYED
SA EXPIRE	UNKEYED	KEYED
SA EXPIRE	(non-existent SPI)	N/A

Suggested Fix

Each else branch should set status to an appropriate error code before falling through to the return statement. For example:

if (sa[spi].sa_state == SA_KEYED)
{
    // ... perform operation ...
}
else
{
    status = CRYPTO_LIB_ERR_SA_NOT_OPERATIONAL;  // or a more specific error
}

And the SPI existence check should similarly return an error:

if (spi < NUM_SA)
{
    // ...
}
else
{
    status = CRYPTO_LIB_ERR_SPI_INDEX_OOB;
}

return status;  // Instead of always returning CRYPTO_LIB_SUCCESS

---

Discovered using the StratoFuzz protocol fuzzing framework.

Branch Name

No response

Reproduction steps

1. Go to '...'
2. Click on '....'
3. Scroll down to '....'
4. See error

Screenshots

![DESCRIPTION](LINK.png)

Logs

OS

Linux