Reworked Cmac hash computation for lower memory consumption #677
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
By allowing the Cmac hash computations to be done on a sequence of blocks, there is no need anymore to memcpy the entire message to the stack. This lowers stack usage by >200 bytes for some frequently used code paths.
mluis1
requested changes
Feb 13, 2019
mluis1
reviewed
Feb 13, 2019
|
That's clear and I understand why you would not want to change the API. But for this particular optimization, providing a list of buffers down to the point where the actual hash is calculated, seemed the best option to us. Every extra copy of the packet is another 1/4 kb of memory lost. You could add an optional function, inlining the concatenation for implementations that do not (yet) support hashing in chunks: src/mac/secure-element.h Or something along those lines. This way there is no urgent need to change existing implementations, while allowing the LoRaMacCrypto code to compute hashes efficiently where possible. |
mluis1
reviewed
Feb 13, 2019
There was a problem hiding this comment.
Please find below a patch that fixes the coding conventions.
I have also made some cleanup.
Fixed an issue reported some time ago which concerns ComputeCmac function that was declared with the same name twice which was confusing.
I have also kept the SecureElementComputeAesCmac API untouched (backwards compatibility).
I have added a pre-processing directive that enables/disables the new SecureElementComputeAesCmacBlocks API.
The new API makes us gain 200 bytes in RAM usage but, it also increases the Flash memory usage by 24 bytes.
Patch file:
diff --git a/src/mac/LoRaMacCrypto.c b/src/mac/LoRaMacCrypto.c
index 0b21c6c0..14a1cb97 100644
--- a/src/mac/LoRaMacCrypto.c
+++ b/src/mac/LoRaMacCrypto.c
@@ -363,49 +363,6 @@ static LoRaMacCryptoStatus_t FOptsEncrypt( uint16_t size, uint32_t address, uint
return LORAMAC_CRYPTO_SUCCESS;
}
-/*
- * Computes cmac.
- *
- * cmac = aes128_cmac(keyID, msg)
- *
- * \param[IN] msg - Message to compute the integrity code
- * \param[IN] len - Length of message
- * \param[IN] keyID - Key identifier
- * \param[OUT] cmac - Computed cmac
- * \retval - Status of the operation
- */
-static LoRaMacCryptoStatus_t ComputeCmac( uint8_t* msg, uint16_t len, KeyIdentifier_t keyID, uint32_t* cmac )
-{
- struct se_block part;
- part.buffer = msg;
- part.size = len;
- if( SecureElementComputeAesCmacParts( &part, 1, keyID, cmac ) != SECURE_ELEMENT_SUCCESS )
- {
- return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
- }
-
- return LORAMAC_CRYPTO_SUCCESS;
-}
-
-/*!
- * Verifies cmac
- *
- * \param[IN] msg - Message to compute the integrity code
- * \param[IN] len - Length of message
- * \param[in] expectedCmac - Expected cmac
- * \param[IN] keyID - Key identifier to determine the AES key to be used
- * \retval - Status of the operation
- */
-static LoRaMacCryptoStatus_t VerifyCmac( uint8_t* msg, uint16_t len, KeyIdentifier_t keyID, uint32_t expectedcmac )
-{
- if( SecureElementVerifyAesCmac( msg, len, expectedcmac, keyID ) != SECURE_ELEMENT_SUCCESS )
- {
- return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
- }
-
- return LORAMAC_CRYPTO_SUCCESS;
-}
-
/*
* Prepares B0 block for cmac computation.
*
@@ -497,22 +454,37 @@ static LoRaMacCryptoStatus_t ComputeCmacB0( uint8_t* msg, uint16_t len, KeyIdent
return LORAMAC_CRYPTO_ERROR_BUF_SIZE;
}
- uint8_t micBuff[MIC_BLOCK_BX_SIZE];
+#if( USE_CMAC_BLOCKS_API == 0 )
+ uint8_t micBuff[CRYPTO_BUFFER_SIZE];
+ memset1( micBuff, 0, CRYPTO_BUFFER_SIZE );
// Initialize the first Block
PrepareB0( len, keyID, isAck, dir, devAddr, fCnt, micBuff );
- struct se_block parts[2];
- parts[0].buffer = micBuff;
- parts[0].size = sizeof( micBuff );
- parts[1].buffer = msg;
- parts[1].size = len;
+ // Copy the given data to the mic computation buffer
+ memcpy1( ( micBuff + MIC_BLOCK_BX_SIZE ), msg, len );
- if( SecureElementComputeAesCmacParts( parts, 2, keyID, cmac ) != SECURE_ELEMENT_SUCCESS )
+ if( SecureElementComputeAesCmac( micBuff, ( len + MIC_BLOCK_BX_SIZE ), keyID, cmac ) != SECURE_ELEMENT_SUCCESS )
{
return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
}
+#else
+ uint8_t micBuff[MIC_BLOCK_BX_SIZE];
+ // Initialize the first Block
+ PrepareB0( len, keyID, isAck, dir, devAddr, fCnt, micBuff );
+
+ SecureElementBlock_t blocks[2];
+ blocks[0].Buffer = micBuff;
+ blocks[0].Size = sizeof( micBuff );
+ blocks[1].Buffer = msg;
+ blocks[1].Size = len;
+
+ if( SecureElementComputeAesCmacBlocks( blocks, 2, keyID, cmac ) != SECURE_ELEMENT_SUCCESS )
+ {
+ return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
+ }
+#endif
return LORAMAC_CRYPTO_SUCCESS;
}
@@ -647,30 +619,43 @@ static LoRaMacCryptoStatus_t ComputeCmacB1( uint8_t* msg, uint16_t len, KeyIdent
return LORAMAC_CRYPTO_ERROR_BUF_SIZE;
}
- uint8_t micBuff[MIC_BLOCK_BX_SIZE];
+#if( USE_CMAC_BLOCKS_API == 0 )
+ uint8_t micBuff[CRYPTO_BUFFER_SIZE];
+ memset1( micBuff, 0, CRYPTO_BUFFER_SIZE );
// Initialize the first Block
PrepareB1( len, keyID, isAck, txDr, txCh, devAddr, fCntUp, micBuff );
- struct se_block parts[2];
- parts[0].buffer = micBuff;
- parts[0].size = sizeof( micBuff );
- parts[1].buffer = msg;
- parts[1].size = len;
+ // Copy the given data to the mic computation buffer
+ memcpy1( ( micBuff + MIC_BLOCK_BX_SIZE ), msg, len );
- if( SecureElementComputeAesCmacParts( parts, 2, keyID, cmac ) != SECURE_ELEMENT_SUCCESS )
+ if( SecureElementComputeAesCmac( micBuff, ( len + MIC_BLOCK_BX_SIZE ), keyID, cmac ) != SECURE_ELEMENT_SUCCESS )
{
return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
}
+#else
+ uint8_t micBuff[MIC_BLOCK_BX_SIZE];
+ // Initialize the first Block
+ PrepareB1( len, keyID, isAck, txDr, txCh, devAddr, fCntUp, micBuff );
+
+ SecureElementBlock_t blocks[2];
+ blocks[0].Buffer = micBuff;
+ blocks[0].Size = sizeof( micBuff );
+ blocks[1].Buffer = msg;
+ blocks[1].Size = len;
+
+ if( SecureElementComputeAesCmacBlocks( blocks, 2, keyID, cmac ) != SECURE_ELEMENT_SUCCESS )
+ {
+ return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
+ }
+#endif
return LORAMAC_CRYPTO_SUCCESS;
}
/*
* Gets security item from list.
*
- * cmac = aes128_cmac(keyID, B0 | msg)
- *
* \param[IN] addrID - Address identifier
* \param[OUT] keyItem - Key item reference
* \retval - Status of the operation
@@ -916,7 +901,7 @@ static void UpdateFCntDown( FCntIdentifier_t fCntID, uint32_t currentDown )
/*!
* Resets the frame counters
*/
-void ResetFCnts( void )
+static void ResetFCnts( void )
{
CryptoCtx.NvmCtx->FCntUp = 0;
@@ -1051,10 +1036,9 @@ LoRaMacCryptoStatus_t LoRaMacCryptoPrepareJoinRequest( LoRaMacMessageJoinRequest
}
// Compute mic
- retval = ComputeCmac( macMsg->Buffer, ( LORAMAC_JOIN_REQ_MSG_SIZE - LORAMAC_MIC_FIELD_SIZE ), micComputationKeyID, &macMsg->MIC );
- if( retval != LORAMAC_CRYPTO_SUCCESS )
+ if( SecureElementComputeAesCmac( macMsg->Buffer, ( LORAMAC_JOIN_REQ_MSG_SIZE - LORAMAC_MIC_FIELD_SIZE ), micComputationKeyID, &macMsg->MIC ) != SECURE_ELEMENT_SUCCESS )
{
- return retval;
+ return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
}
// Reserialize message to add the MIC
@@ -1087,10 +1071,9 @@ LoRaMacCryptoStatus_t LoRaMacCryptoPrepareReJoinType1( LoRaMacMessageReJoinType1
// Compute mic
// cmac = aes128_cmac(JSIntKey, MHDR | RejoinType | JoinEUI| DevEUI | RJcount1)
- LoRaMacCryptoStatus_t retval = ComputeCmac( macMsg->Buffer, ( LORAMAC_RE_JOIN_1_MSG_SIZE - LORAMAC_MIC_FIELD_SIZE ), J_S_INT_KEY, &macMsg->MIC );
- if( retval != LORAMAC_CRYPTO_SUCCESS )
+ if( SecureElementComputeAesCmac( macMsg->Buffer, ( LORAMAC_RE_JOIN_1_MSG_SIZE - LORAMAC_MIC_FIELD_SIZE ), J_S_INT_KEY, &macMsg->MIC ) != SECURE_ELEMENT_SUCCESS )
{
- return retval;
+ return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
}
// Reserialize message to add the MIC
@@ -1127,10 +1110,9 @@ LoRaMacCryptoStatus_t LoRaMacCryptoPrepareReJoinType0or2( LoRaMacMessageReJoinTy
// Compute mic
// cmac = aes128_cmac(SNwkSIntKey, MHDR | Rejoin Type | NetID | DevEUI | RJcount0)
- LoRaMacCryptoStatus_t retval = ComputeCmac( macMsg->Buffer, ( LORAMAC_RE_JOIN_0_2_MSG_SIZE - LORAMAC_MIC_FIELD_SIZE ), S_NWK_S_INT_KEY, &macMsg->MIC );
- if( retval != LORAMAC_CRYPTO_SUCCESS )
+ if( SecureElementComputeAesCmac( macMsg->Buffer, ( LORAMAC_RE_JOIN_0_2_MSG_SIZE - LORAMAC_MIC_FIELD_SIZE ), S_NWK_S_INT_KEY, &macMsg->MIC ) != SECURE_ELEMENT_SUCCESS )
{
- return retval;
+ return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
}
// Reserialize message to add the MIC
@@ -1213,10 +1195,9 @@ LoRaMacCryptoStatus_t LoRaMacCryptoHandleJoinAccept( JoinReqIdentifier_t joinReq
{
// For legacy mode :
// cmac = aes128_cmac(NwkKey, MHDR | JoinNonce | NetID | DevAddr | DLSettings | RxDelay | CFList | CFListType)
- retval = VerifyCmac( macMsg->Buffer, ( macMsg->BufSize - LORAMAC_MIC_FIELD_SIZE ), micComputationKeyID, macMsg->MIC );
- if( retval != LORAMAC_CRYPTO_SUCCESS )
+ if( SecureElementVerifyAesCmac( macMsg->Buffer, ( macMsg->BufSize - LORAMAC_MIC_FIELD_SIZE ), macMsg->MIC, micComputationKeyID ) != SECURE_ELEMENT_SUCCESS )
{
- return retval;
+ return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
}
}
else
@@ -1236,10 +1217,9 @@ LoRaMacCryptoStatus_t LoRaMacCryptoHandleJoinAccept( JoinReqIdentifier_t joinReq
procBuffer[bufItr++] = macMsg->MHDR.Value;
- retval = VerifyCmac( procBuffer, ( macMsg->BufSize + micComputationOffset - LORAMAC_MHDR_FIELD_SIZE - LORAMAC_MIC_FIELD_SIZE ), micComputationKeyID, macMsg->MIC );
- if( retval != LORAMAC_CRYPTO_SUCCESS )
+ if( SecureElementVerifyAesCmac( macMsg->Buffer, ( macMsg->BufSize + micComputationOffset - LORAMAC_MHDR_FIELD_SIZE - LORAMAC_MIC_FIELD_SIZE ), macMsg->MIC, micComputationKeyID ) != SECURE_ELEMENT_SUCCESS )
{
- return retval;
+ return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
}
// Check if the JoinNonce is greater as the previous one
diff --git a/src/mac/secure-element.h b/src/mac/secure-element.h
index 9d8c391b..28de3635 100644
--- a/src/mac/secure-element.h
+++ b/src/mac/secure-element.h
@@ -37,6 +37,11 @@
#include <stdint.h>
#include "LoRaMacCrypto.h"
+/*
+ * When set to 1 the new SecureElementComputeAesCmacBlocks API may be used.
+ */
+#define USE_CMAC_BLOCKS_API 1
+
/*!
* Return values.
*/
@@ -72,6 +77,17 @@ typedef enum eSecureElementStatus
SECURE_ELEMENT_ERROR,
}SecureElementStatus_t;
+#if( USE_CMAC_BLOCKS_API == 1 )
+/*!
+ * CMAC buffer block.
+ */
+typedef struct SecureElementBlock_s
+{
+ uint8_t *Buffer;
+ uint16_t Size;
+}SecureElementBlock_t;
+#endif
+
/*!
* Signature of callback function to be called by the Secure Element driver when the
* non volatile context have to be stored.
@@ -114,19 +130,28 @@ void* SecureElementGetNvmCtx( size_t* seNvmCtxSize );
SecureElementStatus_t SecureElementSetKey( KeyIdentifier_t keyID, uint8_t* key );
/*!
- * Computes a CMAC of a message given in parts
+ * Computes a CMAC of a message
+ *
+ * \param[IN] buffer - Data buffer
+ * \param[IN] size - Data buffer size
+ * \param[IN] keyID - Key identifier to determine the AES key to be used
+ * \param[OUT] cmac - Computed cmac
+ * \retval - Status of the operation
+ */
+SecureElementStatus_t SecureElementComputeAesCmac( uint8_t* buffer, uint16_t size, KeyIdentifier_t keyID, uint32_t* cmac );
+
+#if( USE_CMAC_BLOCKS_API == 1 )
+/*!
+ * Computes a CMAC of a message given in blocks
*
- * \param[IN] buffers - Data buffers
- * \param[IN] numparts - Number of buffers
+ * \param[IN] blocks - Buffer blocks
+ * \param[IN] nbBlocks - Number of buffer blocks
* \param[IN] keyID - Key identifier to determine the AES key to be used
* \param[OUT] cmac - Computed cmac
* \retval - Status of the operation
*/
-struct se_block {
- uint8_t *buffer;
- uint16_t size;
-};
-SecureElementStatus_t SecureElementComputeAesCmacParts( struct se_block *parts, uint16_t numparts, KeyIdentifier_t keyID, uint32_t* cmac );
+SecureElementStatus_t SecureElementComputeAesCmacBlocks( SecureElementBlock_t* blocks, uint16_t nbBlocks, KeyIdentifier_t keyID, uint32_t* cmac );
+#endif
/*!
* Verifies a CMAC (computes and compare with expected cmac)
diff --git a/src/peripherals/soft-se/soft-se.c b/src/peripherals/soft-se/soft-se.c
index 1876f276..6baa168f 100644
--- a/src/peripherals/soft-se/soft-se.c
+++ b/src/peripherals/soft-se/soft-se.c
@@ -100,17 +100,28 @@ SecureElementStatus_t GetKeyByID( KeyIdentifier_t keyID, Key_t** keyItem )
}
/*
- * Computes a CMAC of a message given in parts
+ * Dummy callback in case if the user provides NULL function pointer
+ */
+static void DummyCB( void )
+{
+ return;
+}
+
+#if( USE_CMAC_BLOCKS_API == 0 )
+/*
+ * Computes a CMAC
+ *
+ * cmac = aes128_cmac(keyID, buffer)
*
- * \param[IN] parts - Data buffers
- * \param[IN] num - Number of buffers
+ * \param[IN] buffer - Data buffer
+ * \param[IN] size - Data buffer size
* \param[IN] keyID - Key identifier to determine the AES key to be used
* \param[OUT] cmac - Computed cmac
* \retval - Status of the operation
*/
-SecureElementStatus_t ComputeCmacParts( struct se_block *parts, uint16_t num, KeyIdentifier_t keyID, uint32_t* cmac )
+SecureElementStatus_t ComputeCmac( uint8_t* buffer, uint16_t size, KeyIdentifier_t keyID, uint32_t* cmac )
{
- if( parts == NULL || cmac == NULL || num == 0 )
+ if( buffer == NULL || cmac == NULL )
{
return SECURE_ELEMENT_ERROR_NPE;
}
@@ -126,11 +137,7 @@ SecureElementStatus_t ComputeCmacParts( struct se_block *parts, uint16_t num, Ke
{
AES_CMAC_SetKey( SeNvmCtx.AesCmacCtx, keyItem->KeyValue );
- for ( size_t i = 0; i < num; i++ )
- {
- if ( parts[i].buffer == NULL || parts[i].size == 0 ) continue;
- AES_CMAC_Update( SeNvmCtx.AesCmacCtx, parts[i].buffer, parts[i].size );
- }
+ AES_CMAC_Update( SeNvmCtx.AesCmacCtx, buffer, size );
AES_CMAC_Final( Cmac, SeNvmCtx.AesCmacCtx );
@@ -140,31 +147,55 @@ SecureElementStatus_t ComputeCmacParts( struct se_block *parts, uint16_t num, Ke
return retval;
}
-
+#else
/*
- * Computes a CMAC
+ * Computes a CMAC of a message given in blocks
+ *
+ * cmac = aes128_cmac(keyID, blocks[i].Buffer)
*
- * \param[IN] buffer - Data buffer
- * \param[IN] size - Data buffer size
+ * \param[IN] blocks - Buffer blocks
+ * \param[IN] nbBlocks - Number of buffer blocks
* \param[IN] keyID - Key identifier to determine the AES key to be used
* \param[OUT] cmac - Computed cmac
* \retval - Status of the operation
*/
-SecureElementStatus_t ComputeCmac( uint8_t* buffer, uint16_t size, KeyIdentifier_t keyID, uint32_t* cmac )
+static SecureElementStatus_t ComputeCmacBlocks( SecureElementBlock_t* blocks, uint16_t nbBlocks, KeyIdentifier_t keyID, uint32_t* cmac )
{
- struct se_block part;
- part.buffer = buffer;
- part.size = size;
- return ComputeCmacParts( &part, 1, keyID, cmac );
-}
+ if( ( blocks == NULL ) || ( nbBlocks == 0 ) || ( cmac == NULL ) )
+ {
+ return SECURE_ELEMENT_ERROR_NPE;
+ }
-/*
- * Dummy callback in case if the user provides NULL function pointer
- */
-static void DummyCB( void )
-{
- return;
+ uint8_t Cmac[16];
+
+ AES_CMAC_Init( SeNvmCtx.AesCmacCtx );
+
+ Key_t* keyItem;
+ SecureElementStatus_t retval = GetKeyByID( keyID, &keyItem );
+
+ if( retval == SECURE_ELEMENT_SUCCESS )
+ {
+ AES_CMAC_SetKey( SeNvmCtx.AesCmacCtx, keyItem->KeyValue );
+
+ for( uint16_t i = 0; i < nbBlocks; i++ )
+ {
+ if( ( blocks[i].Buffer == NULL ) ||
+ ( blocks[i].Size == 0 ) )
+ {
+ continue;
+ }
+ AES_CMAC_Update( SeNvmCtx.AesCmacCtx, blocks[i].Buffer, blocks[i].Size );
+ }
+
+ AES_CMAC_Final( Cmac, SeNvmCtx.AesCmacCtx );
+
+ // Bring into the required format
+ *cmac = ( uint32_t )( ( uint32_t ) Cmac[3] << 24 | ( uint32_t ) Cmac[2] << 16 | ( uint32_t ) Cmac[1] << 8 | ( uint32_t ) Cmac[0] );
+ }
+
+ return retval;
}
+#endif
/*
* API functions
@@ -265,7 +296,7 @@ SecureElementStatus_t SecureElementSetKey( KeyIdentifier_t keyID, uint8_t* key )
return SECURE_ELEMENT_ERROR_INVALID_KEY_ID;
}
-SecureElementStatus_t SecureElementComputeAesCmacParts( struct se_block *parts, uint16_t num, KeyIdentifier_t keyID, uint32_t* cmac )
+SecureElementStatus_t SecureElementComputeAesCmac( uint8_t* buffer, uint16_t size, KeyIdentifier_t keyID, uint32_t* cmac )
{
if( keyID >= LORAMAC_CRYPTO_MULITCAST_KEYS )
{
@@ -273,9 +304,29 @@ SecureElementStatus_t SecureElementComputeAesCmacParts( struct se_block *parts,
return SECURE_ELEMENT_ERROR_INVALID_KEY_ID;
}
- return ComputeCmacParts( parts, num, keyID, cmac );
+#if( USE_CMAC_BLOCKS_API == 0 )
+ return ComputeCmac( buffer, size, keyID, cmac );
+#else
+ SecureElementBlock_t block;
+ block.Buffer = buffer;
+ block.Size = size;
+ return ComputeCmacBlocks( &block, 1, keyID, cmac );
+#endif
}
+#if( USE_CMAC_BLOCKS_API == 1 )
+SecureElementStatus_t SecureElementComputeAesCmacBlocks( SecureElementBlock_t* blocks, uint16_t nbBlocks, KeyIdentifier_t keyID, uint32_t* cmac )
+{
+ if( keyID >= LORAMAC_CRYPTO_MULITCAST_KEYS )
+ {
+ //Never accept multicast key identifier for cmac computation
+ return SECURE_ELEMENT_ERROR_INVALID_KEY_ID;
+ }
+
+ return ComputeCmacBlocks( blocks, nbBlocks, keyID, cmac );
+}
+#endif
+
SecureElementStatus_t SecureElementVerifyAesCmac( uint8_t* buffer, uint16_t size, uint32_t expectedCmac, KeyIdentifier_t keyID )
{
if( buffer == NULL )
@@ -285,8 +336,14 @@ SecureElementStatus_t SecureElementVerifyAesCmac( uint8_t* buffer, uint16_t size
SecureElementStatus_t retval = SECURE_ELEMENT_ERROR;
uint32_t compCmac = 0;
-
+#if( USE_CMAC_BLOCKS_API == 0 )
retval = ComputeCmac( buffer, size, keyID, &compCmac );
+#else
+ SecureElementBlock_t block;
+ block.Buffer = buffer;
+ block.Size = size;
+ retval = ComputeCmacBlocks( &block, 1, keyID, &compCmac );
+#endif
if( retval != SECURE_ELEMENT_SUCCESS )
{
return retval;
mluis1
approved these changes
Feb 13, 2019
|
Before merging this pull-request I would like to have @djaeckle feedback first. |
|
I applied the patch, looks fine. This of course is a very generic API now, more generic than currently required. To win back the flash bytes, some speed, some lines of code and even more stack usage, we could change the API to accept an (optional) prefix buffer of 16 bytes (mic) before the message buffer of variable length. The signature would then be:
Just tested, the total size difference compared to the original situation is now zero. If you wish I could commit the patch for this, let me know! |
|
Your proposition looks fair. Concerning the API signature I would recommend the following: /*!
* Computes a CMAC of a message using provided initial Bx block
*
* \param[IN] micBxBuffer - Buffer containing the initial Bx block
* \param[IN] buffer - Data buffer
* \param[IN] size - Data buffer size
* \param[IN] keyID - Key identifier to determine the AES key to be used
* \param[OUT] cmac - Computed cmac
* \retval - Status of the operation
*/
SecureElementStatus_t SecureElementComputeAesCmacBlocks( uint8_t* micBxBuffer, uint8_t* buffer, uint16_t size, KeyIdentifier_t keyID, uint32_t* cmac );Please update your proposition accordingly and we will review it again. |
…onal) prefix plus flexible length message This way the code size stays the same, stack usage is decreased further, and execution speed is faster.
|
Done! |
mluis1
requested changes
Feb 14, 2019
mluis1
approved these changes
Feb 14, 2019
|
The pre-certification tests just finished and they were successful. |
|
Hi @thingsconnected, I discussed with @djaeckle today and we come to the conclusion that with the latest updates we can modify the Could you please apply the below patch in order to update the API? Another point. Could you please check the used file line endings? diff --git a/src/mac/LoRaMacCrypto.c b/src/mac/LoRaMacCrypto.c
index f6953d4c..4256d95a 100644
--- a/src/mac/LoRaMacCrypto.c
+++ b/src/mac/LoRaMacCrypto.c
@@ -454,31 +454,15 @@ static LoRaMacCryptoStatus_t ComputeCmacB0( uint8_t* msg, uint16_t len, KeyIdent
return LORAMAC_CRYPTO_ERROR_BUF_SIZE;
}
-#if( USE_CMAC_BLOCKS_API == 0 )
- uint8_t micBuff[CRYPTO_BUFFER_SIZE];
- memset1( micBuff, 0, CRYPTO_BUFFER_SIZE );
+ uint8_t micBuff[MIC_BLOCK_BX_SIZE];
// Initialize the first Block
PrepareB0( len, keyID, isAck, dir, devAddr, fCnt, micBuff );
- // Copy the given data to the mic computation buffer
- memcpy1( ( micBuff + MIC_BLOCK_BX_SIZE ), msg, len );
-
- if( SecureElementComputeAesCmac( micBuff, ( len + MIC_BLOCK_BX_SIZE ), keyID, cmac ) != SECURE_ELEMENT_SUCCESS )
+ if( SecureElementComputeAesCmac( micBuff, msg, len, keyID, cmac ) != SECURE_ELEMENT_SUCCESS )
{
return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
}
-#else
- uint8_t micBuff[MIC_BLOCK_BX_SIZE];
-
- // Initialize the first Block
- PrepareB0( len, keyID, isAck, dir, devAddr, fCnt, micBuff );
-
- if( SecureElementComputeAesCmacBlocks( micBuff, msg, len, keyID, cmac ) != SECURE_ELEMENT_SUCCESS )
- {
- return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
- }
-#endif
return LORAMAC_CRYPTO_SUCCESS;
}
@@ -613,31 +597,15 @@ static LoRaMacCryptoStatus_t ComputeCmacB1( uint8_t* msg, uint16_t len, KeyIdent
return LORAMAC_CRYPTO_ERROR_BUF_SIZE;
}
-#if( USE_CMAC_BLOCKS_API == 0 )
- uint8_t micBuff[CRYPTO_BUFFER_SIZE];
- memset1( micBuff, 0, CRYPTO_BUFFER_SIZE );
+ uint8_t micBuff[MIC_BLOCK_BX_SIZE];
// Initialize the first Block
PrepareB1( len, keyID, isAck, txDr, txCh, devAddr, fCntUp, micBuff );
- // Copy the given data to the mic computation buffer
- memcpy1( ( micBuff + MIC_BLOCK_BX_SIZE ), msg, len );
-
- if( SecureElementComputeAesCmac( micBuff, ( len + MIC_BLOCK_BX_SIZE ), keyID, cmac ) != SECURE_ELEMENT_SUCCESS )
+ if( SecureElementComputeAesCmac( micBuff, msg, len, keyID, cmac ) != SECURE_ELEMENT_SUCCESS )
{
return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
}
-#else
- uint8_t micBuff[MIC_BLOCK_BX_SIZE];
-
- // Initialize the first Block
- PrepareB1( len, keyID, isAck, txDr, txCh, devAddr, fCntUp, micBuff );
-
- if( SecureElementComputeAesCmacBlocks( micBuff, msg, len, keyID, cmac ) != SECURE_ELEMENT_SUCCESS )
- {
- return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
- }
-#endif
return LORAMAC_CRYPTO_SUCCESS;
}
@@ -889,7 +857,7 @@ static void UpdateFCntDown( FCntIdentifier_t fCntID, uint32_t currentDown )
/*!
* Resets the frame counters
*/
-static void ResetFCnts( void )
+static void ResetFCnts( void )
{
CryptoCtx.NvmCtx->FCntUp = 0;
@@ -1024,9 +992,9 @@ LoRaMacCryptoStatus_t LoRaMacCryptoPrepareJoinRequest( LoRaMacMessageJoinRequest
}
// Compute mic
- if( SecureElementComputeAesCmac( macMsg->Buffer, ( LORAMAC_JOIN_REQ_MSG_SIZE - LORAMAC_MIC_FIELD_SIZE ), micComputationKeyID, &macMsg->MIC ) != SECURE_ELEMENT_SUCCESS )
+ if( SecureElementComputeAesCmac( NULL, macMsg->Buffer, ( LORAMAC_JOIN_REQ_MSG_SIZE - LORAMAC_MIC_FIELD_SIZE ), micComputationKeyID, &macMsg->MIC ) != SECURE_ELEMENT_SUCCESS )
{
- return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
+ return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
}
// Reserialize message to add the MIC
@@ -1059,9 +1027,9 @@ LoRaMacCryptoStatus_t LoRaMacCryptoPrepareReJoinType1( LoRaMacMessageReJoinType1
// Compute mic
// cmac = aes128_cmac(JSIntKey, MHDR | RejoinType | JoinEUI| DevEUI | RJcount1)
- if( SecureElementComputeAesCmac( macMsg->Buffer, ( LORAMAC_RE_JOIN_1_MSG_SIZE - LORAMAC_MIC_FIELD_SIZE ), J_S_INT_KEY, &macMsg->MIC ) != SECURE_ELEMENT_SUCCESS )
+ if( SecureElementComputeAesCmac( NULL, macMsg->Buffer, ( LORAMAC_RE_JOIN_1_MSG_SIZE - LORAMAC_MIC_FIELD_SIZE ), J_S_INT_KEY, &macMsg->MIC ) != SECURE_ELEMENT_SUCCESS )
{
- return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
+ return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
}
// Reserialize message to add the MIC
@@ -1098,9 +1066,9 @@ LoRaMacCryptoStatus_t LoRaMacCryptoPrepareReJoinType0or2( LoRaMacMessageReJoinTy
// Compute mic
// cmac = aes128_cmac(SNwkSIntKey, MHDR | Rejoin Type | NetID | DevEUI | RJcount0)
- if( SecureElementComputeAesCmac( macMsg->Buffer, ( LORAMAC_RE_JOIN_0_2_MSG_SIZE - LORAMAC_MIC_FIELD_SIZE ), S_NWK_S_INT_KEY, &macMsg->MIC ) != SECURE_ELEMENT_SUCCESS )
+ if( SecureElementComputeAesCmac( NULL, macMsg->Buffer, ( LORAMAC_RE_JOIN_0_2_MSG_SIZE - LORAMAC_MIC_FIELD_SIZE ), S_NWK_S_INT_KEY, &macMsg->MIC ) != SECURE_ELEMENT_SUCCESS )
{
- return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
+ return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
}
// Reserialize message to add the MIC
@@ -1183,9 +1151,9 @@ LoRaMacCryptoStatus_t LoRaMacCryptoHandleJoinAccept( JoinReqIdentifier_t joinReq
{
// For legacy mode :
// cmac = aes128_cmac(NwkKey, MHDR | JoinNonce | NetID | DevAddr | DLSettings | RxDelay | CFList | CFListType)
- if( SecureElementVerifyAesCmac( macMsg->Buffer, ( macMsg->BufSize - LORAMAC_MIC_FIELD_SIZE ), macMsg->MIC, micComputationKeyID ) != SECURE_ELEMENT_SUCCESS )
+ if( SecureElementVerifyAesCmac( macMsg->Buffer, ( macMsg->BufSize - LORAMAC_MIC_FIELD_SIZE ), macMsg->MIC, micComputationKeyID ) != SECURE_ELEMENT_SUCCESS )
{
- return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
+ return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
}
}
else
@@ -1205,9 +1173,9 @@ LoRaMacCryptoStatus_t LoRaMacCryptoHandleJoinAccept( JoinReqIdentifier_t joinReq
procBuffer[bufItr++] = macMsg->MHDR.Value;
- if( SecureElementVerifyAesCmac( macMsg->Buffer, ( macMsg->BufSize + micComputationOffset - LORAMAC_MHDR_FIELD_SIZE - LORAMAC_MIC_FIELD_SIZE ), macMsg->MIC, micComputationKeyID ) != SECURE_ELEMENT_SUCCESS )
+ if( SecureElementVerifyAesCmac( macMsg->Buffer, ( macMsg->BufSize + micComputationOffset - LORAMAC_MHDR_FIELD_SIZE - LORAMAC_MIC_FIELD_SIZE ), macMsg->MIC, micComputationKeyID ) != SECURE_ELEMENT_SUCCESS )
{
- return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
+ return LORAMAC_CRYPTO_ERROR_SECURE_ELEMENT_FUNC;
}
// Check if the JoinNonce is greater as the previous one
diff --git a/src/mac/secure-element.h b/src/mac/secure-element.h
index ca6e04ed..e5fc1e16 100644
--- a/src/mac/secure-element.h
+++ b/src/mac/secure-element.h
@@ -37,11 +37,6 @@
#include <stdint.h>
#include "LoRaMacCrypto.h"
-/*
- * When set to 1 the new SecureElementComputeAesCmacBlocks API may be used.
- */
-#define USE_CMAC_BLOCKS_API 1
-
/*!
* Return values.
*/
@@ -119,29 +114,16 @@ void* SecureElementGetNvmCtx( size_t* seNvmCtxSize );
SecureElementStatus_t SecureElementSetKey( KeyIdentifier_t keyID, uint8_t* key );
/*!
- * Computes a CMAC of a message
- *
- * \param[IN] buffer - Data buffer
- * \param[IN] size - Data buffer size
- * \param[IN] keyID - Key identifier to determine the AES key to be used
- * \param[OUT] cmac - Computed cmac
- * \retval - Status of the operation
- */
-SecureElementStatus_t SecureElementComputeAesCmac( uint8_t* buffer, uint16_t size, KeyIdentifier_t keyID, uint32_t* cmac );
-
-#if( USE_CMAC_BLOCKS_API == 1 )
-/*!
- * Computes a CMAC of a message using provided initial Bx block
- *
- * \param[IN] micBxBuffer - Buffer containing the initial Bx block
- * \param[IN] buffer - Data buffer
- * \param[IN] size - Data buffer size
+ * Computes a CMAC of a message using provided initial Bx block
+ *
+ * \param[IN] micBxBuffer - Buffer containing the initial Bx block
+ * \param[IN] buffer - Data buffer
+ * \param[IN] size - Data buffer size
* \param[IN] keyID - Key identifier to determine the AES key to be used
* \param[OUT] cmac - Computed cmac
* \retval - Status of the operation
*/
-SecureElementStatus_t SecureElementComputeAesCmacBlocks( uint8_t* micBxBuffer, uint8_t* buffer, uint16_t size, KeyIdentifier_t keyID, uint32_t* cmac );
-#endif
+SecureElementStatus_t SecureElementComputeAesCmac( uint8_t* micBxBuffer, uint8_t* buffer, uint16_t size, KeyIdentifier_t keyID, uint32_t* cmac );
/*!
* Verifies a CMAC (computes and compare with expected cmac)
diff --git a/src/peripherals/soft-se/soft-se.c b/src/peripherals/soft-se/soft-se.c
index 3e2e3f1f..f767d38e 100644
--- a/src/peripherals/soft-se/soft-se.c
+++ b/src/peripherals/soft-se/soft-se.c
@@ -107,47 +107,6 @@ static void DummyCB( void )
return;
}
-#if( USE_CMAC_BLOCKS_API == 0 )
-/*
- * Computes a CMAC
- *
- * cmac = aes128_cmac(keyID, buffer)
- *
- * \param[IN] buffer - Data buffer
- * \param[IN] size - Data buffer size
- * \param[IN] keyID - Key identifier to determine the AES key to be used
- * \param[OUT] cmac - Computed cmac
- * \retval - Status of the operation
- */
-SecureElementStatus_t ComputeCmac( uint8_t* buffer, uint16_t size, KeyIdentifier_t keyID, uint32_t* cmac )
-{
- if( buffer == NULL || cmac == NULL )
- {
- return SECURE_ELEMENT_ERROR_NPE;
- }
-
- uint8_t Cmac[16];
-
- AES_CMAC_Init( SeNvmCtx.AesCmacCtx );
-
- Key_t* keyItem;
- SecureElementStatus_t retval = GetKeyByID( keyID, &keyItem );
-
- if( retval == SECURE_ELEMENT_SUCCESS )
- {
- AES_CMAC_SetKey( SeNvmCtx.AesCmacCtx, keyItem->KeyValue );
-
- AES_CMAC_Update( SeNvmCtx.AesCmacCtx, buffer, size );
-
- AES_CMAC_Final( Cmac, SeNvmCtx.AesCmacCtx );
-
- // Bring into the required format
- *cmac = ( uint32_t )( ( uint32_t ) Cmac[3] << 24 | ( uint32_t ) Cmac[2] << 16 | ( uint32_t ) Cmac[1] << 8 | ( uint32_t ) Cmac[0] );
- }
-
- return retval;
-}
-#else
/*
* Computes a CMAC of a message using provided initial Bx block
*
@@ -160,7 +119,7 @@ SecureElementStatus_t ComputeCmac( uint8_t* buffer, uint16_t size, KeyIdentifier
* \param[OUT] cmac - Computed cmac
* \retval - Status of the operation
*/
-static SecureElementStatus_t ComputeCmacBlocks( uint8_t *micBxBuffer, uint8_t *buffer, uint16_t size, KeyIdentifier_t keyID, uint32_t* cmac )
+static SecureElementStatus_t ComputeCmac( uint8_t *micBxBuffer, uint8_t *buffer, uint16_t size, KeyIdentifier_t keyID, uint32_t* cmac )
{
if( ( buffer == NULL ) || ( cmac == NULL ) )
{
@@ -193,7 +152,6 @@ static SecureElementStatus_t ComputeCmacBlocks( uint8_t *micBxBuffer, uint8_t *b
return retval;
}
-#endif
/*
* API functions
@@ -294,23 +252,7 @@ SecureElementStatus_t SecureElementSetKey( KeyIdentifier_t keyID, uint8_t* key )
return SECURE_ELEMENT_ERROR_INVALID_KEY_ID;
}
-SecureElementStatus_t SecureElementComputeAesCmac( uint8_t* buffer, uint16_t size, KeyIdentifier_t keyID, uint32_t* cmac )
-{
- if( keyID >= LORAMAC_CRYPTO_MULITCAST_KEYS )
- {
- //Never accept multicast key identifier for cmac computation
- return SECURE_ELEMENT_ERROR_INVALID_KEY_ID;
- }
-
-#if( USE_CMAC_BLOCKS_API == 0 )
- return ComputeCmac( buffer, size, keyID, cmac );
-#else
- return ComputeCmacBlocks( NULL, buffer, size, keyID, cmac );
-#endif
-}
-
-#if( USE_CMAC_BLOCKS_API == 1 )
-SecureElementStatus_t SecureElementComputeAesCmacBlocks( uint8_t *micBxBuffer, uint8_t *buffer, uint16_t size, KeyIdentifier_t keyID, uint32_t* cmac )
+SecureElementStatus_t SecureElementComputeAesCmac( uint8_t *micBxBuffer, uint8_t *buffer, uint16_t size, KeyIdentifier_t keyID, uint32_t* cmac )
{
if( keyID >= LORAMAC_CRYPTO_MULITCAST_KEYS )
{
@@ -318,9 +260,8 @@ SecureElementStatus_t SecureElementComputeAesCmacBlocks( uint8_t *micBxBuffer, u
return SECURE_ELEMENT_ERROR_INVALID_KEY_ID;
}
- return ComputeCmacBlocks( micBxBuffer, buffer, size, keyID, cmac );
+ return ComputeCmac( micBxBuffer, buffer, size, keyID, cmac );
}
-#endif
SecureElementStatus_t SecureElementVerifyAesCmac( uint8_t* buffer, uint16_t size, uint32_t expectedCmac, KeyIdentifier_t keyID )
{
@@ -331,11 +272,7 @@ SecureElementStatus_t SecureElementVerifyAesCmac( uint8_t* buffer, uint16_t size
SecureElementStatus_t retval = SECURE_ELEMENT_ERROR;
uint32_t compCmac = 0;
-#if( USE_CMAC_BLOCKS_API == 0 )
- retval = ComputeCmac( buffer, size, keyID, &compCmac );
-#else
- retval = ComputeCmacBlocks( NULL, buffer, size, keyID, &compCmac );
-#endif
+ retval = ComputeCmac( NULL, buffer, size, keyID, &compCmac );
if( retval != SECURE_ELEMENT_SUCCESS )
{
return retval; |
As of now, the Cmac computation API takes an optional prefix buffer of 16 bytes, followed by the message buffer of variable length. Line endings in LoRaMacCrypto.c and secure-element.h are now strictly Windows (CRLF) soft-se.c line endings remain strictly Unix (LF)
mluis1
previously requested changes
Feb 18, 2019
mluis1
dismissed
their stale review
I see the original file was using Unix line endings.
|
We will fix the line endings in the develop branch. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
By allowing the Cmac hash computations to be done on a sequence of blocks, there is no need anymore to memcpy the entire message to the stack.
This lowers stack usage by >200 bytes for some frequently used code paths.