From 7a45d51ce3268d16409405b9d54d7b4bb77a7fc3 Mon Sep 17 00:00:00 2001 From: Shane Lontis Date: Tue, 9 Mar 2021 17:27:55 +1000 Subject: Use BIO_f_readbuffer() in the decoder to support stdin. Fixes #13185 Fixes #13352 Removed the existing code in file_store that was trying to figure out the input type. Reviewed-by: Tomas Mraz Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/14407) --- crypto/encode_decode/decoder_lib.c | 11 ++ providers/implementations/storemgmt/file_store.c | 170 ++--------------------- test/recipes/20-test_dhparam.t | 6 +- util/libcrypto.num | 1 + 4 files changed, 28 insertions(+), 160 deletions(-) diff --git a/crypto/encode_decode/decoder_lib.c b/crypto/encode_decode/decoder_lib.c index 635a656216..f161c7cd5b 100644 --- a/crypto/encode_decode/decoder_lib.c +++ b/crypto/encode_decode/decoder_lib.c @@ -39,7 +39,14 @@ int OSSL_DECODER_from_bio(OSSL_DECODER_CTX *ctx, BIO *in) { struct decoder_process_data_st data; int ok = 0; + BIO *new_bio = NULL; + if (BIO_tell(in) < 0) { + new_bio = BIO_new(BIO_f_readbuffer()); + if (new_bio == NULL) + return 0; + in = BIO_push(new_bio, in); + } memset(&data, 0, sizeof(data)); data.ctx = ctx; data.bio = in; @@ -52,6 +59,10 @@ int OSSL_DECODER_from_bio(OSSL_DECODER_CTX *ctx, BIO *in) /* Clear any internally cached passphrase */ (void)ossl_pw_clear_passphrase_cache(&ctx->pwdata); + if (new_bio != NULL) { + BIO_pop(new_bio); + BIO_free(new_bio); + } return ok; } diff --git a/providers/implementations/storemgmt/file_store.c b/providers/implementations/storemgmt/file_store.c index ab4b4055d9..1ea820e2a4 100644 --- a/providers/implementations/storemgmt/file_store.c +++ b/providers/implementations/storemgmt/file_store.c @@ -8,30 +8,26 @@ */ #include "e_os.h" /* To get strncasecmp() on Windows */ + #include #include -#include +#include /* isdigit */ #include -#include #include #include #include -#include #include #include -#include #include #include -#include /* The OSSL_STORE_INFO type numbers */ #include +#include /* The OSSL_STORE_INFO type numbers */ #include "internal/cryptlib.h" #include "internal/o_dir.h" -#include "crypto/pem.h" /* For PVK and "blob" PEM headers */ #include "crypto/decoder.h" #include "prov/implementations.h" #include "prov/bio.h" -#include "prov/provider_ctx.h" #include "file_store_local.h" DEFINE_STACK_OF(OSSL_STORE_INFO) @@ -74,10 +70,6 @@ struct file_ctx_st { IS_DIR /* Pass directory entry names */ } type; - /* Flag bits */ - unsigned int flag_attached:1; - unsigned int flag_buffered:1; - union { /* Used with |IS_FILE| */ struct { @@ -299,139 +291,18 @@ static void *file_open(void *provctx, const char *uri) return ctx; } -/* - * Attached input streams must be treated very very carefully to avoid - * nasty surprises. - * - * This implementation tries to support input streams that can't be reset, - * such as standard input. However, OSSL_DECODER assumes resettable streams, - * and because the PEM decoder may read quite a bit of the input file to skip - * past any non-PEM text that precedes the PEM block, we may need to detect - * if the input stream is a PEM file early. - * - * If the input stream supports BIO_tell(), we assume that it also supports - * BIO_seek(), making it a resettable stream and therefore safe to fully - * unleash OSSL_DECODER. - * - * If the input stream doesn't support BIO_tell(), we must assume that we - * have a non-resettable stream, and must tread carefully. We do so by - * trying to detect if the input is PEM, MSBLOB or PVK, and if not, we - * assume that it's DER. - * - * To detect if an input stream is PEM, MSBLOB or PVK, we use the buffer BIO - * filter, which allows us a 4KiB resettable read-ahead. We *hope* that 4KiB - * will be enough to find the start of the PEM block. - * - * It should be possible to use this same technique to detect other file - * types as well. - * - * An alternative technique would be to have an endlessly caching BIO filter. - * That would take away the need for all the detection here, and simply leave - * it for OSSL_DECODER to find out on its own while supporting its demand for - * resettable input streams. - * That's a possible future development. - */ - -# define INPUT_TYPE_ANY NULL -# define INPUT_TYPE_DER "DER" -# define INPUT_TYPE_PEM "PEM" -# define INPUT_TYPE_MSBLOB "MSBLOB" -# define INPUT_TYPE_PVK "PVK" - void *file_attach(void *provctx, OSSL_CORE_BIO *cin) { + struct file_ctx_st *ctx; BIO *new_bio = bio_new_from_core_bio(provctx, cin); - BIO *new_bio_tmp = NULL; - BIO *buff = NULL; - char peekbuf[4096] = { 0, }; - int loc; - const char *input_type = NULL; - unsigned int flag_attached = 1; - unsigned int flag_buffered = 0; - struct file_ctx_st *ctx = NULL; if (new_bio == NULL) - return 0; - - /* Try to get the current position */ - loc = BIO_tell(new_bio); - - if ((buff = BIO_new(BIO_f_buffer())) == NULL - || (new_bio_tmp = BIO_push(buff, new_bio)) == NULL) - goto err; - - /* Assumption, if we can't detect PEM */ - input_type = INPUT_TYPE_DER; - flag_buffered = 1; - new_bio = new_bio_tmp; - - if (BIO_buffer_peek(new_bio, peekbuf, sizeof(peekbuf) - 1) > 0) { -#ifndef OPENSSL_NO_DSA - const unsigned char *p = NULL; - unsigned int magic = 0, bitlen = 0; - int isdss = 0, ispub = -1; -# ifndef OPENSSL_NO_RC4 - unsigned int saltlen = 0, keylen = 0; -# endif -#endif - - peekbuf[sizeof(peekbuf) - 1] = '\0'; - if (strstr(peekbuf, "-----BEGIN ") != NULL) - input_type = INPUT_TYPE_PEM; -#ifndef OPENSSL_NO_DSA - else if (p = (unsigned char *)peekbuf, - ossl_do_blob_header(&p, sizeof(peekbuf), &magic, &bitlen, - &isdss, &ispub)) - input_type = INPUT_TYPE_MSBLOB; -# ifndef OPENSSL_NO_RC4 - else if (p = (unsigned char *)peekbuf, - ossl_do_PVK_header(&p, sizeof(peekbuf), 0, &saltlen, &keylen)) - input_type = INPUT_TYPE_PVK; -# endif -#endif - } - - /* - * After peeking, we know that the underlying source BIO has moved ahead - * from its earlier position and that if it supports BIO_tell(), that - * should be a number that differs from |loc|. Otherwise, we will get - * the same value, which may one of: - * - * - zero (the source BIO doesn't support BIO_tell() / BIO_seek() / - * BIO_reset()) - * - -1 (the underlying operating system / C library routines do not - * support BIO_tell() / BIO_seek() / BIO_reset()) - * - * If it turns out that the source BIO does support BIO_tell(), we pop - * the buffer BIO filter and mark this input as |INPUT_TYPE_ANY|, which - * fully unleashes OSSL_DECODER to do its thing. - */ - if (BIO_tell(new_bio) != loc) { - /* In this case, anything goes */ - input_type = INPUT_TYPE_ANY; - - /* Restore the source BIO like it was when entering this function */ - new_bio = BIO_pop(buff); - BIO_free(buff); - (void)BIO_seek(new_bio, loc); - - flag_buffered = 0; - } - - if ((ctx = file_open_stream(new_bio, NULL, input_type, provctx)) == NULL) - goto err; - - ctx->flag_attached = flag_attached; - ctx->flag_buffered = flag_buffered; + return NULL; + ctx = file_open_stream(new_bio, NULL, NULL, provctx); + if (ctx == NULL) + BIO_free(new_bio); return ctx; - err: - if (flag_buffered) { - new_bio = BIO_pop(buff); - BIO_free(buff); - } - BIO_free(new_bio); /* Removes the provider BIO filter */ - return NULL; } /*- @@ -854,30 +725,11 @@ static int file_close_dir(struct file_ctx_st *ctx) static int file_close_stream(struct file_ctx_st *ctx) { - if (ctx->flag_buffered) { - /* - * file_attach() pushed a BIO_f_buffer() on top of the regular BIO. - * Drop it. - */ - BIO *buff = ctx->_.file.file; - - /* Detach buff */ - ctx->_.file.file = BIO_pop(ctx->_.file.file); - - BIO_free(buff); - } - /* - * If it was attached, we only free the top, as that's the provider BIO - * filter. Otherwise, it was entirely allocated by this implementation, - * and can safely be completely freed. + * This frees either the provider BIO filter (for file_attach()) OR + * the allocated file BIO (for file_open()). */ - if (ctx->flag_attached) - BIO_free(ctx->_.file.file); - else - BIO_free_all(ctx->_.file.file); - - /* To avoid double free */ + BIO_free(ctx->_.file.file); ctx->_.file.file = NULL; free_file_ctx(ctx); diff --git a/test/recipes/20-test_dhparam.t b/test/recipes/20-test_dhparam.t index 9bd947b0ee..78a63508b3 100644 --- a/test/recipes/20-test_dhparam.t +++ b/test/recipes/20-test_dhparam.t @@ -19,7 +19,7 @@ setup("test_dhparam"); plan skip_all => "DH is not supported in this build" if disabled("dh"); -plan tests => 16; +plan tests => 17; sub checkdhparams { my $file = shift; #Filename containing params @@ -171,3 +171,7 @@ SKIP: { checkdhparams("gen-x942-0-512.der", "X9.42", 0, "DER", 512); }; } + +ok(run(app(["openssl", "dhparam", "-noout", "-text"], + stdin => data_file("pkcs3-2-1024.pem"))), + "stdinbuffer input test that uses BIO_gets"); diff --git a/util/libcrypto.num b/util/libcrypto.num index 309676f39b..2ca427c63b 100644 --- a/util/libcrypto.num +++ b/util/libcrypto.num @@ -5313,3 +5313,4 @@ EVP_RAND_CTX_gettable_params ? 3_0_0 EXIST::FUNCTION: EVP_RAND_CTX_settable_params ? 3_0_0 EXIST::FUNCTION: RAND_set_DRBG_type ? 3_0_0 EXIST::FUNCTION: RAND_set_seed_source_type ? 3_0_0 EXIST::FUNCTION: +BIO_f_readbuffer ? 3_0_0 EXIST::FUNCTION: -- cgit v1.2.3