Fixed use of ECB with Botan

[antoinelochet]

Here is my proposed fix for issue 723

[bjosv]

Seems reasonable.
Are all the extra debug printouts needed? I seems that existing printouts are quite sparse with information, maybe for security reasons(?).

As an addition I suggest that we also disable 2 testcases when built WITH_BOTAN.
They will always fail when using Botan as I understand it. WDYT?

1) test: AESTests::testECB (F) line: 518 AESTests.cpp
assertion failed
- Expression: aes->encryptInit(&aesKey128, SymMode::ECB, IV)


2) test: DESTests::testECB (F) line: 537 DESTests.cpp
assertion failed
- Expression: des->encryptInit(&desKey56, SymMode::ECB, IV)

[antoinelochet]

I think that you are right about the tests.

I have added the debug logs because they are often useful. I agree that some security reasons, it may seem bad, but:

• using SoftHSM in production is madness
• using SoftHSM in production WITH debug logs is beyond madness

So I thought this was tolerable. But per your request, I deleted all the offending lines.

[bjosv]

Good comments. Using debug logs in production would be "beyond madness" :)

[anpa8480]

if you revert BotanSymmetricAlgorithm.cpp 8fd89ec and uncomment if (mode == SymMode::ECB) all tests are passing.

Tested with Botan 2.19.3

[antoinelochet]

if you revert BotanSymmetricAlgorithm.cpp 8fd89ec and uncomment if (mode == SymMode::ECB) all tests are passing.

Tested with Botan 2.19.3

That's because ECB is not taken into account during compilation phase. It does not fix the failing tests in src/lib

[anpa8480]

if you revert BotanSymmetricAlgorithm.cpp 8fd89ec and uncomment if (mode == SymMode::ECB) all tests are passing.
Tested with Botan 2.19.3

That's because ECB is not taken into account during compilation phase. It does not fix the failing tests in src/lib

Sorry I dont understand what do you mean "ECB is not taken into account". You mean because Botan_ecb.cpp is missing in CMakelists.txt? The test was performed with dev branch and not with fix-botan-ecb

[antoinelochet]

I will close this PR when #771 is merged since it does a better job than this.

[antoinelochet]

#771 is merged