From 0fda9f7c291de7b3a1e576ce43801dfa91b42f0e Mon Sep 17 00:00:00 2001 From: "Dr. Matthias St. Pierre" Date: Tue, 19 Mar 2019 08:53:35 +0100 Subject: [PATCH] trace: don't pretend success if it's not enabled Partially reverts d33d76168fb7 Don't fail when tracing is disabled Commit d33d76168fb7 fixed the problem that the initialization of libcrypto failed when tracing was disabled, because the unoperational ossl_trace_init() function returned a failure code. The problem was fixed by changing its return value from failure to success. As part of the fix the return values of other unimplemented trace API functions (like OSSL_trace_set_channel(),OSSL_trace_set_callback()) was changed from failure to success, too. This change was not necessary and is a bit problematic IMHO, because nobody expects an unimplemented function to pretend it succeeded. It's the application's duty to handle the case correctly when the trace API is not enabled (i.e., OPENSSL_NO_TRACE is defined), not the API's job to pretend success just to prevent the application from failing. Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/8552) --- apps/openssl.c | 5 +++++ crypto/trace.c | 32 ++++++++++++-------------------- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/apps/openssl.c b/apps/openssl.c index b5ec835d51..44dbda7625 100644 --- a/apps/openssl.c +++ b/apps/openssl.c @@ -118,6 +118,8 @@ static char *make_config_name(void) return p; } + +#ifndef OPENSSL_NO_TRACE typedef struct tracedata_st { BIO *bio; unsigned int ingroup:1; @@ -224,6 +226,7 @@ static void setup_trace(const char *str) OPENSSL_free(val); atexit(cleanup_trace); } +#endif /* OPENSSL_NO_TRACE */ int main(int argc, char *argv[]) { @@ -260,7 +263,9 @@ int main(int argc, char *argv[]) */ atexit(destroy_prefix_method); +#ifndef OPENSSL_NO_TRACE setup_trace(getenv("OPENSSL_TRACE")); +#endif p = getenv("OPENSSL_DEBUG_MEMORY"); if (p != NULL && strcmp(p, "on") == 0) diff --git a/crypto/trace.c b/crypto/trace.c index 613664c3be..6299a688be 100644 --- a/crypto/trace.c +++ b/crypto/trace.c @@ -328,12 +328,11 @@ void ossl_trace_cleanup(void) int OSSL_trace_set_channel(int category, BIO *channel) { #ifndef OPENSSL_NO_TRACE - if (category < 0 || category >= OSSL_TRACE_CATEGORY_NUM - || !set_trace_data(category, SIMPLE_CHANNEL, &channel, NULL, NULL, - trace_attach_cb, trace_detach_cb)) - return 0; + if (category >= 0 && category < OSSL_TRACE_CATEGORY_NUM) + return set_trace_data(category, SIMPLE_CHANNEL, &channel, NULL, NULL, + trace_attach_cb, trace_detach_cb)) #endif - return 1; + return 0; } #ifndef OPENSSL_NO_TRACE @@ -367,7 +366,7 @@ int OSSL_trace_set_callback(int category, OSSL_trace_cb callback, void *data) struct trace_data_st *trace_data = NULL; if (category < 0 || category >= OSSL_TRACE_CATEGORY_NUM) - goto err; + return 0; if (callback != NULL) { if ((channel = BIO_new(&trace_method)) == NULL @@ -386,41 +385,34 @@ int OSSL_trace_set_callback(int category, OSSL_trace_cb callback, void *data) trace_attach_w_callback_cb, trace_detach_cb)) goto err; - goto done; + return 1; err: BIO_free(channel); OPENSSL_free(trace_data); - return 0; - done: #endif - return 1; + + return 0; } int OSSL_trace_set_prefix(int category, const char *prefix) { - int rv = 1; - #ifndef OPENSSL_NO_TRACE - if (category >= 0 || category < OSSL_TRACE_CATEGORY_NUM) + if (category >= 0 && category < OSSL_TRACE_CATEGORY_NUM) return set_trace_data(category, 0, NULL, &prefix, NULL, trace_attach_cb, trace_detach_cb); - rv = 0; #endif - return rv; + return 0; } int OSSL_trace_set_suffix(int category, const char *suffix) { - int rv = 1; - #ifndef OPENSSL_NO_TRACE - if (category >= 0 || category < OSSL_TRACE_CATEGORY_NUM) + if (category >= 0 && category < OSSL_TRACE_CATEGORY_NUM) return set_trace_data(category, 0, NULL, NULL, &suffix, trace_attach_cb, trace_detach_cb); - rv = 0; #endif - return rv; + return 0; } #ifndef OPENSSL_NO_TRACE -- 2.25.1