Changes for base64
authorHardening <rdp.effort@gmail.com>
Sun, 11 May 2014 20:42:01 +0000 (22:42 +0200)
committerHardening <rdp.effort@gmail.com>
Sun, 11 May 2014 20:49:10 +0000 (22:49 +0200)
This patch changes the prototype for decode_base64 so that the encode / decode
method are consistant (encode(BYTE *) => char* and decode(char*) => BYTE*).
It also does some improvements with unrolling loops so that end conditions are
tested only at the end.
The patch also adds some unitary tests.
Before the patch base64_decode() made valgrind complain about uninitialized
bits, after valgrind is happy and very quiet.

client/common/cmdline.c
include/freerdp/crypto/crypto.h
libfreerdp/core/gateway/ncacn_http.c
libfreerdp/crypto/CMakeLists.txt
libfreerdp/crypto/base64.c
libfreerdp/crypto/test/CMakeLists.txt [new file with mode: 0644]
libfreerdp/crypto/test/TestBase64.c [new file with mode: 0644]

index d26206f..85847b3 100644 (file)
@@ -1772,8 +1772,8 @@ int freerdp_client_settings_parse_command_line_arguments(rdpSettings* settings,
                {
                        BYTE *base64;
                        int length;
-                       crypto_base64_decode((BYTE *) (arg->Value),
-                               (int) strlen(arg->Value), &base64, &length);
+                       crypto_base64_decode((const char *) (arg->Value), (int) strlen(arg->Value),
+                                                               &base64, &length);
                        if ((base64 != NULL) && (length == sizeof(ARC_SC_PRIVATE_PACKET)))
                        {
                                memcpy(settings->ServerAutoReconnectCookie, base64, length);
index a022dde..e642deb 100644 (file)
@@ -144,6 +144,6 @@ FREERDP_API void crypto_reverse(BYTE* data, int length);
 FREERDP_API void crypto_nonce(BYTE* nonce, int size);
 
 FREERDP_API char* crypto_base64_encode(const BYTE* data, int length);
-FREERDP_API void crypto_base64_decode(const BYTE* enc_data, int length, BYTE** dec_data, int* res_length);
+FREERDP_API void crypto_base64_decode(const char* enc_data, int length, BYTE** dec_data, int* res_length);
 
 #endif /* FREERDP_CRYPTO_H */
index 76e76d6..270dafb 100644 (file)
@@ -106,7 +106,7 @@ int rpc_ncacn_http_recv_in_channel_response(rdpRpc* rpc)
                        goto out;
 
                ntlm_token_data = NULL;
-               crypto_base64_decode((BYTE*) token64, strlen(token64), &ntlm_token_data, &ntlm_token_length);
+               crypto_base64_decode(token64, strlen(token64), &ntlm_token_data, &ntlm_token_length);
        }
 
        ntlm->inputBuffer[0].pvBuffer = ntlm_token_data;
@@ -238,7 +238,7 @@ int rpc_ncacn_http_recv_out_channel_response(rdpRpc* rpc)
        if (http_response && ListDictionary_Contains(http_response->Authenticates, "NTLM"))
        {
                char *token64 = ListDictionary_GetItemValue(http_response->Authenticates, "NTLM");
-               crypto_base64_decode((BYTE*) token64, strlen(token64), &ntlm_token_data, &ntlm_token_length);
+               crypto_base64_decode(token64, strlen(token64), &ntlm_token_data, &ntlm_token_length);
        }
 
        ntlm->inputBuffer[0].pvBuffer = ntlm_token_data;
index e0f93fc..e33200b 100644 (file)
@@ -71,3 +71,7 @@ else()
 endif()
 
 set_property(TARGET ${MODULE_NAME} PROPERTY FOLDER "FreeRDP/libfreerdp")
+
+if(BUILD_TESTING)
+       add_subdirectory(test)
+endif()
index 3f2b617..d492e89 100644 (file)
@@ -34,38 +34,56 @@ char* crypto_base64_encode(const BYTE* data, int length)
        char* p;
        char* ret;
        int i = 0;
+       int blocks;
 
        q = data;
        p = ret = (char*) malloc((length + 3) * 4 / 3 + 1);
        if (!p)
                return NULL;
 
-       while (i < length)
+       /* b1, b2, b3 are input bytes
+        *
+        * 0         1         2
+        * 012345678901234567890123
+        * |  b1  |  b2   |  b3   |
+        *
+        * [ c1 ]     [  c3 ]
+        *      [  c2 ]     [  c4 ]
+        *
+        * c1, c2, c3, c4 are output chars in base64
+        */
+
+       /* first treat complete blocks */
+       blocks = length - (length % 3);
+       for (i = 0; i < blocks; i += 3, q += 3)
        {
-               c = q[i++];
-
-               c <<= 8;
-               if (i < length)
-                       c += q[i];
-               i++;
-
-               c <<= 8;
-               if (i < length)
-                       c += q[i];
-               i++;
+               c = (q[0] << 16) + (q[1] << 8) + q[2];
 
                *p++ = base64[(c & 0x00FC0000) >> 18];
                *p++ = base64[(c & 0x0003F000) >> 12];
+               *p++ = base64[(c & 0x00000FC0) >> 6];
+               *p++ = base64[c & 0x0000003F];
+       }
 
-               if (i > length + 1)
-                       *p++ = '=';
-               else
-                       *p++ = base64[(c & 0x00000FC0) >> 6];
-
-               if (i > length)
-                       *p++ = '=';
-               else
-                       *p++ = base64[c & 0x0000003F];
+       /* then remainder */
+       switch (length % 3)
+       {
+       case 0:
+               break;
+       case 1:
+               c = (q[0] << 16);
+               *p++ = base64[(c & 0x00FC0000) >> 18];
+               *p++ = base64[(c & 0x0003F000) >> 12];
+               *p++ = '=';
+               *p++ = '=';
+               break;
+       case 2:
+               c = (q[0] << 16) + (q[1] << 8);
+               *p++ = base64[(c & 0x00FC0000) >> 18];
+               *p++ = base64[(c & 0x0003F000) >> 12];
+               *p++ = base64[(c & 0x00000FC0) >> 6];
+               *p++ = '=';
+               break;
        }
 
        *p = 0;
@@ -96,55 +114,82 @@ static int base64_decode_char(char c)
        return -1;
 }
 
-static void* base64_decode(const BYTE* s, int length, int* data_len)
+static void* base64_decode(const char* s, int length, int* data_len)
 {
-       char* p;
        int n[4];
        BYTE* q;
        BYTE* data;
+       int nBlocks, i, outputLen;
 
        if (length % 4)
                return NULL;
 
-       n[0] = n[1] = n[2] = n[3] = 0;
        q = data = (BYTE*) malloc(length / 4 * 3);
 
-       for (p = (char*) s; *p; )
+       /* first treat complete blocks */
+       nBlocks = (length / 4);
+       outputLen = 0;
+
+       for (i = 0; i < nBlocks-1; i++, q += 3)
        {
-               n[0] = base64_decode_char(*p++);
-               n[1] = base64_decode_char(*p++);
-               n[2] = base64_decode_char(*p++);
-               n[3] = base64_decode_char(*p++);
-
-               if ((n[0] == -1) || (n[1] == -1))
-               {
-                       free(data);
-                       return NULL;
-               }
-
-               if ((n[2] == -1) && (n[3] != -1))
-               {
-                       free(data);
-                       return NULL;
-               }
+               n[0] = base64_decode_char(*s++);
+               n[1] = base64_decode_char(*s++);
+               n[2] = base64_decode_char(*s++);
+               n[3] = base64_decode_char(*s++);
+
+               if ((n[0] == -1) || (n[1] == -1) || (n[2] == -1) || (n[3] == -1))
+                       goto out_free;
 
                q[0] = (n[0] << 2) + (n[1] >> 4);
+               q[1] = ((n[1] & 15) << 4) + (n[2] >> 2);
+               q[2] = ((n[2] & 3) << 6) + n[3];
+               outputLen += 3;
+       }
 
-               if (n[2] != -1)
-                       q[1] = ((n[1] & 15) << 4) + (n[2] >> 2);
+       /* treat last block */
+       n[0] = base64_decode_char(*s++);
+       n[1] = base64_decode_char(*s++);
+       if ((n[0] == -1) || (n[1] == -1))
+               goto out_free;
 
+       n[2] = base64_decode_char(*s++);
+       n[3] = base64_decode_char(*s++);
+
+       q[0] = (n[0] << 2) + (n[1] >> 4);
+       if (n[2] == -1)
+       {
+               /* XX== */
+               outputLen += 1;
                if (n[3] != -1)
-                       q[2] = ((n[2] & 3) << 6) + n[3];
+                       goto out_free;
 
-               q += 3;
+               q[1] = ((n[1] & 15) << 4);
+       }
+       else if (n[3] == -1)
+       {
+               /* yyy= */
+               outputLen += 2;
+               q[1] = ((n[1] & 15) << 4) + (n[2] >> 2);
+               q[2] = ((n[2] & 3) << 6);
+       }
+       else
+       {
+               /* XXXX */
+               outputLen += 3;
+               q[0] = (n[0] << 2) + (n[1] >> 4);
+               q[1] = ((n[1] & 15) << 4) + (n[2] >> 2);
+               q[2] = ((n[2] & 3) << 6) + n[3];
        }
 
-       *data_len = q - data - (n[2] == -1) - (n[3] == -1);
+       *data_len = outputLen;
 
        return data;
+out_free:
+       free(data);
+       return NULL;
 }
 
-void crypto_base64_decode(const BYTE* enc_data, int length, BYTE** dec_data, int* res_length)
+void crypto_base64_decode(const char* enc_data, int length, BYTE** dec_data, int* res_length)
 {
        *dec_data = base64_decode(enc_data, length, res_length);
 }
diff --git a/libfreerdp/crypto/test/CMakeLists.txt b/libfreerdp/crypto/test/CMakeLists.txt
new file mode 100644 (file)
index 0000000..4333b01
--- /dev/null
@@ -0,0 +1,31 @@
+
+set(MODULE_NAME "TestFreeRDPCrypto")
+set(MODULE_PREFIX "TEST_FREERDP_CRYPTO")
+
+set(${MODULE_PREFIX}_DRIVER ${MODULE_NAME}.c)
+
+set(${MODULE_PREFIX}_TESTS
+       TestBase64.c)
+
+create_test_sourcelist(${MODULE_PREFIX}_SRCS
+       ${${MODULE_PREFIX}_DRIVER}
+       ${${MODULE_PREFIX}_TESTS})
+
+add_executable(${MODULE_NAME} ${${MODULE_PREFIX}_SRCS})
+
+set_complex_link_libraries(VARIABLE ${MODULE_PREFIX}_LIBS
+       MONOLITHIC ${MONOLITHIC_BUILD}
+       MODULE freerdp
+       MODULES freerdp-crypto)
+
+target_link_libraries(${MODULE_NAME} ${${MODULE_PREFIX}_LIBS})
+
+set_target_properties(${MODULE_NAME} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${TESTING_OUTPUT_DIRECTORY}")
+
+foreach(test ${${MODULE_PREFIX}_TESTS})
+       get_filename_component(TestName ${test} NAME_WE)
+       add_test(${TestName} ${TESTING_OUTPUT_DIRECTORY}/${MODULE_NAME} ${TestName})
+endforeach()
+
+set_property(TARGET ${MODULE_NAME} PROPERTY FOLDER "FreeRDP/Test")
+
diff --git a/libfreerdp/crypto/test/TestBase64.c b/libfreerdp/crypto/test/TestBase64.c
new file mode 100644 (file)
index 0000000..5c32ea4
--- /dev/null
@@ -0,0 +1,110 @@
+/**
+ * Copyright © 2014 Thincast Technologies GmbH
+ * Copyright © 2014 Hardening <contact@hardening-consulting.com>
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and
+ * its documentation for any purpose is hereby granted without fee, provided
+ * that the above copyright notice appear in all copies and that both that
+ * copyright notice and this permission notice appear in supporting
+ * documentation, and that the name of the copyright holders not be used in
+ * advertising or publicity pertaining to distribution of the software
+ * without specific, written prior permission.  The copyright holders make
+ * no representations about the suitability of this software for any
+ * purpose.  It is provided "as is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
+ * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
+ * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
+ * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER
+ * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF
+ * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <freerdp/crypto/crypto.h>
+
+struct Encode64test {
+       const char *input;
+       int len;
+       const char *output;
+};
+
+struct Encode64test encodeTests[] = {
+               {"\x00",                        1, "AA=="},
+               {"\x00\x00",            2, "AAA="},
+               {"\x00\x00\x00",        3, "AAAA"},
+               {"0123456",                     7, "MDEyMzQ1Ng=="},
+               {"90123456",            8, "OTAxMjM0NTY="},
+               {"890123456",           9, "ODkwMTIzNDU2"},
+               {"7890123456",          10, "Nzg5MDEyMzQ1Ng=="},
+
+               {NULL, -1, NULL},  /*  /!\ last one  /!\ */
+};
+
+
+int TestBase64(int argc, char* argv[])
+{
+       int i, testNb = 0;
+       int outLen;
+       BYTE *decoded;
+
+       testNb++;
+       fprintf(stderr, "%d:encode base64...", testNb);
+
+       for (i = 0; encodeTests[i].input; i++)
+       {
+               char *encoded = crypto_base64_encode((const BYTE *)encodeTests[i].input, encodeTests[i].len);
+
+               if (strcmp(encodeTests[i].output, encoded))
+               {
+                       fprintf(stderr, "ko, error for string %d\n", i);
+                       return -1;
+               }
+
+               free(encoded);
+       }
+       fprintf(stderr, "ok\n");
+
+       testNb++;
+       fprintf(stderr, "%d:decode base64...", testNb);
+       for (i = 0; encodeTests[i].input; i++)
+       {
+               crypto_base64_decode(encodeTests[i].output, strlen(encodeTests[i].output), &decoded, &outLen);
+
+               if (!decoded || (outLen != encodeTests[i].len) || memcmp(encodeTests[i].input, decoded, outLen))
+               {
+                       fprintf(stderr, "ko, error for string %d\n", i);
+                       return -1;
+               }
+
+               free(decoded);
+       }
+       fprintf(stderr, "ok\n");
+
+       testNb++;
+       fprintf(stderr, "%d:decode base64 errors...", testNb);
+       crypto_base64_decode("000", 3, &decoded, &outLen);
+       if (decoded)
+       {
+               fprintf(stderr, "ko, badly padded string\n");
+               return -1;
+       }
+
+       crypto_base64_decode("0=00", 4, &decoded, &outLen);
+       if (decoded)
+       {
+               fprintf(stderr, "ko, = in a wrong place\n");
+               return -1;
+       }
+
+       crypto_base64_decode("00=0", 4, &decoded, &outLen);
+       if (decoded)
+       {
+               fprintf(stderr, "ko, = in a wrong place\n");
+               return -1;
+       }
+       fprintf(stderr, "ok\n");
+
+
+       return 0;
+}