[openssl] master update

matthias.st.pierre at ncp-e.com matthias.st.pierre at ncp-e.com
Fri Mar 29 23:08:32 UTC 2019


The branch master has been updated
       via  3a8269b3194f7528e3657cef70fe2db1ed38b755 (commit)
       via  02bd2d7f5ce30928baf11226fa31125337251e49 (commit)
       via  6a411436a5642a4c2ca934a649eced7118f3793d (commit)
       via  0fda9f7c291de7b3a1e576ce43801dfa91b42f0e (commit)
      from  2e6b615f795e8ca8ae830a00079c4ea064eaae42 (commit)


- Log -----------------------------------------------------------------
commit 3a8269b3194f7528e3657cef70fe2db1ed38b755
Author: Dr. Matthias St. Pierre <Matthias.St.Pierre at ncp-e.com>
Date:   Thu Mar 21 18:59:13 2019 +0100

    trace: rename the default trace category from 'ANY' to 'ALL'
    
    It seems more intuitive to set `OPENSSL_TRACE=all` instead of
    `OPENSSL_TRACE=any` to obtain trace output for all categories.
    
    Reviewed-by: Paul Dale <paul.dale at oracle.com>
    (Merged from https://github.com/openssl/openssl/pull/8552)

commit 02bd2d7f5ce30928baf11226fa31125337251e49
Author: Dr. Matthias St. Pierre <Matthias.St.Pierre at ncp-e.com>
Date:   Thu Mar 21 18:27:50 2019 +0100

    trace: apps/openssl: print the correct category name
    
    Previously, if the openssl application was run with OPENSSL_TRACE=any,
    all trace output would just show 'ANY' as the category name, which was
    not very useful. To get the correct category name printed in the trace
    output, the openssl application now registers separate channels for
    each category.
    
    The trace API is unchanged, it is still possible for an application to
    register a single channel for the 'ANY' category to see all outputt,
    if it does not need this level of detail.
    
    Reviewed-by: Paul Dale <paul.dale at oracle.com>
    (Merged from https://github.com/openssl/openssl/pull/8552)

commit 6a411436a5642a4c2ca934a649eced7118f3793d
Author: Dr. Matthias St. Pierre <Matthias.St.Pierre at ncp-e.com>
Date:   Thu Mar 21 00:56:36 2019 +0100

    trace: fix out-of-bound memory access
    
    When OSSL_trace_get_category_num() is called with an unknown category
    name, it returns -1. This case needs to be considered in order to
    avoid out-of-bound memory access to the `trace_channels` array.
    
    Reviewed-by: Paul Dale <paul.dale at oracle.com>
    (Merged from https://github.com/openssl/openssl/pull/8552)

commit 0fda9f7c291de7b3a1e576ce43801dfa91b42f0e
Author: Dr. Matthias St. Pierre <Matthias.St.Pierre at ncp-e.com>
Date:   Tue Mar 19 08:53:35 2019 +0100

    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 <paul.dale at oracle.com>
    (Merged from https://github.com/openssl/openssl/pull/8552)

-----------------------------------------------------------------------

Summary of changes:
 apps/openssl.c                      | 57 +++++++++++++++++++++++++------------
 crypto/trace.c                      | 42 +++++++++++++--------------
 doc/man3/OSSL_trace_enabled.pod     |  2 +-
 doc/man3/OSSL_trace_set_channel.pod |  8 +++++-
 include/openssl/trace.h             |  8 ++++--
 5 files changed, 72 insertions(+), 45 deletions(-)

diff --git a/apps/openssl.c b/apps/openssl.c
index b5ec835..9c0d933 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;
@@ -183,6 +185,33 @@ static void cleanup_trace(void)
     sk_tracedata_pop_free(trace_data_stack, tracedata_free);
 }
 
+static void setup_trace_category(int category)
+{
+    BIO *channel;
+    tracedata *trace_data;
+
+    if (OSSL_trace_enabled(category))
+        return;
+
+    channel = BIO_push(BIO_new(apps_bf_prefix()),
+                       dup_bio_err(FORMAT_TEXT));
+    trace_data = OPENSSL_zalloc(sizeof(*trace_data));
+
+    if (trace_data == NULL
+        || (trace_data->bio = channel) == NULL
+        || OSSL_trace_set_callback(category, internal_trace_cb,
+                                   trace_data) == 0
+        || sk_tracedata_push(trace_data_stack, trace_data) == 0) {
+
+        fprintf(stderr,
+                "warning: unable to setup trace callback for category '%s'.\n",
+                OSSL_trace_get_category_name(category));
+
+        OSSL_trace_set_callback(category, NULL, NULL);
+        BIO_free_all(channel);
+    }
+}
+
 static void setup_trace(const char *str)
 {
     char *val;
@@ -197,26 +226,15 @@ static void setup_trace(const char *str)
         for (valp = val; (item = strtok(valp, ",")) != NULL; valp = NULL) {
             int category = OSSL_trace_get_category_num(item);
 
-            if (category >= 0) {
-                BIO *channel = BIO_push(BIO_new(apps_bf_prefix()),
-                                        dup_bio_err(FORMAT_TEXT));
-                tracedata *trace_data = OPENSSL_zalloc(sizeof(*trace_data));
-
-                if (trace_data == NULL
-                    || (trace_data->bio = channel) == NULL
-                    || OSSL_trace_set_callback(category, internal_trace_cb,
-                                               trace_data) == 0
-                    || sk_tracedata_push(trace_data_stack, trace_data) == 0) {
-                    OSSL_trace_set_callback(category, NULL, NULL);
-                    BIO_free_all(channel);
-                    fprintf(stderr,
-                            "warning: unable to setup trace callback for category '%s'.\n",
-                            item);
-                }
+            if (category == OSSL_TRACE_CATEGORY_ALL) {
+                while (++category < OSSL_TRACE_CATEGORY_NUM)
+                    setup_trace_category(category);
+                break;
+            } else if (category > 0) {
+                setup_trace_category(category);
             } else {
                 fprintf(stderr,
-                        "warning: unknown trace category: '%s'.\n",
-                        item);
+                        "warning: unknown trace category: '%s'.\n", item);
             }
         }
     }
@@ -224,6 +242,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 +279,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 613664c..efcf8be 100644
--- a/crypto/trace.c
+++ b/crypto/trace.c
@@ -119,7 +119,7 @@ struct trace_category_st {
 #define TRACE_CATEGORY_(name)       { #name, OSSL_TRACE_CATEGORY_##name }
 
 static const struct trace_category_st trace_categories[] = {
-    TRACE_CATEGORY_(ANY),
+    TRACE_CATEGORY_(ALL),
     TRACE_CATEGORY_(TRACE),
     TRACE_CATEGORY_(INIT),
     TRACE_CATEGORY_(TLS),
@@ -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
@@ -430,7 +422,7 @@ static int ossl_trace_get_category(int category)
         return -1;
     if (trace_channels[category].bio != NULL)
         return category;
-    return OSSL_TRACE_CATEGORY_ANY;
+    return OSSL_TRACE_CATEGORY_ALL;
 }
 #endif
 
@@ -439,7 +431,8 @@ int OSSL_trace_enabled(int category)
     int ret = 0;
 #ifndef OPENSSL_NO_TRACE
     category = ossl_trace_get_category(category);
-    ret = trace_channels[category].bio != NULL;
+    if (category >= 0)
+        ret = trace_channels[category].bio != NULL;
 #endif
     return ret;
 }
@@ -451,6 +444,9 @@ BIO *OSSL_trace_begin(int category)
     char *prefix = NULL;
 
     category = ossl_trace_get_category(category);
+    if (category < 0)
+        return NULL;
+
     channel = trace_channels[category].bio;
     prefix = trace_channels[category].prefix;
 
diff --git a/doc/man3/OSSL_trace_enabled.pod b/doc/man3/OSSL_trace_enabled.pod
index 98e3bd6..e26dee5 100644
--- a/doc/man3/OSSL_trace_enabled.pod
+++ b/doc/man3/OSSL_trace_enabled.pod
@@ -40,7 +40,7 @@ The tracing output is divided into types which are enabled
 individually by the application.
 The tracing types are described in detail in
 L<OSSL_trace_set_callback(3)/Trace types>.
-The fallback type C<OSSL_TRACE_CATEGORY_ANY> should I<not> be used
+The fallback type C<OSSL_TRACE_CATEGORY_ALL> should I<not> be used
 with the functions described here.
 
 Tracing for a specific category is enabled if a so called
diff --git a/doc/man3/OSSL_trace_set_channel.pod b/doc/man3/OSSL_trace_set_channel.pod
index 773a6b1..46e248f 100644
--- a/doc/man3/OSSL_trace_set_channel.pod
+++ b/doc/man3/OSSL_trace_set_channel.pod
@@ -178,9 +178,15 @@ Traces BIGNUM context operations.
 
 =back
 
-There is also C<OSSL_TRACE_CATEGORY_ANY>, which works as a fallback
+There is also C<OSSL_TRACE_CATEGORY_ALL>, which works as a fallback
 and can be used to get I<all> trace output.
 
+Note, however, that in this case all trace output will effectively be
+associated with the 'ALL' category, which is undesirable if the
+application intends to include the category name in the trace output.
+In this case it is better to register separate channels for each
+trace category instead.
+
 =head1 RETURN VALUES
 
 OSSL_trace_set_channel(), OSSL_trace_set_prefix(),
diff --git a/include/openssl/trace.h b/include/openssl/trace.h
index 767b19f..48c98ca 100644
--- a/include/openssl/trace.h
+++ b/include/openssl/trace.h
@@ -30,9 +30,13 @@ extern "C" {
  * BIO which sends all trace output it receives to the registered application
  * callback.
  *
- * The ANY category is used as a fallback category.
+ * The ALL category can be used as a fallback category to register a single
+ * channel which receives the output from all categories. However, if the
+ * application intends to print the trace channel name in the line prefix,
+ * it is better to register channels for all categories separately.
+ * (This is how the openssl application does it.)
  */
-# define OSSL_TRACE_CATEGORY_ANY                 0 /* The fallback */
+# define OSSL_TRACE_CATEGORY_ALL                 0 /* The fallback */
 # define OSSL_TRACE_CATEGORY_TRACE               1
 # define OSSL_TRACE_CATEGORY_INIT                2
 # define OSSL_TRACE_CATEGORY_TLS                 3


More information about the openssl-commits mailing list