[openssl] master update
Richard Levitte
levitte at openssl.org
Tue Feb 9 10:16:03 UTC 2021
The branch master has been updated
via 604b86d8d360e36fc2fc0d1611d05bf38699d297 (commit)
from e60a748a13a244e8b13bacca18bad9bb3505aa90 (commit)
- Log -----------------------------------------------------------------
commit 604b86d8d360e36fc2fc0d1611d05bf38699d297
Author: Petr Gotthard <petr.gotthard at centrum.cz>
Date: Sat Feb 6 21:47:20 2021 +0100
Enhanced integer parsing in OSSL_PARAM_allocate_from_text
Fixes #14041 and additional bugs discovered by the newly created
tests.
This patch:
- Introduces support for 0x prefixed integers
- Fixes parsing of negative integers (negative numbers were
shifted by -2)
- Fixes ability to parse maximal unsigned numbers ("too small
buffer" error used to be reported incorrectly)
- Fixes a memory leak when OSSL_PARAM_allocate_from_text fails
leaving a temporary BN allocated
Reviewed-by: Matt Caswell <matt at openssl.org>
Reviewed-by: Paul Dale <pauli at openssl.org>
Reviewed-by: Richard Levitte <levitte at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14093)
-----------------------------------------------------------------------
Summary of changes:
crypto/params_from_text.c | 21 ++++++---
doc/man3/OSSL_PARAM_allocate_from_text.pod | 11 +++--
test/params_test.c | 73 ++++++++++++++++++++++++++++++
3 files changed, 94 insertions(+), 11 deletions(-)
diff --git a/crypto/params_from_text.c b/crypto/params_from_text.c
index ddc3c38aa4..b019744f9b 100644
--- a/crypto/params_from_text.c
+++ b/crypto/params_from_text.c
@@ -28,6 +28,7 @@ static int prepare_from_text(const OSSL_PARAM *paramdefs, const char *key,
size_t *buf_n, BIGNUM **tmpbn, int *found)
{
const OSSL_PARAM *p;
+ size_t buf_bits;
/*
* ishex is used to translate legacy style string controls in hex format
@@ -50,7 +51,7 @@ static int prepare_from_text(const OSSL_PARAM *paramdefs, const char *key,
if (*ishex)
BN_hex2bn(tmpbn, value);
else
- BN_dec2bn(tmpbn, value);
+ BN_asc2bn(tmpbn, value);
if (*tmpbn == NULL)
return 0;
@@ -62,20 +63,25 @@ static int prepare_from_text(const OSSL_PARAM *paramdefs, const char *key,
* buffer, i.e. if it's negative, we need to deal with it. We do
* it by subtracting 1 here and inverting the bytes in
* construct_from_text() below.
+ * To subtract 1 from an absolute value of a negative number we
+ * actually have to add 1: -3 - 1 = -4, |-3| = 3 + 1 = 4.
*/
if (p->data_type == OSSL_PARAM_INTEGER && BN_is_negative(*tmpbn)
- && !BN_sub_word(*tmpbn, 1)) {
+ && !BN_add_word(*tmpbn, 1)) {
return 0;
}
- *buf_n = BN_num_bytes(*tmpbn);
+ buf_bits = (size_t)BN_num_bits(*tmpbn);
+ *buf_n = (buf_bits + 7) / 8;
/*
* TODO(v3.0) is this the right way to do this? This code expects
* a zero data size to simply mean "arbitrary size".
*/
if (p->data_size > 0) {
- if (*buf_n >= p->data_size) {
+ if (buf_bits > p->data_size * 8
+ || (p->data_type == OSSL_PARAM_INTEGER
+ && 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;
@@ -184,11 +190,11 @@ int OSSL_PARAM_allocate_from_text(OSSL_PARAM *to,
if (!prepare_from_text(paramdefs, key, value, value_n,
¶mdef, &ishex, &buf_n, &tmpbn, found))
- return 0;
+ goto err;
if ((buf = OPENSSL_zalloc(buf_n > 0 ? buf_n : 1)) == NULL) {
ERR_raise(ERR_LIB_CRYPTO, ERR_R_MALLOC_FAILURE);
- return 0;
+ goto err;
}
ok = construct_from_text(to, paramdef, value, value_n, ishex,
@@ -197,4 +203,7 @@ int OSSL_PARAM_allocate_from_text(OSSL_PARAM *to,
if (!ok)
OPENSSL_free(buf);
return ok;
+ err:
+ BN_free(tmpbn);
+ return 0;
}
diff --git a/doc/man3/OSSL_PARAM_allocate_from_text.pod b/doc/man3/OSSL_PARAM_allocate_from_text.pod
index ef68f0e10c..80ba555a8f 100644
--- a/doc/man3/OSSL_PARAM_allocate_from_text.pod
+++ b/doc/man3/OSSL_PARAM_allocate_from_text.pod
@@ -55,15 +55,16 @@ depending on that item's I<data_type>, as follows:
=item B<OSSL_PARAM_INTEGER> and B<OSSL_PARAM_UNSIGNED_INTEGER>
-If I<key> started with "hex", I<value> is assumed to contain
-I<value_n> hexadecimal characters, which are decoded, and the
-resulting bytes become the number stored in the I<< to->data >>
-storage.
-
If I<key> didn't start with "hex", I<value> is assumed to contain
I<value_n> decimal characters, which are decoded, and the resulting
bytes become the number stored in the I<< to->data >> storage.
+If I<value> starts with "0x", it is assumed to contain I<value_n>
+hexadecimal characters.
+
+If I<key> started with "hex", I<value> is assumed to contain
+I<value_n> hexadecimal characters without the "0x" prefix.
+
If I<value> contains characters that couldn't be decoded as
hexadecimal or decimal characters, OSSL_PARAM_allocate_from_text()
considers that an error.
diff --git a/test/params_test.c b/test/params_test.c
index 8ee2e1594c..913df9eb8a 100644
--- a/test/params_test.c
+++ b/test/params_test.c
@@ -541,8 +541,81 @@ static int test_case(int i)
test_cases[i].prov));
}
+/*-
+ * OSSL_PARAM_allocate_from_text() tests
+ * =====================================
+ */
+
+static const OSSL_PARAM params_from_text[] = {
+ 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)),
+ OSSL_PARAM_END,
+};
+
+struct int_from_text_test_st {
+ const char *argname;
+ const char *strval;
+ long int intval;
+ int res;
+};
+
+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 */
+ /* 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 */
+};
+
+static int check_int_from_text(const struct int_from_text_test_st a)
+{
+ OSSL_PARAM param;
+ long int val = 0;
+ int res;
+
+ 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;
+ }
+
+ 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);
+ return 0;
+ }
+
+ return a.res;
+}
+
+static int test_allocate_from_text(int i)
+{
+ return check_int_from_text(int_from_text_test_cases[i]);
+}
+
int setup_tests(void)
{
ADD_ALL_TESTS(test_case, OSSL_NELEM(test_cases));
+ ADD_ALL_TESTS(test_allocate_from_text, OSSL_NELEM(int_from_text_test_cases));
return 1;
}
More information about the openssl-commits
mailing list