fix all paddding issues

abumq · Aug 29, 2017 · bd448bb · bd448bb
1 parent 9c7852b
commit bd448bb
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 29 deletions.
diff --git a/src/aes.cc b/src/aes.cc
@@ -622,6 +622,26 @@ std::string AES::resolveOutputMode(const ByteArray& input, Encoding outputMode)
     return Base64::encode(input.begin(), input.end());
 }
 
+std::size_t AES::getPaddingIndex(const ByteArray& byteArr)
+{
+    char lastChar = byteArr[kBlockSize - 1];
+    int c = lastChar & 0xff;
+    if (c > 0 && c < kBlockSize - 1) {
+        bool validPadding = true;
+        for (int chkIdx = kBlockSize - c; chkIdx < kBlockSize; ++chkIdx) {
+            if ((byteArr[chkIdx] & 0xff) != c) {
+                // with openssl we found padding
+                validPadding = false;
+                break;
+            }
+        }
+        if (validPadding) {
+            return kBlockSize - c;
+        }
+    }
+    return kBlockSize;
+}
+
 // public
 
 ByteArray AES::encrypt(const ByteArray& input, const Key* key, bool pkcs5Padding)
@@ -685,6 +705,10 @@ ByteArray AES::decrypt(const ByteArray& input, const Key* key)
         }
         ByteArray outputBlock = decryptSingleBlock(inputBlock.begin(), key, &keySchedule);
 
+        if (i + kBlockSize == inputSize) {
+            // check padding
+            j = getPaddingIndex(outputBlock);
+        }
         std::copy_n(outputBlock.begin(), j, std::back_inserter(result));
     }
     return result;
@@ -785,21 +809,7 @@ ByteArray AES::decrypt(const ByteArray& input, const Key* key, ByteArray& iv)
 
         if (i + kBlockSize == inputSize) {
             // check padding
-            char lastChar = outputBlock[kBlockSize - 1];
-            int c = lastChar & 0xff;
-            if (c > 0 && c < kBlockSize - 1) {
-                bool validPadding = true;
-                for (int chkIdx = c; chkIdx < kBlockSize - 2; ++chkIdx) {
-                    if ((outputBlock[chkIdx] & 0xff) != c) {
-                        // with openssl we found padding
-                        validPadding = false;
-                        break;
-                    }
-                }
-                if (validPadding) {
-                    j = c;
-                }
-            }
+            j = getPaddingIndex(outputBlock);
         } else {
             nextXorWithBeg = input.begin() + i;
             nextXorWithEnd = input.begin() + i + kBlockSize;

diff --git a/src/aes.h b/src/aes.h
@@ -368,6 +368,11 @@ class AES {
     ///
     static ByteArray stateToByteArray(const State* state);
 
+    ///
+    /// \brief Get padding index for stripping the padding
+    ///
+    static std::size_t getPaddingIndex(const ByteArray& byteArr);
+
     AES() = delete;
     AES(const AES&) = delete;
     AES& operator=(const AES&) = delete;

diff --git a/test/aes-test.h b/test/aes-test.h
@@ -782,10 +782,13 @@ TEST(AESTest, CbcDecipher)
     }
 }
 
-TEST(AESTest, RealDataIssuesTest)
+TEST(AESTest, CrossAppsDataTest)
 {
-    std::string expected = "WQ73OMIum+OHKGHnAhQKJX1tByfBq4BhSpw2X+SgtjY=";
     std::string iv = "a14c54563269e9e368f56b325f04ff00";
+    const std::string key = "CBD437FA37772C66051A47D72367B38E";
+
+    // genearted using online tool
+    std::string expected = "WQ73OMIum+OHKGHnAhQKJX1tByfBq4BhSpw2X+SgtjY=";
     std::string output = AES::encrypt("test this test this",
                           "CBD437FA37772C66051A47D72367B38E",
                           iv,
@@ -794,10 +797,19 @@ TEST(AESTest, RealDataIssuesTest)
 
     ASSERT_STRCASEEQ(expected.c_str(), output.c_str());
 
+    std::string nextexp = "test this test this";
+    output = AES::decrypt("WQ73OMIum+OHKGHnAhQKJX1tByfBq4BhSpw2X+SgtjY=",
+                          key,
+                          iv,
+                          AES::Encoding::Base64,
+                          AES::Encoding::Raw);
+
+    ASSERT_STRCASEEQ(nextexp.c_str(), output.c_str());
+
     expected = "EtYr5JFo/7kqYWxooMvU2DJ+upNhUMDii9X6IEHYxvUNXSVGk34IakT5H7GbyzL5/JIMMAQCLnUU824RI3ymgQ==";
 
     output = AES::encrypt(R"({"_t":1503928197,"logger_id":"default","access_code":"default"})",
-                          "CBD437FA37772C66051A47D72367B38E",
+                          key,
                           iv,
                           AES::Encoding::Raw,
                           AES::Encoding::Base64);
@@ -809,29 +821,35 @@ TEST(AESTest, RealDataIssuesTest)
     //
     expected = R"({"_t":1503928197,"logger_id":"default","access_code":"default"})";
     output = AES::decrypt("EtYr5JFo/7kqYWxooMvU2DJ+upNhUMDii9X6IEHYxvUNXSVGk34IakT5H7GbyzL5/JIMMAQCLnUU824RI3ymgQ==",
-                                       "CBD437FA37772C66051A47D72367B38E",
-                                       "a14c54563269e9e368f56b325f04ff00",
+                                       key,
+                                       iv,
                                        AES::Encoding::Base64,
                                        AES::Encoding::Raw);
 
     ASSERT_STRCASEEQ(expected.c_str(), output.c_str());
 
+    // generated with ripe
+    // echo test this test this | ripe -e --aes --key CBD437FA37772C66051A47D72367B38E --iv a14c54563269e9e368f56b325f04ff00
     expected = "test this test this";
-    output = AES::decrypt("WQ73OMIum+OHKGHnAhQKJc/uwM2APneVOH9mBq15bOk=",
-                                       "CBD437FA37772C66051A47D72367B38E",
-                                       "a14c54563269e9e368f56b325f04ff00",
+    output = AES::decrypt("WQ73OMIum+OHKGHnAhQKJX1tByfBq4BhSpw2X+SgtjY=",
+                                       key,
+                                       iv,
                                        AES::Encoding::Base64,
                                        AES::Encoding::Raw);
 
 
     ASSERT_STRCASEEQ(expected.c_str(), output.c_str());
 
-    expected = R"({"_t":1503928197,"logger_id":"default","access_code":"default"})";
-    output = AES::decrypt("12D62BE49168FFB92A616C68A0CBD4D8327EBA936150C0E28BD5FA2041D8C6F50D5D2546937E086A44F91FB19BCB32F9FC920C3004022E7514F36E11237CA681",
-                                       "CBD437FA37772C66051A47D72367B38E",
-                                       "a14c54563269e9e368f56b325f04ff00",
-                                       AES::Encoding::Base16,
+     // generated with openssl
+    // echo test this test this | openssl enc -aes-128-cbc -K CBD437FA37772C66051A47D72367B38E -iv a14c54563269e9e368f56b325f04ff00 -base64
+    expected = "test this test this\n"; // openssl adds newline char
+    output = AES::decrypt("WQ73OMIum+OHKGHnAhQKJdSsXR5NwysOnq+cuf5C6cs=",
+                                       key,
+                                       iv,
+                                       AES::Encoding::Base64,
                                        AES::Encoding::Raw);
+
+
     ASSERT_STRCASEEQ(expected.c_str(), output.c_str());
 }
 

diff --git a/test/main.cc b/test/main.cc
@@ -2,7 +2,7 @@
 #include "base16-test.h"
 #include "base64-test.h"
 //#include "zlib-test.h"
-//#include "rsa-test.h"
+#include "rsa-test.h"
 #include "aes-test.h"
 
 INITIALIZE_EASYLOGGINGPP