Tongsuo Performance up and fix some bugs[up to openssl 3.0.13]#808
Open
kafei-cy wants to merge 69 commits intoTongsuo-Project:8.4.1-devfrom
Open
Tongsuo Performance up and fix some bugs[up to openssl 3.0.13]#808kafei-cy wants to merge 69 commits intoTongsuo-Project:8.4.1-devfrom
kafei-cy wants to merge 69 commits intoTongsuo-Project:8.4.1-devfrom
Conversation
This otherwise breaks compilation of applications using ssl.h on MingW. (Merged from openssl/openssl#22316)
Test case amended from code initially written by Bernd Edlinger. (Merged from openssl/openssl#22421)
Include 3 commits: 1. x509_print_ex:Use correct constant for nmflag comparison 2. Fix X509_REQ_print_ex bug 3. x509_print_ex: Remove unused setting when XN_FLAG_COMPAT is set (Merged from openssl/openssl#19963)
When X509_ALGOR_set0() fails, ownership of the the ASN1 object "los"
(label octet string) has not been passed on to the X509_ALGOR object
"oaep->pSourceFunc", so we need to free "los" in that case.
Check return value of X509_ALGOR_set0(), change the scope of "los" and
ensure it is freed on failure (on success, set it to NULL so it is not
freed inside the function).
Testing:
You can use the following script to test cms encryption with rsa-oaep:
#!/bin/bash -x
OSSLCMD="apps/openssl"
# check we are calling the right openssl app
LD_LIBRARY_PATH=. valgrind $OSSLCMD version
echo "this is a confidential message." > msg.txt
LD_LIBRARY_PATH=. valgrind $OSSLCMD cms -encrypt -in msg.txt \
-stream -out msg.txt.cms \
-recip test/smime-certs/smrsa1.pem \
-keyopt rsa_padding_mode:oaep \
-keyopt rsa_oaep_md:sha256 \
-keyopt rsa_oaep_label:deadbeef
(Merged from openssl/openssl#22556)
The `aarch64-linux-android33-clang` cross-compiler (v14.0.6)
complains twice about an unsupported '%n' format specifier,
preventing a successful `--strict-warnings` build:
error: '%n' specifier not supported on this platform [-Werror,-Wformat]
BIO_snprintf(buf, buflen, "%s%s%n%08x.%s%d",
This is a false positive, because BIO_snprintf() implements its
own format parsing (which is implemented in the _dopr() function).
This commit fixes the problem by rewriting the code to dispense with
the dubious '%n' format specifier. As a side-effect, the code becomes
a little bit more comprehensible and self-explaining.
(Merged from openssl/openssl#22511)
Fix spelling $cppfags2 => $cppflags2 in file Configurations/windows-makefile.tmpl (Merged from openssl/openssl#22771)
Several error cases leak either the X509 object or the pkey or the danetls_record object. (Merged from openssl/openssl#22743)
When PKCS7_add_signed_attribute fails, the ASN1_STRING object may be leaked. (Merged from openssl/openssl#22744)
When an error happens after cms_encode_Receipt the ASN1_OCTET_STRING object "os" may be leaked. (Merged from openssl/openssl#22758)
This may happen when ssl_cert_dup calls custom_exts_copy, where a possible memory allocation error causes custom_exts_free to be called twice: once in the error handling of custom_exts_copy and a second time in the error handling of ssl_cert_dup. (Merged from openssl/openssl#22772)
When PKCS7_add_signed_attribute fails, the ASN1_TIME object may be leaked when it was not passed in as input parameter. (Merged from openssl/openssl#22772)
ctx->propq that strdup from input parameter propq in sm2sig_newctx, is not released. It should be released in sm2sig_freectx and copied to dstctx in sm2sig_dupctx. And dstctx->id and dstctx->propq should be set NULL to avoid releasing id/propq of srcctx when err occurs. (Merged from openssl/openssl#22796)
Include 4 commits: 1. When changing IV length invalidate previously set IV 2. Return error if key is not set 3. Add negative test for iv length change 4. Add negative test for key length change (Merged from openssl/openssl#22613)
if we allocate a new hm_frament in dtls1_buffer_message with dtls1_hm_fragment_new, the returned fragment contains uninitalized data in the msg_header field. If an error then occurs, and we free the fragment, dtls_hm_fragment_free interrogates the msg_header field (which is garbage), and potentially references undefined values, or worse, accidentally references available memory that is not owned, leading to various corruptions. (Merged from openssl/openssl#2261)
…uffer When we are clearing the sent messages queue we should ensure we free any old enc_write_ctx/write_hash that are no longer in use. Previously this logic was in dtls1_hm_fragment_free() - but this can end up freeing the current enc_write_ctx/write_hash under certain error conditions. (Merged from openssl/openssl#2261)
Instead of trying to move the doomed sct back to the src stack, which may fail as well, simply free the sct object, as the src list will be deleted anyway. (Merged from openssl/openssl#22762)
Include 2 commits: 1. Fix freshly introduced double-free. 2. Add last missing TLSA usage/selector/mtype test case (Merged from openssl/openssl#22821)
The little-endian optimization is doing some type-punning in a way violating the C standard aliasing rule by loading or storing through a lvalue with type "unsigned int" but the memory location has effective type "unsigned long" or "unsigned long long" (BN_ULONG). Convert these accesses to use memcpy instead, as memcpy is defined as-is "accessing through the lvalues with type char" and char is aliasing with all types. GCC does a good job to optimize away the temporary copies introduced with the change. Ideally copying to a temporary unsigned int array, doing the calculation, and then copying back to `r_d` will make the code look better, but unfortunately GCC would fail to optimize away this temporary array then. I've not touched the LE optimization in BN_nist_mod_224 because it's guarded by BN_BITS2!=64, then BN_BITS2 must be 32 and BN_ULONG must be unsigned int, thus there is no aliasing issue in BN_nist_mod_224. (Merged from openssl/openssl#22816)
And clean up partially created choice objects, which have still the default type = -1 from ASIdentifierChoice_new(). (Merged from openssl/openssl#22745)
When the CMS_ReceiptRequest cannot be created, the rct_to and rct_from may be leaked. (Merged from openssl/openssl#22742)
Add null check to cmac_size(). This avoids a seg-fault encountered with cmac when EVP_MAC_CTX_get_mac_size() is called before init. Extend mac testing in evp_test.c to check that the sizes returned by EVP_MAC_CTX_get_mac_size() before and after init make sense (this also ensures that we no longer seg-fault). (Merged from openssl/openssl#22858)
The return code of X509_ALGOR_set0 was not checked, and if it fails the key will be leaked. (Merged from openssl/openssl#22741)
(Merged from openssl/openssl#22918)
(Merged from openssl/openssl#22919)
The OPENSSL_DIR_end was missing in case of error. (Merged from openssl/openssl#22920)
The ASN1_OBJECT otmp was leaked if X509_VERIFY_PARAM_add0_policy fails. (Merged from openssl/openssl#22922)
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. (Merged from openssl/openssl#22967)
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) (Merged from openssl/openssl#23034)
If you decrypt a random input using RSAES-PKCS-v1_5, then there is a non-negligible chance that the result will look like a valid plaintext (that is why RSAES-PKCS-v1_5 shouldn't be used anymore). This was the cause of an intermittent failure in a test that did a cms-encrypt operation targetting multiple recipients. The failure happened during key-only decrypt. The recipient decrypts every RSA ciphertext -- only one is supposed to decrypt successfully, which would reveal the right content-key. Occassionally, more than one decrypted successfully. Update the test by specifying the recipient cert in the decrypt op (this avoids looping over all RSA ciphertexts). Add a new test to get coverage for key-only decrypt, but use RSA-OAEP during the encrypt op. Fixes openssl/project#380 Testing: $ make TESTS='test_cms' test (Merged from openssl/openssl#23055)
Include 3 commits: 1. Always apply all configuration settings from the ssl section 2. Test that incorrect entry in the ssl section is not fatal 3. Consolidate raising errors in SSL_CONF_cmd() (Merged from openssl/openssl#23048)
The newly introduced test case do not work
when configured with no-des, fix that by
choosing -aes128 as cipher.
Fixes ffed597882ba ("cms: avoid intermittent test failure")
(Merged from openssl/openssl#23086)
Add a missing check of the return code of X509_ALGOR_set0. otherwise a memory leak may occur. (Merged from openssl/openssl#22999)
Add a missing check of the return code of X509_ALGOR_set0, otherwise the ASN1_STRING object wrap_str may be leaked. (Merged from openssl/openssl#22998)
Fixed - Windows compilation issue - unbale to find correct definitions of _InterlockedExchangeAdd. Issue number - openssl/openssl#21080 Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from openssl/openssl#23087)
Include 3 commits: 1. Fix memleak in rsa_cms_decrypt 2. Limit RSA-OAEP related functions to RSA keys only 3. Add missing settable entry OSSL_ASYM_CIPHER_PARAM_OAEP_DIGEST_PROPS for RSA asym (Merged from openssl/openssl#20319)
…openssl#22898] If a malformed config file is provided such as the following: openssl_conf = openssl_init [openssl_init] providers = provider_sect [provider_sect] = provider_sect The config parsing library will crash overflowing the stack, as it recursively parses the same provider_sect ad nauseum. Prevent this by maintaing a list of visited nodes as we recurse through referenced sections, and erroring out in the event we visit any given section node more than once. Note, adding the test for this revealed that our diagnostic code inadvertently pops recorded errors off the error stack because provider_conf_load returns success even in the event that a configuration parse failed. The call path to provider_conf_load has been updated in this commit to address that shortcoming, allowing recorded errors to be visibile to calling applications. (Merged from openssl/openssl#23120)
If a name is passed to EVP_<OBJ>_fetch of the form: name1:name2:name3 The names are parsed on the separator ':' and added to the store, but during the lookup in inner_evp_generic_fetch, the subsequent search of the store uses the full name1:name2:name3 string, which fails lookup, and causes subsequent assertion failures in evp_method_id. instead catch the failure in inner_evp_generic_fetch and return an error code if the name_id against a colon separated list of names fails. This provides a graceful error return path without asserts, and leaves room for a future feature in which such formatted names can be parsed and searched for iteratively Add a simple test to verify that providing a colon separated name results in an error indicating an invalid lookup. (Merged from openssl/openssl#23110)
When using pbkdf1 key deriviation, it is possible to request a key length larger than the maximum digest size a given digest can produce, leading to a read of random stack memory. fix it by returning an error if the requested key size n is larger than the EVP_MD_size of the digest (Merged from openssl/openssl#23174)
Include 7 commits: 1. make inability to dup/clone ciphers an error 2. Add dupctx support to aead ciphers 3. implement dupctx for aes_WRAP methods 4. implement dupctx for chacha20_poly1305 5. Add dupctx support to rc4_hmac_md5 algo 6. Fix a key repointing in various ciphers 7. Also with SM4 for Tongsuo, delete some codes tend to CI error. (Merged from openssl/openssl#23102)
There are several points during x509 extension creation which rely on configuration options which may have been incorrectly parsed due to invalid settings. Preform a value check for null in those locations to avoid various crashes/undefined behaviors (Merged from openssl/openssl#23183)
kdf_pbkdf1_do_derive stores key derivation information in a stack variable, which is left uncleansed prior to returning. Ensure that the stack information is zeroed prior to return to avoid potential leaks of key information (Merged from openssl/openssl#23194)
Fixes Coverity 1560046 (Merged from openssl/openssl#23211)
Include 2 commits: 1. Fix a possible memory leak in sxnet_v2i 2. Fix a similar memory leak in SXNET_add_id_INTEGER (Merged from openssl/openssl#23234)
Include 3 commits: 1. Fix partial block encryption in cfb and ofb for s390x 2. Fix partial block encryption in cfb and ofb for s390x (legacy) 3. Add tests for re-using cipher contexts (Merged from openssl/openssl#23201)
When parsing the stable section of a config such as this: openssl_conf = openssl_init [openssl_init] stbl_section = mstbl [mstbl] id-tc26 = min Can lead to a SIGSEGV, as the parsing code doesnt recognize min as a proper section name without a trailing colon to associate it with a value. As a result the stack of configuration values has an entry with a null value in it, which leads to the SIGSEGV in do_tcreate when we attempt to pass NULL to strtoul. Fix it by skipping any entry in the config name/value list that has a null value, prior to passing it to stroul (Merged from openssl/openssl#22988)
If the value of a->length is large (>= 2^12), then an integer overflow will occur for the signed type, which according to the C standard is UB. (Merged from openssl/openssl#23274)
if the private key is output to stdout using the HARNESS_OSSL_PREFIX,
out is a stack of BIOs and must therefore free'd using BIO_free_all.
Steps to reproduce:
$ HARNESS_OSSL_PREFIX=x OPENSSL_CONF=apps/openssl.cnf util/shlib_wrap.sh apps/openssl req -new -keyout - -passout pass: </dev/null
[...]
Direct leak of 128 byte(s) in 1 object(s) allocated from:
0x7f6f692b89cf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
0x7f6f686eda00 in CRYPTO_malloc crypto/mem.c:202
0x7f6f686edba0 in CRYPTO_zalloc crypto/mem.c:222
0x7f6f68471bdf in BIO_new_ex crypto/bio/bio_lib.c:83
0x7f6f68491a8f in BIO_new_fp crypto/bio/bss_file.c:95
0x555c5f58b378 in dup_bio_out apps/lib/apps.c:3014
0x555c5f58f9ac in bio_open_default_ apps/lib/apps.c:3175
0x555c5f58f9ac in bio_open_default apps/lib/apps.c:3203
0x555c5f528537 in req_main apps/req.c:683
0x555c5f50e315 in do_cmd apps/openssl.c:426
0x555c5f4c5575 in main apps/openssl.c:307
0x7f6f680461c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
SUMMARY: AddressSanitizer: 128 byte(s) leaked in 1 allocation(s).
(Merged from openssl/openssl#23365)
The failure would be caught later on, so this went unnoticed, until someone tried with just one hex digit, which was simply ignored. (Merged from openssl/openssl#23374)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
这个PR会一直持续到Tongsuo性能提升至openssl3.5.4,目前进行的修改为:
openssl3.0.12至openssl3.0.13 共168个补丁,除去Tongsuo已移除的、已合并的补丁,合并为69个commit
修复方式:
对openssl3.0.2到3.0.13所有的commit进行审查,将对Tongsuo有意义的补丁保留,在本地fork的Tongsuo中进行修改
补丁内容介绍:
详细见各commit中描述
本地已通过CI。