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)

diff --git a/crypto/bio/build.info b/crypto/bio/build.info
index 227071f0ce7de176815023b92c05010f1162dfc1..2bee64fc6287d711d82ede13f53e257773247e24 100644 (file)
--- 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 (file)
index 0000000..328302e
--- /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 f161c7cd5b3dbd3ef344bb726c4ef0879fafc38c..9c01dc894b37b9502bb614d1d4954e95af39c1e7 100644 (file)
--- 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 d15fb27fde3ebffc9f132f5fdf07c4259fe2d6d3..b25529e073cf00dba5a8f363a016120c50a604e4 100644 (file)
--- 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 1326f83f7e7a00f00f4e6da7a8be3e3f44b638f6..9536cb65d11d80f701e049e4506e36af510e6821 100644 (file)
--- 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 5d0b3e739740df2443fbbca36e74ff845dbf3230..1aaf9f89a4e44187f7a174e0c851bdb118db70a6 100644 (file)
--- 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 e057298318a956dc795260646e4036286be85b99..b905845a1af5cfd00da89a080fef10b23a217938 100644 (file)
--- 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 afdeb80d802cad04e57c3f351a125a91a3cef26d..47f04c47e2422f89f78aeee1066ae7d9ff46f7b8 100644 (file)
--- 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 9dd9f44bada460e5a459daaca47656bbdd5e61b2..fc8237a249e104173b0420c56e7fff87afe02ba1 100644 (file)
--- 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 74a18eb70b2a65af36c5669efa6b45bc58689365..d34e90540dc27d998b43f760f01f55bbb3883b7f 100644 (file)
--- 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 7424aed0dd573857cf52524200feb42d2b978c87..e06c0b55def6fd8a09daf0d2a9915b07ab577e63 100644 (file)
--- 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);