3

TrueCrypt (7.1a) で Serpent の実装をブラウズしていたのですが、何かが正しくないようです! このアルゴリズムのインターフェースは次のとおりです。

void serpent_set_key(const unsigned __int8 userKey[], int keylen, unsigned __int8 *ks);
void serpent_encrypt(const unsigned __int8 *inBlock, unsigned __int8 *outBlock, unsigned __int8 *ks);
void serpent_decrypt(const unsigned __int8 *inBlock,  unsigned __int8 *outBlock, unsigned __int8 *ks);

ここで注目する関数はserpent_set_key. ユーザー キーの長さは 32 バイトで、 はkeylenそのサイズである必要があり、ksは暗号化/復号化に使用される出力キーです。問題は実装です。冒頭の関連するフラグメントは次のとおりです。

unsigned __int32 a,b,c,d,e;
unsigned __int32 *k = (unsigned __int32 *)ks;
unsigned __int32 t;
int i;

for (i = 0; i < keylen / (int)sizeof(__int32); i++)
    k[i] = LE32(((unsigned __int32*)userKey)[i]);

for ループは、実際にはユーザー キーから実装キーにデータをコピーしています。これは、データを 4 バイトの整数として「見る」ことによって行われます。ここで、キー len がバイトとして送信されれば問題ありません (32 が正しい値です) が...

trueCrypt のすべての実装で、これは 2 つの場所で呼び出されます。これが最初のものです: CipherInit では、次のように呼び出されます:

case SERPENT:
    serpent_set_key (key, CipherGetKeySize(SERPENT) * 8, ks);
    break;

CipherGetKeySize(SERPENT)32 (バイト) を返すため、渡されたパラメーターの値は 256 になります! キーの長さに関しては正しいのですが、この実装ではそうではありません! for ループが 8 回ではなく 64 回実行されるため、これにより 'serpent_set_key' でバッファ オーバーフローが発生します。これが呼び出される他の場所は、次のように EAInit にあります。

serpent_set_key (キー、32 * 8、ks);

ここで、渡されたパラメーターが 256 になることは明らかです。

他の人がこれについてどう思うか興味がありますか？他の誰かがこのバグを確認できますか?

4

1 に答える 1

3

As VeraCrypt main developer, a user has redirected me to this post since VeraCrypt is based on TrueCrypt source .

After studying the issue you raised, I can confirm that this is indeed a mistake in the code and that calls to serpent_set_key should pass 32 instead of 256 as a parameter.

Luckily, this mistake has no effect on the correctness or security during the execution of the program, this is why no body discovered it before you. Thus, we can NOT qualify this as a bug.

Let me explain this in three points :

  1. let's look at the Serpent algorithm implementation of serpent_set_key : the parameter keylen is used only for copying user key into the ks buffer which is guaranteed to have a minimum size of 560 (look at the SERPENT_KS define in crypt.h). Thus, even if keylen is 256 instead of 32, we will never write beyond the allocated memory of ks.
  2. the internal key expansion that comes after this loop will build the expanded Serpent key using the first 32 bytes of userKey only as in the Serpent algorithm specification. Thus, all bytes coming after the first 32 ones will be discarded and they will be never used. This explains why the calculation result is correct even if 256 bytes are passed to it instead of the expected 32 bytes.
  3. if we list all the runtime calls the leads to serpent_set_key we'll notice that, except for the case of auto tests, all the calls use a 256 bytes buffer for the userKey parameter even if its first 32 bytes are filled (look at MASTER_KEYDATA_SIZE in crypto.h). So, during runtime, we'll never read beyond the allocate buffer space. It remains the case of the auto tests (e.i. in Tests.c or CipherTestDialogProc in Dlgcode.c) where a 32 bytes buffer is used for userKey : here we will read beyond the allocated space but in practice it doesn't cause any harm because the memory around this buffer is readable.

I hope this clarifies why this mistake is harmless. That being said, it has to be corrected and that's what we'll do in VeraCrypt.

For the record, it appears that this mistake was caused by mix-up between twofish_set_key and serpent_set_key : the declaration of the two functions have the same type of parameters but twofish_set_key expects the user key length in bits whereas serpent_set_key expect it in bytes! Clearly, we should have the same convention for the size of a key.

于 2014-09-27T07:05:39.487 に答える