Fix EVP_DecodeBlock and add tests.

Another signedness error. Leave a TODO to possibly resolve EVP_DecodeBlock's
ignoring padding. Document some of the Init/Update/Finish versions' behavior.

Change-Id: I78a72c3163f8543172a7008b2d09fb10e003d957
Reviewed-on: https://boringssl-review.googlesource.com/1230
Reviewed-by: Adam Langley <agl@google.com>

crypto/base64/CMakeLists.txt

@@ -7,3 +7,11 @@ add_library(
 
 	base64.c
 )
+
+add_executable(
+	base64_test
+
+	base64_test.c
+)
+
+target_link_libraries(base64_test crypto)

crypto/base64/base64.c

@@ -63,6 +63,7 @@ static const unsigned char data_bin2ascii[65] =
     "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
 
 #define conv_bin2ascii(a) (data_bin2ascii[(a) & 0x3f])
+/* TODO(davidben): This doesn't error on bytes above 127. */
 #define conv_ascii2bin(a) (data_ascii2bin[(a) & 0x7f])
 
 /* 64 char lines
@@ -357,7 +358,7 @@ int EVP_DecodeFinal(EVP_ENCODE_CTX *ctx, uint8_t *out, int *outl) {
   }
 }
 
-size_t EVP_DecodeBlock(uint8_t *dst, const uint8_t *src, size_t src_len) {
+ssize_t EVP_DecodeBlock(uint8_t *dst, const uint8_t *src, size_t src_len) {
   int a, b, c, d;
   uint32_t l;
   size_t i, ret = 0;

crypto/base64/base64_test.c

@@ -0,0 +1,104 @@
+/* Copyright (c) 2014, Google Inc.
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY
+ * SPECIAL, DIRECT, 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 <stdio.h>
+#include <string.h>
+
+#include <openssl/base64.h>
+#include <openssl/err.h>
+
+typedef struct {
+  const char *decoded;
+  const char *encoded;
+} TEST_VECTOR;
+
+/* Test vectors from RFC 4648. */
+static const TEST_VECTOR test_vectors[] = {
+  { "", "" },
+  { "f" , "Zg==" },
+  { "fo", "Zm8=" },
+  { "foo", "Zm9v" },
+  { "foob", "Zm9vYg==" },
+  { "fooba", "Zm9vYmE=" },
+  { "foobar", "Zm9vYmFy" },
+};
+static const size_t kNumTests = sizeof(test_vectors) / sizeof(test_vectors[0]);
+
+static int test_encode() {
+  uint8_t out[8];
+  size_t i;
+  ssize_t len;
+
+  for (i = 0; i < kNumTests; i++) {
+    const TEST_VECTOR *t = &test_vectors[i];
+    len = EVP_EncodeBlock(out, (const uint8_t*)t->decoded, strlen(t->decoded));
+    if (len != strlen(t->encoded) ||
+        memcmp(out, t->encoded, len) != 0) {
+      fprintf(stderr, "encode(\"%s\") = \"%.*s\", want \"%s\"\n",
+              t->decoded, (int)len, (const char*)out, t->encoded);
+      return 0;
+    }
+  }
+  return 1;
+}
+
+static int test_decode() {
+  uint8_t out[6];
+  size_t i;
+  ssize_t len;
+
+  for (i = 0; i < kNumTests; i++) {
+    const TEST_VECTOR *t = &test_vectors[i];
+    size_t expected_len = strlen(t->decoded);
+    len = EVP_DecodeBlock(out, (const uint8_t*)t->encoded, strlen(t->encoded));
+    /* TODO(davidben): EVP_DecodeBlock doesn't take padding into account. Is
+     * this behavior we can change? */
+    if (expected_len % 3 != 0) {
+      len -= 3 - (expected_len % 3);
+    }
+    if (len != strlen(t->decoded) ||
+        memcmp(out, t->decoded, len) != 0) {
+      fprintf(stderr, "decode(\"%s\") = \"%.*s\", want \"%s\"\n",
+              t->encoded, (int)len, (const char*)out, t->decoded);
+      return 0;
+    }
+  }
+
+  if (EVP_DecodeBlock(out, (const uint8_t*)"a!bc", 4) >= 0) {
+    fprintf(stderr, "Failed to reject invalid characters in the middle.\n");
+    return 0;
+  }
+
+  if (EVP_DecodeBlock(out, (const uint8_t*)"abc", 3) >= 0) {
+    fprintf(stderr, "Failed to reject invalid input length.\n");
+    return 0;
+  }
+
+  return 1;
+}
+
+int main() {
+  ERR_load_crypto_strings();
+
+  if (!test_encode()) {
+    return 1;
+  }
+
+  if (!test_decode()) {
+    return 1;
+  }
+
+  printf("PASS\n");
+  return 0;
+}

include/openssl/base64.h

@@ -75,8 +75,12 @@ typedef struct evp_encode_ctx_st EVP_ENCODE_CTX;
 
 /* Encoding */
 
-/* EVP_EncodeInit initialises |*ctx|, which is typically stack allocated, for
- * an encoding operation. */
+/* EVP_EncodeInit initialises |*ctx|, which is typically stack
+ * allocated, for an encoding operation.
+ *
+ * NOTE: The encoding operation breaks its output with newlines every
+ * 64 characters of output (48 characters of input). Use
+ * EVP_EncodeBlock to encode raw base64. */
 void EVP_EncodeInit(EVP_ENCODE_CTX *ctx);
 
 /* EVP_EncodeUpdate encodes |in_len| bytes from |in| and writes an encoded
@@ -98,7 +102,10 @@ size_t EVP_EncodeBlock(uint8_t *dst, const uint8_t *src, size_t src_len);
 /* Decoding */
 
 /* EVP_DecodeInit initialises |*ctx|, which is typically stack allocated, for
- * a decoding operation. */
+ * a decoding operation.
+ *
+ * TODO(davidben): This isn't a straight-up base64 decode either. Document
+ * and/or fix exactly what's going on here; maximum line length and such. */
 void EVP_DecodeInit(EVP_ENCODE_CTX *ctx);
 
 /* EVP_DecodeUpdate decodes |in_len| bytes from |in| and writes the decoded
@@ -117,8 +124,11 @@ int EVP_DecodeUpdate(EVP_ENCODE_CTX *ctx, uint8_t *out, int *out_len,
 int EVP_DecodeFinal(EVP_ENCODE_CTX *ctx, uint8_t *out, int *out_len);
 
 /* EVP_DecodeBlock encodes |src_len| bytes from |src| and writes the result to
- * |dst|. It returns the number of bytes written. */
-size_t EVP_DecodeBlock(uint8_t *dst, const uint8_t *src, size_t src_len);
+ * |dst|. It returns the number of bytes written or -1 on error.
+ *
+ * WARNING: EVP_DecodeBlock's return value does not take padding into
+ * account. TODO(davidben): Possible or worth it to fix or add new API? */
+ssize_t EVP_DecodeBlock(uint8_t *dst, const uint8_t *src, size_t src_len);
 
 
 struct evp_encode_ctx_st {

util/all_tests.sh

@@ -19,6 +19,7 @@ TESTS="
 ./crypto/cipher/aead_test aes-256-gcm ../crypto/cipher/aes_256_gcm_tests.txt
 ./crypto/cipher/aead_test chacha20-poly1305 ../crypto/cipher/chacha20_poly1305_tests.txt
 ./crypto/cipher/aead_test rc4-md5 ../crypto/cipher/rc4_md5_tests.txt
+./crypto/base64/base64_test
 ./crypto/bio/bio_test
 ./crypto/bn/bn_test
 ./crypto/cipher/cipher_test ../crypto/cipher/cipher_test.txt