The branch openssl-3.0 has been updated
via f838730d15b6942d3fc401f73715c8789d05b385 (commit)
via de54b77ec35c47d4b8da40eeeecc84ab4ea17058 (commit)
via b4175902ffa8cd9c971f650f03c0f48eb418a0ad (commit)
from 0eade1589543f4ce9e3dd72449fe03851cc48b6c (commit)
- Log -----------------------------------------------------------------
commit f838730d15b6942d3fc401f73715c8789d05b385
Author: Richard Levitte <levi...@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 <to...@openssl.org>
Reviewed-by: Paul Dale <pa...@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/17104)
(cherry picked from commit 946bc0e3ec19ca019fcfa95f93c37f34e12fe0bd)
commit de54b77ec35c47d4b8da40eeeecc84ab4ea17058
Author: Richard Levitte <levi...@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 <to...@openssl.org>
Reviewed-by: Paul Dale <pa...@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/17104)
(cherry picked from commit 8585b5bc62d0bf394ca6adf24f8590e9b9b18402)
commit b4175902ffa8cd9c971f650f03c0f48eb418a0ad
Author: Richard Levitte <levi...@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 <to...@openssl.org>
Reviewed-by: Paul Dale <pa...@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/17104)
(cherry picked from commit b556713a6f2884eadc7f56428bc82a844e9a49e0)
-----------------------------------------------------------------------
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 50f48fdb7e..360f8933e1 100644
--- a/crypto/params_from_text.c
+++ b/crypto/params_from_text.c
@@ -57,8 +57,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
@@ -73,6 +79,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;
/*
@@ -80,9 +100,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;
@@ -132,7 +150,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(¶m, 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(¶m, &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)
