From f3338b17e4a6d2a86fe0bd79d6332c7929685e3d Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Tue, 28 Nov 2023 13:54:37 -0500 Subject: [PATCH 01/24] Add overflow checks to parse_number/parse_hex/parse_oct Test the next arithmetic operation to safely determine if adding the next digit in the passed property string will overflow Also, noted a bug in the parse_hex code. When parsing non-digit characters (i.e. a-f and A-F), we do a tolower conversion (which is fine), and then subtract 'a' to get the hex value from the ascii (which is definately wrong). We should subtract 'W' to convert tolower converted hex digits in the range a-f to their hex value counterparts Add tests to test_property_parse_error to ensure overflow checks work Reviewed-by: Richard Levitte Reviewed-by: Tomas Mraz Reviewed-by: Tom Cosgrove (Merged from https://github.com/openssl/openssl/pull/22874) (cherry picked from commit 986c48c4eb26861f25bc68ea252d8f2aad592735) Signed-off-by: fly2x --- crypto/property/property_parse.c | 50 +++++++++++++++++++++++++------- test/property_test.c | 7 +++++ 2 files changed, 47 insertions(+), 10 deletions(-) diff --git a/crypto/property/property_parse.c b/crypto/property/property_parse.c index 8ba42a0dcb..f94d0d3d41 100644 --- a/crypto/property/property_parse.c +++ b/crypto/property/property_parse.c @@ -97,9 +97,18 @@ static int parse_number(const char *t[], OSSL_PROPERTY_DEFINITION *res) const char *s = *t; int64_t v = 0; - if (!ossl_isdigit(*s)) - return 0; do { + if (!ossl_isdigit(*s)) { + ERR_raise_data(ERR_LIB_PROP, PROP_R_NOT_A_DECIMAL_DIGIT, + "HERE-->%s", *t); + return 0; + } + /* overflow check */ + if (v > ((INT64_MAX - (*s - '0')) / 10)) { + ERR_raise_data(ERR_LIB_PROP, PROP_R_PARSE_FAILED, + "Property %s overflows", *t); + return 0; + } v = v * 10 + (*s++ - '0'); } while (ossl_isdigit(*s)); if (!ossl_isspace(*s) && *s != '\0' && *s != ',') { @@ -117,15 +126,27 @@ static int parse_hex(const char *t[], OSSL_PROPERTY_DEFINITION *res) { const char *s = *t; int64_t v = 0; + int sval; - if (!ossl_isxdigit(*s)) - return 0; do { + if (ossl_isdigit(*s)) { + sval = *s - '0'; + } else if (ossl_isxdigit(*s)) { + sval = ossl_tolower(*s) - 'a' + 10; + } else { + ERR_raise_data(ERR_LIB_PROP, PROP_R_NOT_AN_HEXADECIMAL_DIGIT, + "%s", *t); + return 0; + } + + if (v > ((INT64_MAX - sval) / 16)) { + ERR_raise_data(ERR_LIB_PROP, PROP_R_PARSE_FAILED, + "Property %s overflows", *t); + return 0; + } + v <<= 4; - if (ossl_isdigit(*s)) - v += *s - '0'; - else - v += ossl_tolower(*s) - 'a'; + v += sval; } while (ossl_isxdigit(*++s)); if (!ossl_isspace(*s) && *s != '\0' && *s != ',') { ERR_raise_data(ERR_LIB_PROP, PROP_R_NOT_AN_HEXADECIMAL_DIGIT, @@ -143,9 +164,18 @@ static int parse_oct(const char *t[], OSSL_PROPERTY_DEFINITION *res) const char *s = *t; int64_t v = 0; - if (*s == '9' || *s == '8' || !ossl_isdigit(*s)) - return 0; do { + if (*s == '9' || *s == '8' || !ossl_isdigit(*s)) { + ERR_raise_data(ERR_LIB_PROP, PROP_R_NOT_AN_OCTAL_DIGIT, + "HERE-->%s", *t); + return 0; + } + if (v > ((INT64_MAX - (*s - '0')) / 8)) { + ERR_raise_data(ERR_LIB_PROP, PROP_R_PARSE_FAILED, + "Property %s overflows", *t); + return 0; + } + v = (v << 3) + (*s - '0'); } while (ossl_isdigit(*++s) && *s != '9' && *s != '8'); if (!ossl_isspace(*s) && *s != '\0' && *s != ',') { diff --git a/test/property_test.c b/test/property_test.c index bba96fac0a..18f8cc8740 100644 --- a/test/property_test.c +++ b/test/property_test.c @@ -136,6 +136,10 @@ static const struct { { "n=0x3", "n=3", 1 }, { "n=0x3", "n=-3", -1 }, { "n=0x33", "n=51", 1 }, + { "n=0x123456789abcdef", "n=0x123456789abcdef", 1 }, + { "n=0x7fffffffffffffff", "n=0x7fffffffffffffff", 1 }, /* INT64_MAX */ + { "n=9223372036854775807", "n=9223372036854775807", 1 }, /* INT64_MAX */ + { "n=0777777777777777777777", "n=0777777777777777777777", 1 }, /* INT64_MAX */ { "n=033", "n=27", 1 }, { "n=0", "n=00", 1 }, { "n=0x0", "n=0", 1 }, @@ -198,6 +202,9 @@ static const struct { { 1, "a=2, n=012345678" }, /* Bad octal digit */ { 0, "n=0x28FG, a=3" }, /* Bad hex digit */ { 0, "n=145d, a=2" }, /* Bad decimal digit */ + { 0, "n=0x8000000000000000, a=3" }, /* Hex overflow */ + { 0, "n=922337203000000000d, a=2" }, /* Decimal overflow */ + { 0, "a=2, n=1000000000000000000000" }, /* Octal overflow */ { 1, "@='hello'" }, /* Invalid name */ { 1, "n0123456789012345678901234567890123456789" "0123456789012345678901234567890123456789" -- Gitee From 156b523784b0e74fa72ec9ed2358671c2809f0f9 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Tue, 28 Nov 2023 15:55:43 +0100 Subject: [PATCH 02/24] Modify 'out-of-source-and-install' to work with a read-only source tree Fixes #22907 Reviewed-by: Tomas Mraz Reviewed-by: Neil Horman (Merged from https://github.com/openssl/openssl/pull/22934) Signed-off-by: fly2x --- .github/workflows/ci.yml | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 830e2d75f7..00be656245 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -251,19 +251,30 @@ jobs: - name: make test run: make test HARNESS_JOBS=${HARNESS_JOBS:-4} - out-of-source-and-install: + # out-of-source-and-install checks multiple things at the same time: + # - That building, testing and installing works from an out-of-source + # build tree + # - That building, testing and installing works with a read-only source + # tree + out-of-readonly-source-and-install: strategy: matrix: os: [ubuntu-latest, macos-latest ] runs-on: ${{matrix.os}} steps: - uses: actions/checkout@v4 - - name: extra preparations + with: + path: ./source + - name: make source read-only + run: chmod -R a-w ./source + - name: create build and install directories run: | mkdir ./build mkdir ./install - name: config - run: ../config --banner=Configured enable-fips enable-acvp-tests --strict-warnings --prefix=$(cd ../install; pwd) && perl configdata.pm --dump + run: | + ../source/config --banner=Configured enable-fips enable-acvp-tests --strict-warnings --prefix=$(cd ../install; pwd) + perl configdata.pm --dump working-directory: ./build - name: make run: make -s -j4 -- Gitee From 139fcf200ea27bb4f2caac5558813009e0489a95 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Tue, 28 Nov 2023 23:41:32 +0100 Subject: [PATCH 03/24] Configure: Refuse to make directories in the source tree Fixes #22907 Reviewed-by: Tomas Mraz Reviewed-by: Neil Horman (Merged from https://github.com/openssl/openssl/pull/22934) Signed-off-by: fly2x --- Configure | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/Configure b/Configure index d058451d8a..fc044854dd 100755 --- a/Configure +++ b/Configure @@ -1822,11 +1822,12 @@ if ($builder eq "unified") { my $base = shift; my $dir = shift; my $relativeto = shift || "."; + my $no_mkpath = shift // 0; $dir = catdir($base,$dir) unless isabsolute($dir); # Make sure the directories we're building in exists - mkpath($dir); + mkpath($dir) unless $no_mkpath; my $res = abs2rel(absolutedir($dir), rel2abs($relativeto)); #print STDERR "DEBUG[cleandir]: $dir , $base => $res\n"; @@ -1837,6 +1838,7 @@ if ($builder eq "unified") { my $base = shift; my $file = shift; my $relativeto = shift || "."; + my $no_mkpath = shift // 0; $file = catfile($base,$file) unless isabsolute($file); @@ -1844,7 +1846,7 @@ if ($builder eq "unified") { my $f = basename($file); # Make sure the directories we're building in exists - mkpath($d); + mkpath($d) unless $no_mkpath; my $res = abs2rel(catfile(absolutedir($d), $f), rel2abs($relativeto)); #print STDERR "DEBUG[cleanfile]: $d , $f => $res\n"; @@ -1874,7 +1876,7 @@ if ($builder eq "unified") { } # Then, look in our standard directory push @build_file_templates, - ( map { cleanfile($srcdir, catfile("Configurations", $_), $blddir) } + ( map { cleanfile($srcdir, catfile("Configurations", $_), $blddir, 1) } @build_file_template_names ); my $build_file_template; @@ -1889,7 +1891,7 @@ if ($builder eq "unified") { } $config{build_file_templates} = [ cleanfile($srcdir, catfile("Configurations", "common0.tmpl"), - $blddir), + $blddir, 1), $build_file_template ]; my @build_dirs = ( [ ] ); # current directory @@ -1898,7 +1900,7 @@ if ($builder eq "unified") { # We want to detect configdata.pm in the source tree, so we # don't use it if the build tree is different. - my $src_configdata = cleanfile($srcdir, "configdata.pm", $blddir); + my $src_configdata = cleanfile($srcdir, "configdata.pm", $blddir, 1); # Any source file that we recognise is placed in this hash table, with # the list of its intended destinations as value. When everything has @@ -2251,7 +2253,7 @@ EOF my $dest = $_; my $ddest = cleanfile($buildd, $_, $blddir); foreach (@{$sources{$dest}}) { - my $s = cleanfile($sourced, $_, $blddir); + my $s = cleanfile($sourced, $_, $blddir, 1); # If it's generated or we simply don't find it in the source # tree, we assume it's in the build tree. @@ -2296,7 +2298,7 @@ EOF my $dest = $_; my $ddest = cleanfile($buildd, $_, $blddir); foreach (@{$shared_sources{$dest}}) { - my $s = cleanfile($sourced, $_, $blddir); + my $s = cleanfile($sourced, $_, $blddir, 1); # If it's generated or we simply don't find it in the source # tree, we assume it's in the build tree. @@ -2351,7 +2353,7 @@ EOF if scalar @{$generate{$_}} > 1; my @generator = split /\s+/, $generate{$dest}->[0]; my $gen = $generator[0]; - $generator[0] = cleanfile($sourced, $gen, $blddir); + $generator[0] = cleanfile($sourced, $gen, $blddir, 1); # If the generator is itself generated, it's in the build tree if ($generate{$gen} || ! -f $generator[0]) { @@ -2377,7 +2379,7 @@ EOF } elsif ($dest eq '') { $ddest = ''; } else { - $ddest = cleanfile($sourced, $_, $blddir); + $ddest = cleanfile($sourced, $_, $blddir, 1); # If the destination doesn't exist in source, it can only be # a generated file in the build tree. @@ -2386,7 +2388,7 @@ EOF } } foreach (@{$depends{$dest}}) { - my $d = cleanfile($sourced, $_, $blddir); + my $d = cleanfile($sourced, $_, $blddir, 1); my $d2 = cleanfile($buildd, $_, $blddir); # If we know it's generated, or assume it is because we can't @@ -2409,7 +2411,7 @@ EOF foreach (keys %includes) { my $dest = $_; - my $ddest = cleanfile($sourced, $_, $blddir); + my $ddest = cleanfile($sourced, $_, $blddir, 1); # If the destination doesn't exist in source, it can only be # a generated file in the build tree. @@ -2417,7 +2419,7 @@ EOF $ddest = cleanfile($buildd, $_, $blddir); } foreach (@{$includes{$dest}}) { - my $is = cleandir($sourced, $_, $blddir); + my $is = cleandir($sourced, $_, $blddir, 1); my $ib = cleandir($buildd, $_, $blddir); push @{$unified_info{includes}->{$ddest}->{source}}, $is unless grep { $_ eq $is } @{$unified_info{includes}->{$ddest}->{source}}; @@ -2430,7 +2432,7 @@ EOF my $ddest; if ($dest ne "") { - $ddest = cleanfile($sourced, $dest, $blddir); + $ddest = cleanfile($sourced, $dest, $blddir, 1); # If the destination doesn't exist in source, it can only # be a generated file in the build tree. @@ -2812,7 +2814,7 @@ my %template_vars = ( my $configdata_outname = 'configdata.pm'; open CONFIGDATA, ">$configdata_outname.new" or die "Trying to create $configdata_outname.new: $!"; -my $configdata_tmplname = cleanfile($srcdir, "configdata.pm.in", $blddir); +my $configdata_tmplname = cleanfile($srcdir, "configdata.pm.in", $blddir, 1); my $configdata_tmpl = OpenSSL::Template->new(TYPE => 'FILE', SOURCE => $configdata_tmplname); $configdata_tmpl->fill_in( -- Gitee From f1cd0f850545ef42b61dc967cb7f4aeb33988426 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Tue, 5 Dec 2023 09:21:35 +0100 Subject: [PATCH 04/24] Make sure that the test / tests build target run 'run_tests' last Fixes #22943 Reviewed-by: Tomas Mraz Reviewed-by: Neil Horman (Merged from https://github.com/openssl/openssl/pull/22948) Signed-off-by: fly2x --- Configurations/descrip.mms.tmpl | 3 ++- Configurations/unix-Makefile.tmpl | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Configurations/descrip.mms.tmpl b/Configurations/descrip.mms.tmpl index c69a1cc1f8..d0b8d42628 100644 --- a/Configurations/descrip.mms.tmpl +++ b/Configurations/descrip.mms.tmpl @@ -478,7 +478,8 @@ build_all_generated : $(GENERATED_MANDATORY) $(GENERATED) build_docs all : build_sw build_docs test : tests -{- dependmagic('tests'); -} : build_programs_nodep, build_modules_nodep run_tests +{- dependmagic('tests'); -} : build_programs_nodep, build_modules_nodep + $(MMS) $(MMSQUALIFIERS) run_tests run_tests : @ ! {- output_off() if $disabled{tests}; "" -} DEFINE SRCTOP "$(SRCDIR)" diff --git a/Configurations/unix-Makefile.tmpl b/Configurations/unix-Makefile.tmpl index a48fae5fb8..3754595d38 100644 --- a/Configurations/unix-Makefile.tmpl +++ b/Configurations/unix-Makefile.tmpl @@ -526,8 +526,9 @@ build_all_generated: $(GENERATED_MANDATORY) $(GENERATED) build_docs all: build_sw build_docs test: tests -{- dependmagic('tests'); -}: build_programs_nodep build_modules_nodep link-utils run_tests -run_tests: +{- dependmagic('tests'); -}: build_programs_nodep build_modules_nodep link-utils + $(MAKE) run_tests +run_tests: FORCE @ : {- output_off() if $disabled{tests}; "" -} ( SRCTOP=$(SRCDIR) \ BLDTOP=$(BLDDIR) \ -- Gitee From 2927e2eb48a39c2f5a88ca3bcbd5afd84cc0acae Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Tue, 5 Dec 2023 09:26:36 +0100 Subject: [PATCH 05/24] Add the 'run_tests' target to the Windows build file template as well For some reason, it was added to the Unix and VMS build templates, but Windows was forgotten. Reviewed-by: Tomas Mraz Reviewed-by: Neil Horman (Merged from https://github.com/openssl/openssl/pull/22948) Signed-off-by: fly2x --- Configurations/windows-makefile.tmpl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Configurations/windows-makefile.tmpl b/Configurations/windows-makefile.tmpl index ae6daa2256..504e4bc803 100644 --- a/Configurations/windows-makefile.tmpl +++ b/Configurations/windows-makefile.tmpl @@ -440,6 +440,8 @@ all: build_sw build_docs test: tests {- dependmagic('tests'); -}: build_programs_nodep build_modules_nodep copy-utils + $(MAKE) /$(MAKEFLAGS) run_tests +run_tests: @{- output_off() if $disabled{tests}; "\@rem" -} cmd /C "set "SRCTOP=$(SRCDIR)" & set "BLDTOP=$(BLDDIR)" & set "PERL=$(PERL)" & set "FIPSKEY=$(FIPSKEY)" & "$(PERL)" "$(SRCDIR)\test\run_tests.pl" $(TESTS)" @{- if ($disabled{tests}) { output_on(); } else { output_off(); } "" -} -- Gitee From adc7a2cc8b78de0ac80f8590afcd4f63d0181515 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Thu, 30 Nov 2023 08:48:33 +0100 Subject: [PATCH 06/24] test/recipes/01-test_symbol_presence.t: Ignore symbols starting with '__' On some platforms, the compiler may add symbols that aren't ours and that we should ignore. They are generally expected to start with a double underscore, and thereby easy to detect. (backport of commit 6c63b7e861819db439551b52ea5594faec04b65c) Reviewed-by: Matt Caswell Reviewed-by: Neil Horman (Merged from https://github.com/openssl/openssl/pull/22929) Signed-off-by: fly2x --- test/recipes/01-test_symbol_presence.t | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/recipes/01-test_symbol_presence.t b/test/recipes/01-test_symbol_presence.t index 5530ade0ad..8aaea51a88 100644 --- a/test/recipes/01-test_symbol_presence.t +++ b/test/recipes/01-test_symbol_presence.t @@ -80,7 +80,13 @@ foreach my $libname (@libnames) { # Return the result $_ } - grep(m|.* [BCDST] .*|, @nm_lines); + # Drop any symbol starting with a double underscore, they + # are reserved for the compiler / system ABI and are none + # of our business + grep !m|^__|, + # Only look at external definitions + grep m|.* [BCDST] .*|, + @nm_lines; # Massage the mkdef.pl output to only contain global symbols # The output we got is in Unix .map format, which has a global -- Gitee From bc8f70e2f0c1534f527b595b4e6b8783046ab252 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Thu, 30 Nov 2023 09:02:25 +0100 Subject: [PATCH 07/24] test/recipes/01-test_symbol_presence.t: Treat common symbols specially Common symbols (type 'C' in the 'nm' output) are allowed to be defined more than once. This makes test/recipes/01-test_symbol_presence.t reflect that. (backport of commit 4ff5137ff5fb896e0273d274110517e3c7adb8cf) Reviewed-by: Matt Caswell Reviewed-by: Neil Horman (Merged from https://github.com/openssl/openssl/pull/22929) Signed-off-by: fly2x --- test/recipes/01-test_symbol_presence.t | 42 +++++++++++++++++--------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/test/recipes/01-test_symbol_presence.t b/test/recipes/01-test_symbol_presence.t index 8aaea51a88..3de3d2ccf1 100644 --- a/test/recipes/01-test_symbol_presence.t +++ b/test/recipes/01-test_symbol_presence.t @@ -70,23 +70,35 @@ foreach my $libname (@libnames) { note "Number of lines in \@def_lines before massaging: ", scalar @def_lines; # Massage the nm output to only contain defined symbols + # Common symbols need separate treatment + my %commons; + foreach (@nm_lines) { + if (m|^(.*) C .*|) { + $commons{$1}++; + } + } + foreach (sort keys %commons) { + note "Common symbol: $_"; + } + @nm_lines = sort - map { - # Drop the first space and everything following it - s| .*||; - # Drop OpenSSL dynamic version information if there is any - s|\@\@.+$||; - # Return the result - $_ - } - # Drop any symbol starting with a double underscore, they - # are reserved for the compiler / system ABI and are none - # of our business - grep !m|^__|, - # Only look at external definitions - grep m|.* [BCDST] .*|, - @nm_lines; + ( map { + # Drop the first space and everything following it + s| .*||; + # Drop OpenSSL dynamic version information if there is any + s|\@\@.+$||; + # Return the result + $_ + } + # Drop any symbol starting with a double underscore, they + # are reserved for the compiler / system ABI and are none + # of our business + grep !m|^__|, + # Only look at external definitions + grep m|.* [BDST] .*|, + @nm_lines ), + keys %commons; # Massage the mkdef.pl output to only contain global symbols # The output we got is in Unix .map format, which has a global -- Gitee From a34d359b5c70fad7c43a05db6d299bf501e08276 Mon Sep 17 00:00:00 2001 From: "Matthias St. Pierre" Date: Wed, 29 Nov 2023 22:12:45 +0100 Subject: [PATCH 08/24] doc: improve documentation of EVP in-place encryption The EVP interface explicitly allows in-place encryption/decryption, but this fact is just 'partially' documented in `EVP_EncryptUpdate(3)` (pun intended): the manual page mentions only operation failure in case of 'partial' overlaps. This is not even correct, because the check for partially overlapping buffers is only implemented in legacy code paths. Currently, in-place encryption/decryption is only documented for RSA (`RSA_public_encrypt(3)`) and DES (`DES_ecb_encrypt(3)`), as well as in the provider interface (`provider-cipher(7)`). This commit amends `EVP_EncryptUpdate(3)` and `provider-cipher(7)` to make the front-end and back-end documentation consistent. Reviewed-by: Tomas Mraz Reviewed-by: Tom Cosgrove (Merged from https://github.com/openssl/openssl/pull/22875) (cherry picked from commit 6ebdbba76a45294e22006ede1442847cdee24f03) Signed-off-by: fly2x --- doc/man3/EVP_EncryptInit.pod | 14 +++++++++----- doc/man7/provider-cipher.pod | 10 +++++++--- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/doc/man3/EVP_EncryptInit.pod b/doc/man3/EVP_EncryptInit.pod index 56b29b52e1..a2004e37d7 100644 --- a/doc/man3/EVP_EncryptInit.pod +++ b/doc/man3/EVP_EncryptInit.pod @@ -373,7 +373,12 @@ exists. =item EVP_EncryptUpdate() Encrypts I bytes from the buffer I and writes the encrypted version to -I. This function can be called multiple times to encrypt successive blocks +I. The pointers I and I may point to the same location, in which +case the encryption will be done in-place. If I and I point to different +locations, the two buffers must be disjoint, otherwise the operation might fail +or the outcome might be undefined. + +This function can be called multiple times to encrypt successive blocks of data. The amount of data written depends on the block alignment of the encrypted data. For most ciphers and modes, the amount of data written can be anything @@ -382,10 +387,9 @@ For wrap cipher modes, the amount of data written can be anything from zero bytes to (inl + cipher_block_size) bytes. For stream ciphers, the amount of data written can be anything from zero bytes to inl bytes. -Thus, I should contain sufficient room for the operation being performed. -The actual number of bytes written is placed in I. It also -checks if I and I are partially overlapping, and if they are -0 is returned to indicate failure. +Thus, the buffer pointed to by I must contain sufficient room for the +operation being performed. +The actual number of bytes written is placed in I. If padding is enabled (the default) then EVP_EncryptFinal_ex() encrypts the "final" data, that is any data that remains in a partial block. diff --git a/doc/man7/provider-cipher.pod b/doc/man7/provider-cipher.pod index 14ff581c72..eaad3cf2ff 100644 --- a/doc/man7/provider-cipher.pod +++ b/doc/man7/provider-cipher.pod @@ -148,9 +148,13 @@ It is the responsibility of the cipher implementation to handle input lengths that are not multiples of the block length. In such cases a cipher implementation will typically cache partial blocks of input data until a complete block is obtained. -I may be the same location as I but it should not partially overlap. -The same expectations apply to I as documented for -L and L. +The pointers I and I may point to the same location, in which +case the encryption must be done in-place. If I and I point to different +locations, the requirements of L and L +guarantee that the two buffers are disjoint. +Similarly, the requirements of L and L +ensure that the buffer pointed to by I contains sufficient room for the +operation being performed. OSSL_FUNC_cipher_final() completes an encryption or decryption started through previous OSSL_FUNC_cipher_encrypt_init() or OSSL_FUNC_cipher_decrypt_init(), and OSSL_FUNC_cipher_update() -- Gitee From 64d0442d845044a0842188eb2c51d9db8efe9440 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Sun, 10 Dec 2023 10:18:19 +0100 Subject: [PATCH 09/24] Fix a possible memory leak in do_othername Since the gen->type will not be set in a2i_GENERAL_NAME the gen->d.otherName will not be automatically cleaned up by GENERAL_NAME_free. Also fixed a similar leak in a2i_GENERAL_NAME, where ASN1_STRING_set may fail but gen->d.ia5 will not be automatically cleaned up. Reviewed-by: Shane Lontis Reviewed-by: Tom Cosgrove Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/22996) (cherry picked from commit 1c078212f1548d7f647a1f0f12ed6df257c85cc3) Signed-off-by: fly2x --- crypto/x509/v3_san.c | 13 ++++++++++--- test/recipes/25-test_req.t | 6 +++++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/crypto/x509/v3_san.c b/crypto/x509/v3_san.c index c081f02e19..34ca16a6d7 100644 --- a/crypto/x509/v3_san.c +++ b/crypto/x509/v3_san.c @@ -581,6 +581,8 @@ GENERAL_NAME *a2i_GENERAL_NAME(GENERAL_NAME *out, if ((gen->d.ia5 = ASN1_IA5STRING_new()) == NULL || !ASN1_STRING_set(gen->d.ia5, (unsigned char *)value, strlen(value))) { + ASN1_IA5STRING_free(gen->d.ia5); + gen->d.ia5 = NULL; ERR_raise(ERR_LIB_X509V3, ERR_R_MALLOC_FAILURE); goto err; } @@ -651,16 +653,21 @@ static int do_othername(GENERAL_NAME *gen, const char *value, X509V3_CTX *ctx) */ ASN1_TYPE_free(gen->d.otherName->value); if ((gen->d.otherName->value = ASN1_generate_v3(p + 1, ctx)) == NULL) - return 0; + goto err; objlen = p - value; objtmp = OPENSSL_strndup(value, objlen); if (objtmp == NULL) - return 0; + goto err; gen->d.otherName->type_id = OBJ_txt2obj(objtmp, 0); OPENSSL_free(objtmp); if (!gen->d.otherName->type_id) - return 0; + goto err; return 1; + + err: + OTHERNAME_free(gen->d.otherName); + gen->d.otherName = NULL; + return 0; } static int do_dirname(GENERAL_NAME *gen, const char *value, X509V3_CTX *ctx) diff --git a/test/recipes/25-test_req.t b/test/recipes/25-test_req.t index 6ae09c2b8f..8035add857 100644 --- a/test/recipes/25-test_req.t +++ b/test/recipes/25-test_req.t @@ -15,7 +15,7 @@ use OpenSSL::Test qw/:DEFAULT srctop_file/; setup("test_req"); -plan tests => 46; +plan tests => 48; require_ok(srctop_file('test', 'recipes', 'tconversion.pl')); @@ -40,10 +40,14 @@ my @addext_args = ( "openssl", "req", "-new", "-out", "testreq.pem", "-key", srctop_file("test", "certs", "ee-key.pem"), "-config", srctop_file("test", "test.cnf"), @req_new ); my $val = "subjectAltName=DNS:example.com"; +my $val1 = "subjectAltName=otherName:1.2.3.4;UTF8:test,email:info\@example.com"; my $val2 = " " . $val; my $val3 = $val; $val3 =~ s/=/ =/; ok( run(app([@addext_args, "-addext", $val]))); +ok( run(app([@addext_args, "-addext", $val1]))); +$val1 =~ s/UTF8/XXXX/; # execute the error handling in do_othername +ok(!run(app([@addext_args, "-addext", $val1]))); ok(!run(app([@addext_args, "-addext", $val, "-addext", $val]))); ok(!run(app([@addext_args, "-addext", $val, "-addext", $val2]))); ok(!run(app([@addext_args, "-addext", $val, "-addext", $val3]))); -- Gitee From 2474f57d716423dc587e49be8863777465ed7bd7 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 6 Dec 2023 11:09:53 +0000 Subject: [PATCH 10/24] Avoid an infinite loop in BN_GF2m_mod_inv If p is set to 1 when calling BN_GF2m_mod_inv then an infinite loop will result. Calling this function set 1 when applications call this directly is a non-sensical value - so this would be considered a bug in the caller. It does not seem possible to cause OpenSSL internal callers of BN_GF2m_mod_inv to call it with a value of 1. So, for the above reasons, this is not considered a security issue. Reported by Bing Shi. Reviewed-by: Tomas Mraz Reviewed-by: Todd Short (Merged from https://github.com/openssl/openssl/pull/22960) (cherry picked from commit 9c1b8f17ce2471ca37ee3936d07aed29aab10975) Signed-off-by: fly2x --- crypto/bn/bn_gf2m.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crypto/bn/bn_gf2m.c b/crypto/bn/bn_gf2m.c index 304c2ea08d..c811ae82d6 100644 --- a/crypto/bn/bn_gf2m.c +++ b/crypto/bn/bn_gf2m.c @@ -734,14 +734,20 @@ int BN_GF2m_mod_inv(BIGNUM *r, const BIGNUM *a, const BIGNUM *p, BN_CTX *ctx) { BIGNUM *b = NULL; int ret = 0; + int numbits; BN_CTX_start(ctx); if ((b = BN_CTX_get(ctx)) == NULL) goto err; + /* Fail on a non-sensical input p value */ + numbits = BN_num_bits(p); + if (numbits <= 1) + goto err; + /* generate blinding value */ do { - if (!BN_priv_rand_ex(b, BN_num_bits(p) - 1, + if (!BN_priv_rand_ex(b, numbits - 1, BN_RAND_TOP_ANY, BN_RAND_BOTTOM_ANY, 0, ctx)) goto err; } while (BN_is_zero(b)); -- Gitee From 6ad06057d1763b2abb6f479293300fdd5c3f760d Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 6 Dec 2023 11:19:24 +0000 Subject: [PATCH 11/24] Extend the test of BN_GF2m_mod_inv Test that input value of 1 for p is treated as an error Reviewed-by: Tomas Mraz Reviewed-by: Todd Short (Merged from https://github.com/openssl/openssl/pull/22960) (cherry picked from commit b83c719ecb884f609ade7ad7f52bd5e09737585b) Signed-off-by: fly2x --- test/bntest.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/bntest.c b/test/bntest.c index e8c8ca4f8d..f9bde5ef4d 100644 --- a/test/bntest.c +++ b/test/bntest.c @@ -907,6 +907,14 @@ static int test_gf2m_modinv(void) || !TEST_ptr(d = BN_new())) goto err; + /* Test that a non-sensical, too small value causes a failure */ + if (!TEST_true(BN_one(b[0]))) + goto err; + if (!TEST_true(BN_bntest_rand(a, 512, 0, 0))) + goto err; + if (!TEST_false(BN_GF2m_mod_inv(c, a, b[0], ctx))) + goto err; + if (!(TEST_true(BN_GF2m_arr2poly(p0, b[0])) && TEST_true(BN_GF2m_arr2poly(p1, b[1])))) goto err; -- Gitee From 676bf0a8e2d96488f83ef24639bfc9a699ed9baa Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 6 Dec 2023 11:51:01 +0000 Subject: [PATCH 12/24] Fix some invalid use of sscanf sscanf can return -1 on an empty input string. We need to appropriately handle such an invalid case. The instance in OSSL_HTTP_parse_url could cause an uninitialised read of sizeof(unsigned int) bytes (typically 4). In many cases this uninit read will immediately fail on the following check (i.e. if the read value >65535). If the top 2 bytes of a 4 byte unsigned int are zero then the value will be <=65535 and the uninitialised value will be returned to the caller and could represent arbitrary data on the application stack. The OpenSSL security team has assessed this issue and consider it to be a bug only (i.e. not a CVE). Reviewed-by: Todd Short Reviewed-by: Tomas Mraz Reviewed-by: Shane Lontis (Merged from https://github.com/openssl/openssl/pull/22961) (cherry picked from commit 322517d817ecb5c1a3a8b0e7e038fa146857b4d4) Signed-off-by: fly2x --- apps/errstr.c | 2 +- crypto/http/http_lib.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/errstr.c b/apps/errstr.c index 782705a78a..21349d21cb 100644 --- a/apps/errstr.c +++ b/apps/errstr.c @@ -62,7 +62,7 @@ int errstr_main(int argc, char **argv) /* All remaining arg are error code. */ ret = 0; for (argv = opt_rest(); *argv != NULL; argv++) { - if (sscanf(*argv, "%lx", &l) == 0) { + if (sscanf(*argv, "%lx", &l) <= 0) { ret++; } else { ERR_error_string_n(l, buf, sizeof(buf)); diff --git a/crypto/http/http_lib.c b/crypto/http/http_lib.c index 3164d01d9e..cd0e25c85e 100644 --- a/crypto/http/http_lib.c +++ b/crypto/http/http_lib.c @@ -118,7 +118,7 @@ int OSSL_parse_url(const char *url, char **pscheme, char **puser, char **phost, port = ++p; /* remaining port spec handling is also done for the default values */ /* make sure a decimal port number is given */ - if (!sscanf(port, "%u", &portnum) || portnum > 65535) { + if (sscanf(port, "%u", &portnum) <= 0 || portnum > 65535) { ERR_raise_data(ERR_LIB_HTTP, HTTP_R_INVALID_PORT_NUMBER, "%s", port); goto err; } -- Gitee From 3e1408eb475e80bbacd3c79704049bdb52fa34f8 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 6 Dec 2023 12:51:34 +0000 Subject: [PATCH 13/24] Add a test case for OSSL_HTTP_parse_url Ensure we test the case where the port value is empty in the URL. Reviewed-by: Todd Short Reviewed-by: Tomas Mraz Reviewed-by: Shane Lontis (Merged from https://github.com/openssl/openssl/pull/22961) (cherry picked from commit a36d10dfb7e77614c8d3da602ff3800a2e9f4989) Signed-off-by: fly2x --- test/http_test.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/http_test.c b/test/http_test.c index b9f7452744..b6897a17fd 100644 --- a/test/http_test.c +++ b/test/http_test.c @@ -298,7 +298,8 @@ static int test_http_url_invalid_prefix(void) static int test_http_url_invalid_port(void) { - return test_http_url_invalid("https://1.2.3.4:65536/pkix"); + return test_http_url_invalid("https://1.2.3.4:65536/pkix") + && test_http_url_invalid("https://1.2.3.4:"); } static int test_http_url_invalid_path(void) -- Gitee From 7eff227f5f92502feff2a6f260b2e14ccf731359 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 29 Nov 2023 11:30:07 +0000 Subject: [PATCH 14/24] Add a test for late loading of an ENGINE in TLS Confirm that using an ENGINE works as expected with TLS even if it is loaded late (after construction of the SSL_CTX). (cherry picked from commit a9c97da4910648790387d035afb12963158778fb) Reviewed-by: Tomas Mraz Reviewed-by: Todd Short (Merged from https://github.com/openssl/openssl/pull/22865) Signed-off-by: fly2x --- test/sslapitest.c | 56 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 42 insertions(+), 14 deletions(-) diff --git a/test/sslapitest.c b/test/sslapitest.c index 20e3594613..0c62f62e01 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -10322,6 +10322,27 @@ end: #endif /* OSSL_NO_USABLE_TLS1_3 */ #if !defined(OPENSSL_NO_TLS1_2) && !defined(OPENSSL_NO_DYNAMIC_ENGINE) + +static ENGINE *load_dasync(void) +{ + ENGINE *e; + + if (!TEST_ptr(e = ENGINE_by_id("dasync"))) + return NULL; + + if (!TEST_true(ENGINE_init(e))) { + ENGINE_free(e); + return NULL; + } + + if (!TEST_true(ENGINE_register_ciphers(e))) { + ENGINE_free(e); + return NULL; + } + + return e; +} + /* * Test TLSv1.2 with a pipeline capable cipher. TLSv1.3 and DTLS do not * support this yet. The only pipeline capable cipher that we have is in the @@ -10337,6 +10358,8 @@ end: * Test 4: Client has pipelining enabled, server does not: more data than all * the available pipelines can take * Test 5: Client has pipelining enabled, server does not: Maximum size pipeline + * Test 6: Repeat of test 0, but the engine is loaded late (after the SSL_CTX + * is created) */ static int test_pipelining(int idx) { @@ -10349,25 +10372,28 @@ static int test_pipelining(int idx) size_t written, readbytes, offset, msglen, fragsize = 10, numpipes = 5; size_t expectedreads; unsigned char *buf = NULL; - ENGINE *e; - - if (!TEST_ptr(e = ENGINE_by_id("dasync"))) - return 0; + ENGINE *e = NULL; - if (!TEST_true(ENGINE_init(e))) { - ENGINE_free(e); - return 0; + if (idx != 6) { + e = load_dasync(); + if (e == NULL) + return 0; } - if (!TEST_true(ENGINE_register_ciphers(e))) - goto end; - if (!TEST_true(create_ssl_ctx_pair(libctx, TLS_server_method(), TLS_client_method(), 0, TLS1_2_VERSION, &sctx, &cctx, cert, privkey))) goto end; + if (idx == 6) { + e = load_dasync(); + if (e == NULL) + goto end; + /* Now act like test 0 */ + idx = 0; + } + if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl, NULL, NULL))) goto end; @@ -10503,9 +10529,11 @@ end: SSL_free(clientssl); SSL_CTX_free(sctx); SSL_CTX_free(cctx); - ENGINE_unregister_ciphers(e); - ENGINE_finish(e); - ENGINE_free(e); + if (e != NULL) { + ENGINE_unregister_ciphers(e); + ENGINE_finish(e); + ENGINE_free(e); + } OPENSSL_free(buf); if (fragsize == SSL3_RT_MAX_PLAIN_LENGTH) OPENSSL_free(msg); @@ -10903,7 +10931,7 @@ int setup_tests(void) ADD_ALL_TESTS(test_serverinfo_custom, 4); #endif #if !defined(OPENSSL_NO_TLS1_2) && !defined(OPENSSL_NO_DYNAMIC_ENGINE) - ADD_ALL_TESTS(test_pipelining, 6); + ADD_ALL_TESTS(test_pipelining, 7); #endif ADD_ALL_TESTS(test_handshake_retry, 16); return 1; -- Gitee From 57dd265613da84a9120f5dc1946a046a27b886b9 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 29 Nov 2023 11:45:12 +0000 Subject: [PATCH 15/24] Don't attempt to set provider params on an ENGINE based cipher If an ENGINE has been loaded after the SSL_CTX has been created then the cipher we have cached might be provider based, but the cipher we actually end up using might not be. Don't try to set provider params on a cipher that is actually ENGINE based. Reviewed-by: Tomas Mraz Reviewed-by: Todd Short (Merged from https://github.com/openssl/openssl/pull/22865) Signed-off-by: fly2x --- ssl/s3_enc.c | 6 +++++- ssl/t1_enc.c | 7 ++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/ssl/s3_enc.c b/ssl/s3_enc.c index 2ca3f74ae7..ee4f58e75e 100644 --- a/ssl/s3_enc.c +++ b/ssl/s3_enc.c @@ -225,7 +225,11 @@ int ssl3_change_cipher_state(SSL *s, int which) goto err; } - if (EVP_CIPHER_get0_provider(c) != NULL + /* + * The cipher we actually ended up using in the EVP_CIPHER_CTX may be + * different to that in c if we have an ENGINE in use + */ + if (EVP_CIPHER_get0_provider(EVP_CIPHER_CTX_get0_cipher(dd)) != NULL && !tls_provider_set_tls_params(s, dd, c, m)) { /* SSLfatal already called */ goto err; diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c index 91238e6457..6cb7baaf7c 100644 --- a/ssl/t1_enc.c +++ b/ssl/t1_enc.c @@ -427,7 +427,12 @@ int tls1_change_cipher_state(SSL *s, int which) SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); goto err; } - if (EVP_CIPHER_get0_provider(c) != NULL + + /* + * The cipher we actually ended up using in the EVP_CIPHER_CTX may be + * different to that in c if we have an ENGINE in use + */ + if (EVP_CIPHER_get0_provider(EVP_CIPHER_CTX_get0_cipher(dd)) != NULL && !tls_provider_set_tls_params(s, dd, c, m)) { /* SSLfatal already called */ goto err; -- Gitee From a9cdc9ee6c906f6513138d89379def879db1454d Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Sun, 3 Dec 2023 11:24:18 +0100 Subject: [PATCH 16/24] Fix a possible memleak in cms_main Reviewed-by: Shane Lontis Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/22918) (cherry picked from commit 3457a550c64ab8009c7cd0175675ac140cab33c2) Signed-off-by: fly2x --- apps/cms.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/cms.c b/apps/cms.c index 12095b9641..3994cb0fcd 100644 --- a/apps/cms.c +++ b/apps/cms.c @@ -620,7 +620,8 @@ int cms_main(int argc, char **argv) "recipient certificate file"); if (cert == NULL) goto end; - sk_X509_push(encerts, cert); + if (!sk_X509_push(encerts, cert)) + goto end; cert = NULL; } else { recipfile = opt_arg(); @@ -831,7 +832,8 @@ int cms_main(int argc, char **argv) "recipient certificate file"); if (cert == NULL) goto end; - sk_X509_push(encerts, cert); + if (!sk_X509_push(encerts, cert)) + goto end; cert = NULL; } } -- Gitee From 3582a901e3d8416ba995fb59b19f521bc402c010 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Sun, 3 Dec 2023 11:34:37 +0100 Subject: [PATCH 17/24] Fix a possible memleak in smime_main Reviewed-by: Shane Lontis Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/22919) (cherry picked from commit ba4d833f6e24a83bc3e74ba55f52d8916b70fb59) Signed-off-by: fly2x --- apps/smime.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/smime.c b/apps/smime.c index a2ff0b5be7..52b4a01c23 100644 --- a/apps/smime.c +++ b/apps/smime.c @@ -453,7 +453,8 @@ int smime_main(int argc, char **argv) "recipient certificate file"); if (cert == NULL) goto end; - sk_X509_push(encerts, cert); + if (!sk_X509_push(encerts, cert)) + goto end; cert = NULL; argv++; } -- Gitee From 10df696f1ce68be6709ce774e94257c7403ca8da Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Sun, 3 Dec 2023 11:29:52 +0100 Subject: [PATCH 18/24] Fix a possible memleak in apps/rehash.c The OPENSSL_DIR_end was missing in case of error. Reviewed-by: Shane Lontis Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/22920) (cherry picked from commit 01709fcb8b609cfc47e277d20492c333bafb113e) Signed-off-by: fly2x --- apps/rehash.c | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/rehash.c b/apps/rehash.c index d63a0909a2..85eee38579 100644 --- a/apps/rehash.c +++ b/apps/rehash.c @@ -383,6 +383,7 @@ static int do_dir(const char *dirname, enum Hash h) if ((copy = OPENSSL_strdup(filename)) == NULL || sk_OPENSSL_STRING_push(files, copy) == 0) { OPENSSL_free(copy); + OPENSSL_DIR_end(&d); BIO_puts(bio_err, "out of memory\n"); errs = 1; goto err; -- Gitee From 27e76666defdde3112cbd010575b823895cdee84 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Sun, 3 Dec 2023 11:41:51 +0100 Subject: [PATCH 19/24] Fix a possible memleak in opt_verify The ASN1_OBJECT otmp was leaked if X509_VERIFY_PARAM_add0_policy fails. Reviewed-by: Shane Lontis Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/22922) (cherry picked from commit d6688e45fa2f987f3ffd324e19922468beee5ddc) Signed-off-by: fly2x --- apps/lib/opt.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/apps/lib/opt.c b/apps/lib/opt.c index 157367982d..d56964dbe7 100644 --- a/apps/lib/opt.c +++ b/apps/lib/opt.c @@ -696,7 +696,12 @@ int opt_verify(int opt, X509_VERIFY_PARAM *vpm) opt_printf_stderr("%s: Invalid Policy %s\n", prog, opt_arg()); return 0; } - X509_VERIFY_PARAM_add0_policy(vpm, otmp); + if (!X509_VERIFY_PARAM_add0_policy(vpm, otmp)) { + ASN1_OBJECT_free(otmp); + opt_printf_stderr("%s: Internal error adding Policy %s\n", + prog, opt_arg()); + return 0; + } break; case OPT_V_PURPOSE: /* purpose name -> purpose index */ -- Gitee From 5160f7982e7eb9940584b81cdcae3d4b2cda1bb4 Mon Sep 17 00:00:00 2001 From: "Dr. David von Oheimb" Date: Fri, 27 Oct 2023 08:58:48 +0200 Subject: [PATCH 20/24] provider-storemgmt.pod: fix nits (unclosed '<' around name) Reviewed-by: Hugo Landau Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/22942) (cherry picked from commit a149e8e108263718daede1858d2855d68dde5652) Signed-off-by: fly2x --- doc/man7/provider-storemgmt.pod | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/man7/provider-storemgmt.pod b/doc/man7/provider-storemgmt.pod index cde95f66e1..f39f345fc8 100644 --- a/doc/man7/provider-storemgmt.pod +++ b/doc/man7/provider-storemgmt.pod @@ -162,12 +162,12 @@ fingerprint, computed with the given digest. Indicates that the caller wants to search for an object with the given alias (some call it a "friendly name"). -=item "properties" (B +=item "properties" (B) Property string to use when querying for algorithms such as the B decoder implementations. -=item "input-type" (B +=item "input-type" (B) Type of the input format as a hint to use when decoding the objects in the store. -- Gitee From 518d9e2a7c8f91977ee137f41ac48cc48a7d0ecd Mon Sep 17 00:00:00 2001 From: James Muir Date: Wed, 6 Dec 2023 16:49:11 -0500 Subject: [PATCH 21/24] ossl-params: check length returned by strlen() In param_build.c, the functions OSSL_PARAM_BLD_push_utf8_string() and OSSL_PARAM_BLD_push_utf8_ptr() use strlen() to compute the length of the string when bsize is zero. However, the size_t returned by strlen() might be too large (it is stored in an intermediate "int"), so check for that. There are analogous functions in params.c, but they do not use an intermediate "int" to store the size_t returned by strlen(). So there is some inconsistency between the implementations. Credit to Viktor D and Tomas M for spotting these missing checks. Reviewed-by: Matt Caswell Reviewed-by: Shane Lontis Reviewed-by: Viktor Dukhovni Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/22967) (cherry picked from commit d4d6694aa710c9970410a6836070daa6486a0ac0) Signed-off-by: fly2x --- crypto/param_build.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crypto/param_build.c b/crypto/param_build.c index 4fc6c0319f..acbb01928d 100644 --- a/crypto/param_build.c +++ b/crypto/param_build.c @@ -241,9 +241,9 @@ int OSSL_PARAM_BLD_push_utf8_string(OSSL_PARAM_BLD *bld, const char *key, OSSL_PARAM_BLD_DEF *pd; int secure; - if (bsize == 0) { + if (bsize == 0) bsize = strlen(buf); - } else if (bsize > INT_MAX) { + if (bsize > INT_MAX) { ERR_raise(ERR_LIB_CRYPTO, CRYPTO_R_STRING_TOO_LONG); return 0; } @@ -260,9 +260,9 @@ int OSSL_PARAM_BLD_push_utf8_ptr(OSSL_PARAM_BLD *bld, const char *key, { OSSL_PARAM_BLD_DEF *pd; - if (bsize == 0) { + if (bsize == 0) bsize = strlen(buf); - } else if (bsize > INT_MAX) { + if (bsize > INT_MAX) { ERR_raise(ERR_LIB_CRYPTO, CRYPTO_R_STRING_TOO_LONG); return 0; } -- Gitee From 3ed20084d736abcf8d60eab05f55c930a303af1c Mon Sep 17 00:00:00 2001 From: "fangming.fang" Date: Thu, 7 Dec 2023 06:17:51 +0000 Subject: [PATCH 22/24] Enable BTI feature for md5 on aarch64 Fixes: #22959 Reviewed-by: Tom Cosgrove Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/22971) (cherry picked from commit ad347c9ff0fd93bdd2fa2085611c65b88e94829f) Signed-off-by: fly2x --- crypto/md5/asm/md5-aarch64.pl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crypto/md5/asm/md5-aarch64.pl b/crypto/md5/asm/md5-aarch64.pl index 3200a0fa9b..5a86080696 100755 --- a/crypto/md5/asm/md5-aarch64.pl +++ b/crypto/md5/asm/md5-aarch64.pl @@ -28,10 +28,13 @@ open OUT,"| \"$^X\" $xlate $flavour \"$output\"" *STDOUT=*OUT; $code .= < Date: Thu, 7 Dec 2023 10:23:49 -0500 Subject: [PATCH 23/24] doc: fix list display in man page "=over 1" is too small. Use "=over 2" so that list items are displayed correctly in the generated man-page. You can check the man-page using the following command: cd doc && pod2man man3/OSSL_PARAM_int.pod | man /dev/stdin Reviewed-by: Neil Horman Reviewed-by: Shane Lontis (Merged from https://github.com/openssl/openssl/pull/22974) (cherry picked from commit 7f4bf1857321d2a2ebcbbb2742946a965e463b79) Signed-off-by: fly2x --- doc/man3/OSSL_PARAM_int.pod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/man3/OSSL_PARAM_int.pod b/doc/man3/OSSL_PARAM_int.pod index d357818ff1..105fe3241f 100644 --- a/doc/man3/OSSL_PARAM_int.pod +++ b/doc/man3/OSSL_PARAM_int.pod @@ -112,7 +112,7 @@ OSSL_PARAM_UNMODIFIED, OSSL_PARAM_modified, OSSL_PARAM_set_all_unmodified A collection of utility functions that simplify and add type safety to the L arrays. The following B> names are supported: -=over 1 +=over 2 =item * -- Gitee From 61863cb7597d53dad397a785f5cb6af2c8629d4a Mon Sep 17 00:00:00 2001 From: "Dr. David von Oheimb" Date: Sat, 2 Dec 2023 15:54:27 +0100 Subject: [PATCH 24/24] CONTRIBUTING.md: add reference to util/check-format.pl and fix several nits Reviewed-by: Hugo Landau Reviewed-by: Richard Levitte Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/22911) (cherry picked from commit 260d97229c467d17934ca3e2e0455b1b5c0994a6) Signed-off-by: fly2x --- CONTRIBUTING.md | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0117fce16f..15490fd9f6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -9,19 +9,22 @@ Development is done on GitHub in the [openssl/openssl] repository. [openssl/openssl]: -To request new features or report bugs, please open an issue on GitHub +To request new a feature, ask a question, or report a bug, +please open an [issue on GitHub](https://github.com/openssl/openssl/issues). -To submit a patch, please open a pull request on GitHub. If you are thinking -of making a large contribution, open an issue for it before starting work, -to get comments from the community. Someone may be already working on -the same thing, or there may be reasons why that feature isn't implemented. +To submit a patch or implement a new feature, please open a +[pull request on GitHub](https://github.com/openssl/openssl/pulls). +If you are thinking of making a large contribution, +open an issue for it before starting work, to get comments from the community. +Someone may be already working on the same thing, +or there may be special reasons why a feature is not implemented. To make it easier to review and accept your pull request, please follow these guidelines: 1. Anything other than a trivial contribution requires a [Contributor License Agreement] (CLA), giving us permission to use your code. - If your contribution is too small to require a CLA (e.g. fixing a spelling + If your contribution is too small to require a CLA (e.g., fixing a spelling mistake), then place the text "`CLA: trivial`" on a line by itself below the rest of your commit message separated by an empty line, like this: @@ -64,22 +67,24 @@ guidelines: often. We do not accept merge commits, you will have to remove them (usually by rebasing) before it will be acceptable. - 4. Patches should follow our [coding style] and compile without warnings. + 4. Code provided should follow our [coding style] and compile without warnings. + There is a [Perl tool](util/check-format.pl) that helps + finding code formatting mistakes and other coding style nits. Where `gcc` or `clang` is available, you should use the `--strict-warnings` `Configure` option. OpenSSL compiles on many varied - platforms: try to ensure you only use portable features. Clean builds via - GitHub Actions and AppVeyor are required, and they are started automatically - whenever a PR is created or updated. + platforms: try to ensure you only use portable features. + Clean builds via GitHub Actions are required. They are started automatically + whenever a PR is created or updated by committers. [coding style]: https://www.openssl.org/policies/technical/coding-style.html - 5. When at all possible, patches should include tests. These can + 5. When at all possible, code contributions should include tests. These can either be added to an existing test, or completely new. Please see [test/README.md](test/README.md) for information on the test framework. 6. New features or changed functionality must include - documentation. Please look at the "pod" files in doc/man[1357] for - examples of our style. Run "make doc-nits" to make sure that your + documentation. Please look at the `.pod` files in `doc/man[1357]` for + examples of our style. Run `make doc-nits` to make sure that your documentation changes are clean. 7. For user visible changes (API changes, behaviour changes, ...), @@ -89,7 +94,7 @@ guidelines: Have a look through existing entries for inspiration. Please note that this is NOT simply a copy of git-log one-liners. Also note that security fixes get an entry in [CHANGES.md](CHANGES.md). - This file helps users get more in depth information of what comes + This file helps users get more in-depth information of what comes with a specific release without having to sift through the higher noise ratio in git-log. -- Gitee