[erlang-patches] [PATCH] fix handling of ssl_cipher:block_decipher/5 failure

[Ingela Anderton Andin]

Hi!

Thank you for the explanation I am with you now.

Regards Ingela Erlang/OTP team - Ericsson AB


Andreas Schultz wrote:
> Hi,
>
> ----- Original Message -----
>   
>> Hi!
>>
>> This patch is to late for R14B04, but I think it is a good patch and
>> I want to include it for the next release.
>> I was just thinking that  should not the aes_decipher_fail-test test
>> that the Contetent and Mac values are
>> not the expected ones and not only test that they are "something" of
>> the expected size?
>>     
>
> I am not sure that this would actually make a difference. I was not trying
> to check if AES works (thats a test for the crypto module). The test is
> supposed to check that handle padding correctly and deal with the fallout
> of an decryption failure in the right way.
>
> It is not really obvious from the test, but the expected size in the good case
> is actually 22, so getting something different means that the decryption must
> have failed. The plain text contains a 10 byte padding, which can not be stripped
> when the encryption fails.
>
> Also, the real test is that we get the three element tupple and not a ?BAD_RECORD_MAC
> alert or (as would happen without the generic_block_cipher_from_bin change) a badmatch
> error. I did choose the bad key in a way that it would lead to a padding length greater
> than the text length.
>
> Regards
> Andreas
>
>   
>> Regards Ingela Erlang/OTP team - Ericsson AB
>>
>> Andreas Schultz wrote:
>>     
>>> Hi all,
>>>
>>> Included is a change to fix a badmatch error in
>>> ssl_cipher:generic_block_cipher_from_bin/2 and implement a CBC
>>> timming attack counter measure in ssl_cipher:block_decipher/5.
>>> Both changes are closely related.
>>>
>>>    git fetch git@REDACTED:RoadRunnr/otp.git ssl-cbc-fix
>>>
>>> ssl_cipher:generic_block_cipher_from_bin/2 would generate a
>>> badmatch
>>> error when the padding length was greater than the overall data.
>>> This
>>> can happen when the decryption resulted in invalid data. It seems
>>> to
>>> me, that the try in block_decipher/5 was supposed to catch that,
>>> but
>>> it did not.
>>>
>>> Also, RFC 5246 suggests a counter measure for a CBC timing attack
>>> on
>>> the MAC calculation. This can easily be implemented by not
>>> generating
>>> the error alert in block_decipher/5 and invalidating the decoded
>>> text.
>>>
>>> It would also be possible to extend the return value of
>>> block_decipher
>>> with the result of the padding check and test that value later.
>>> However,
>>> this would also require changes to generic_block_cipher_from_bin/2.
>>>
>>> Thanks
>>> Andreas
>>>
>>>   
>>>       
>>     
>
>