From 2e37f7601380a09ac635941581387d4ac6b54f12 Mon Sep 17 00:00:00 2001 From: Never Date: Fri, 9 Dec 2016 13:51:55 +0100 Subject: Rewrote bc unpad functions as const time operations. The unpad functions return the blocksize as padding position, if the padding is invalid. . --- src/lib/modes/cbc/cbc.cpp | 8 ++- src/lib/modes/mode_pad/mode_pad.cpp | 98 ++++++++++++++++++++++--------------- src/tests/test_pad.cpp | 11 ++--- 3 files changed, 70 insertions(+), 47 deletions(-) (limited to 'src') diff --git a/src/lib/modes/cbc/cbc.cpp b/src/lib/modes/cbc/cbc.cpp index 592ff95e9..9fbb7023f 100644 --- a/src/lib/modes/cbc/cbc.cpp +++ b/src/lib/modes/cbc/cbc.cpp @@ -19,8 +19,8 @@ CBC_Mode::CBC_Mode(BlockCipher* cipher, BlockCipherModePaddingMethod* padding) : { if(m_padding && !m_padding->valid_blocksize(cipher->block_size())) throw Invalid_Argument("Padding " + m_padding->name() + - " cannot be used with " + - cipher->name() + "/CBC"); + " cannot be used with " + + cipher->name() + "/CBC"); } void CBC_Mode::clear() @@ -243,6 +243,10 @@ void CBC_Decryption::finish(secure_vector& buffer, size_t offset) const size_t pad_bytes = BS - padding().unpad(&buffer[buffer.size()-BS], BS); buffer.resize(buffer.size() - pad_bytes); // remove padding + if(pad_bytes == 0 && padding().name() != "NoPadding") + { + throw Decoding_Error(name()); + } } void CBC_Decryption::reset() diff --git a/src/lib/modes/mode_pad/mode_pad.cpp b/src/lib/modes/mode_pad/mode_pad.cpp index eb4ae42be..893795089 100644 --- a/src/lib/modes/mode_pad/mode_pad.cpp +++ b/src/lib/modes/mode_pad/mode_pad.cpp @@ -8,6 +8,7 @@ #include #include +#include namespace Botan { @@ -52,16 +53,23 @@ void PKCS7_Padding::add_padding(secure_vector& buffer, */ size_t PKCS7_Padding::unpad(const byte block[], size_t size) const { - size_t position = block[size-1]; + CT::poison(block,size); + size_t bad_input = 0; + const byte last_byte = block[size-1]; - if(position > size) - throw Decoding_Error("Bad padding in " + name()); + bad_input |= CT::expand_mask(last_byte > size); - for(size_t j = size-position; j != size-1; ++j) - if(block[j] != position) - throw Decoding_Error("Bad padding in " + name()); + size_t pad_pos = size - last_byte; + size_t i = size - 2; + while(i) + { + bad_input |= ~CT::is_equal(block[i],last_byte) & CT::expand_mask(i >= pad_pos); + --i; + } - return (size-position); + CT::conditional_copy_mem(bad_input,&pad_pos,&size,&pad_pos,1); + CT::unpoison(block,size); + return pad_pos; } /* @@ -85,13 +93,22 @@ void ANSI_X923_Padding::add_padding(secure_vector& buffer, */ size_t ANSI_X923_Padding::unpad(const byte block[], size_t size) const { - size_t position = block[size-1]; - if(position > size) - throw Decoding_Error(name()); - for(size_t j = size-position; j != size-1; ++j) - if(block[j] != 0) - throw Decoding_Error(name()); - return (size-position); + CT::poison(block,size); + size_t bad_input = 0; + const size_t last_byte = block[size-1]; + + bad_input |= CT::expand_mask(last_byte > size); + + size_t pad_pos = size - last_byte; + size_t i = size - 2; + while(i) + { + bad_input |= ~CT::is_zero(block[i]) & CT::expand_mask(i >= pad_pos); + --i; + } + CT::conditional_copy_mem(bad_input,&pad_pos,&size,&pad_pos,1); + CT::unpoison(block,size); + return pad_pos; } /* @@ -112,25 +129,33 @@ void OneAndZeros_Padding::add_padding(secure_vector& buffer, */ size_t OneAndZeros_Padding::unpad(const byte block[], size_t size) const { - while(size) + CT::poison(block, size); + byte bad_input = 0; + byte seen_one = 0; + size_t pad_pos = size - 1; + size_t i = size; + + while(i) { - if(block[size-1] == 0x80) - break; - if(block[size-1] != 0x00) - throw Decoding_Error(name()); - size--; + seen_one |= CT::is_equal(block[i-1],0x80); + pad_pos -= CT::select(~seen_one, 1, 0); + bad_input |= ~CT::is_zero(block[i-1]) & ~seen_one; + i--; } - if(!size) - throw Decoding_Error(name()); - return (size-1); + bad_input |= ~seen_one; + + CT::conditional_copy_mem(size_t(bad_input),&pad_pos,&size,&pad_pos,1); + CT::unpoison(block, size); + + return pad_pos; } /* * Pad with ESP Padding Method */ void ESP_Padding::add_padding(secure_vector& buffer, - size_t last_byte_pos, - size_t block_size) const + size_t last_byte_pos, + size_t block_size) const { byte pad_value = 0x01; @@ -145,26 +170,21 @@ void ESP_Padding::add_padding(secure_vector& buffer, */ size_t ESP_Padding::unpad(const byte block[], size_t size) const { - const byte last_byte = block[size-1]; - if(last_byte > size) - { - throw Decoding_Error(name()); - } + CT::poison(block,size); + + const size_t last_byte = block[size-1]; + size_t bad_input = 0; + bad_input |= CT::expand_mask(last_byte > size); - // try to do this in const time by looping over the entire block - const size_t pad_pos = size - last_byte; + size_t pad_pos = size - last_byte; size_t i = size - 1; while(i) { - if(block[i-1] != block[i]-1) - { - if(i > pad_pos) - { - throw Decoding_Error(name()); - } - } + bad_input |= ~CT::is_equal(size_t(block[i-1]),size_t(block[i])-1) & CT::expand_mask(i > pad_pos); --i; } + CT::conditional_copy_mem(bad_input,&pad_pos,&size,&pad_pos,1); + CT::unpoison(block, size); return pad_pos; } diff --git a/src/tests/test_pad.cpp b/src/tests/test_pad.cpp index bf9e64c0d..c27e2607f 100644 --- a/src/tests/test_pad.cpp +++ b/src/tests/test_pad.cpp @@ -7,7 +7,7 @@ #include "tests.h" #if defined(BOTAN_HAS_CIPHER_MODE_PADDING) - #include + #include #endif namespace Botan_Tests { @@ -18,7 +18,7 @@ class Cipher_Mode_Padding_Tests : public Text_Based_Test { public: Cipher_Mode_Padding_Tests() : - Text_Based_Test("", "pad.vec", {"In", "Blocksize"},{"Out"}) + Text_Based_Test("", "pad.vec", {"In", "Blocksize"}, {"Out"}) {} Test::Result run_one_test(const std::string& header, const VarMap& vars) override @@ -50,12 +50,11 @@ class Cipher_Mode_Padding_Tests : public Text_Based_Test if(expected.empty()) { // This is an unpad an invalid input and ensure we reject - try + if(pad->unpad(input.data(), block_size) != block_size) { - pad->unpad(input.data(), block_size); result.test_failure("Did not reject invalid padding", Botan::hex_encode(input)); } - catch(Botan::Decoding_Error&) + else { result.test_success("Rejected invalid padding"); } @@ -69,7 +68,7 @@ class Cipher_Mode_Padding_Tests : public Text_Based_Test buf.assign(expected.begin(), expected.end()); - const size_t last_block = ( buf.size() < block_size ) ? 0 : buf.size() - block_size; + const size_t last_block = (buf.size() < block_size) ? 0 : buf.size() - block_size; const size_t pad_bytes = block_size - pad->unpad(&buf[last_block], block_size); buf.resize(buf.size() - pad_bytes); // remove padding result.test_eq("unpad", buf, input); -- cgit v1.2.3 From 672ca8bfff4075a5feb5cfe4313d7ad2c6c1fe4a Mon Sep 17 00:00:00 2001 From: Never Date: Fri, 9 Dec 2016 14:32:56 +0100 Subject: forgot to unpoison return vals --- src/lib/modes/mode_pad/mode_pad.cpp | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src') diff --git a/src/lib/modes/mode_pad/mode_pad.cpp b/src/lib/modes/mode_pad/mode_pad.cpp index 893795089..c84c2030e 100644 --- a/src/lib/modes/mode_pad/mode_pad.cpp +++ b/src/lib/modes/mode_pad/mode_pad.cpp @@ -69,6 +69,7 @@ size_t PKCS7_Padding::unpad(const byte block[], size_t size) const CT::conditional_copy_mem(bad_input,&pad_pos,&size,&pad_pos,1); CT::unpoison(block,size); + CT::unpoison(pad_pos); return pad_pos; } @@ -108,6 +109,7 @@ size_t ANSI_X923_Padding::unpad(const byte block[], size_t size) const } CT::conditional_copy_mem(bad_input,&pad_pos,&size,&pad_pos,1); CT::unpoison(block,size); + CT::unpoison(pad_pos); return pad_pos; } @@ -146,6 +148,7 @@ size_t OneAndZeros_Padding::unpad(const byte block[], size_t size) const CT::conditional_copy_mem(size_t(bad_input),&pad_pos,&size,&pad_pos,1); CT::unpoison(block, size); + CT::unpoison(pad_pos); return pad_pos; } @@ -185,6 +188,7 @@ size_t ESP_Padding::unpad(const byte block[], size_t size) const } CT::conditional_copy_mem(bad_input,&pad_pos,&size,&pad_pos,1); CT::unpoison(block, size); + CT::unpoison(pad_pos); return pad_pos; } -- cgit v1.2.3