[aes-ccm] misc enhancements (#5015)

- add a new enumeration type `Mode` to indicate the encryption vs.
  decryption mode in `AesCcm::Payload()` method.
- add `GetTagLength()` method and simplify `Finalize()`.
- change `AesCcm::Init()` to assert on bad tag length input param
  instead of returning `OT_ERROR_INVALID_ARGS`.
- reorder AesCcm member variable to help with alignments
- use CHAR_BIT
- remove check of tag length from Finalize
- use `memcpy`/`memset` to copy/clear buffers

src/core/api/crypto_api.cpp

@@ -72,13 +72,12 @@ void otCryptoAesCcm(const uint8_t *aKey,
                     bool           aEncrypt,
                     void *         aTag)
 {
-    AesCcm  aesCcm;
+    AesCcm aesCcm;
-    uint8_t tagLength;
 
     OT_ASSERT((aKey != NULL) && (aNonce != NULL) && (aPlainText != NULL) && (aCipherText != NULL) && (aTag != NULL));
 
     aesCcm.SetKey(aKey, aKeyLength);
-    SuccessOrExit(aesCcm.Init(aHeaderLength, aLength, aTagLength, aNonce, aNonceLength));
+    aesCcm.Init(aHeaderLength, aLength, aTagLength, aNonce, aNonceLength);
 
     if (aHeaderLength != 0)
     {
@@ -86,13 +85,8 @@ void otCryptoAesCcm(const uint8_t *aKey,
         aesCcm.Header(aHeader, aHeaderLength);
     }
 
-    aesCcm.Payload(aPlainText, aCipherText, aLength, aEncrypt);
+    aesCcm.Payload(aPlainText, aCipherText, aLength, aEncrypt ? AesCcm::kEncrypt : AesCcm::kDecrypt);
-    aesCcm.Finalize(aTag, &tagLength);
+    aesCcm.Finalize(aTag);
-
-    OT_ASSERT(aTagLength == tagLength);
-
-exit:
-    return;
 }
 
 #if OPENTHREAD_CONFIG_ECDSA_ENABLE

src/core/crypto/aes_ccm.cpp

@@ -33,6 +33,8 @@
 
 #include "aes_ccm.hpp"
 
+#include <limits.h>
+
 #include "common/code_utils.hpp"
 #include "common/debug.hpp"
 #include "common/encoding.hpp"
@@ -42,7 +44,7 @@ namespace Crypto {
 
 void AesCcm::SetKey(const uint8_t *aKey, uint16_t aKeyLength)
 {
-    mEcb.SetKey(aKey, 8 * aKeyLength);
+    mEcb.SetKey(aKey, CHAR_BIT * aKeyLength);
 }
 
 void AesCcm::SetKey(const Mac::Key &aMacKey)
@@ -50,30 +52,20 @@ void AesCcm::SetKey(const Mac::Key &aMacKey)
     SetKey(aMacKey.GetKey(), Mac::Key::kSize);
 }
 
-otError AesCcm::Init(uint32_t    aHeaderLength,
+void AesCcm::Init(uint32_t    aHeaderLength,
-                     uint32_t    aPlainTextLength,
+                  uint32_t    aPlainTextLength,
-                     uint8_t     aTagLength,
+                  uint8_t     aTagLength,
-                     const void *aNonce,
+                  const void *aNonce,
-                     uint8_t     aNonceLength)
+                  uint8_t     aNonceLength)
 {
     const uint8_t *nonceBytes  = reinterpret_cast<const uint8_t *>(aNonce);
-    otError        error       = OT_ERROR_NONE;
     uint8_t        blockLength = 0;
     uint32_t       len;
     uint8_t        L;
     uint8_t        i;
 
-    // aTagLength must be even
+    // Tag length must be even and within [kMinTagLength, kMaxTagLength]
-    aTagLength &= ~1;
+    OT_ASSERT(((aTagLength & 0x1) == 0) && (kMinTagLength <= aTagLength) && (aTagLength <= kMaxTagLength));
-
-    if (aTagLength > sizeof(mBlock))
-    {
-        aTagLength = sizeof(mBlock);
-    }
-    else if (aTagLength < kTagLengthMin)
-    {
-        ExitNow(error = OT_ERROR_INVALID_ARGS);
-    }
 
     L = 0;
 
@@ -111,10 +103,7 @@ otError AesCcm::Init(uint32_t    aHeaderLength,
                  static_cast<uint8_t>(L - 1));
 
     // write nonce
-    for (i = 0; i < aNonceLength; i++)
+    memcpy(&mBlock[1], nonceBytes, aNonceLength);
-    {
-        mBlock[1 + i] = nonceBytes[i];
-    }
 
     // write len
     len = aPlainTextLength;
@@ -150,16 +139,8 @@ otError AesCcm::Init(uint32_t    aHeaderLength,
 
     // init counter
     mCtr[0] = L - 1;
-
+    memcpy(&mCtr[1], nonceBytes, aNonceLength);
-    for (i = 0; i < aNonceLength; i++)
+    memset(&mCtr[aNonceLength + 1], 0, sizeof(mCtr) - aNonceLength - 1);
-    {
-        mCtr[1 + i] = nonceBytes[i];
-    }
-
-    for (i = i + 1; i < sizeof(mCtr); i++)
-    {
-        mCtr[i] = 0;
-    }
 
     mNonceLength     = aNonceLength;
     mHeaderLength    = aHeaderLength;
@@ -169,9 +150,6 @@ otError AesCcm::Init(uint32_t    aHeaderLength,
     mBlockLength     = blockLength;
     mCtrLength       = sizeof(mCtrPad);
     mTagLength       = aTagLength;
-
-exit:
-    return error;
 }
 
 void AesCcm::Header(const void *aHeader, uint32_t aHeaderLength)
@@ -206,7 +184,7 @@ void AesCcm::Header(const void *aHeader, uint32_t aHeaderLength)
     }
 }
 
-void AesCcm::Payload(void *aPlainText, void *aCipherText, uint32_t aLength, bool aEncrypt)
+void AesCcm::Payload(void *aPlainText, void *aCipherText, uint32_t aLength, Mode aMode)
 {
     uint8_t *plaintextBytes  = reinterpret_cast<uint8_t *>(aPlainText);
     uint8_t *ciphertextBytes = reinterpret_cast<uint8_t *>(aCipherText);
@@ -230,7 +208,7 @@ void AesCcm::Payload(void *aPlainText, void *aCipherText, uint32_t aLength, bool
             mCtrLength = 0;
         }
 
-        if (aEncrypt)
+        if (aMode == kEncrypt)
         {
             byte               = plaintextBytes[i];
             ciphertextBytes[i] = byte ^ mCtrPad[mCtrLength++];
@@ -260,32 +238,21 @@ void AesCcm::Payload(void *aPlainText, void *aCipherText, uint32_t aLength, bool
         }
 
         // reset counter
-        for (uint8_t i = mNonceLength + 1; i < sizeof(mCtr); i++)
+        memset(&mCtr[mNonceLength + 1], 0, sizeof(mCtr) - mNonceLength - 1);
-        {
-            mCtr[i] = 0;
-        }
     }
 }
 
-void AesCcm::Finalize(void *aTag, uint8_t *aTagLength)
+void AesCcm::Finalize(void *aTag)
 {
     uint8_t *tagBytes = reinterpret_cast<uint8_t *>(aTag);
 
     OT_ASSERT(mPlainTextCur == mPlainTextLength);
 
-    if (mTagLength > 0)
+    mEcb.Encrypt(mCtr, mCtrPad);
-    {
-        mEcb.Encrypt(mCtr, mCtrPad);
 
-        for (int i = 0; i < mTagLength; i++)
+    for (int i = 0; i < mTagLength; i++)
-        {
-            tagBytes[i] = mBlock[i] ^ mCtrPad[i];
-        }
-    }
-
-    if (aTagLength)
     {
-        *aTagLength = mTagLength;
+        tagBytes[i] = mBlock[i] ^ mCtrPad[i];
     }
 }
 

src/core/crypto/aes_ccm.hpp

@@ -62,7 +62,19 @@ class AesCcm
 public:
     enum
     {
-        kNonceSize = 13, ///< Size of IEEE 802.15.4 Nonce (bytes).
+        kMinTagLength = 4,                  ///< Minimum tag length (in bytes).
+        kMaxTagLength = AesEcb::kBlockSize, ///< Maximum tag length (in bytes).
+        kNonceSize    = 13,                 ///< Size of IEEE 802.15.4 Nonce (in bytes).
+    };
+
+    /**
+     * This enumeration type represent the encryption vs decryption mode.
+     *
+     */
+    enum Mode
+    {
+        kEncrypt, // Encryption mode.
+        kDecrypt, // Decryption mode.
     };
 
     /**
@@ -87,19 +99,16 @@ public:
      *
      * @param[in]  aHeaderLength     Length of header in bytes.
      * @param[in]  aPlainTextLength  Length of plaintext in bytes.
-     * @param[in]  aTagLength        Length of tag in bytes.
+     * @param[in]  aTagLength        Length of tag in bytes (must be even and in `[kMinTagLength, kMaxTagLength]`).
      * @param[in]  aNonce            A pointer to the nonce.
      * @param[in]  aNonceLength      Length of nonce in bytes.
      *
-     * @retval OT_ERROR_NONE          Initialization was successful.
-     * @retval OT_ERROR_INVALID_ARGS  Initialization failed.
-     *
      */
-    otError Init(uint32_t    aHeaderLength,
+    void Init(uint32_t    aHeaderLength,
-                 uint32_t    aPlainTextLength,
+              uint32_t    aPlainTextLength,
-                 uint8_t     aTagLength,
+              uint8_t     aTagLength,
-                 const void *aNonce,
+              const void *aNonce,
-                 uint8_t     aNonceLength);
+              uint8_t     aNonceLength);
 
     /**
      * This method processes the header.
@@ -116,19 +125,26 @@ public:
      * @param[inout]  aPlainText   A pointer to the plaintext.
      * @param[inout]  aCipherText  A pointer to the ciphertext.
      * @param[in]     aLength      Payload length in bytes.
-     * @param[in]     aEncrypt     TRUE on encrypt and FALSE on decrypt.
+     * @param[in]     aMode        Mode to indicate whether to encrypt (`kEncrypt`) or decrypt (`kDecrypt`).
      *
      */
-    void Payload(void *aPlainText, void *aCipherText, uint32_t aLength, bool aEncrypt);
+    void Payload(void *aPlainText, void *aCipherText, uint32_t aLength, Mode aMode);
+
+    /**
+     * This method returns the tag length in bytes.
+     *
+     * @returns The tag length in bytes.
+     *
+     */
+    uint8_t GetTagLength(void) const { return mTagLength; }
 
     /**
      * This method generates the tag.
      *
-     * @param[out]  aTag        A pointer to the tag.
+     * @param[out]  aTag        A pointer to the tag (must have `GetTagLength()` bytes).
-     * @param[out]  aTagLength  Length of the tag in bytes.
      *
      */
-    void Finalize(void *aTag, uint8_t *aTagLength);
+    void Finalize(void *aTag);
 
     /**
      * This static method generates IEEE 802.15.4 nonce byte sequence.
@@ -145,22 +161,17 @@ public:
                               uint8_t *              aNonce);
 
 private:
-    enum
-    {
-        kTagLengthMin = 4,
-    };
-
     AesEcb   mEcb;
     uint8_t  mBlock[AesEcb::kBlockSize];
     uint8_t  mCtr[AesEcb::kBlockSize];
     uint8_t  mCtrPad[AesEcb::kBlockSize];
-    uint8_t  mNonceLength;
     uint32_t mHeaderLength;
     uint32_t mHeaderCur;
     uint32_t mPlainTextLength;
     uint32_t mPlainTextCur;
     uint16_t mBlockLength;
     uint16_t mCtrLength;
+    uint8_t  mNonceLength;
     uint8_t  mTagLength;
 };
 

src/core/mac/mac.cpp

@@ -1518,17 +1518,17 @@ otError Mac::ProcessReceiveSecurity(RxFrame &aFrame, const Address &aSrcAddr, Ne
 
     aesCcm.SetKey(*macKey);
 
-    SuccessOrExit(aesCcm.Init(aFrame.GetHeaderLength(), aFrame.GetPayloadLength(), tagLength, nonce, sizeof(nonce)));
+    aesCcm.Init(aFrame.GetHeaderLength(), aFrame.GetPayloadLength(), tagLength, nonce, sizeof(nonce));
-
     aesCcm.Header(aFrame.GetHeader(), aFrame.GetHeaderLength());
+
 #ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
-    aesCcm.Payload(aFrame.GetPayload(), aFrame.GetPayload(), aFrame.GetPayloadLength(), false);
+    aesCcm.Payload(aFrame.GetPayload(), aFrame.GetPayload(), aFrame.GetPayloadLength(), Crypto::AesCcm::kDecrypt);
 #else
     // For fuzz tests, execute AES but do not alter the payload
     uint8_t fuzz[OT_RADIO_FRAME_MAX_SIZE];
-    aesCcm.Payload(fuzz, aFrame.GetPayload(), aFrame.GetPayloadLength(), false);
+    aesCcm.Payload(fuzz, aFrame.GetPayload(), aFrame.GetPayloadLength(), Crypto::AesCcm::kDecrypt);
 #endif
-    aesCcm.Finalize(tag, &tagLength);
+    aesCcm.Finalize(tag);
 
 #ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
     VerifyOrExit(memcmp(tag, aFrame.GetFooter(), tagLength) == 0, OT_NOOP);

src/core/mac/mac_frame.cpp

@@ -1052,12 +1052,10 @@ void TxFrame::ProcessTransmitAesCcm(const ExtAddress &aExtAddress)
     aesCcm.SetKey(GetAesKey());
     tagLength = GetFooterLength() - Frame::kFcsSize;
 
-    error = aesCcm.Init(GetHeaderLength(), GetPayloadLength(), tagLength, nonce, sizeof(nonce));
+    aesCcm.Init(GetHeaderLength(), GetPayloadLength(), tagLength, nonce, sizeof(nonce));
-    OT_ASSERT(error == OT_ERROR_NONE);
-
     aesCcm.Header(GetHeader(), GetHeaderLength());
-    aesCcm.Payload(GetPayload(), GetPayload(), GetPayloadLength(), true);
+    aesCcm.Payload(GetPayload(), GetPayload(), GetPayloadLength(), Crypto::AesCcm::kEncrypt);
-    aesCcm.Finalize(GetFooter(), &tagLength);
+    aesCcm.Finalize(GetFooter());
 
     SetIsSecurityProcessed(true);
 

src/core/thread/mle.cpp

@@ -2521,7 +2521,6 @@ otError Mle::SendMessage(Message &aMessage, const Ip6::Address &aDestination)
     uint32_t         keySequence;
     uint8_t          nonce[Crypto::AesCcm::kNonceSize];
     uint8_t          tag[4];
-    uint8_t          tagLength;
     Crypto::AesCcm   aesCcm;
     uint8_t          buf[64];
     uint16_t         length;
@@ -2542,9 +2541,8 @@ otError Mle::SendMessage(Message &aMessage, const Ip6::Address &aDestination)
                                       Mac::Frame::kSecEncMic32, nonce);
 
         aesCcm.SetKey(Get<KeyManager>().GetCurrentMleKey());
-        error = aesCcm.Init(16 + 16 + header.GetHeaderLength(), aMessage.GetLength() - (header.GetLength() - 1),
+        aesCcm.Init(16 + 16 + header.GetHeaderLength(), aMessage.GetLength() - (header.GetLength() - 1), sizeof(tag),
-                            sizeof(tag), nonce, sizeof(nonce));
+                    nonce, sizeof(nonce));
-        OT_ASSERT(error == OT_ERROR_NONE);
 
         aesCcm.Header(&mLinkLocal64.GetAddress(), sizeof(mLinkLocal64.GetAddress()));
         aesCcm.Header(&aDestination, sizeof(aDestination));
@@ -2555,14 +2553,13 @@ otError Mle::SendMessage(Message &aMessage, const Ip6::Address &aDestination)
         while (aMessage.GetOffset() < aMessage.GetLength())
         {
             length = aMessage.Read(aMessage.GetOffset(), sizeof(buf), buf);
-            aesCcm.Payload(buf, buf, length, true);
+            aesCcm.Payload(buf, buf, length, Crypto::AesCcm::kEncrypt);
             aMessage.Write(aMessage.GetOffset(), length, buf);
             aMessage.MoveOffset(length);
         }
 
-        tagLength = sizeof(tag);
+        aesCcm.Finalize(tag);
-        aesCcm.Finalize(tag, &tagLength);
+        SuccessOrExit(error = aMessage.Append(tag, sizeof(tag)));
-        SuccessOrExit(error = aMessage.Append(tag, tagLength));
 
         Get<KeyManager>().IncrementMleFrameCounter();
     }
@@ -2616,7 +2613,6 @@ void Mle::HandleUdpReceive(Message &aMessage, const Ip6::MessageInfo &aMessageIn
     uint8_t         buf[64];
     uint16_t        length;
     uint8_t         tag[4];
-    uint8_t         tagLength;
     uint8_t         command;
     Neighbor *      neighbor;
 
@@ -2677,10 +2673,8 @@ void Mle::HandleUdpReceive(Message &aMessage, const Ip6::MessageInfo &aMessageIn
     Crypto::AesCcm::GenerateNonce(macAddr, frameCounter, Mac::Frame::kSecEncMic32, nonce);
 
     aesCcm.SetKey(*mleKey);
-    SuccessOrExit(error = aesCcm.Init(sizeof(aMessageInfo.GetPeerAddr()) + sizeof(aMessageInfo.GetSockAddr()) +
+    aesCcm.Init(sizeof(aMessageInfo.GetPeerAddr()) + sizeof(aMessageInfo.GetSockAddr()) + header.GetHeaderLength(),
-                                          header.GetHeaderLength(),
+                aMessage.GetLength() - aMessage.GetOffset(), sizeof(messageTag), nonce, sizeof(nonce));
-                                      aMessage.GetLength() - aMessage.GetOffset(), sizeof(messageTag), nonce,
-                                      sizeof(nonce)));
 
     aesCcm.Header(&aMessageInfo.GetPeerAddr(), sizeof(aMessageInfo.GetPeerAddr()));
     aesCcm.Header(&aMessageInfo.GetSockAddr(), sizeof(aMessageInfo.GetSockAddr()));
@@ -2691,15 +2685,14 @@ void Mle::HandleUdpReceive(Message &aMessage, const Ip6::MessageInfo &aMessageIn
     while (aMessage.GetOffset() < aMessage.GetLength())
     {
         length = aMessage.Read(aMessage.GetOffset(), sizeof(buf), buf);
-        aesCcm.Payload(buf, buf, length, false);
+        aesCcm.Payload(buf, buf, length, Crypto::AesCcm::kDecrypt);
 #ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
         aMessage.Write(aMessage.GetOffset(), length, buf);
 #endif
         aMessage.MoveOffset(length);
     }
 
-    tagLength = sizeof(tag);
+    aesCcm.Finalize(tag);
-    aesCcm.Finalize(tag, &tagLength);
 #ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
     VerifyOrExit(memcmp(messageTag, tag, sizeof(tag)) == 0, error = OT_ERROR_SECURITY);
 #endif

tests/unit/test_aes.cpp

@@ -68,15 +68,17 @@ void TestMacBeaconFrame(void)
     VerifyOrQuit(instance != NULL, "Null OpenThread instance");
 
     aesCcm.SetKey(key, sizeof(key));
-    SuccessOrQuit(aesCcm.Init(headerLength, payloadLength, tagLength, nonce, sizeof(nonce)), "AesCcm::Init() failed");
+    aesCcm.Init(headerLength, payloadLength, tagLength, nonce, sizeof(nonce));
     aesCcm.Header(test, headerLength);
-    aesCcm.Finalize(test + headerLength, &tagLength);
+    VerifyOrQuit(aesCcm.GetTagLength() == tagLength, "AesCcm::GetTagLength() failed");
+    aesCcm.Finalize(test + headerLength);
 
     VerifyOrQuit(memcmp(test, encrypted, sizeof(encrypted)) == 0, "TestMacBeaconFrame encrypt failed");
 
-    SuccessOrQuit(aesCcm.Init(headerLength, payloadLength, tagLength, nonce, sizeof(nonce)), "AesCcm::Init() failed");
+    aesCcm.Init(headerLength, payloadLength, tagLength, nonce, sizeof(nonce));
     aesCcm.Header(test, headerLength);
-    aesCcm.Finalize(test + headerLength, &tagLength);
+    VerifyOrQuit(aesCcm.GetTagLength() == tagLength, "AesCcm::GetTagLength() failed");
+    aesCcm.Finalize(test + headerLength);
 
     VerifyOrQuit(memcmp(test, decrypted, sizeof(decrypted)) == 0, "TestMacBeaconFrame decrypt failed");
 
@@ -119,16 +121,18 @@ void TestMacCommandFrame()
 
     ot::Crypto::AesCcm aesCcm;
     aesCcm.SetKey(key, sizeof(key));
-    SuccessOrQuit(aesCcm.Init(headerLength, payloadLength, tagLength, nonce, sizeof(nonce)), "AesCcm::Init() failed");
+    aesCcm.Init(headerLength, payloadLength, tagLength, nonce, sizeof(nonce));
     aesCcm.Header(test, headerLength);
-    aesCcm.Payload(test + headerLength, test + headerLength, payloadLength, true);
+    aesCcm.Payload(test + headerLength, test + headerLength, payloadLength, ot::Crypto::AesCcm::kEncrypt);
-    aesCcm.Finalize(test + headerLength + payloadLength, &tagLength);
+    VerifyOrQuit(aesCcm.GetTagLength() == tagLength, "AesCcm::GetTagLength() failed");
+    aesCcm.Finalize(test + headerLength + payloadLength);
     VerifyOrQuit(memcmp(test, encrypted, sizeof(encrypted)) == 0, "TestMacCommandFrame encrypt failed");
 
-    SuccessOrQuit(aesCcm.Init(headerLength, payloadLength, tagLength, nonce, sizeof(nonce)), "AesCcm::Init() failed");
+    aesCcm.Init(headerLength, payloadLength, tagLength, nonce, sizeof(nonce));
     aesCcm.Header(test, headerLength);
-    aesCcm.Payload(test + headerLength, test + headerLength, payloadLength, false);
+    aesCcm.Payload(test + headerLength, test + headerLength, payloadLength, ot::Crypto::AesCcm::kDecrypt);
-    aesCcm.Finalize(test + headerLength + payloadLength, &tagLength);
+    VerifyOrQuit(aesCcm.GetTagLength() == tagLength, "AesCcm::GetTagLength() failed");
+    aesCcm.Finalize(test + headerLength + payloadLength);
 
     VerifyOrQuit(memcmp(test, decrypted, sizeof(decrypted)) == 0, "TestMacCommandFrame decrypt failed");
 }