From: Richard Levitte <levitte@openssl.org>
Date: Fri, 2 Oct 2020 11:56:54 +0000 (+0200)
Subject: DECODER: Handle abstract object data type
X-Git-Tag: openssl-3.0.0-alpha7~73
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ecadfdadde491572b0bdf3c5a95e7a6a004585c6;p=thirdparty%2Fopenssl.git

DECODER: Handle abstract object data type

The PEM->DER decoder passes the data type of its contents, something
that decoder_process() ignored.

On the other hand, the PEM->DER decoder passed nonsense.

Both issues are fixed here.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/13060)
---

diff --git a/crypto/encode_decode/decoder_lib.c b/crypto/encode_decode/decoder_lib.c
index 0411da41f4a..ab7c5370382 100644
--- a/crypto/encode_decode/decoder_lib.c
+++ b/crypto/encode_decode/decoder_lib.c
@@ -430,6 +430,7 @@ 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;
 
     memset(&new_data, 0, sizeof(new_data));
     new_data.ctx = data->ctx;
@@ -471,6 +472,11 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
         if (new_data.bio == NULL)
             goto end;
         bio = new_data.bio;
+
+        /* Get the object 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))
+            goto end;
     }
 
     /*
@@ -513,6 +519,13 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
         if (decoder != NULL && !OSSL_DECODER_is_a(decoder, new_input_type))
             continue;
 
+        /*
+         * If the previous decoder gave us an object 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))
+            continue;
+
         /*
          * Checking the return value of BIO_reset() or BIO_seek() is unsafe.
          * Furthermore, BIO_reset() is unsafe to use if the source BIO happens
diff --git a/providers/implementations/encode_decode/decode_pem2der.c b/providers/implementations/encode_decode/decode_pem2der.c
index 99ca7e8210b..9ddc0ae3bbc 100644
--- a/providers/implementations/encode_decode/decode_pem2der.c
+++ b/providers/implementations/encode_decode/decode_pem2der.c
@@ -21,6 +21,7 @@
 #include <openssl/err.h>
 #include <openssl/params.h>
 #include <openssl/pem.h>
+#include "internal/nelem.h"
 #include "prov/bio.h"
 #include "prov/implementations.h"
 #include "prov/providercommonerr.h"
@@ -109,8 +110,27 @@ static int pem2der_decode(void *vctx, OSSL_CORE_BIO *cin,
                           OSSL_CALLBACK *data_cb, void *data_cbarg,
                           OSSL_PASSPHRASE_CALLBACK *pw_cb, void *pw_cbarg)
 {
+    /* Strings to peal off the pem name */
+    static const char *pealable_pem_name_endings[] = {
+        /*
+         * These entries should be in longest to shortest order to avoid
+         * mixups.
+         */
+        "ENCRYPTED PRIVATE KEY",
+        "PRIVATE KEY",
+        "PUBLIC KEY",
+        "PARAMETERS"
+
+        /*
+         * Libcrypto currently only supports decoding keys with provider side
+         * decoders, so we don't try to peal any other PEM name.  That's an
+         * exercise for when libcrypto starts to treat other types of objects
+         * via providers.
+         */
+    };
     struct pem2der_ctx_st *ctx = vctx;
     char *pem_name = NULL, *pem_header = NULL;
+    size_t pem_name_len, i;
     unsigned char *der = NULL;
     long der_len = 0;
     int ok = 0;
@@ -137,16 +157,46 @@ static int pem2der_decode(void *vctx, OSSL_CORE_BIO *cin,
             goto end;
     }
 
+    /*
+     * Peal off certain strings from the end of |pem_name|, as they serve
+     * no further purpose.
+     */
+    for (i = 0, pem_name_len = strlen(pem_name);
+         i < OSSL_NELEM(pealable_pem_name_endings);
+         i++) {
+        size_t peal_len = strlen(pealable_pem_name_endings[i]);
+        size_t pem_name_offset;
+
+        if (peal_len <= pem_name_len) {
+            pem_name_offset = pem_name_len - peal_len;
+            if (strcmp(pem_name + pem_name_offset,
+                       pealable_pem_name_endings[i]) == 0) {
+
+                do {
+                    pem_name[pem_name_offset] = '\0';
+                } while (pem_name_offset > 0
+                         && pem_name[--pem_name_offset] == ' ');
+
+                if (pem_name[0] == '\0') {
+                    OPENSSL_free(pem_name);
+                    pem_name = NULL;
+                }
+                break;
+            }
+        }
+    }
+
     {
-        OSSL_PARAM params[3];
+        OSSL_PARAM params[3], *p = params;
 
-        params[0] =
-            OSSL_PARAM_construct_utf8_string(OSSL_OBJECT_PARAM_DATA_TYPE,
-                                             pem_name, 0);
-        params[1] =
+        if (pem_name != NULL)
+            *p++ =
+                OSSL_PARAM_construct_utf8_string(OSSL_OBJECT_PARAM_DATA_TYPE,
+                                                 pem_name, 0);
+        *p++ =
             OSSL_PARAM_construct_octet_string(OSSL_OBJECT_PARAM_DATA,
                                               der, der_len);
-        params[2] = OSSL_PARAM_construct_end();
+        *p = OSSL_PARAM_construct_end();
 
         ok = data_cb(params, data_cbarg);
     }