[openssl-dev] [openssl.org #4094] Nonsensical pointer comparison in PACKET_buf_init
As of 2015-10-14, the function PACKET_buf_init in ssl/packet_locl.h reads: static inline int PACKET_buf_init(PACKET *pkt, unsigned char *buf, size_t len) { /* Sanity check for negative values. */ if (buf + len < buf) return 0; pkt->curr = buf; pkt->remaining = len; return 1; } Link: https://github.com/openssl/openssl/blob/310115448188415e270bb0bef958c7c130939838/ssl/packet_locl.h#L113 The rules of pointer arithmetics are such that the condition (buf + len < buf) is always false when it is defined. A modern compiler, or even an old compiler, could suppress the entire conditional from the generated code without a second thought and without a warning. Please read https://lwn.net/Articles/278137/ . Note that in that very similar instance, the GCC developers did not acknowledge any bug in their compiler, did not change the compiler's behavior, and simply regretted that "their project has been singled out in an unfair way". That LWN story is not a about a compiler bug, or in any case, it is about a compiler bug that is here to stay. And according to GCC developers, to be common to several C compilers. As far as I can tell, no current compiler recognizes that the very same reasoning as in that old LWN story justifies the suppression of the conditional. I tested the compilers currently available on https://gcc.godbolt.org . However, any compiler willing to miscompile the examples in the LWN article would gladly miscompile the function PACKET_buf_init given the chance. If the function is intended to return 0 for large values of len, then the test should look like: if (len > (size_t)(SIZE_MAX / 2)) ... Here I have chosen a constant that happens to give a behavior close to the one obtained by luck with current compilers. If another constant makes sense, then that other constant can be used. The current implementation is an invitation for the compiler to generate code that, even when len is above the limit, sets pkt->curr to buf, sets pkt->remaining to len, and returns 1, which is not what is intended, and could have serious security-related consequences latter on. If, as the comment implies, the function is not intended to be called with a len so large that it causes (uintptr_t)buf + len to be lower than (uintptr_t)buf, then please, please, please simplify the function by removing this nonsensical "sanity check", and make this function, which cannot fail, return void-as the history of the rest of OpenSSL shows, remembering of testing all error codes returned by called functions is difficult enough, even when only functions that have reason to fail return such codes. PACKET_buf_init is called with (size_t)-1 for len in test/packettest.c: https://github.com/openssl/openssl/blob/310115448188415e270bb0bef958c7c130939838/test/packettest.c#L341 Since PACKET_buf_init could no longer fail, that test could be simplified, which is good. If a sanity check is indispensable in PACKET_buf_init, but is really a check for something not supposed to happen, then in order to keep the type returned by the function as void, the check could be expressed as: if (len > (size_t)(SIZE_MAX / 2)) abort(); or assert (len <= (size_t)(SIZE_MAX / 2)); ___ openssl-bugs-mod mailing list openssl-bugs-...@openssl.org https://mta.openssl.org/mailman/listinfo/openssl-bugs-mod___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [openssl.org #4093] AutoReply: Problem loading engine from config
Hello! The attached patch fixes it. On Wed, Oct 14, 2015 at 10:10 PM, The default queue via RT wrote: > > Greetings, > > This message has been automatically generated in response to the > creation of a trouble ticket regarding: > "Problem loading engine from config", > a summary of which appears below. > > There is no need to reply to this message right now. Your ticket has been > assigned an ID of [openssl.org #4093]. > > Please include the string: > > [openssl.org #4093] > > in the subject line of all future correspondence about this issue. To do > so, > you may reply to this message. > > Thank you, > r...@openssl.org > > - > Hello, > > I have a problem when I load an engine from config file in master. > > OpenSSL cmdline: /home/build/openssl-shell/openssl/apps/openssl dgst > -md_gost94 dgst.dat > > Error configuring OpenSSL modules > 47445915269832:error:25066067:DSO support routines:DLFCN_LOAD:could not > load the shared library:dso_dlfcn.c:172:filename(libengines.so): > libengines.so: cannot open shared object file: No such file or directory > 47445915269832:error:25070067:DSO support routines:DSO_load:could not load > the shared library:dso_lib.c:227: > 47445915269832:error:0E07506E:configuration file > routines:MODULE_LOAD_DSO:error loading dso:conf_mod.c:270:module=engines, > path=engines > 47445915269832:error:0E076071:configuration file > routines:MODULE_RUN:unknown module name:conf_mod.c:212:module=engines > > The openssl.cnf file I use is: > > > openssl_conf = openssl_def > [openssl_def] > engines = engine_section > [engine_section] > gost=gost_section > [gost_section] > dynamic_path=/home/build/openssl-shell/openssl/engines/ccgost/libgost.so > engine_id=gost > default_algorithms = ALL > > > The gost engine I build is from the master. > > If I delete the lines > > engines = engine_section > [engine_section] > > I get another error: > > dgst: Unknown digest md_gost94 > dgst: Use -help for summary. > > The behavior seems to be changed after the commit > > https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=a0a82324f965bbcc4faed4e1ee3fcaf81ea52166 > > Thank you! > > -- > SY, Dmitry Belyavsky > > -- SY, Dmitry Belyavsky --- a/apps/openssl.c +++ b/apps/openssl.c @@ -175,6 +175,10 @@ static int apps_startup() ERR_load_crypto_strings(); ERR_load_SSL_strings(); +OPENSSL_load_builtin_modules(); +#ifndef OPENSSL_NO_ENGINE +ENGINE_load_builtin_engines(); +#endif if (!app_load_modules(NULL)) { ERR_print_errors(bio_err); BIO_printf(bio_err, "Error loading default configuration\n"); @@ -183,12 +187,8 @@ static int apps_startup() OpenSSL_add_all_algorithms(); OpenSSL_add_ssl_algorithms(); -OPENSSL_load_builtin_modules(); setup_ui_method(); /*SSL_library_init();*/ -#ifndef OPENSSL_NO_ENGINE -ENGINE_load_builtin_engines(); -#endif return 1; } ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
[openssl-dev] [openssl.org #4093] Problem loading engine from config
Hello, I have a problem when I load an engine from config file in master. OpenSSL cmdline: /home/build/openssl-shell/openssl/apps/openssl dgst -md_gost94 dgst.dat Error configuring OpenSSL modules 47445915269832:error:25066067:DSO support routines:DLFCN_LOAD:could not load the shared library:dso_dlfcn.c:172:filename(libengines.so): libengines.so: cannot open shared object file: No such file or directory 47445915269832:error:25070067:DSO support routines:DSO_load:could not load the shared library:dso_lib.c:227: 47445915269832:error:0E07506E:configuration file routines:MODULE_LOAD_DSO:error loading dso:conf_mod.c:270:module=engines, path=engines 47445915269832:error:0E076071:configuration file routines:MODULE_RUN:unknown module name:conf_mod.c:212:module=engines The openssl.cnf file I use is: openssl_conf = openssl_def [openssl_def] engines = engine_section [engine_section] gost=gost_section [gost_section] dynamic_path=/home/build/openssl-shell/openssl/engines/ccgost/libgost.so engine_id=gost default_algorithms = ALL The gost engine I build is from the master. If I delete the lines engines = engine_section [engine_section] I get another error: dgst: Unknown digest md_gost94 dgst: Use -help for summary. The behavior seems to be changed after the commit https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=a0a82324f965bbcc4faed4e1ee3fcaf81ea52166 Thank you! -- SY, Dmitry Belyavsky ___ openssl-bugs-mod mailing list openssl-bugs-...@openssl.org https://mta.openssl.org/mailman/listinfo/openssl-bugs-mod___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
[openssl-dev] [openssl.org #3896] unable to install to openssldir
Nope, not doing anything wrong. makedepend is bust and can't find the headers. Our clang and OS/X configurations were a bit off - I've changed them to use clang for 'make depend' as well when clang is the compiler, see commit c97c7f8d53dda12f4fda24fc7542281999df97f6. Cheers, Emilia ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
[openssl-dev] [openssl.org #4088] RE: [Bug] Openssl caused CPU high to 100%
OK thanks for the update. Ticket closed. Steve. -- Dr Stephen N. Henson. OpenSSL project core developer. Commercial tech support now available see: http://www.openssl.org ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] who wants to fix travis builds?
>>> I've opened the following PR to add support for GCC v5 and >>> address sanitizer (not sure if we want valgrind as well...): >>> https://github.com/openssl/openssl/pull/429 I've commented there on other -fsanitize options as well as about option to execute them without --debug and/or --strict-warnings. But I failed to recall all the details from last time the question was risen in context of RT#3422 through 3324. -fsanitize=undefined is expected to work only with --strict-warnings (or more specifically with -DPEDANTIC) and I wouldn't be surprised if -fsanitize=memory works only with -DPURIFY. ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev