From f0cb9cfb830212caf607111669f103bef6492e36 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Tue, 5 Dec 2023 09:21:35 +0100 Subject: [PATCH 01/28] Make sure that the test / tests build target run 'run_tests' last Fixes #22943 Reviewed-by: Tomas Mraz Reviewed-by: Todd Short (Merged from https://github.com/openssl/openssl/pull/22947) (cherry picked from commit f882753f43ff7d3e03dc53e6c7450a1308265628) 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 828e1e91fb..e9fb13f24f 100644 --- a/Configurations/descrip.mms.tmpl +++ b/Configurations/descrip.mms.tmpl @@ -516,7 +516,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 6714699178..b3350a1048 100644 --- a/Configurations/unix-Makefile.tmpl +++ b/Configurations/unix-Makefile.tmpl @@ -544,8 +544,9 @@ help: ## Show this help screen ##@ Testing test: tests ## Run tests (alias of "tests") -{- dependmagic('tests', 'Run tests'); -}: build_programs_nodep build_modules_nodep link-utils run_tests -run_tests: +{- dependmagic('tests', 'Run 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 8bea956e16f4677cb70db069bce873a471b79ac1 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Tue, 5 Dec 2023 09:26:36 +0100 Subject: [PATCH 02/28] 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: Todd Short (Merged from https://github.com/openssl/openssl/pull/22947) (cherry picked from commit ae64a116f05bade92fa2794394e44127bdaa7436) 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 1f260e49d4..d09773e71a 100644 --- a/Configurations/windows-makefile.tmpl +++ b/Configurations/windows-makefile.tmpl @@ -440,6 +440,8 @@ all: build_sw {- "build_docs" if !$disabled{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 b9ca625298e12eee061097f5acd062af3729f68f Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Tue, 28 Nov 2023 15:55:43 +0100 Subject: [PATCH 03/28] Modify 'out-of-source-and-install' to work with a read-only source tree This also adds the configuration options 'enable-quic'. Fixes #22907 Reviewed-by: Tomas Mraz Reviewed-by: Todd Short (Merged from https://github.com/openssl/openssl/pull/22908) (cherry picked from commit 266a3553d743f5335ccdff196a07916f03d34d0d) Signed-off-by: fly2x --- .github/workflows/ci.yml | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2ba14153c2..38ad82fe69 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -336,21 +336,33 @@ 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 + with: + path: ./source - name: checkout fuzz/corpora submodule run: git submodule update --init --depth 1 fuzz/corpora - - name: extra preparations + working-directory: ./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-quic enable-acvp-tests --strict-warnings --prefix=$(cd ../install; pwd) + perl configdata.pm --dump working-directory: ./build - name: make run: make -s -j4 -- Gitee From 5ee424761e8bdc173a0aa7de06daf5e49f9040d9 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Tue, 28 Nov 2023 23:41:32 +0100 Subject: [PATCH 04/28] Configure: Refuse to make directories in the source tree Fixes #22907 Reviewed-by: Tomas Mraz Reviewed-by: Todd Short (Merged from https://github.com/openssl/openssl/pull/22908) (cherry picked from commit 504ff2a4ef5f26990a48ca3d664ac1e5d9cb20b9) Signed-off-by: fly2x --- Configure | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/Configure b/Configure index cbba1749b5..cca1ac8d16 100755 --- a/Configure +++ b/Configure @@ -1891,11 +1891,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"; @@ -1906,6 +1907,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); @@ -1913,7 +1915,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"; @@ -1943,7 +1945,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; @@ -1958,7 +1960,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 @@ -1967,7 +1969,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 @@ -2320,7 +2322,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. @@ -2365,7 +2367,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. @@ -2420,7 +2422,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]) { @@ -2446,7 +2448,7 @@ EOF } elsif ($dest eq '') { $ddest = ''; } else { - $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. @@ -2471,12 +2473,12 @@ EOF && $f =~ m/^(.*?)\|(.*)$/) { $i = $1; $m = $2; - $i = cleanfile($sourced, $i, $blddir); + $i = cleanfile($sourced, $i, $blddir, 1); $i2 = cleanfile($buildd, $i, $blddir); - $d = cleanfile($sourced, "$i/$m", $blddir); + $d = cleanfile($sourced, "$i/$m", $blddir, 1); $d2 = cleanfile($buildd, "$i/$m", $blddir); } else { - $d = cleanfile($sourced, $f, $blddir); + $d = cleanfile($sourced, $f, $blddir, 1); $d2 = cleanfile($buildd, $f, $blddir); } @@ -2507,7 +2509,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. @@ -2515,7 +2517,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}}; @@ -2528,7 +2530,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. @@ -2912,7 +2914,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 536094a3cc4cfa88466013e52b2001e14d43944d Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Tue, 28 Nov 2023 13:54:37 -0500 Subject: [PATCH 05/28] 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 520d83d3f9e215d4f1cac1f0d940f78fde4af680 Mon Sep 17 00:00:00 2001 From: "Matthias St. Pierre" Date: Wed, 29 Nov 2023 22:12:45 +0100 Subject: [PATCH 06/28] 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 a04e6d102f..955340b94d 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 00e9ba58d48061c3339dfb083f8dd74afea10132 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Sun, 10 Dec 2023 10:18:19 +0100 Subject: [PATCH 07/28] 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 7798505eec..9adf494707 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_ASN1_LIB); 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 32dc4ded8c..20e338b46f 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 => 104; +plan tests => 106; require_ok(srctop_file('test', 'recipes', 'tconversion.pl')); @@ -40,10 +40,14 @@ my @addext_args = ( "openssl", "req", "-new", "-out", "testreq.pem", "-key", srctop_file(@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 b9690450f2082e2379682ee2c3c8732f3e76efc5 Mon Sep 17 00:00:00 2001 From: "Randall S. Becker" Date: Wed, 22 Nov 2023 20:45:24 +0000 Subject: [PATCH 08/28] Deprecate SPT threading support on NonStop. This fix removes explicit support for the SPT threading model in configurations. This also reverts commit f63e1b48ac893dd6110452e70ed08f191547cd89 that were required for SPT but broke other models. Fixes: #22798 Signed-off-by: Randall S. Becker Reviewed-by: Richard Levitte Reviewed-by: Neil Horman Reviewed-by: Hugo Landau (Merged from https://github.com/openssl/openssl/pull/22807) (cherry picked from commit 5cd17920167a8b4f7a81722a1ed3b514115702de) Signed-off-by: fly2x --- Configurations/50-nonstop.conf | 55 ---------------------------------- NOTES-NONSTOP.md | 25 ++++++---------- crypto/bio/bio_sock.c | 2 +- 3 files changed, 10 insertions(+), 72 deletions(-) diff --git a/Configurations/50-nonstop.conf b/Configurations/50-nonstop.conf index 827a13b4ee..873e398169 100644 --- a/Configurations/50-nonstop.conf +++ b/Configurations/50-nonstop.conf @@ -170,24 +170,6 @@ '_REENTRANT', '_THREAD_SUPPORT_FUNCTIONS'], ex_libs => '-lput', }, - 'nonstop-model-spt' => { - template => 1, - cflags => add('-Wnowarn=140'), - defines => ['_SPT_MODEL_', - 'SPT_THREAD_AWARE_NONBLOCK', - '_REENTRANT'], - ex_libs => '-lspt', - }, - - # Additional floss model that can be combined with any of the other models. - # If used without any of the other models, the entry that does so must - # disable threads. - 'nonstop-model-floss' => { - template => 1, - defines => ['OPENSSL_TANDEM_FLOSS', '_ENABLE_FLOSS_THREADS'], - includes => ['/usr/local/include'], - ex_libs => '-lfloss', - }, ###################################################################### # Now for the entries themselves, let's combine things! @@ -225,25 +207,6 @@ multilib => '64-put', multibin => '64-put', }, - 'nonstop-nsx_spt' => { - inherit_from => [ 'nonstop-common', - 'nonstop-archenv-x86_64-oss', - 'nonstop-ilp32', - 'nonstop-efloat-x86_64', - 'nonstop-model-spt' ], - multilib => '-spt', - multibin => '-spt', - }, - 'nonstop-nsx_spt_floss' => { - inherit_from => [ 'nonstop-common', - 'nonstop-archenv-x86_64-oss', - 'nonstop-ilp32', - 'nonstop-efloat-x86_64', - 'nonstop-model-floss', - 'nonstop-model-spt'], - multilib => '-spt', - multibin => '-spt', - }, 'nonstop-nsx_g' => { inherit_from => [ 'nonstop-common', 'nonstop-archenv-x86_64-guardian', @@ -293,24 +256,6 @@ multilib => '64-put', multibin => '64-put', }, - 'nonstop-nse_spt' => { - inherit_from => [ 'nonstop-common', - 'nonstop-archenv-itanium-oss', - 'nonstop-ilp32', - 'nonstop-efloat-itanium', - 'nonstop-model-spt' ], - multilib => '-spt', - multibin => '-spt', - }, - 'nonstop-nse_spt_floss' => { - inherit_from => [ 'nonstop-common', - 'nonstop-archenv-itanium-oss', - 'nonstop-ilp32', - 'nonstop-efloat-itanium', - 'nonstop-model-floss', 'nonstop-model-spt' ], - multilib => '-spt', - multibin => '-spt', - }, 'nonstop-nse_g' => { inherit_from => [ 'nonstop-common', 'nonstop-archenv-itanium-guardian', diff --git a/NOTES-NONSTOP.md b/NOTES-NONSTOP.md index 68438b9988..65bfc1087d 100644 --- a/NOTES-NONSTOP.md +++ b/NOTES-NONSTOP.md @@ -26,15 +26,16 @@ is the only FLOSS variant that has been broadly tested. Threading Models ---------------- -OpenSSL can be built using unthreaded, POSIX User Threads (PUT), or Standard -POSIX Threads (SPT). Select the following build configuration for each on -the TNS/X (L-Series) platform: +OpenSSL can be built either using the POSIX User Threads (PUT) threading model, +or with threading support disabled. Select the following build configuration +for each on the TNS/X (L-Series) platform: - * `nonstop-nsx` or default will select an unthreaded build. + * `nonstop-nsx` or default will select an unthreaded 32-bit build. + * `nonstop-nsx_64` selects an unthreaded 64-bit memory and file length build. * `nonstop-nsx_put` selects the PUT build. - * `nonstop-nsx_64_put` selects the 64 bit file length PUT build. - * `nonstop-nsx_spt_floss` selects the SPT build with FLOSS. FLOSS is - required for SPT builds because of a known hang when using SPT on its own. + * `nonstop-nsx_64_put` selects the 64-bit memory and file length PUT build. + +The SPT threading model is no longer supported as of OpenSSL 3.2. ### TNS/E Considerations @@ -145,9 +146,7 @@ update this list: - nonstop-nsx_64_put **Note:** Cross-compile builds for TNS/E have not been attempted, but should -follow the same considerations as for TNS/X above. SPT builds generally require -FLOSS, which is not available for workstation builds. As a result, SPT builds -of OpenSSL cannot be cross-compiled. +follow the same considerations as for TNS/X above. Also see the NSDEE discussion below for more historical information. @@ -223,9 +222,6 @@ assumes that your PWD is set according to your installation standards. ./Configure nonstop-nsx_put --prefix=${PWD} \ --openssldir=${PWD}/ssl threads "-D_REENTRANT" \ --with-rand-seed=rdcpu ${CIPHENABLES} ${DBGFLAG} ${SYSTEMLIBS} - ./Configure nonstop-nsx_spt_floss --prefix=${PWD} \ - --openssldir=${PWD}/ssl threads "-D_REENTRANT" \ - --with-rand-seed=rdcpu ${CIPHENABLES} ${DBGFLAG} ${SYSTEMLIBS} ./Configure nonstop-nsx_64 --prefix=${PWD} \ --openssldir=${PWD}/ssl no-threads \ --with-rand-seed=rdcpu ${CIPHENABLES} ${DBGFLAG} ${SYSTEMLIBS} @@ -245,9 +241,6 @@ assumes that your PWD is set according to your installation standards. ./Configure nonstop-nse_put --prefix=${PWD} \ --openssldir=${PWD}/ssl threads "-D_REENTRANT" \ --with-rand-seed=egd ${CIPHENABLES} ${DBGFLAG} ${SYSTEMLIBS} - ./Configure nonstop-nse_spt_floss --prefix=${PWD} \ - --openssldir=${PWD}/ssl threads "-D_REENTRANT" \ - --with-rand-seed=egd ${CIPHENABLES} ${DBGFLAG} ${SYSTEMLIBS} ./Configure nonstop-nse_64 --prefix=${PWD} \ --openssldir=${PWD}/ssl no-threads \ --with-rand-seed=egd ${CIPHENABLES} ${DBGFLAG} ${SYSTEMLIBS} diff --git a/crypto/bio/bio_sock.c b/crypto/bio/bio_sock.c index 3c8b28501c..ee3ee5b78f 100644 --- a/crypto/bio/bio_sock.c +++ b/crypto/bio/bio_sock.c @@ -354,7 +354,7 @@ int BIO_socket_nbio(int s, int mode) int l; l = mode; -# if defined(FIONBIO) && !defined(OPENSSL_SYS_TANDEM) +# ifdef FIONBIO l = mode; ret = BIO_socket_ioctl(s, FIONBIO, &l); -- Gitee From 7c74f7ac8de829c87720cfd1c4e2463f54bc0701 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 29 Nov 2023 11:30:07 +0000 Subject: [PATCH 09/28] 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). Reviewed-by: Tomas Mraz Reviewed-by: Todd Short (Merged from https://github.com/openssl/openssl/pull/22864) (cherry picked from commit 7765d25ffe4f2a60b2082d469dec3b40f3418024) 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 88294af16a..7e01b72328 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -10586,6 +10586,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 @@ -10601,6 +10622,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) { @@ -10613,25 +10636,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; @@ -10767,9 +10793,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); @@ -11489,7 +11517,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_version, 6); ADD_TEST(test_rstate_string); -- Gitee From 4b6b35bdfcf411f07452cd168ad6f20960700533 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 29 Nov 2023 11:45:12 +0000 Subject: [PATCH 10/28] 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/22864) (cherry picked from commit afcc12c41ad82c5b63194502592de015604dbd47) Signed-off-by: fly2x --- ssl/record/methods/ssl3_meth.c | 6 +++++- ssl/record/methods/tls1_meth.c | 11 +++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/ssl/record/methods/ssl3_meth.c b/ssl/record/methods/ssl3_meth.c index 76a108e443..810dc0716b 100644 --- a/ssl/record/methods/ssl3_meth.c +++ b/ssl/record/methods/ssl3_meth.c @@ -64,7 +64,11 @@ static int ssl3_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, return OSSL_RECORD_RETURN_FATAL; } - if (EVP_CIPHER_get0_provider(ciph) != NULL + /* + * The cipher we actually ended up using in the EVP_CIPHER_CTX may be + * different to that in ciph if we have an ENGINE in use + */ + if (EVP_CIPHER_get0_provider(EVP_CIPHER_CTX_get0_cipher(ciph_ctx)) != NULL && !ossl_set_tls_provider_parameters(rl, ciph_ctx, ciph, md)) { /* ERR_raise already called */ return OSSL_RECORD_RETURN_FATAL; diff --git a/ssl/record/methods/tls1_meth.c b/ssl/record/methods/tls1_meth.c index 46a83ad8f4..f13d530a05 100644 --- a/ssl/record/methods/tls1_meth.c +++ b/ssl/record/methods/tls1_meth.c @@ -117,9 +117,16 @@ static int tls1_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR); return OSSL_RECORD_RETURN_FATAL; } - if (EVP_CIPHER_get0_provider(ciph) != NULL - && !ossl_set_tls_provider_parameters(rl, ciph_ctx, ciph, md)) + + /* + * The cipher we actually ended up using in the EVP_CIPHER_CTX may be + * different to that in ciph if we have an ENGINE in use + */ + if (EVP_CIPHER_get0_provider(EVP_CIPHER_CTX_get0_cipher(ciph_ctx)) != NULL + && !ossl_set_tls_provider_parameters(rl, ciph_ctx, ciph, md)) { + /* ERR_raise already called */ return OSSL_RECORD_RETURN_FATAL; + } /* Calculate the explicit IV length */ if (RLAYER_USE_EXPLICIT_IV(rl)) { -- Gitee From 24b6a647b6507211b81472f686178c966c3146fe Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 29 Nov 2023 11:55:17 +0000 Subject: [PATCH 11/28] Remove some redundant code We remove a function that was left behind and is no longer called after the record layer refactor Reviewed-by: Tomas Mraz Reviewed-by: Todd Short (Merged from https://github.com/openssl/openssl/pull/22864) (cherry picked from commit e46a6b1a5de0759023c5c9c2143ead4621f20d20) Signed-off-by: fly2x --- ssl/ssl_local.h | 4 ---- ssl/t1_enc.c | 38 -------------------------------------- 2 files changed, 42 deletions(-) diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h index d1ef358932..0d3acfbe66 100644 --- a/ssl/ssl_local.h +++ b/ssl/ssl_local.h @@ -2927,10 +2927,6 @@ const EVP_MD *ssl_evp_md_fetch(OSSL_LIB_CTX *libctx, int ssl_evp_md_up_ref(const EVP_MD *md); void ssl_evp_md_free(const EVP_MD *md); -int tls_provider_set_tls_params(SSL_CONNECTION *s, EVP_CIPHER_CTX *ctx, - const EVP_CIPHER *ciph, - const EVP_MD *md); - void tls_engine_finish(ENGINE *e); const EVP_CIPHER *tls_get_cipher_from_engine(int nid); const EVP_MD *tls_get_digest_from_engine(int nid); diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c index 673a53ad36..94f68eb999 100644 --- a/ssl/t1_enc.c +++ b/ssl/t1_enc.c @@ -101,44 +101,6 @@ static int tls1_generate_key_block(SSL_CONNECTION *s, unsigned char *km, return ret; } -int tls_provider_set_tls_params(SSL_CONNECTION *s, EVP_CIPHER_CTX *ctx, - const EVP_CIPHER *ciph, - const EVP_MD *md) -{ - /* - * Provided cipher, the TLS padding/MAC removal is performed provider - * side so we need to tell the ctx about our TLS version and mac size - */ - OSSL_PARAM params[3], *pprm = params; - size_t macsize = 0; - int imacsize = -1; - - if ((EVP_CIPHER_get_flags(ciph) & EVP_CIPH_FLAG_AEAD_CIPHER) == 0 - /* - * We look at s->ext.use_etm instead of SSL_READ_ETM() or - * SSL_WRITE_ETM() because this test applies to both reading - * and writing. - */ - && !s->ext.use_etm) - imacsize = EVP_MD_get_size(md); - if (imacsize >= 0) - macsize = (size_t)imacsize; - - *pprm++ = OSSL_PARAM_construct_int(OSSL_CIPHER_PARAM_TLS_VERSION, - &s->version); - *pprm++ = OSSL_PARAM_construct_size_t(OSSL_CIPHER_PARAM_TLS_MAC_SIZE, - &macsize); - *pprm = OSSL_PARAM_construct_end(); - - if (!EVP_CIPHER_CTX_set_params(ctx, params)) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); - return 0; - } - - return 1; -} - - static int tls_iv_length_within_key_block(const EVP_CIPHER *c) { /* If GCM/CCM mode only part of IV comes from PRF */ -- Gitee From 3b20258119b31716811ea6654c37671a57cfa984 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Thu, 30 Nov 2023 09:24:26 +0000 Subject: [PATCH 12/28] Fix detection for riscv64/riscv32 Fixes #22871 Reviewed-by: Tomas Mraz Reviewed-by: Kurt Roeckx (Merged from https://github.com/openssl/openssl/pull/22881) (cherry picked from commit ff279597692f9f19dca5b147944d3d96f2e109f8) Signed-off-by: fly2x --- providers/implementations/ciphers/cipher_aes_ccm_hw.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/providers/implementations/ciphers/cipher_aes_ccm_hw.c b/providers/implementations/ciphers/cipher_aes_ccm_hw.c index 575a8ba88d..b050cf3edd 100644 --- a/providers/implementations/ciphers/cipher_aes_ccm_hw.c +++ b/providers/implementations/ciphers/cipher_aes_ccm_hw.c @@ -61,9 +61,9 @@ static const PROV_CCM_HW aes_ccm = { # include "cipher_aes_ccm_hw_aesni.inc" #elif defined(SPARC_AES_CAPABLE) # include "cipher_aes_ccm_hw_t4.inc" -#elif defined(__riscv) && __riscv_xlen == 64 +#elif defined(OPENSSL_CPUID_OBJ) && defined(__riscv) && __riscv_xlen == 64 # include "cipher_aes_ccm_hw_rv64i.inc" -#elif defined(__riscv) && __riscv_xlen == 32 +#elif defined(OPENSSL_CPUID_OBJ) && defined(__riscv) && __riscv_xlen == 32 # include "cipher_aes_ccm_hw_rv32i.inc" #else const PROV_CCM_HW *ossl_prov_aes_hw_ccm(size_t keybits) -- Gitee From 7c285cb69aac096fc2d59960a1cb94172b5138a1 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 6 Dec 2023 11:09:53 +0000 Subject: [PATCH 13/28] 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 83e1f11e18..444c5ca7a3 100644 --- a/crypto/bn/bn_gf2m.c +++ b/crypto/bn/bn_gf2m.c @@ -730,14 +730,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 db586b799107fb169ef853a7cf3edca0bf4eb88f Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 6 Dec 2023 11:19:24 +0000 Subject: [PATCH 14/28] 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 2ffff10ef1..20020cac42 100644 --- a/test/bntest.c +++ b/test/bntest.c @@ -910,6 +910,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 5936c105f6e5c82f4135fa66642dcdf3dfbcf163 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 6 Dec 2023 11:51:01 +0000 Subject: [PATCH 15/28] 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 33e288115b7d29c5624f1aea522ea527b6004332 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 6 Dec 2023 12:51:34 +0000 Subject: [PATCH 16/28] 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 49e770cd88..3f70f6223a 100644 --- a/test/http_test.c +++ b/test/http_test.c @@ -347,7 +347,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 d96d30181b881727c9f360323ef26fc3dbd59a13 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Sun, 3 Dec 2023 11:24:18 +0100 Subject: [PATCH 17/28] 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 a16318f37c..f93c98ac92 100644 --- a/apps/cms.c +++ b/apps/cms.c @@ -628,7 +628,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(); @@ -837,7 +838,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 a29c3f46ee6a9e78b2277e2d563cf1169157ae42 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Sun, 3 Dec 2023 11:34:37 +0100 Subject: [PATCH 18/28] 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 88b0475d2d..b59e14b0b5 100644 --- a/apps/smime.c +++ b/apps/smime.c @@ -484,7 +484,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 8793307e068b2b6846d6983fd7c095468db0c878 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Sun, 3 Dec 2023 11:29:52 +0100 Subject: [PATCH 19/28] 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 4fea16fa38..56a6ccea09 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 ad61cb659f06a704959497fb0bcdb588b6bc2849 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Sun, 3 Dec 2023 11:41:51 +0100 Subject: [PATCH 20/28] 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 2d61ac9a78..0490c39c25 100644 --- a/apps/lib/opt.c +++ b/apps/lib/opt.c @@ -726,7 +726,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 8a0e2197ab1891e56ce185a84ea2e8fb551f6f3c Mon Sep 17 00:00:00 2001 From: "Dr. David von Oheimb" Date: Fri, 27 Oct 2023 08:58:48 +0200 Subject: [PATCH 21/28] 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 81d407a4b8..2210600217 100644 --- a/doc/man7/provider-storemgmt.pod +++ b/doc/man7/provider-storemgmt.pod @@ -184,12 +184,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 9ecec5a888b93fec02846afd82a6f8e29fa14710 Mon Sep 17 00:00:00 2001 From: James Muir Date: Wed, 6 Dec 2023 16:49:11 -0500 Subject: [PATCH 22/28] 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 2392e5909c..799094da9b 100644 --- a/crypto/param_build.c +++ b/crypto/param_build.c @@ -255,9 +255,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; } @@ -274,9 +274,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 b5c13ed5ac6aba5a5fdc07069b47bab02114660e Mon Sep 17 00:00:00 2001 From: "fangming.fang" Date: Thu, 7 Dec 2023 06:17:51 +0000 Subject: [PATCH 23/28] 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 24/28] 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 29cefe673c..dae0de083a 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 90fcf217efa44b8cfeb8339d3b467929cfb04828 Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Mon, 11 Dec 2023 15:03:08 +0100 Subject: [PATCH 25/28] pkcs12: Do not forcibly load the config file This was added as part of commit e869c86 but later it was made unnecessary by commit 21f7a09. Fixes #22994 Reviewed-by: Neil Horman Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/23005) (cherry picked from commit 58eeb4350ca89c52d603b42119a0893129a25c09) Signed-off-by: fly2x --- apps/pkcs12.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/apps/pkcs12.c b/apps/pkcs12.c index 1fa0abd3d4..117b673643 100644 --- a/apps/pkcs12.c +++ b/apps/pkcs12.c @@ -14,7 +14,6 @@ #include #include "apps.h" #include "progs.h" -#include #include #include #include @@ -535,7 +534,6 @@ int pkcs12_main(int argc, char **argv) EVP_MD *macmd = NULL; unsigned char *catmp = NULL; int i; - CONF *conf = NULL; ASN1_OBJECT *obj = NULL; if ((options & (NOCERTS | NOKEYS)) == (NOCERTS | NOKEYS)) { @@ -681,12 +679,6 @@ int pkcs12_main(int argc, char **argv) if (!twopass) OPENSSL_strlcpy(macpass, pass, sizeof(macpass)); - /* Load the config file */ - if ((conf = app_load_config(default_config_file)) == NULL) - goto export_end; - if (!app_load_modules(conf)) - goto export_end; - if (jdktrust != NULL) { obj = OBJ_txt2obj(jdktrust, 0); } @@ -731,7 +723,6 @@ int pkcs12_main(int argc, char **argv) OSSL_STACK_OF_X509_free(certs); OSSL_STACK_OF_X509_free(untrusted_certs); X509_free(ee_cert); - NCONF_free(conf); ASN1_OBJECT_free(obj); ERR_print_errors(bio_err); goto end; -- Gitee From 2e83a0e4397f2fc5b6224b83fbcd62aa88dd1a1f Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Tue, 5 Dec 2023 14:50:01 -0500 Subject: [PATCH 26/28] Fix genstr/genconf option in asn1parse At some point the asn1parse applet was changed to default the inform to PEM, and defalt input file to stdin. Doing so broke the -genstr|conf options, in that, before we attempt to generate an ASN1 block from the provided genstr string, we attempt to read a PEM input from stdin. As a result, this command: openssl asn1parse -genstr OID:1.2.3.4 hangs because we are attempting a blocking read on stdin, waiting for data that never arrives Fix it by giving priority to genstr|genconf, such that, if set, will just run do_generate on that string and exit Reviewed-by: Hugo Landau Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/22957) (cherry picked from commit 749fcc0e3ce796474a15d6fac221e57daeacff1e) Signed-off-by: fly2x --- apps/asn1parse.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/asn1parse.c b/apps/asn1parse.c index 097b0cc1ed..6597a6180b 100644 --- a/apps/asn1parse.c +++ b/apps/asn1parse.c @@ -178,7 +178,7 @@ int asn1parse_main(int argc, char **argv) if ((buf = BUF_MEM_new()) == NULL) goto end; - if (informat == FORMAT_PEM) { + if (genstr == NULL && informat == FORMAT_PEM) { if (PEM_read_bio(in, &name, &header, &str, &num) != 1) { BIO_printf(bio_err, "Error reading PEM file\n"); ERR_print_errors(bio_err); -- Gitee From 6aec932ddd89bc4f0825dd2154029c171740a25a Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Tue, 5 Dec 2023 15:24:20 -0500 Subject: [PATCH 27/28] Harden asn1 oid loader to invalid inputs In the event that a config file contains this sequence: ======= openssl_conf = openssl_init config_diagnostics = 1 [openssl_init] oid_section = oids [oids] testoid1 = 1.2.3.4.1 testoid2 = A Very Long OID Name, 1.2.3.4.2 testoid3 = ,1.2.3.4.3 ====== The leading comma in testoid3 can cause a heap buffer overflow, as the parsing code will move the string pointer back 1 character, thereby pointing to an invalid memory space correct the parser to detect this condition and handle it by treating it as if the comma doesn't exist (i.e. an empty long oid name) Reviewed-by: Hugo Landau Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/22957) (cherry picked from commit a552c23c6502592c1b3c67d93dd7e5ffbe958aa4) Signed-off-by: fly2x --- apps/asn1parse.c | 2 +- crypto/asn1/asn_moid.c | 4 ++++ test/recipes/04-test_asn1_parse.t | 26 ++++++++++++++++++++++++++ test/test_asn1_parse.cnf | 12 ++++++++++++ 4 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 test/recipes/04-test_asn1_parse.t create mode 100644 test/test_asn1_parse.cnf diff --git a/apps/asn1parse.c b/apps/asn1parse.c index 6597a6180b..bf62f85947 100644 --- a/apps/asn1parse.c +++ b/apps/asn1parse.c @@ -178,7 +178,7 @@ int asn1parse_main(int argc, char **argv) if ((buf = BUF_MEM_new()) == NULL) goto end; - if (genstr == NULL && informat == FORMAT_PEM) { + if (genconf == NULL && genstr == NULL && informat == FORMAT_PEM) { if (PEM_read_bio(in, &name, &header, &str, &num) != 1) { BIO_printf(bio_err, "Error reading PEM file\n"); ERR_print_errors(bio_err); diff --git a/crypto/asn1/asn_moid.c b/crypto/asn1/asn_moid.c index 6f816307af..1e183f4f18 100644 --- a/crypto/asn1/asn_moid.c +++ b/crypto/asn1/asn_moid.c @@ -67,6 +67,10 @@ static int do_create(const char *value, const char *name) if (p == NULL) { ln = name; ostr = value; + } else if (p == value) { + /* we started with a leading comma */ + ln = name; + ostr = p + 1; } else { ln = value; ostr = p + 1; diff --git a/test/recipes/04-test_asn1_parse.t b/test/recipes/04-test_asn1_parse.t new file mode 100644 index 0000000000..f3af436592 --- /dev/null +++ b/test/recipes/04-test_asn1_parse.t @@ -0,0 +1,26 @@ +#! /usr/bin/env perl +# Copyright 2023 The OpenSSL Project Authors. All Rights Reserved. +# +# Licensed under the Apache License 2.0 (the "License"). You may not use +# this file except in compliance with the License. You can obtain a copy +# in the file LICENSE in the source distribution or at +# https://www.openssl.org/source/license.html + +use strict; +use OpenSSL::Test qw(:DEFAULT srctop_file); +use OpenSSL::Test::Utils; + +setup("test_asn1_parse"); + +plan tests => 3; + +$ENV{OPENSSL_CONF} = srctop_file("test", "test_asn1_parse.cnf"); + +ok(run(app(([ 'openssl', 'asn1parse', + '-genstr', 'OID:1.2.3.4.1'])))); + +ok(run(app(([ 'openssl', 'asn1parse', + '-genstr', 'OID:1.2.3.4.2'])))); + +ok(run(app(([ 'openssl', 'asn1parse', + '-genstr', 'OID:1.2.3.4.3'])))); diff --git a/test/test_asn1_parse.cnf b/test/test_asn1_parse.cnf new file mode 100644 index 0000000000..5f0305657e --- /dev/null +++ b/test/test_asn1_parse.cnf @@ -0,0 +1,12 @@ +openssl_conf = openssl_init + +# Comment out the next line to ignore configuration errors +config_diagnostics = 1 + +[openssl_init] +oid_section = oids + +[oids] +testoid1 = 1.2.3.4.1 +testoid2 = A Very Long OID Name, 1.2.3.4.2 +testoid3 = ,1.2.3.4.3 -- Gitee From d9173d11ba7ecfaf748d5986d5dc0837c6abab25 Mon Sep 17 00:00:00 2001 From: "Dr. David von Oheimb" Date: Sat, 2 Dec 2023 15:54:27 +0100 Subject: [PATCH 28/28] 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 11c31e0d69..9773af905f 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