Add a real type for OSSL_CORE_BIO which is distinct from and not castable to BIO

Providers (particularly the FIPS provider) needs access to BIOs from libcrypto.
Libcrypto is allowed to change the internal format of the BIO structure and it
is still expected to work with providers that were already built.  This means
that the libcrypto BIO must be distinct from and not castable to the provider
side OSSL_CORE_BIO.

Unfortunately, this requirement was broken in both directions.  This fixes
things by forcing the two to be different and any casts break loudly.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14419)

11 files changed, 213 insertions, 33 deletions

diff --git a/crypto/bio/build.info b/crypto/bio/build.info
index 227071f0ce..2bee64fc62 100644
--- a/crypto/bio/build.info
+++ b/crypto/bio/build.info
@@ -5,7 +5,7 @@ SOURCE[../../libcrypto]=\
         bio_lib.c bio_cb.c bio_err.c \
         b_print.c b_dump.c b_addr.c \
         b_sock.c b_sock2.c \
-        bio_meth.c
+        bio_meth.c core_bio.c
 
 # Source / sink implementations
 SOURCE[../../libcrypto]=\
diff --git a/crypto/bio/core_bio.c b/crypto/bio/core_bio.c
new file mode 100644
index 0000000000..328302ea34
--- /dev/null
+++ b/crypto/bio/core_bio.c
@@ -0,0 +1,124 @@
+/*
+ * Copyright 2021 The OpenSSL Project Authors. All Rights Reserved.
+ *
+ * Licensed under the Apache License 2.0 (the "License").  You may not use
+ * this file except in compliance with the License.  You can obtain a copy
+ * in the file LICENSE in the source distribution or at
+ * https://www.openssl.org/source/license.html
+ */
+
+#include <openssl/core.h>
+#include "bio_local.h"
+
+/*-
+ * Core BIO structure
+ * This is distinct from a BIO to prevent casting between the two which could
+ * lead to versioning problems.
+ */
+struct ossl_core_bio_st {
+    CRYPTO_REF_COUNT ref_cnt;
+    CRYPTO_RWLOCK *ref_lock;
+    BIO *bio;
+};
+
+static OSSL_CORE_BIO *core_bio_new(void)
+{
+    OSSL_CORE_BIO *cb = OPENSSL_malloc(sizeof(*cb));
+
+    if (cb == NULL || (cb->ref_lock = CRYPTO_THREAD_lock_new()) == NULL) {
+        OPENSSL_free(cb);
+        return NULL;
+    }
+    cb->ref_cnt = 1;
+    return cb;
+}
+
+int ossl_core_bio_up_ref(OSSL_CORE_BIO *cb)
+{
+    int ref = 0;
+
+    return CRYPTO_UP_REF(&cb->ref_cnt, &ref, cb->ref_lock);
+}
+
+int ossl_core_bio_free(OSSL_CORE_BIO *cb)
+{
+    int ref = 0, res = 1;
+
+    if (cb != NULL) {
+        CRYPTO_DOWN_REF(&cb->ref_cnt, &ref, cb->ref_lock);
+        if (ref <= 0) {
+            res = BIO_free(cb->bio);
+            CRYPTO_THREAD_lock_free(cb->ref_lock);
+            OPENSSL_free(cb);
+        }
+    }
+    return res;
+}
+
+OSSL_CORE_BIO *ossl_core_bio_new_from_bio(BIO *bio)
+{
+    OSSL_CORE_BIO *cb = core_bio_new();
+
+    if (cb == NULL || !BIO_up_ref(bio)) {
+        ossl_core_bio_free(cb);
+        return NULL;
+    }
+    cb->bio = bio;
+    return cb;
+}
+
+static OSSL_CORE_BIO *core_bio_new_from_new_bio(BIO *bio)
+{
+    OSSL_CORE_BIO *cb = NULL;
+
+    if (bio == NULL)
+        return NULL;
+    if ((cb = core_bio_new()) == NULL) {
+        BIO_free(bio);
+        return NULL;
+    }
+    cb->bio = bio;
+    return cb;
+}
+
+OSSL_CORE_BIO *ossl_core_bio_new_file(const char *filename, const char *mode)
+{
+    return core_bio_new_from_new_bio(BIO_new_file(filename, mode));
+}
+
+OSSL_CORE_BIO *ossl_core_bio_new_mem_buf(const void *buf, int len)
+{
+    return core_bio_new_from_new_bio(BIO_new_mem_buf(buf, len));
+}
+
+int ossl_core_bio_read_ex(OSSL_CORE_BIO *cb, void *data, size_t dlen,
+                          size_t *readbytes)
+{
+    return BIO_read_ex(cb->bio, data, dlen, readbytes);
+}
+
+int ossl_core_bio_write_ex(OSSL_CORE_BIO *cb, const void *data, size_t dlen,
+                           size_t *written)
+{
+    return BIO_write_ex(cb->bio, data, dlen, written);
+}
+
+int ossl_core_bio_gets(OSSL_CORE_BIO *cb, char *buf, int size)
+{
+    return BIO_gets(cb->bio, buf, size);
+}
+
+int ossl_core_bio_puts(OSSL_CORE_BIO *cb, const char *buf)
+{
+    return BIO_puts(cb->bio, buf);
+}
+
+long ossl_core_bio_ctrl(OSSL_CORE_BIO *cb, int cmd, long larg, void *parg)
+{
+    return BIO_ctrl(cb->bio, cmd, larg, parg);
+}
+
+int ossl_core_bio_vprintf(OSSL_CORE_BIO *cb, const char *format, va_list args)
+{
+    return BIO_vprintf(cb->bio, format, args);
+}
diff --git a/crypto/encode_decode/decoder_lib.c b/crypto/encode_decode/decoder_lib.c
index f161c7cd5b..9c01dc894b 100644
--- a/crypto/encode_decode/decoder_lib.c
+++ b/crypto/encode_decode/decoder_lib.c
@@ -17,6 +17,7 @@
 #include <openssl/x509err.h>
 #include <openssl/trace.h>
 #include "internal/passphrase.h"
+#include "internal/bio.h"
 #include "crypto/decoder.h"
 #include "encoder_local.h"
 #include "e_os.h"
@@ -520,6 +521,7 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
     OSSL_DECODER_CTX *ctx = data->ctx;
     OSSL_DECODER_INSTANCE *decoder_inst = NULL;
     OSSL_DECODER *decoder = NULL;
+    OSSL_CORE_BIO *cbio = NULL;
     BIO *bio = data->bio;
     long loc;
     size_t i;
@@ -633,6 +635,11 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
         goto end;
     }
 
+    if ((cbio = ossl_core_bio_new_from_bio(bio)) == NULL) {
+        ERR_raise(ERR_LIB_OSSL_DECODER, ERR_R_MALLOC_FAILURE);
+        goto end;
+    }
+
     for (i = data->current_decoder_inst_index; i-- > 0;) {
         OSSL_DECODER_INSTANCE *new_decoder_inst =
             sk_OSSL_DECODER_INSTANCE_value(ctx->decoder_insts, i);
@@ -740,7 +747,7 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
         } OSSL_TRACE_END(DECODER);
 
         new_data.current_decoder_inst_index = i;
-        ok = new_decoder->decode(new_decoderctx, (OSSL_CORE_BIO *)bio,
+        ok = new_decoder->decode(new_decoderctx, cbio,
                                  new_data.ctx->selection,
                                  decoder_process, &new_data,
                                  ossl_pw_passphrase_callback_dec,
@@ -776,6 +783,7 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
     }
 
  end:
+    ossl_core_bio_free(cbio);
     BIO_free(new_data.bio);
     return ok;
 }
diff --git a/crypto/encode_decode/encoder_lib.c b/crypto/encode_decode/encoder_lib.c
index d15fb27fde..b25529e073 100644
--- a/crypto/encode_decode/encoder_lib.c
+++ b/crypto/encode_decode/encoder_lib.c
@@ -15,6 +15,7 @@
 #include <openssl/params.h>
 #include <openssl/provider.h>
 #include <openssl/trace.h>
+#include "internal/bio.h"
 #include "encoder_local.h"
 
 struct encoder_process_data_st {
@@ -604,6 +605,7 @@ static int encoder_process(struct encoder_process_data_st *data)
         /* Calling the encoder implementation */
 
         if (ok) {
+            OSSL_CORE_BIO *cbio = NULL;
             BIO *current_out = NULL;
 
             /*
@@ -616,9 +618,10 @@ static int encoder_process(struct encoder_process_data_st *data)
                      == NULL)
                 ok = 0;     /* Assume BIO_new() recorded an error */
 
+            if (ok)
+                ok = (cbio = ossl_core_bio_new_from_bio(current_out)) != NULL;
             if (ok) {
-                ok = current_encoder->encode(current_encoder_ctx,
-                                             (OSSL_CORE_BIO *)current_out,
+                ok = current_encoder->encode(current_encoder_ctx, cbio,
                                              original_data, current_abstract,
                                              data->ctx->selection,
                                              ossl_pw_passphrase_callback_enc,
@@ -631,6 +634,7 @@ static int encoder_process(struct encoder_process_data_st *data)
                 } OSSL_TRACE_END(ENCODER);
             }
 
+            ossl_core_bio_free(cbio);
             data->prev_encoder_inst = current_encoder_inst;
         }
     }
diff --git a/crypto/provider_core.c b/crypto/provider_core.c
index 1326f83f7e..9536cb65d1 100644
--- a/crypto/provider_core.c
+++ b/crypto/provider_core.c
@@ -20,6 +20,7 @@
 #include "internal/thread_once.h"
 #include "internal/provider.h"
 #include "internal/refcount.h"
+#include "internal/bio.h"
 #include "provider_local.h"
 #ifndef FIPS_MODULE
 # include <openssl/self_test.h>
@@ -1191,15 +1192,16 @@ static const OSSL_DISPATCH core_dispatch_[] = {
     { OSSL_FUNC_CORE_CLEAR_LAST_ERROR_MARK,
       (void (*)(void))core_clear_last_error_mark },
     { OSSL_FUNC_CORE_POP_ERROR_TO_MARK, (void (*)(void))core_pop_error_to_mark },
-    { OSSL_FUNC_BIO_NEW_FILE, (void (*)(void))BIO_new_file },
-    { OSSL_FUNC_BIO_NEW_MEMBUF, (void (*)(void))BIO_new_mem_buf },
-    { OSSL_FUNC_BIO_READ_EX, (void (*)(void))BIO_read_ex },
-    { OSSL_FUNC_BIO_WRITE_EX, (void (*)(void))BIO_write_ex },
-    { OSSL_FUNC_BIO_GETS, (void (*)(void))BIO_gets },
-    { OSSL_FUNC_BIO_PUTS, (void (*)(void))BIO_puts },
-    { OSSL_FUNC_BIO_CTRL, (void (*)(void))BIO_ctrl },
-    { OSSL_FUNC_BIO_FREE, (void (*)(void))BIO_free },
-    { OSSL_FUNC_BIO_VPRINTF, (void (*)(void))BIO_vprintf },
+    { OSSL_FUNC_BIO_NEW_FILE, (void (*)(void))ossl_core_bio_new_file },
+    { OSSL_FUNC_BIO_NEW_MEMBUF, (void (*)(void))ossl_core_bio_new_mem_buf },
+    { OSSL_FUNC_BIO_READ_EX, (void (*)(void))ossl_core_bio_read_ex },
+    { OSSL_FUNC_BIO_WRITE_EX, (void (*)(void))ossl_core_bio_write_ex },
+    { OSSL_FUNC_BIO_GETS, (void (*)(void))ossl_core_bio_gets },
+    { OSSL_FUNC_BIO_PUTS, (void (*)(void))ossl_core_bio_puts },
+    { OSSL_FUNC_BIO_CTRL, (void (*)(void))ossl_core_bio_ctrl },
+    { OSSL_FUNC_BIO_UP_REF, (void (*)(void))ossl_core_bio_up_ref },
+    { OSSL_FUNC_BIO_FREE, (void (*)(void))ossl_core_bio_free },
+    { OSSL_FUNC_BIO_VPRINTF, (void (*)(void))ossl_core_bio_vprintf },
     { OSSL_FUNC_BIO_VSNPRINTF, (void (*)(void))BIO_vsnprintf },
     { OSSL_FUNC_SELF_TEST_CB, (void (*)(void))OSSL_SELF_TEST_get_callback },
     { OSSL_FUNC_GET_ENTROPY, (void (*)(void))ossl_rand_get_entropy },
diff --git a/crypto/store/store_lib.c b/crypto/store/store_lib.c
index 5d0b3e7397..1aaf9f89a4 100644
--- a/crypto/store/store_lib.c
+++ b/crypto/store/store_lib.c
@@ -26,6 +26,7 @@
 #include "internal/thread_once.h"
 #include "internal/cryptlib.h"
 #include "internal/provider.h"
+#include "internal/bio.h"
 #include "crypto/store.h"
 #include "store_local.h"
 
@@ -941,9 +942,10 @@ OSSL_STORE_CTX *OSSL_STORE_attach(BIO *bp, const char *scheme,
         const OSSL_PROVIDER *provider =
             OSSL_STORE_LOADER_provider(fetched_loader);
         void *provctx = OSSL_PROVIDER_get0_provider_ctx(provider);
+        OSSL_CORE_BIO *cbio = ossl_core_bio_new_from_bio(bp);
 
-        if ((loader_ctx =
-             fetched_loader->p_attach(provctx, (OSSL_CORE_BIO *)bp)) == NULL) {
+        if (cbio == NULL
+            || (loader_ctx = fetched_loader->p_attach(provctx, cbio)) == NULL) {
             OSSL_STORE_LOADER_free(fetched_loader);
             fetched_loader = NULL;
         } else if (propq != NULL) {
@@ -961,6 +963,7 @@ OSSL_STORE_CTX *OSSL_STORE_attach(BIO *bp, const char *scheme,
             }
         }
         loader = fetched_loader;
+        ossl_core_bio_free(cbio);
     }
 
     if (loader_ctx == NULL) {
diff --git a/include/internal/bio.h b/include/internal/bio.h
index e057298318..b905845a1a 100644
--- a/include/internal/bio.h
+++ b/include/internal/bio.h
@@ -11,6 +11,7 @@
 # define OSSL_INTERNAL_BIO_H
 # pragma once
 
+# include <openssl/core.h>
 # include <openssl/bio.h>
 
 struct bio_method_st {
@@ -70,4 +71,19 @@ int bread_conv(BIO *bio, char *data, size_t datal, size_t *read);
 # define BIO_clear_ktls_ctrl_msg(b) \
      BIO_ctrl(b, BIO_CTRL_CLEAR_KTLS_TX_CTRL_MSG, 0, NULL)
 
+/* Functions to allow the core to offer the CORE_BIO type to providers */
+OSSL_CORE_BIO *ossl_core_bio_new_from_bio(BIO *bio);
+OSSL_CORE_BIO *ossl_core_bio_new_file(const char *filename, const char *mode);
+OSSL_CORE_BIO *ossl_core_bio_new_mem_buf(const void *buf, int len);
+int ossl_core_bio_read_ex(OSSL_CORE_BIO *cb, void *data, size_t dlen,
+                          size_t *readbytes);
+int ossl_core_bio_write_ex(OSSL_CORE_BIO *cb, const void *data, size_t dlen,
+                           size_t *written);
+int ossl_core_bio_gets(OSSL_CORE_BIO *cb, char *buf, int size);
+int ossl_core_bio_puts(OSSL_CORE_BIO *cb, const char *buf);
+long ossl_core_bio_ctrl(OSSL_CORE_BIO *cb, int cmd, long larg, void *parg);
+int ossl_core_bio_up_ref(OSSL_CORE_BIO *cb);
+int ossl_core_bio_free(OSSL_CORE_BIO *cb);
+int ossl_core_bio_vprintf(OSSL_CORE_BIO *cb, const char *format, va_list args);
+
 #endif
diff --git a/providers/common/bio_prov.c b/providers/common/bio_prov.c
index afdeb80d80..47f04c47e2 100644
--- a/providers/common/bio_prov.c
+++ b/providers/common/bio_prov.c
@@ -19,6 +19,7 @@ static OSSL_FUNC_BIO_write_ex_fn *c_bio_write_ex = NULL;
 static OSSL_FUNC_BIO_gets_fn *c_bio_gets = NULL;
 static OSSL_FUNC_BIO_puts_fn *c_bio_puts = NULL;
 static OSSL_FUNC_BIO_ctrl_fn *c_bio_ctrl = NULL;
+static OSSL_FUNC_BIO_up_ref_fn *c_bio_up_ref = NULL;
 static OSSL_FUNC_BIO_free_fn *c_bio_free = NULL;
 static OSSL_FUNC_BIO_vprintf_fn *c_bio_vprintf = NULL;
 
@@ -54,6 +55,10 @@ int ossl_prov_bio_from_dispatch(const OSSL_DISPATCH *fns)
             if (c_bio_ctrl == NULL)
                 c_bio_ctrl = OSSL_FUNC_BIO_ctrl(fns);
             break;
+        case OSSL_FUNC_BIO_UP_REF:
+            if (c_bio_up_ref == NULL)
+                c_bio_up_ref = OSSL_FUNC_BIO_up_ref(fns);
+            break;
         case OSSL_FUNC_BIO_FREE:
             if (c_bio_free == NULL)
                 c_bio_free = OSSL_FUNC_BIO_free(fns);
@@ -119,6 +124,13 @@ int ossl_prov_bio_ctrl(OSSL_CORE_BIO *bio, int cmd, long num, void *ptr)
     return c_bio_ctrl(bio, cmd, num, ptr);
 }
 
+int ossl_prov_bio_up_ref(OSSL_CORE_BIO *bio)
+{
+    if (c_bio_up_ref == NULL)
+        return 0;
+    return c_bio_up_ref(bio);
+}
+
 int ossl_prov_bio_free(OSSL_CORE_BIO *bio)
 {
     if (c_bio_free == NULL)
@@ -186,6 +198,7 @@ static int bio_core_new(BIO *bio)
 static int bio_core_free(BIO *bio)
 {
     BIO_set_init(bio, 0);
+    ossl_prov_bio_free(BIO_get_data(bio));
 
     return 1;
 }
@@ -218,10 +231,13 @@ BIO *bio_new_from_core_bio(PROV_CTX *provctx, OSSL_CORE_BIO *corebio)
     if (corebiometh == NULL)
         return NULL;
 
-    outbio = BIO_new(corebiometh);
-    if (outbio != NULL)
-        BIO_set_data(outbio, corebio);
-
+    if ((outbio = BIO_new(corebiometh)) == NULL)
+        return NULL;
+    if (!ossl_prov_bio_up_ref(corebio)) {
+        BIO_free(outbio);
+        return NULL;
+    }
+    BIO_set_data(outbio, corebio);
     return outbio;
 }
 
diff --git a/providers/common/include/prov/bio.h b/providers/common/include/prov/bio.h
index 9dd9f44bad..fc8237a249 100644
--- a/providers/common/include/prov/bio.h
+++ b/providers/common/include/prov/bio.h
@@ -23,6 +23,7 @@ int ossl_prov_bio_write_ex(OSSL_CORE_BIO *bio, const void *data, size_t data_len
 int ossl_prov_bio_gets(OSSL_CORE_BIO *bio, char *buf, int size);
 int ossl_prov_bio_puts(OSSL_CORE_BIO *bio, const char *str);
 int ossl_prov_bio_ctrl(OSSL_CORE_BIO *bio, int cmd, long num, void *ptr);
+int ossl_prov_bio_up_ref(OSSL_CORE_BIO *bio);
 int ossl_prov_bio_free(OSSL_CORE_BIO *bio);
 int ossl_prov_bio_vprintf(OSSL_CORE_BIO *bio, const char *format, va_list ap);
 int ossl_prov_bio_printf(OSSL_CORE_BIO *bio, const char *format, ...);
diff --git a/providers/implementations/storemgmt/file_store_der2obj.c b/providers/implementations/storemgmt/file_store_der2obj.c
index 74a18eb70b..d34e90540d 100644
--- a/providers/implementations/storemgmt/file_store_der2obj.c
+++ b/providers/implementations/storemgmt/file_store_der2obj.c
@@ -85,10 +85,13 @@ static int der2obj_decode(void *provctx, OSSL_CORE_BIO *cin, int selection,
      * We're called from file_store.c, so we know that OSSL_CORE_BIO is a
      * BIO in this case.
      */
-    BIO *in = (BIO *)cin;
+    BIO *in = bio_new_from_core_bio(provctx, cin);
     BUF_MEM *mem = NULL;
     int err, ok;
 
+    if (in == NULL)
+        return 0;
+
     ERR_set_mark();
     ok = (asn1_d2i_read_bio(in, &mem) >= 0);
     /*
@@ -117,6 +120,7 @@ static int der2obj_decode(void *provctx, OSSL_CORE_BIO *cin, int selection,
         OPENSSL_free(mem->data);
         OPENSSL_free(mem);
     }
+    BIO_free(in);
     return ok;
 }
 
diff --git a/test/ossl_store_test.c b/test/ossl_store_test.c
index 7424aed0dd..e06c0b55de 100644
--- a/test/ossl_store_test.c
+++ b/test/ossl_store_test.c
@@ -139,19 +139,21 @@ static int test_store_get_params(int idx)
 static int test_store_attach_unregistered_scheme(void)
 {
     int ret;
-    OSSL_STORE_CTX *store_ctx;
-    OSSL_PROVIDER *provider;
-    OSSL_LIB_CTX *libctx;
-    BIO *bio;
-    libctx = OSSL_LIB_CTX_new();
-    provider = OSSL_PROVIDER_load(libctx, "default");
-    bio = BIO_new_file("test/certs/sm2-root.crt", "r");
-
-    ret = TEST_ptr(store_ctx = OSSL_STORE_attach(bio, "file", libctx, NULL,
-                                                 NULL, NULL, NULL, NULL)) &&
-          TEST_int_ne(ERR_GET_LIB(ERR_peek_error()), ERR_LIB_OSSL_STORE) &&
-          TEST_int_ne(ERR_GET_REASON(ERR_peek_error()),
-                      OSSL_STORE_R_UNREGISTERED_SCHEME);
+    OSSL_STORE_CTX *store_ctx = NULL;
+    OSSL_PROVIDER *provider = NULL;
+    OSSL_LIB_CTX *libctx = NULL;
+    BIO *bio = NULL;
+    char *input = test_mk_file_path(inputdir, sm2file);
+
+    ret = TEST_ptr(input)
+          && TEST_ptr(libctx = OSSL_LIB_CTX_new())
+          && TEST_ptr(provider = OSSL_PROVIDER_load(libctx, "default"))
+          && TEST_ptr(bio = BIO_new_file(input, "r"))
+          && TEST_ptr(store_ctx = OSSL_STORE_attach(bio, "file", libctx, NULL,
+                                                    NULL, NULL, NULL, NULL))
+          && TEST_int_ne(ERR_GET_LIB(ERR_peek_error()), ERR_LIB_OSSL_STORE)
+          && TEST_int_ne(ERR_GET_REASON(ERR_peek_error()),
+                         OSSL_STORE_R_UNREGISTERED_SCHEME);
 
     BIO_free(bio);
     OSSL_STORE_close(store_ctx);