Improve ckm deserialization errors detection 63/213463/4
authorAlicja Kluczek <a.kluczek@samsung.com>
Tue, 3 Sep 2019 10:10:13 +0000 (12:10 +0200)
committerKrzysztof Jackiewicz <k.jackiewicz@samsung.com>
Mon, 9 Sep 2019 07:33:31 +0000 (07:33 +0000)
Add a check to TZSerializableBinary::Deserialize making sure
that deserialized buffer has adequate size.
    * In case of fixed-size data, buffer size should be equal to the
      size given in constructor.
    * In case of variable-size data, buffer size should be less or equal
      to the size given in constructor.

Change-Id: Ie0f80169adb8b758cb7aa2370551bd30410dc8b0

src/manager/crypto/tz-backend/tz-context.cpp
src/manager/crypto/tz-backend/tz-serializer.cpp
src/manager/crypto/tz-backend/tz-serializer.h

index 917716e..9dcd13f 100644 (file)
@@ -176,14 +176,6 @@ void TrustZoneContext::generateSKeyPwd(tz_algo_type algo,
        sOut.Deserialize(outMemory);
        sOut.Pull(keyId);
        sOut.Pull(pwdTag);
-
-       if (keyId.size() != KM_KEY_ID_SIZE) {
-               ThrowErr(Exc::Crypto::InternalError, "Deserialized incorrect key ID");
-       }
-
-       if (pwdTag.size() != Params::DEFAULT_AES_GCM_TAG_LEN_BYTES) {
-               ThrowErr(Exc::Crypto::InternalError, "Deserialized incorrect key tag");
-       }
 }
 
 void TrustZoneContext::GenerateAKey(tz_command commandId,
@@ -219,9 +211,13 @@ void TrustZoneContext::GenerateAKey(tz_command commandId,
 
        TZSerializer sOut;
        sOut.Push(new TZSerializableBinary(KM_KEY_ID_SIZE));
-       sOut.Push(new TZSerializableBinary(pubTagSize));
+       if (pubTagSize) {
+               sOut.Push(new TZSerializableBinary(pubTagSize));
+       }
        sOut.Push(new TZSerializableBinary(KM_KEY_ID_SIZE));
-       sOut.Push(new TZSerializableBinary(privTagSize));
+       if (privTagSize) {
+               sOut.Push(new TZSerializableBinary(privTagSize));
+       }
 
        TrustZoneMemory outMemory(m_Context, sOut.GetSize(), TEEC_MEM_OUTPUT);
 
@@ -240,18 +236,12 @@ void TrustZoneContext::GenerateAKey(tz_command commandId,
        sOut.Deserialize(outMemory);
 
        sOut.Pull(pubKeyId);
-       if (pubKeyId.size() != KM_KEY_ID_SIZE) {
-               ThrowErr(Exc::Crypto::InternalError, "Failed to deserialize public key ID");
-       }
 
        if (pubPwdExists) {
                sOut.Pull(pubKeyTag);
        }
 
        sOut.Pull(privKeyId);
-       if (privKeyId.size() != KM_KEY_ID_SIZE) {
-               ThrowErr(Exc::Crypto::InternalError, "Failed to deserialize private key ID");
-       }
 
        if (privPwdExists) {
                sOut.Pull(privKeyTag);
@@ -358,7 +348,7 @@ void TrustZoneContext::executeCrypt(tz_command cmd,
        }
 
        TZSerializer sOut;
-       sOut.Push(new TZSerializableBinary(outMemorySize));
+       sOut.Push(new TZSerializableBinary(outMemorySize, false));
        TrustZoneMemory outMemory(m_Context, sOut.GetSize(), TEEC_MEM_OUTPUT);
 
        TEEC_Operation op;
@@ -413,7 +403,7 @@ void TrustZoneContext::executeEncryptAE(const RawBuffer &keyId,
        uint32_t tagSizeBytes = (tagSizeBits + 7) / 8;
 
        TZSerializer sOut;
-       sOut.Push(new TZSerializableBinary(outMemorySize));
+       sOut.Push(new TZSerializableBinary(outMemorySize, false));
        sOut.Push(new TZSerializableBinary(tagSizeBytes));
        TrustZoneMemory outMemory(m_Context, sOut.GetSize(), TEEC_MEM_OUTPUT);
 
@@ -515,7 +505,7 @@ void TrustZoneContext::executeSign(tz_algo_type algo,
        sIn.Serialize(inMemory);
 
        TZSerializer sOut;
-       sOut.Push(new TZSerializableBinary(MAX_KEY_SIZE.at(algo)));
+       sOut.Push(new TZSerializableBinary(MAX_KEY_SIZE.at(algo), false));
        TrustZoneMemory outMemory(m_Context, sOut.GetSize(), TEEC_MEM_OUTPUT);
 
        TEEC_Operation op;
index d8fc107..764c532 100644 (file)
@@ -47,16 +47,20 @@ void TZSerializable::Pull(uint32_t &) const
 
 
 // TZSerializableBinary
-TZSerializableBinary::TZSerializableBinary(uint32_t data_size)
+TZSerializableBinary::TZSerializableBinary(uint32_t data_size, bool is_size_fixed)
 {
        m_data.data = nullptr;
        m_data.data_size = data_size;
+       m_isSizeFixed = is_size_fixed;
+       m_expectedSize = data_size;
 }
 
 TZSerializableBinary::TZSerializableBinary(const RawBuffer &data)
 {
        m_data.data = data.empty() ? nullptr : const_cast<unsigned char *>(data.data());
        m_data.data_size = data.size();
+       m_isSizeFixed = true;
+       m_expectedSize = data.size();
 }
 
 uint32_t TZSerializableBinary::GetSize() const
@@ -71,7 +75,17 @@ int TZSerializableBinary::Serialize(void **buffer, uint32_t *size_guard) const
 
 int TZSerializableBinary::Deserialize(void **buffer, uint32_t *size_guard)
 {
-       return KM_DeserializeBinaryData(buffer, size_guard, &m_data);
+       int ret = KM_DeserializeBinaryData(buffer, size_guard, &m_data);
+       if (m_isSizeFixed) {
+               if (m_data.data_size != m_expectedSize) {
+                       ThrowErr(Exc::Crypto::InternalError, "Size of deserialized data differ from size given in constructor.");
+               }
+       } else {
+               if (m_data.data_size > m_expectedSize) {
+                       ThrowErr(Exc::Crypto::InternalError, "Size of deserialized data is bigger than size given in constructor.");
+               }
+       }
+       return ret;
 }
 
 void TZSerializableBinary::Pull(RawBuffer &buffer) const
index 6a5b572..a3fbbd9 100644 (file)
@@ -55,7 +55,7 @@ public:
 
 class TZSerializableBinary : public TZSerializable {
 public:
-       explicit TZSerializableBinary(uint32_t data_size);
+       explicit TZSerializableBinary(uint32_t data_size, bool is_size_fixed = true);
        explicit TZSerializableBinary(const RawBuffer &data);
        uint32_t GetSize() const override;
        int Serialize(void **buffer, uint32_t *size_guard) const override;
@@ -63,6 +63,8 @@ public:
        void Pull(RawBuffer &buffer) const override;
 private:
        KM_BinaryData m_data;
+       bool m_isSizeFixed;
+       uint32_t m_expectedSize;
 };