[openssl] master update

Richard Levitte levitte at openssl.org
Sat Feb 20 20:16:04 UTC 2021


The branch master has been updated
       via  57acc56bdcdf2a7f084cf480f6f1d8f250735b0c (commit)
       via  acf497b53b0a349af13ca5e89665f331e1096af8 (commit)
      from  f16e52b67c9261bdc7e1284a50502a802921ac6d (commit)


- Log -----------------------------------------------------------------
commit 57acc56bdcdf2a7f084cf480f6f1d8f250735b0c
Author: Richard Levitte <levitte at openssl.org>
Date:   Fri Feb 19 10:16:04 2021 +0100

    DECODER: Add better tracing of the chain walking process
    
    Reviewed-by: Matt Caswell <matt at openssl.org>
    Reviewed-by: Paul Dale <pauli at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/14233)

commit acf497b53b0a349af13ca5e89665f331e1096af8
Author: Richard Levitte <levitte at openssl.org>
Date:   Thu Feb 18 13:18:53 2021 +0100

    DECODER: Use the data structure from the last decoder to select the next
    
    Any decoder can now also declare the name of the data structure for
    the object it decoded in the OSSL_PARAM array they pass back to the
    decoding process.  The decoding process will use that as another
    criterion to select the next decoder in the chain to consider.
    
    Together with declaring the data type, this becomes a means to refine
    how the decoded data is treated along the chain.
    
    Fixes #13539
    
    Reviewed-by: Matt Caswell <matt at openssl.org>
    Reviewed-by: Paul Dale <pauli at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/14233)

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

Summary of changes:
 crypto/encode_decode/decoder_lib.c | 135 +++++++++++++++++++++++++++++++++----
 1 file changed, 123 insertions(+), 12 deletions(-)

diff --git a/crypto/encode_decode/decoder_lib.c b/crypto/encode_decode/decoder_lib.c
index 8e9af13bbb..6503b46d63 100644
--- a/crypto/encode_decode/decoder_lib.c
+++ b/crypto/encode_decode/decoder_lib.c
@@ -28,6 +28,8 @@ struct decoder_process_data_st {
 
     /* Index of the current decoder instance to be processed */
     size_t current_decoder_inst_index;
+    /* For tracing, count recursion level */
+    size_t recursion;
 };
 
 static int decoder_process(const OSSL_PARAM params[], void *arg);
@@ -512,20 +514,34 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
     int err, ok = 0;
     /* For recursions */
     struct decoder_process_data_st new_data;
-    const char *object_type = NULL;
+    const char *data_type = NULL;
+    const char *data_structure = NULL;
 
     memset(&new_data, 0, sizeof(new_data));
     new_data.ctx = data->ctx;
+    new_data.recursion = data->recursion + 1;
+
+#define LEVEL_STR ">>>>>>>>>>>>>>>>"
+#define LEVEL (new_data.recursion < sizeof(LEVEL_STR)                   \
+               ? &LEVEL_STR[sizeof(LEVEL_STR) - new_data.recursion - 1] \
+               : LEVEL_STR "...")
 
     if (params == NULL) {
         /* First iteration, where we prepare for what is to come */
 
+        OSSL_TRACE_BEGIN(DECODER) {
+            BIO_printf(trc_out,
+                       "(ctx %p) starting to walk the decoder chain\n",
+                       (void *)new_data.ctx);
+        } OSSL_TRACE_END(DECODER);
+
         data->current_decoder_inst_index =
             OSSL_DECODER_CTX_get_num_decoders(ctx);
 
         bio = data->bio;
     } else {
         const OSSL_PARAM *p;
+        const char *trace_data_structure;
 
         decoder_inst =
             sk_OSSL_DECODER_INSTANCE_value(ctx->decoder_insts,
@@ -555,10 +571,42 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
             goto end;
         bio = new_data.bio;
 
-        /* Get the object type if there is one */
+        /* Get the data type if there is one */
         p = OSSL_PARAM_locate_const(params, OSSL_OBJECT_PARAM_DATA_TYPE);
-        if (p != NULL && !OSSL_PARAM_get_utf8_string_ptr(p, &object_type))
+        if (p != NULL && !OSSL_PARAM_get_utf8_string_ptr(p, &data_type))
             goto end;
+
+        /* Get the data structure if there is one */
+        p = OSSL_PARAM_locate_const(params, OSSL_OBJECT_PARAM_DATA_STRUCTURE);
+        if (p != NULL && !OSSL_PARAM_get_utf8_string_ptr(p, &data_structure))
+            goto end;
+
+        /*
+         * If the data structure is "type-specific" and the data type is
+         * given, we drop the data structure.  The reasoning is that the
+         * data type is already enough to find the applicable next decoder,
+         * so an additional "type-specific" data structure is extraneous.
+         *
+         * Furthermore, if the OSSL_DECODER caller asked for a type specific
+         * structure under another name, such as "DH", we get a mismatch
+         * if the data structure we just received is "type-specific".
+         * There's only so much you can do without infusing this code with
+         * too special knowledge.
+         */
+        trace_data_structure = data_structure;
+        if (data_type != NULL
+            && strcasecmp(data_structure, "type-specific") == 0)
+            data_structure = NULL;
+
+        OSSL_TRACE_BEGIN(DECODER) {
+            BIO_printf(trc_out,
+                       "(ctx %p) %s incoming from previous decoder (%p):\n"
+                       "    data type: %s, data structure: %s%s\n",
+                       (void *)new_data.ctx, LEVEL, (void *)decoder,
+                       data_type, trace_data_structure,
+                       (trace_data_structure == data_structure
+                        ? "" : " (dropped)"));
+        } OSSL_TRACE_END(DECODER);
     }
 
     /*
@@ -582,6 +630,19 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
             OSSL_DECODER_INSTANCE_get_decoder_ctx(new_decoder_inst);
         const char *new_input_type =
             OSSL_DECODER_INSTANCE_get_input_type(new_decoder_inst);
+        int n_i_s_was_set = 0;   /* We don't care here */
+        const char *new_input_structure =
+            OSSL_DECODER_INSTANCE_get_input_structure(new_decoder_inst,
+                                                      &n_i_s_was_set);
+
+        OSSL_TRACE_BEGIN(DECODER) {
+            BIO_printf(trc_out,
+                       "(ctx %p) %s [%u] Considering decoder instance %p, which has:\n"
+                       "    input type: %s, input structure: %s, decoder: %p\n",
+                       (void *)new_data.ctx, LEVEL, (unsigned int)i,
+                       (void *)new_decoder_inst, new_input_type,
+                       new_input_structure, (void *)new_decoder);
+        } OSSL_TRACE_END(DECODER);
 
         /*
          * If |decoder| is NULL, it means we've just started, and the caller
@@ -589,24 +650,60 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
          * that's the case, we do this extra check.
          */
         if (decoder == NULL && ctx->start_input_type != NULL
-            && strcasecmp(ctx->start_input_type, new_input_type) != 0)
+            && strcasecmp(ctx->start_input_type, new_input_type) != 0) {
+            OSSL_TRACE_BEGIN(DECODER) {
+                BIO_printf(trc_out,
+                           "(ctx %p) %s [%u] the start input type '%s' doesn't match the input type of the considered decoder, skipping...\n",
+                           (void *)new_data.ctx, LEVEL, (unsigned int)i,
+                           ctx->start_input_type);
+            } OSSL_TRACE_END(DECODER);
             continue;
+        }
 
         /*
          * If we have a previous decoder, we check that the input type
          * of the next to be used matches the type of this previous one.
-         * input_type is a cache of the parameter "input-type" value for
-         * that decoder.
+         * |new_input_type| holds the value of the "input-type" parameter
+         * for the decoder we're currently considering.
          */
-        if (decoder != NULL && !OSSL_DECODER_is_a(decoder, new_input_type))
+        if (decoder != NULL && !OSSL_DECODER_is_a(decoder, new_input_type)) {
+            OSSL_TRACE_BEGIN(DECODER) {
+                BIO_printf(trc_out,
+                           "(ctx %p) %s [%u] the input type doesn't match the name of the previous decoder (%p), skipping...\n",
+                           (void *)new_data.ctx, LEVEL, (unsigned int)i,
+                           (void *)decoder);
+            } OSSL_TRACE_END(DECODER);
             continue;
+        }
 
         /*
-         * If the previous decoder gave us an object type, we check to see
+         * If the previous decoder gave us a data type, we check to see
          * if that matches the decoder we're currently considering.
          */
-        if (object_type != NULL && !OSSL_DECODER_is_a(new_decoder, object_type))
+        if (data_type != NULL && !OSSL_DECODER_is_a(new_decoder, data_type)) {
+            OSSL_TRACE_BEGIN(DECODER) {
+                BIO_printf(trc_out,
+                           "(ctx %p) %s [%u] the previous decoder's data type doesn't match the name of the considered decoder, skipping...\n",
+                           (void *)new_data.ctx, LEVEL, (unsigned int)i);
+            } OSSL_TRACE_END(DECODER);
             continue;
+        }
+
+        /*
+         * If the previous decoder gave us a data structure name, we check
+         * to see that it matches the input data structure of the decoder
+         * we're currently considering.
+         */
+        if (data_structure != NULL
+            && (new_input_structure == NULL
+                || strcasecmp(data_structure, new_input_structure) != 0)) {
+            OSSL_TRACE_BEGIN(DECODER) {
+                BIO_printf(trc_out,
+                           "(ctx %p) %s [%u] the previous decoder's data structure doesn't match the input structure of the considered decoder, skipping...\n",
+                           (void *)new_data.ctx, LEVEL, (unsigned int)i);
+            } OSSL_TRACE_END(DECODER);
+            continue;
+        }
 
         /*
          * Checking the return value of BIO_reset() or BIO_seek() is unsafe.
@@ -623,6 +720,13 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
             goto end;
 
         /* Recurse */
+        OSSL_TRACE_BEGIN(DECODER) {
+            BIO_printf(trc_out,
+                       "(ctx %p) %s [%u] Running decoder instance %p\n",
+                       (void *)new_data.ctx, LEVEL, (unsigned int)i,
+                       (void *)new_decoder_inst);
+        } OSSL_TRACE_END(DECODER);
+
         new_data.current_decoder_inst_index = i;
         ok = new_decoder->decode(new_decoderctx, (OSSL_CORE_BIO *)bio,
                                  new_data.ctx->selection,
@@ -632,12 +736,19 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
 
         OSSL_TRACE_BEGIN(DECODER) {
             BIO_printf(trc_out,
-                       "(ctx %p) Running decoder instance %p => %d\n",
-                       (void *)new_data.ctx, (void *)new_decoder_inst, ok);
+                       "(ctx %p) %s [%u] Running decoder instance %p => %d\n",
+                       (void *)new_data.ctx, LEVEL, (unsigned int)i,
+                       (void *)new_decoder_inst, ok);
         } OSSL_TRACE_END(DECODER);
 
         if (ok)
             break;
+
+        /*
+         * These errors are assumed to come from ossl_store_handle_load_result()
+         * in crypto/store/store_result.c.  They are currently considered fatal
+         * errors, so we preserve them in the error queue and stop.
+         */
         err = ERR_peek_last_error();
         if ((ERR_GET_LIB(err) == ERR_LIB_EVP
              && ERR_GET_REASON(err) == EVP_R_UNSUPPORTED_PRIVATE_KEY_ALGORITHM)
@@ -647,7 +758,7 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
 #endif
             || (ERR_GET_LIB(err) == ERR_LIB_X509
                 && ERR_GET_REASON(err) == X509_R_UNSUPPORTED_ALGORITHM))
-            break; /* fatal error; preserve it on the error queue and stop */
+            goto end;
     }
 
  end:


More information about the openssl-commits mailing list