diff --git a/CHANGES b/CHANGES index a7dab6cd12e417723da1f809ed9084d5713a9416..3aa9dc813b8b8512b791dc12df4ca4df228f361f 100644 --- a/CHANGES +++ b/CHANGES @@ -4,6 +4,12 @@ Changes between 1.0.2 and 1.1.0 [xx XXX xxxx] + *) Rewrite EVP_DecodeUpdate (base64 decoding) to fix several bugs. + This changes the decoding behaviour for some invalid messages, + though the change is mostly in the more lenient direction, and + legacy behaviour is preserved as much as possible. + [Emilia Käsper] + *) New testing framework The testing framework has been largely rewritten and is now using perl and the perl modules Test::Harness and an extended variant of diff --git a/crypto/evp/encode.c b/crypto/evp/encode.c index 4d3c5c873efafe6705d4dcb09e7e378994c24419..985fd29d8fa4f841159c8e1655b2a0d29796ddc5 100644 --- a/crypto/evp/encode.c +++ b/crypto/evp/encode.c @@ -103,6 +103,7 @@ abcdefghijklmnopqrstuvwxyz0123456789+/"; #define B64_WS 0xE0 #define B64_ERROR 0xFF #define B64_NOT_BASE64(a) (((a)|0x13) == 0xF3) +#define B64_BASE64(a) !B64_NOT_BASE64(a) static const unsigned char data_ascii2bin[128] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, @@ -218,8 +219,9 @@ int EVP_EncodeBlock(unsigned char *t, const unsigned char *f, int dlen) void EVP_DecodeInit(EVP_ENCODE_CTX *ctx) { - ctx->length = 30; + /* Only ctx->num is used during decoding. */ ctx->num = 0; + ctx->length = 0; ctx->line_num = 0; ctx->expect_nl = 0; } @@ -228,139 +230,123 @@ void EVP_DecodeInit(EVP_ENCODE_CTX *ctx) * -1 for error * 0 for last line * 1 for full line + * + * Note: even though EVP_DecodeUpdate attempts to detect and report end of + * content, the context doesn't currently remember it and will accept more data + * in the next call. Therefore, the caller is responsible for checking and + * rejecting a 0 return value in the middle of content. + * + * Note: even though EVP_DecodeUpdate has historically tried to detect end of + * content based on line length, this has never worked properly. Therefore, + * we now return 0 when one of the following is true: + * - Padding or B64_EOF was detected and the last block is complete. + * - Input has zero-length. + * -1 is returned if: + * - Invalid characters are detected. + * - There is extra trailing padding, or data after padding. + * - B64_EOF is detected after an incomplete base64 block. */ int EVP_DecodeUpdate(EVP_ENCODE_CTX *ctx, unsigned char *out, int *outl, const unsigned char *in, int inl) { - int seof = -1, eof = 0, rv = -1, ret = 0, i, v, tmp, n, ln, exp_nl; + int seof = 0, eof = 0, rv = -1, ret = 0, i, v, tmp, n, decoded_len; unsigned char *d; n = ctx->num; d = ctx->enc_data; - ln = ctx->line_num; - exp_nl = ctx->expect_nl; - /* last line of input. */ - if ((inl == 0) || ((n == 0) && (conv_ascii2bin(in[0]) == B64_EOF))) { + if (n > 0 && d[n - 1] == '=') { + eof++; + if (n > 1 && d[n - 2] == '=') + eof++; + } + + /* Legacy behaviour: an empty input chunk signals end of input. */ + if (inl == 0) { rv = 0; goto end; } - /* We parse the input data */ for (i = 0; i < inl; i++) { - /* If the current line is > 80 characters, scream a lot */ - if (ln >= 80) { - rv = -1; - goto end; - } - - /* Get char and put it into the buffer */ tmp = *(in++); v = conv_ascii2bin(tmp); - /* only save the good data :-) */ - if (!B64_NOT_BASE64(v)) { - OPENSSL_assert(n < (int)sizeof(ctx->enc_data)); - d[n++] = tmp; - ln++; - } else if (v == B64_ERROR) { + if (v == B64_ERROR) { rv = -1; goto end; } - /* - * have we seen a '=' which is 'definitly' the last input line. seof - * will point to the character that holds it. and eof will hold how - * many characters to chop off. - */ if (tmp == '=') { - if (seof == -1) - seof = n; eof++; + } else if (eof > 0 && B64_BASE64(v)) { + /* More data after padding. */ + rv = -1; + goto end; } - if (v == B64_CR) { - ln = 0; - if (exp_nl) - continue; + if (eof > 2) { + rv = -1; + goto end; } - /* eoln */ - if (v == B64_EOLN) { - ln = 0; - if (exp_nl) { - exp_nl = 0; - continue; - } - } - exp_nl = 0; - - /* - * If we are at the end of input and it looks like a line, process - * it. - */ - if (((i + 1) == inl) && (((n & 3) == 0) || eof)) { - v = B64_EOF; - /* - * In case things were given us in really small records (so two - * '=' were given in separate updates), eof may contain the - * incorrect number of ending bytes to skip, so let's redo the - * count - */ - eof = 0; - if (d[n - 1] == '=') - eof++; - if (d[n - 2] == '=') - eof++; - /* There will never be more than two '=' */ + if (v == B64_EOF) { + seof = 1; + goto tail; } - if ((v == B64_EOF && (n & 3) == 0) || (n >= 64)) { - /* - * This is needed to work correctly on 64 byte input lines. We - * process the line and then need to accept the '\n' - */ - if ((v != B64_EOF) && (n >= 64)) - exp_nl = 1; - if (n > 0) { - v = EVP_DecodeBlock(out, d, n); - n = 0; - if (v < 0) { - rv = 0; - goto end; - } - if (eof > v) { - rv = -1; - goto end; - } - ret += (v - eof); - } else { - eof = 1; - v = 0; - } - - /* - * This is the case where we have had a short but valid input - * line - */ - if ((v < ctx->length) && eof) { - rv = 0; + /* Only save valid base64 characters. */ + if (B64_BASE64(v)) { + if (n >= 64) { + /* + * We increment n once per loop, and empty the buffer as soon as + * we reach 64 characters, so this can only happen if someone's + * manually messed with the ctx. Refuse to write any more data. + */ + rv = -1; goto end; - } else - ctx->length = v; + } + OPENSSL_assert(n < (int)sizeof(ctx->enc_data)); + d[n++] = tmp; + } - if (seof >= 0) { - rv = 0; + if (n == 64) { + decoded_len = EVP_DecodeBlock(out, d, n); + n = 0; + if (decoded_len < 0 || eof > decoded_len) { + rv = -1; goto end; } - out += v; + ret += decoded_len - eof; + out += decoded_len - eof; } } - rv = 1; - end: + + /* + * Legacy behaviour: if the current line is a full base64-block (i.e., has + * 0 mod 4 base64 characters), it is processed immediately. We keep this + * behaviour as applications may not be calling EVP_DecodeFinal properly. + */ +tail: + if (n > 0) { + if ((n & 3) == 0) { + decoded_len = EVP_DecodeBlock(out, d, n); + n = 0; + if (decoded_len < 0 || eof > decoded_len) { + rv = -1; + goto end; + } + ret += (decoded_len - eof); + } else if (seof) { + /* EOF in the middle of a base64 block. */ + rv = -1; + goto end; + } + } + + rv = seof || (n == 0 && eof) ? 0 : 1; +end: + /* Legacy behaviour. This should probably rather be zeroed on error. */ *outl = ret; ctx->num = n; - ctx->line_num = ln; - ctx->expect_nl = exp_nl; return (rv); } diff --git a/test/evp_test.c b/test/evp_test.c index c5c6e073238f016d31b7da25c405be298d08f9a8..eb7ef5017afbe0d32f4c4fb7418a824a2a4cedbd 100644 --- a/test/evp_test.c +++ b/test/evp_test.c @@ -124,12 +124,58 @@ static int parse_line(char **pkw, char **pval, char *linebuf) return 1; } +/* + * Unescape some escape sequences in string literals. + * Return the result in a newly allocated buffer. + * Currently only supports '\n'. + * If the input length is 0, returns a valid 1-byte buffer, but sets + * the length to 0. + */ +static unsigned char* unescape(const char *input, size_t input_len, + size_t *out_len) +{ + unsigned char *ret, *p; + size_t i; + if (input_len == 0) { + *out_len = 0; + return OPENSSL_zalloc(1); + } + + /* Escaping is non-expanding; over-allocate original size for simplicity. */ + ret = p = OPENSSL_malloc(input_len); + if (ret == NULL) + return NULL; + + for (i = 0; i < input_len; i++) { + if (input[i] == '\\') { + if (i == input_len - 1 || input[i+1] != 'n') + goto err; + *p++ = '\n'; + i++; + } else { + *p++ = input[i]; + } + } + + *out_len = p - ret; + return ret; + + err: + OPENSSL_free(ret); + return NULL; +} + /* For a hex string "value" convert to a binary allocated buffer */ static int test_bin(const char *value, unsigned char **buf, size_t *buflen) { long len; if (!*value) { - /* Don't return NULL for zero length buffer */ + /* + * Don't return NULL for zero length buffer. + * This is needed for some tests with empty keys: HMAC_Init_ex() expects + * a non-NULL key buffer even if the key length is 0, in order to detect + * key reset. + */ *buf = OPENSSL_malloc(1); if (!*buf) return 0; @@ -145,19 +191,12 @@ static int test_bin(const char *value, unsigned char **buf, size_t *buflen) if (value[vlen - 1] != '"') return 0; vlen--; - if (vlen == 0) { - *buf = OPENSSL_malloc(1); - if (*buf == NULL) - return 0; - **buf = 0; - } else { - *buf = BUF_memdup(value, vlen); - if (*buf == NULL) - return 0; - } - *buflen = vlen; + *buf = unescape(value, vlen, buflen); + if (*buf == NULL) + return 0; return 1; } + *buf = string_to_hex(value, &len); if (!*buf) { fprintf(stderr, "Value=%s\n", value); @@ -217,9 +256,10 @@ struct evp_test { /* Number of tests skipped */ int nskip; /* If output mismatch expected and got value */ - unsigned char *out_got; + unsigned char *out_received; + size_t out_received_len; unsigned char *out_expected; - size_t out_len; + size_t out_expected_len; /* test specific data */ void *data; /* Current test should be skipped */ @@ -252,6 +292,7 @@ static const struct evp_test_method psign_test_method, pverify_test_method; static const struct evp_test_method pdecrypt_test_method; static const struct evp_test_method pverify_recover_test_method; static const struct evp_test_method pbe_test_method; +static const struct evp_test_method encode_test_method; static const struct evp_test_method *evp_test_list[] = { &digest_test_method, @@ -262,6 +303,7 @@ static const struct evp_test_method *evp_test_list[] = { &pdecrypt_test_method, &pverify_recover_test_method, &pbe_test_method, + &encode_test_method, NULL }; @@ -290,17 +332,21 @@ static void free_expected(struct evp_test *t) OPENSSL_free(t->expected_err); t->expected_err = NULL; OPENSSL_free(t->out_expected); - OPENSSL_free(t->out_got); + OPENSSL_free(t->out_received); t->out_expected = NULL; - t->out_got = NULL; + t->out_received = NULL; + t->out_expected_len = 0; + t->out_received_len = 0; + /* Literals. */ + t->err = NULL; } static void print_expected(struct evp_test *t) { - if (t->out_expected == NULL) + if (t->out_expected == NULL && t->out_received == NULL) return; - hex_print("Expected:", t->out_expected, t->out_len); - hex_print("Got: ", t->out_got, t->out_len); + hex_print("Expected:", t->out_expected, t->out_expected_len); + hex_print("Got: ", t->out_received, t->out_received_len); free_expected(t); } @@ -496,21 +542,37 @@ static int process_test(struct evp_test *t, char *buf, int verbose) return 1; } -static int check_output(struct evp_test *t, const unsigned char *expected, - const unsigned char *got, size_t len) +static int check_var_length_output(struct evp_test *t, + const unsigned char *expected, + size_t expected_len, + const unsigned char *received, + size_t received_len) { - if (!memcmp(expected, got, len)) + if (expected_len == received_len && + memcmp(expected, received, expected_len) == 0) { return 0; - t->out_expected = BUF_memdup(expected, len); - t->out_got = BUF_memdup(got, len); - t->out_len = len; - if (t->out_expected == NULL || t->out_got == NULL) { + } + + /* The result printing code expects a non-NULL buffer. */ + t->out_expected = BUF_memdup(expected, expected_len ? expected_len : 1); + t->out_expected_len = expected_len; + t->out_received = BUF_memdup(received, received_len ? received_len : 1); + t->out_received_len = received_len; + if (t->out_expected == NULL || t->out_received == NULL) { fprintf(stderr, "Memory allocation error!\n"); exit(1); } return 1; } +static int check_output(struct evp_test *t, + const unsigned char *expected, + const unsigned char *received, + size_t len) +{ + return check_var_length_output(t, expected, len, received, len); +} + int main(int argc, char **argv) { FILE *in = NULL; @@ -528,17 +590,7 @@ int main(int argc, char **argv) OpenSSL_add_all_algorithms(); memset(&t, 0, sizeof(t)); - t.meth = NULL; - t.public = NULL; - t.private = NULL; - t.err = NULL; - t.line = 0; t.start_line = -1; - t.errors = 0; - t.ntests = 0; - t.out_expected = NULL; - t.out_got = NULL; - t.out_len = 0; in = fopen(argv[1], "r"); t.in = in; while (fgets(buf, sizeof(buf), in)) { @@ -1473,3 +1525,132 @@ static const struct evp_test_method pbe_test_method = { pbe_test_parse, pbe_test_run }; + +/* Base64 tests */ + +typedef enum { + BASE64_CANONICAL_ENCODING = 0, + BASE64_VALID_ENCODING = 1, + BASE64_INVALID_ENCODING = 2 +} base64_encoding_type; + +struct encode_data { + /* Input to encoding */ + unsigned char *input; + size_t input_len; + /* Expected output */ + unsigned char *output; + size_t output_len; + base64_encoding_type encoding; +}; + +static int encode_test_init(struct evp_test *t, const char *encoding) +{ + struct encode_data *edata = OPENSSL_zalloc(sizeof(*edata)); + + if (strcmp(encoding, "canonical") == 0) { + edata->encoding = BASE64_CANONICAL_ENCODING; + } else if (strcmp(encoding, "valid") == 0) { + edata->encoding = BASE64_VALID_ENCODING; + } else if (strcmp(encoding, "invalid") == 0) { + edata->encoding = BASE64_INVALID_ENCODING; + t->expected_err = BUF_strdup("DECODE_ERROR"); + if (t->expected_err == NULL) + return 0; + } else { + fprintf(stderr, "Bad encoding: %s. Should be one of " + "{canonical, valid, invalid}\n", encoding); + return 0; + } + t->data = edata; + return 1; +} + +static void encode_test_cleanup(struct evp_test *t) +{ + struct encode_data *edata = t->data; + test_free(edata->input); + test_free(edata->output); + memset(edata, 0, sizeof(*edata)); +} + +static int encode_test_parse(struct evp_test *t, + const char *keyword, const char *value) +{ + struct encode_data *edata = t->data; + if (strcmp(keyword, "Input") == 0) + return test_bin(value, &edata->input, &edata->input_len); + if (strcmp(keyword, "Output") == 0) + return test_bin(value, &edata->output, &edata->output_len); + return 0; +} + +static int encode_test_run(struct evp_test *t) +{ + struct encode_data *edata = t->data; + unsigned char *encode_out = NULL, *decode_out = NULL; + int output_len, chunk_len; + const char *err = "INTERNAL_ERROR"; + EVP_ENCODE_CTX decode_ctx; + + if (edata->encoding == BASE64_CANONICAL_ENCODING) { + EVP_ENCODE_CTX encode_ctx; + encode_out = OPENSSL_malloc(EVP_ENCODE_LENGTH(edata->input_len)); + if (encode_out == NULL) + goto err; + + EVP_EncodeInit(&encode_ctx); + EVP_EncodeUpdate(&encode_ctx, encode_out, &chunk_len, + edata->input, edata->input_len); + output_len = chunk_len; + + EVP_EncodeFinal(&encode_ctx, encode_out + chunk_len, &chunk_len); + output_len += chunk_len; + + if (check_var_length_output(t, edata->output, edata->output_len, + encode_out, output_len)) { + err = "BAD_ENCODING"; + goto err; + } + } + + decode_out = OPENSSL_malloc(EVP_DECODE_LENGTH(edata->output_len)); + if (decode_out == NULL) + goto err; + + EVP_DecodeInit(&decode_ctx); + if (EVP_DecodeUpdate(&decode_ctx, decode_out, &chunk_len, edata->output, + edata->output_len) < 0) { + err = "DECODE_ERROR"; + goto err; + } + output_len = chunk_len; + + if (EVP_DecodeFinal(&decode_ctx, decode_out + chunk_len, &chunk_len) != 1) { + err = "DECODE_ERROR"; + goto err; + } + output_len += chunk_len; + + if (edata->encoding != BASE64_INVALID_ENCODING && + check_var_length_output(t, edata->input, edata->input_len, + decode_out, output_len)) { + err = "BAD_DECODING"; + goto err; + } + + err = NULL; + err: + t->err = err; + OPENSSL_free(encode_out); + OPENSSL_free(decode_out); + return 1; +} + +static const struct evp_test_method encode_test_method = { + "Encoding", + encode_test_init, + encode_test_cleanup, + encode_test_parse, + encode_test_run, +}; diff --git a/test/evptests.txt b/test/evptests.txt index 2b0b7ba6e2d31cbdd8a0d05518065dcef84c05a7..e8de2c16ecf3bc38e170f76bf483affecfc05cd8 100644 --- a/test/evptests.txt +++ b/test/evptests.txt @@ -2624,3 +2624,176 @@ Salt = 7361006c74 iter = 4096 MD = sha512 Key = 9d9e9c4cd21fe4be24d5b8244c759665 + +# Base64 tests + +Encoding = canonical +Input = "" +Output = "" + +Encoding = canonical +Input = "h" +Output = "aA==\n" + +Encoding = canonical +Input = "hello" +Output = "aGVsbG8=\n" + +Encoding = canonical +Input = "hello world!" +Output = "aGVsbG8gd29ybGQh\n" + +Encoding = canonical +Input = 00010203040506070809a0b0c0d0e0f000 +Output = "AAECAwQFBgcICaCwwNDg8AA=\n" + +# Missing padding +Encoding = invalid +Output = "aGVsbG8" + +Encoding = invalid +Output = "aGVsbG8\n" + +# Tolerate missing newline +Encoding = valid +Input = "hello" +Output = "aGVsbG8=" + +# Don't tolerate extra trailing '=' +Encoding = invalid +Input = "hello" +Output = "aGVsbG8==\n" + +Encoding = invalid +Output = "aGVsbG8===\n" + +# Don't tolerate data after '=' +Encoding = invalid +Output = "aGV=sbG8=\n" + +# Newlines are ignored +Encoding = valid +Input = "hello" +Output = "aGV\nsbG8=\n" + +Encoding = canonical +Input = "hello" +Output = 614756736247383d0a + +# Invalid characters +Encoding = invalid +Output = 614756736247383d0a00 + +Encoding = invalid +Output = 61475600736247383d0a + +Encoding = invalid +Output = 61475601736247383d0a + +Encoding = canonical +Input = "OpenSSLOpenSSL\n" +Output = "T3BlblNTTE9wZW5TU0wK\n" + +Encoding = valid +Input = "OpenSSLOpenSSL\n" +Output = "T3BlblNTTE9wZW5TU0wK" + +# Truncate 1-3 chars +Encoding = invalid +Output = "T3BlblNTTE9wZW5TU0w" + +Encoding = invalid +Output = "T3BlblNTTE9wZW5TU0" + +Encoding = invalid +Output = "T3BlblNTTE9wZW5TU" + +Encoding = invalid +Output = "T3BlblNTTE9wZW5TU0wK====" + +Encoding = invalid +Output = "T3BlblNTTE9wZW5TU0wK============================================\n" + +Encoding = invalid +Output = "YQ==YQ==YQ==\n" + +Encoding = invalid +Output = "A" + +Encoding = invalid +Output = "A\n" + +Encoding = invalid +Output = "A=" + +Encoding = invalid +Output = "A==\n" + +Encoding = invalid +Output = "A===\n" + +Encoding = invalid +Output = "A====\n" + +Encoding = valid +Input = "OpenSSLOpenSSL\n" +Output = "T3BlblNTTE9wZW5TU0wK\n\n" + +Encoding = valid +Input = "OpenSSLOpenSSL\n" +Output = "T3BlblNTTE\n9wZW5TU0wK" + +# CVE 2015-0292 +Encoding = invalid +Output = "ZW5jb2RlIG1lCg==================================================================\n" + +Encoding = canonical +Input = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" +Output = "eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eA==\n" + +Encoding = valid +Input = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" +Output = "eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eA\n==\n" + +Encoding = valid +Input = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" +Output = "eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eA=\n=\n" + +Encoding = invalid +Output = "eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eA====\n" + +# Multiline output without padding +Encoding = canonical +Input = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" +Output = "eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4\neHh4eHh4eHh4eHh4\n" + +# Multiline output with padding +Encoding = canonical +Input = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" +Output = "eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4\neHh4eHh4eHh4eHh4eHh4eA==\n" + +# Multiline output with line break in the middle of a b64 block is accepted +Encoding = valid +Input = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" +Output = "eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh\n4eHh4eHh4eHh4eHh4eHh4eA==\n" + +# Long lines are accepted +Encoding = valid +Input = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" +Output = "eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eA==\n" + +# Multiline input with data after '='. +Encoding = invalid +Output = "eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eA==\neHh4eHh4eHh4eHh4eHh4eHh4\n" + +Encoding = invalid +Output = "eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4\neA==eHh4eHh4eHh4eHh4eHh4\n" + +# B64_EOF ('-') terminates input and trailing bytes are ignored +Encoding = valid +Input = "OpenSSLOpenSSL\n" +Output = "T3BlblNTTE9wZW5TU0wK\n-abcd" + +Encoding = valid +Input = "OpenSSLOpenSSL\n" +Output = "T3BlblNTTE9wZW5TU0wK-abcd"