[openssl] master update

Richard Levitte levitte at openssl.org
Wed Nov 24 18:20:20 UTC 2021


The branch master has been updated
       via  946bc0e3ec19ca019fcfa95f93c37f34e12fe0bd (commit)
       via  8585b5bc62d0bf394ca6adf24f8590e9b9b18402 (commit)
       via  b556713a6f2884eadc7f56428bc82a844e9a49e0 (commit)
      from  8c86529fe1b9ade0794c6f557ca8936f0c0de431 (commit)


- Log -----------------------------------------------------------------
commit 946bc0e3ec19ca019fcfa95f93c37f34e12fe0bd
Author: Richard Levitte <levitte at openssl.org>
Date:   Mon Nov 22 17:10:10 2021 +0100

    Allow sign extension in OSSL_PARAM_allocate_from_text()
    
    This is done for the data type OSSL_PARAM_INTEGER by checking if the
    most significant bit is set, and adding 8 to the number of buffer bits
    if that is the case.  Everything else is already in place.
    
    Fixes #17103
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    Reviewed-by: Paul Dale <pauli at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/17104)

commit 8585b5bc62d0bf394ca6adf24f8590e9b9b18402
Author: Richard Levitte <levitte at openssl.org>
Date:   Mon Nov 22 17:08:19 2021 +0100

    Have OSSL_PARAM_allocate_from_text() raise error on unexpected neg number
    
    When the parameter definition has the data type OSSL_PARAM_UNSIGNED_INTEGER,
    negative input values should not be accepted.
    
    Fixes #17103
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    Reviewed-by: Paul Dale <pauli at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/17104)

commit b556713a6f2884eadc7f56428bc82a844e9a49e0
Author: Richard Levitte <levitte at openssl.org>
Date:   Mon Nov 22 16:38:43 2021 +0100

    Test the performance of OSSL_PARAM_allocate_from_text with arbitrary size ints
    
    With arbitrary size ints, we get to know exactly how large the minimum
    buffer must be.
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    Reviewed-by: Paul Dale <pauli at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/17104)

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

Summary of changes:
 crypto/cpt_err.c            |  2 +
 crypto/err/openssl.txt      |  1 +
 crypto/params_from_text.c   | 28 ++++++++++---
 include/openssl/cryptoerr.h |  1 +
 test/params_test.c          | 99 ++++++++++++++++++++++++++++++++-------------
 5 files changed, 98 insertions(+), 33 deletions(-)

diff --git a/crypto/cpt_err.c b/crypto/cpt_err.c
index 79c1a90595..8574f31a81 100644
--- a/crypto/cpt_err.c
+++ b/crypto/cpt_err.c
@@ -29,6 +29,8 @@ static const ERR_STRING_DATA CRYPTO_str_reasons[] = {
     "insufficient param size"},
     {ERR_PACK(ERR_LIB_CRYPTO, 0, CRYPTO_R_INSUFFICIENT_SECURE_DATA_SPACE),
     "insufficient secure data space"},
+    {ERR_PACK(ERR_LIB_CRYPTO, 0, CRYPTO_R_INVALID_NEGATIVE_VALUE),
+    "invalid negative value"},
     {ERR_PACK(ERR_LIB_CRYPTO, 0, CRYPTO_R_INVALID_NULL_ARGUMENT),
     "invalid null argument"},
     {ERR_PACK(ERR_LIB_CRYPTO, 0, CRYPTO_R_INVALID_OSSL_PARAM_TYPE),
diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
index b47293a27a..777a0de19d 100644
--- a/crypto/err/openssl.txt
+++ b/crypto/err/openssl.txt
@@ -434,6 +434,7 @@ CRYPTO_R_ILLEGAL_HEX_DIGIT:102:illegal hex digit
 CRYPTO_R_INSUFFICIENT_DATA_SPACE:106:insufficient data space
 CRYPTO_R_INSUFFICIENT_PARAM_SIZE:107:insufficient param size
 CRYPTO_R_INSUFFICIENT_SECURE_DATA_SPACE:108:insufficient secure data space
+CRYPTO_R_INVALID_NEGATIVE_VALUE:122:invalid negative value
 CRYPTO_R_INVALID_NULL_ARGUMENT:109:invalid null argument
 CRYPTO_R_INVALID_OSSL_PARAM_TYPE:110:invalid ossl param type
 CRYPTO_R_ODD_NUMBER_OF_DIGITS:103:odd number of digits
diff --git a/crypto/params_from_text.c b/crypto/params_from_text.c
index 84851edc47..b681d8f564 100644
--- a/crypto/params_from_text.c
+++ b/crypto/params_from_text.c
@@ -54,8 +54,14 @@ static int prepare_from_text(const OSSL_PARAM *paramdefs, const char *key,
         if (r == 0 || *tmpbn == NULL)
             return 0;
 
+        if (p->data_type == OSSL_PARAM_UNSIGNED_INTEGER
+            && BN_is_negative(*tmpbn)) {
+            ERR_raise(ERR_LIB_CRYPTO, CRYPTO_R_INVALID_NEGATIVE_VALUE);
+            return 0;
+        }
+
         /*
-         * 2s complement negate, part 1
+         * 2's complement negate, part 1
          *
          * BN_bn2nativepad puts the absolute value of the number in the
          * buffer, i.e. if it's negative, we need to deal with it.  We do
@@ -70,6 +76,20 @@ static int prepare_from_text(const OSSL_PARAM *paramdefs, const char *key,
         }
 
         buf_bits = (size_t)BN_num_bits(*tmpbn);
+
+        /*
+         * Compensate for cases where the most significant bit in
+         * the resulting OSSL_PARAM buffer will be set after the
+         * BN_bn2nativepad() call, as the implied sign may not be
+         * correct after the second part of the 2's complement
+         * negation has been performed.
+         * We fix these cases by extending the buffer by one byte
+         * (8 bits), which will give some padding.  The second part
+         * of the 2's complement negation will do the rest.
+         */
+        if (p->data_type == OSSL_PARAM_INTEGER && buf_bits % 8 == 0)
+            buf_bits += 8;
+
         *buf_n = (buf_bits + 7) / 8;
 
         /*
@@ -77,9 +97,7 @@ static int prepare_from_text(const OSSL_PARAM *paramdefs, const char *key,
          * range checking if a size is specified.
          */
         if (p->data_size > 0) {
-            if (buf_bits > p->data_size * 8
-                || (p->data_type == OSSL_PARAM_INTEGER
-                    && buf_bits == p->data_size * 8)) {
+            if (buf_bits > p->data_size * 8) {
                 ERR_raise(ERR_LIB_CRYPTO, CRYPTO_R_TOO_SMALL_BUFFER);
                 /* Since this is a different error, we don't break */
                 return 0;
@@ -129,7 +147,7 @@ static int construct_from_text(OSSL_PARAM *to, const OSSL_PARAM *paramdef,
             BN_bn2nativepad(tmpbn, buf, buf_n);
 
             /*
-             * 2s complement negate, part two.
+             * 2's complement negation, part two.
              *
              * Because we did the first part on the BIGNUM itself, we can just
              * invert all the bytes here and be done with it.
diff --git a/include/openssl/cryptoerr.h b/include/openssl/cryptoerr.h
index 6799668089..c6a04d9b97 100644
--- a/include/openssl/cryptoerr.h
+++ b/include/openssl/cryptoerr.h
@@ -28,6 +28,7 @@
 # define CRYPTO_R_INSUFFICIENT_DATA_SPACE                 106
 # define CRYPTO_R_INSUFFICIENT_PARAM_SIZE                 107
 # define CRYPTO_R_INSUFFICIENT_SECURE_DATA_SPACE          108
+# define CRYPTO_R_INVALID_NEGATIVE_VALUE                  122
 # define CRYPTO_R_INVALID_NULL_ARGUMENT                   109
 # define CRYPTO_R_INVALID_OSSL_PARAM_TYPE                 110
 # define CRYPTO_R_ODD_NUMBER_OF_DIGITS                    103
diff --git a/test/params_test.c b/test/params_test.c
index 13cfb9d19e..6a970feaa4 100644
--- a/test/params_test.c
+++ b/test/params_test.c
@@ -551,40 +551,64 @@ static int test_case(int i)
  */
 
 static const OSSL_PARAM params_from_text[] = {
+    /* Fixed size buffer */
     OSSL_PARAM_int32("int", NULL),
     OSSL_PARAM_DEFN("short", OSSL_PARAM_INTEGER, NULL, sizeof(int16_t)),
     OSSL_PARAM_DEFN("ushort", OSSL_PARAM_UNSIGNED_INTEGER, NULL, sizeof(uint16_t)),
+    /* Arbitrary size buffer.  Make sure the result fits in a long */
+    OSSL_PARAM_DEFN("num", OSSL_PARAM_INTEGER, NULL, 0),
+    OSSL_PARAM_DEFN("unum", OSSL_PARAM_UNSIGNED_INTEGER, NULL, 0),
     OSSL_PARAM_END,
 };
 
 struct int_from_text_test_st {
     const char *argname;
     const char *strval;
-    long int intval;
-    int res;
+    long int expected_intval;
+    int expected_res;
+    size_t expected_bufsize;
 };
 
 static struct int_from_text_test_st int_from_text_test_cases[] = {
-    { "int",               "",          0, 0 },
-    { "int",              "0",          0, 1 },
-    { "int",            "101",        101, 1 },
-    { "int",           "-102",       -102, 1 },
-    { "int",            "12A",         12, 1 }, /* incomplete */
-    { "int",          "0x12B",      0x12B, 1 },
-    { "hexint",         "12C",      0x12C, 1 },
-    { "hexint",       "0x12D",          0, 1 }, /* zero */
+    { "int",               "",          0, 0, 0 },
+    { "int",              "0",          0, 1, 4 },
+    { "int",            "101",        101, 1, 4 },
+    { "int",           "-102",       -102, 1, 4 },
+    { "int",            "12A",         12, 1, 4 }, /* incomplete */
+    { "int",          "0x12B",      0x12B, 1, 4 },
+    { "hexint",         "12C",      0x12C, 1, 4 },
+    { "hexint",       "0x12D",          0, 1, 4 }, /* zero */
     /* test check of the target buffer size */
-    { "int",     "0x7fffffff",  INT32_MAX, 1 },
-    { "int",     "2147483647",  INT32_MAX, 1 },
-    { "int",     "2147483648",          0, 0 }, /* too small buffer */
-    { "int",    "-2147483648",  INT32_MIN, 1 },
-    { "int",    "-2147483649",          0, 0 }, /* too small buffer */
-    { "short",       "0x7fff",  INT16_MAX, 1 },
-    { "short",        "32767",  INT16_MAX, 1 },
-    { "short",        "32768",          0, 0 }, /* too small buffer */
-    { "ushort",      "0xffff", UINT16_MAX, 1 },
-    { "ushort",       "65535", UINT16_MAX, 1 },
-    { "ushort",       "65536",          0, 0 }, /* too small buffer */
+    { "int",     "0x7fffffff",  INT32_MAX, 1, 4 },
+    { "int",     "2147483647",  INT32_MAX, 1, 4 },
+    { "int",     "2147483648",          0, 0, 0 }, /* too small buffer */
+    { "int",    "-2147483648",  INT32_MIN, 1, 4 },
+    { "int",    "-2147483649",          0, 0, 4 }, /* too small buffer */
+    { "short",       "0x7fff",  INT16_MAX, 1, 2 },
+    { "short",        "32767",  INT16_MAX, 1, 2 },
+    { "short",        "32768",          0, 0, 0 }, /* too small buffer */
+    { "ushort",      "0xffff", UINT16_MAX, 1, 2 },
+    { "ushort",       "65535", UINT16_MAX, 1, 2 },
+    { "ushort",       "65536",          0, 0, 0 }, /* too small buffer */
+    /* test check of sign extension in arbitrary size results */
+    { "num",              "0",          0, 1, 1 },
+    { "num",              "0",          0, 1, 1 },
+    { "num",           "0xff",       0xff, 1, 2 }, /* sign extension */
+    { "num",          "-0xff",      -0xff, 1, 2 }, /* sign extension */
+    { "num",           "0x7f",       0x7f, 1, 1 }, /* no sign extension */
+    { "num",          "-0x7f",      -0x7f, 1, 1 }, /* no sign extension */
+    { "num",           "0x80",       0x80, 1, 2 }, /* sign extension */
+    { "num",          "-0x80",      -0x80, 1, 1 }, /* no sign extension */
+    { "num",           "0x81",       0x81, 1, 2 }, /* sign extension */
+    { "num",          "-0x81",      -0x81, 1, 2 }, /* sign extension */
+    { "unum",          "0xff",       0xff, 1, 1 },
+    { "unum",         "-0xff",      -0xff, 0, 0 }, /* invalid neg number */
+    { "unum",          "0x7f",       0x7f, 1, 1 },
+    { "unum",         "-0x7f",      -0x7f, 0, 0 }, /* invalid neg number */
+    { "unum",          "0x80",       0x80, 1, 1 },
+    { "unum",         "-0x80",      -0x80, 0, 0 }, /* invalid neg number */
+    { "unum",          "0x81",       0x81, 1, 1 },
+    { "unum",         "-0x81",      -0x81, 0, 0 }, /* invalid neg number */
 };
 
 static int check_int_from_text(const struct int_from_text_test_st a)
@@ -595,21 +619,40 @@ static int check_int_from_text(const struct int_from_text_test_st a)
 
     if (!OSSL_PARAM_allocate_from_text(&param, params_from_text,
                                        a.argname, a.strval, 0, NULL)) {
-        if (a.res)
-            TEST_error("errant %s param \"%s\"", a.argname, a.strval);
-        return !a.res;
+        if (a.expected_res)
+            TEST_error("unexpected OSSL_PARAM_allocate_from_text() return for %s \"%s\"",
+                       a.argname, a.strval);
+        return !a.expected_res;
     }
 
+    /* For data size zero, OSSL_PARAM_get_long() may crash */
+    if (param.data_size == 0) {
+        OPENSSL_free(param.data);
+        TEST_error("unexpected zero size for %s \"%s\"",
+                   a.argname, a.strval);
+        return 0;
+    }
     res = OSSL_PARAM_get_long(&param, &val);
     OPENSSL_free(param.data);
 
-    if (res ^ a.res || val != a.intval) {
-        TEST_error("errant %s \"%s\" %li != %li",
-                   a.argname, a.strval, a.intval, val);
+    if (res ^ a.expected_res) {
+        TEST_error("unexpected OSSL_PARAM_get_long() return for %s \"%s\": "
+                   "%d != %d", a.argname, a.strval, a.expected_res, res);
+        return 0;
+    }
+    if (val != a.expected_intval) {
+        TEST_error("unexpected result for %s \"%s\":  %li != %li",
+                   a.argname, a.strval, a.expected_intval, val);
+        return 0;
+    }
+    if (param.data_size != a.expected_bufsize) {
+        TEST_error("unexpected size for %s \"%s\":  %d != %d",
+                   a.argname, a.strval,
+                   (int)a.expected_bufsize, (int)param.data_size);
         return 0;
     }
 
-    return a.res;
+    return a.expected_res;
 }
 
 static int test_allocate_from_text(int i)


More information about the openssl-commits mailing list