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>
diff --git a/crypto/base64/CMakeLists.txt b/crypto/base64/CMakeLists.txt
index 2601abd..dec67ea 100644
--- a/crypto/base64/CMakeLists.txt
+++ b/crypto/base64/CMakeLists.txt
@@ -7,3 +7,11 @@
base64.c
)
+
+add_executable(
+ base64_test
+
+ base64_test.c
+)
+
+target_link_libraries(base64_test crypto)
diff --git a/crypto/base64/base64.c b/crypto/base64/base64.c
index 58e6151..f5525b1 100644
--- a/crypto/base64/base64.c
+++ b/crypto/base64/base64.c
@@ -63,6 +63,7 @@
"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 @@
}
}
-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;
diff --git a/crypto/base64/base64_test.c b/crypto/base64/base64_test.c
new file mode 100644
index 0000000..ea0d3d1
--- /dev/null
+++ b/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;
+}
diff --git a/include/openssl/base64.h b/include/openssl/base64.h
index afafaef..c4d312c 100644
--- a/include/openssl/base64.h
+++ b/include/openssl/base64.h
@@ -75,8 +75,12 @@
/* 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 @@
/* 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_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 {
diff --git a/util/all_tests.sh b/util/all_tests.sh
index def97ff..a7e961e 100644
--- a/util/all_tests.sh
+++ b/util/all_tests.sh
@@ -19,6 +19,7 @@
./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
