From 7882dd15b68f284824b4cfa1e7ce9769b2860811 Mon Sep 17 00:00:00 2001 From: Michael Baentsch <57787676+baentsch@users.noreply.github.com> Date: Sat, 2 Sep 2023 06:28:37 +0200 Subject: [PATCH 1/5] corner case object creation added --- oqsprov/oqsprov.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/oqsprov/oqsprov.c b/oqsprov/oqsprov.c index ddceff4f..520d4498 100644 --- a/oqsprov/oqsprov.c +++ b/oqsprov/oqsprov.c @@ -742,6 +742,13 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, return 0; } + /* create object (NID) again to avoid setup corner case problems + * see https://github.com/openssl/openssl/discussions/21903 + * Not testing for errors is intentional. + */ + OBJ_create(oqs_oid_alg_list[i], oqs_oid_alg_list[i + 1], + oqs_oid_alg_list[i + 1]); + if (!oqs_set_nid((char *)oqs_oid_alg_list[i + 1], OBJ_sn2nid(oqs_oid_alg_list[i + 1]))) { ERR_raise(ERR_LIB_USER, OQSPROV_R_OBJ_CREATE_ERR); @@ -764,6 +771,7 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, fprintf(stderr, "OQS PROV: Impossible error: NID unregistered for %s.\n", oqs_oid_alg_list[i + 1]); + ERR_raise(ERR_LIB_USER, OQSPROV_R_OBJ_CREATE_ERR); return 0; } } From 696d944d31c1b2ec58c18cf939472b0393a37f6c Mon Sep 17 00:00:00 2001 From: Michael Baentsch <57787676+baentsch@users.noreply.github.com> Date: Sun, 3 Sep 2023 10:17:42 +0200 Subject: [PATCH 2/5] enable 3.1.0 workaround --- oqsprov/oqsprov.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/oqsprov/oqsprov.c b/oqsprov/oqsprov.c index 520d4498..ef5e4725 100644 --- a/oqsprov/oqsprov.c +++ b/oqsprov/oqsprov.c @@ -691,6 +691,11 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, BIO_METHOD *corebiometh; OSSL_LIB_CTX *libctx = NULL; int i, rc = 0; + char *opensslv; + const char *versionp; + OSSL_PARAM version_request[] = {{"openssl-version", OSSL_PARAM_UTF8_PTR, + &opensslv, sizeof(&opensslv), 0}, + {NULL, 0, NULL, 0, 0}}; OQS_init(); @@ -729,9 +734,15 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, } // we need these functions: - if (c_obj_create == NULL || c_obj_add_sigid == NULL) + if (c_obj_create == NULL || c_obj_add_sigid == NULL || c_get_params == NULL) return 0; + // we need to know the version of the calling core to activate + // suitable bug workarounds + if (c_get_params(handle, version_request)) { + versionp = *(void **)version_request[0].data; + } + // insert all OIDs to the global objects list for (i = 0; i < OQS_OID_CNT; i += 2) { if (!c_obj_create(handle, oqs_oid_alg_list[i], oqs_oid_alg_list[i + 1], @@ -745,9 +756,12 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, /* create object (NID) again to avoid setup corner case problems * see https://github.com/openssl/openssl/discussions/21903 * Not testing for errors is intentional. + * At least one core version hangs up; so don't do this there: */ - OBJ_create(oqs_oid_alg_list[i], oqs_oid_alg_list[i + 1], - oqs_oid_alg_list[i + 1]); + if (strcmp("3.1.0", versionp)) { + OBJ_create(oqs_oid_alg_list[i], oqs_oid_alg_list[i + 1], + oqs_oid_alg_list[i + 1]); + } if (!oqs_set_nid((char *)oqs_oid_alg_list[i + 1], OBJ_sn2nid(oqs_oid_alg_list[i + 1]))) { From bb196e7b4a6a88c06f46485182db64262706229b Mon Sep 17 00:00:00 2001 From: Michael Baentsch <57787676+baentsch@users.noreply.github.com> Date: Mon, 4 Sep 2023 07:15:35 +0200 Subject: [PATCH 3/5] output more info on error --- oqsprov/oqsprov.c | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/oqsprov/oqsprov.c b/oqsprov/oqsprov.c index ef5e4725..e2b56e60 100644 --- a/oqsprov/oqsprov.c +++ b/oqsprov/oqsprov.c @@ -692,7 +692,7 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, OSSL_LIB_CTX *libctx = NULL; int i, rc = 0; char *opensslv; - const char *versionp; + const char *ossl_versionp = NULL; OSSL_PARAM version_request[] = {{"openssl-version", OSSL_PARAM_UTF8_PTR, &opensslv, sizeof(&opensslv), 0}, {NULL, 0, NULL, 0, 0}}; @@ -700,17 +700,17 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, OQS_init(); if (!oqs_prov_bio_from_dispatch(in)) - return 0; + goto end_init; if (!oqs_patch_codepoints()) - return 0; + goto end_init; if (!oqs_patch_oids()) - return 0; + goto end_init; #ifdef USE_ENCODING_LIB if (!oqs_patch_encodings()) - return 0; + goto end_init; #endif for (; in->function_id != 0; in++) { @@ -735,12 +735,12 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, // we need these functions: if (c_obj_create == NULL || c_obj_add_sigid == NULL || c_get_params == NULL) - return 0; + goto end_init; // we need to know the version of the calling core to activate // suitable bug workarounds if (c_get_params(handle, version_request)) { - versionp = *(void **)version_request[0].data; + ossl_versionp = *(void **)version_request[0].data; } // insert all OIDs to the global objects list @@ -750,7 +750,7 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, ERR_raise(ERR_LIB_USER, OQSPROV_R_OBJ_CREATE_ERR); fprintf(stderr, "error registering NID for %s\n", oqs_oid_alg_list[i + 1]); - return 0; + goto end_init; } /* create object (NID) again to avoid setup corner case problems @@ -758,7 +758,7 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, * Not testing for errors is intentional. * At least one core version hangs up; so don't do this there: */ - if (strcmp("3.1.0", versionp)) { + if (strcmp("3.1.0", ossl_versionp)) { OBJ_create(oqs_oid_alg_list[i], oqs_oid_alg_list[i + 1], oqs_oid_alg_list[i + 1]); } @@ -766,7 +766,7 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, if (!oqs_set_nid((char *)oqs_oid_alg_list[i + 1], OBJ_sn2nid(oqs_oid_alg_list[i + 1]))) { ERR_raise(ERR_LIB_USER, OQSPROV_R_OBJ_CREATE_ERR); - return 0; + goto end_init; } if (!c_obj_add_sigid(handle, oqs_oid_alg_list[i + 1], "", @@ -774,7 +774,7 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, fprintf(stderr, "error registering %s with no hash\n", oqs_oid_alg_list[i + 1]); ERR_raise(ERR_LIB_USER, OQSPROV_R_OBJ_CREATE_ERR); - return 0; + goto end_init; } if (OBJ_sn2nid(oqs_oid_alg_list[i + 1]) != 0) { @@ -786,7 +786,7 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, "OQS PROV: Impossible error: NID unregistered for %s.\n", oqs_oid_alg_list[i + 1]); ERR_raise(ERR_LIB_USER, OQSPROV_R_OBJ_CREATE_ERR); - return 0; + goto end_init; } } @@ -814,9 +814,15 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, end_init: if (!rc) { - OSSL_LIB_CTX_free(libctx); - oqsprovider_teardown(*provctx); - *provctx = NULL; + if (ossl_versionp) + OQS_PROV_PRINTF2("oqsprovider init failed for OpenSSL core version %s\n", ossl_versionp); + else + OQS_PROV_PRINTF("oqsprovider init failed for OpenSSL\n"); + if (libctx) OSSL_LIB_CTX_free(libctx); + if (provctx && *provctx) { + oqsprovider_teardown(*provctx); + *provctx = NULL; + } } return rc; } From 62fec76826b457ed9dfd4ad77ee3207a62e85b8d Mon Sep 17 00:00:00 2001 From: Michael Baentsch <57787676+baentsch@users.noreply.github.com> Date: Mon, 4 Sep 2023 07:23:36 +0200 Subject: [PATCH 4/5] correct format error --- oqsprov/oqsprov.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/oqsprov/oqsprov.c b/oqsprov/oqsprov.c index e2b56e60..cd32a5a3 100644 --- a/oqsprov/oqsprov.c +++ b/oqsprov/oqsprov.c @@ -815,10 +815,13 @@ int OQS_PROVIDER_ENTRYPOINT_NAME(const OSSL_CORE_HANDLE *handle, end_init: if (!rc) { if (ossl_versionp) - OQS_PROV_PRINTF2("oqsprovider init failed for OpenSSL core version %s\n", ossl_versionp); + OQS_PROV_PRINTF2( + "oqsprovider init failed for OpenSSL core version %s\n", + ossl_versionp); else OQS_PROV_PRINTF("oqsprovider init failed for OpenSSL\n"); - if (libctx) OSSL_LIB_CTX_free(libctx); + if (libctx) + OSSL_LIB_CTX_free(libctx); if (provctx && *provctx) { oqsprovider_teardown(*provctx); *provctx = NULL; From 4f841a225e1ba24518718721fab213151900ac05 Mon Sep 17 00:00:00 2001 From: Michael Baentsch <57787676+baentsch@users.noreply.github.com> Date: Mon, 4 Sep 2023 08:44:31 +0200 Subject: [PATCH 5/5] enhanced bug report template [skip ci] --- .github/ISSUE_TEMPLATE/bug_report.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 1966badb..22234bd7 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -35,6 +35,16 @@ Please run the following commands to obtain the version information: If `oqsprovider` is not listed as active, be sure to first follow all [USAGE guidance](https://github.com/open-quantum-safe/oqs-provider/blob/main/USAGE.md). +If reporting bugs triggered by OpenSSL API integrations, e.g. running +a provider build [statically](https://github.com/open-quantum-safe/oqs-provider/blob/main/CONFIGURE.md#oqs_provider_build_static) +or directly invoking any OpenSSL API, be sure to retrieve and report all errors +reported by using the OpenSSL [ERR_get_error_all](https://www.openssl.org/docs/man3.1/man3/ERR_get_error_all.html) +function. + +Bug reports generated from [Debug builds](https://github.com/open-quantum-safe/oqs-provider/wiki/Debugging) +wth the debug environment variable "OQSPROV=1" set will be particularly helpful to find underlying +problems. + **Additional context** Add any other context about the problem here.