Re: [openssl.org #2749] SSL_shutdown() doesn't need to ever return 0
John Gardiner Myers via RT wrote: There's no good reason for SSL_shutdown() to ever return a value of 0. The attached patch simplifies things. One point of view is: Maybe so. But this is how it has always worked and is documented as such. Your patch does not attempt to update the documentation which talks of the return value 0 which now won't exist. However my point of view is: Actually there is. It is important for OpenSSL to convey back to the application when it has successfully carried out all the following tasks: * to encode SSL control packet (with the way OpenSSL is imlemented this actually means to have flushed all outstanding application payload data down) * to enqueue SSL control packet * to push SSL control packet into BIO / kernel layers In reference to the data that makes up the SSL control packet indicating end-of-encrypted-stream. Any one of these operations might fail due to network conditions. Knowing this state has occurred is important if you want to call TCP shutdown(fd, SHUT_WR) on the underlying socket. Which is a TCP level end-of-write-stream indicator. A single return value of 1 can not indicate both points in the proceedings. * The point the SEND shutdown control packet has been push into BIO / kernel layers. * The point in the proceedings when both the SEND and the RECEIVE end-of-encrypted-stream control packet have been fully processed. So maybe for you it seems like a simplification as you have a simpler mental model of what is going on :) If there was a chance to start over I think a much better interface would be to return the state of the SSL_RECEIVED_SHUTDOWN and SSL_SENT_SHUTDOWN flags, i.e. just like SSL_get_shutdown() would instead of 0 or 1 scenarios that currently exist. Regards, Darryl __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
[openssl.org #2474] PATCH to provide cmd.exe (native) MinGW complication support against openssl-1.0.0d
This patch is against version openssl-1.0.0d.
Please find attached a patch that works for me. Works means it builds
without error, the test.bat executes the tests ok and even the make
install works to some degree. It has also been test built against
zlib zlib-dynamic options.
Background to this work. This work has been done to provide the QtJambi
project with a MinGW based build, this project are Java bindings library
for the GUI / platform toolkit Qt. Due to the nature and complexity of
the resulting project (a collection of multiple open source libraries
end up loaded into a Java JVM) toolchain compatibility and
interoperability is critical to this project.
This patch should be tested against other MinGW build environments, such
as using MinGW under CYGWIN shell or MSYS shell. To make sure that
process is not broken, also to see if the ms/mingw32.bat script still
works for them. But I guess no one builds this way when they have a
bash shell. So maybe this batch file can be exclusively for he purpose
of building with MinGW from cmd.exe.
$ git diff | diffstat
Configure | 11 +--
INSTALL.W32| 22 ++
ms/mingw32.bat | 41 -
ms/test.bat|2 ++
util/mk1mf.pl |7 +++
util/pl/Mingw32.pl | 10 ++
6 files changed, 74 insertions(+), 19 deletions(-)
File Notes:
Configure - This includes MinGW64 additions but another patch/ticket
will be opened for that target. I have used the term native-mingw to
mean from cmd.exe and without access to any Unix like shell. Compile
options brought roughly info line with those in util/pl/Mingw32.pl
however they are on the new target native-mingw so should not affect
anyone else.
INSTALL.W32 - Tried to update some documentation about mingw to clarify
builds with Unix like shell (CYGWIN/MSYS) to those without.
ms/mingw32.bat - This tries to detect if a bash.exe is present to setup
the correct argument to perl Configure ... this needs proper testing
with MinGW builds under CYGWIN/MSYS. There is provision for manually
setting up debug builds by editing the top of the mingw32.bat file, see
the comments. Additional errorlevel checks are put in place to fail
upon any error with any part of the process. It does not seem necessary
at all to perform any of the ASM generation since the mk1mf.pl has been
fixed to do the correct thing now.
ms/test.bat - This has been updated to allow optional first argument as
the directory to change into before executing the test EXE files.
util/mk1mf.pl - Multiple fixes here.
Fix the test target to not execute a cd out as one command and then
run ..\ms\test as another command. I don't think this even works on
Unix shell systems, you'd need to put them on the same line with a
semicolon for it to work. But on Windows it is more complicated so to
make the make test work out-the-box the directory to change into is
now passed as the first argument to the test.bat file.
Fix the install target to not have the shell glob pattern *.[ch] for
installing the include files. There are no *.c file to install. The
Windows copy command does not understand the *.[ch] glob syntax. The
Windows copy command returns error status of no files were found to
copy. So the *.c has been removed, with just *.h remaining.
util/pl/Mingw32.pl - Compile options tweaked. Missing
$asmtype='gaswin', maybe this is the reason why ms/mingw32.bat manually
generated the ASM, since the resulting ms/mingw32a.mak Makefile is
broken with working out-the-box without $asmtype being set. Added
$exep='.exe' since the make install breaks without it and the linker
auto-corrects the resulting exe name so doesn't care either way.
Darryl
diff --git a/Configure b/Configure
index 429ab2e..ccfa30c 100755
--- a/Configure
+++ b/Configure
@@ -506,6 +506,9 @@ my %table=(
# MinGW
mingw, gcc:-mno-cygwin -DL_ENDIAN -DWIN32_LEAN_AND_MEAN -fomit-frame-pointer -O3 -march=i486 -Wall::-D_MT:MINGW32:-lws2_32 -lgdi32 -lcrypt32:BN_LLONG ${x86_gcc_des} ${x86_gcc_opts} EXPORT_VAR_AS_FN:${x86_asm}:coff:win32:cygwin-shared:-D_WINDLL -DOPENSSL_USE_APPLINK:-mno-cygwin:.dll.a,
+# *-native-mingw means without Bash (no CYGWIN, no MSYS, no bash, no sh), see ms/mingw32.bat
+debug-native-mingw, gcc:-mno-cygwin -DL_ENDIAN -DDSO_WIN32 -DWIN32_LEAN_AND_MEAN -g2 -ggdb -march=i486 -Wall::-D_MT:MINGW32:-lws2_32 -lgdi32 -lcrypt32:BN_LLONG ${x86_gcc_des} ${x86_gcc_opts} EXPORT_VAR_AS_FN:${x86_asm}:coff:win32:cygwin-shared:-D_WINDLL -DOPENSSL_USE_APPLINK:-mno-cygwin:.dll.a,
+native-mingw, gcc:-mno-cygwin -DL_ENDIAN -DDSO_WIN32 -DWIN32_LEAN_AND_MEAN -fomit-frame-pointer -O3 -march=i486 -Wall::-D_MT:MINGW32:-lws2_32 -lgdi32 -lcrypt32:BN_LLONG ${x86_gcc_des} ${x86_gcc_opts} EXPORT_VAR_AS_FN:${x86_asm}:coff:win32:cygwin-shared:-D_WINDLL -DOPENSSL_USE_APPLINK:-mno-cygwin:.dll.a,
# As for OPENSSL_USE_APPLINK. Applink makes it
Re: [openssl.org #1833] [PATCH] Abbreviated Renegotiations
Robin Seggelmann via RT wrote: Note that since we need to retain binary compatibility between 1.0.0 and 1.0.1 we will need to either avoid having to add a new field to ssl.h or move it to the end of the structure. As things are any application accessing a field after the new member would misbehave. Can you cite the mechanism via which an application achieves this (misbehaving) ? Do you need a patch which moves the int renegotiate; to the end of the struct for 1.0.1? Which internal members of the openssl/ssl.h (struct ssl_st) are visible outside of the OpenSSL implementation (i.e. by the application) ? My understanding is that providing there are no macro's directly accessing members of the struct from application code the order issue is moot. If the application programmer has read ssl.h and decided he is going to access internal members of (struct ssl_st) directly, when it has not been documented as safe to do so; should he not be left to burn ? If there are functions/macros/mechanisms that can be compiled into application code which do access and expect structure members to be at specific offsets, WHY IS THIS THE DEFAULT ANYWAY ? i.e. why doesn't the application programmer have to define some -DOPENSSL_UNSAFE_DIRECT_ACCESS disable those accesses that indirect through a function (inside the OpenSSL implementation library) to those implemented as macros and therefore embedded inside applications. But first please confirm the API calls put at risk with you concern with this patch/feature. A larger concern to me is the increasing of the size of the (struct ssl_st) a matter you seem to place at a lower priority than struct member order. If it is possible and accepted usage that an application might allocate a fixed amount of storage, such as static global variables, local stack variables, embedding the (SSL) inside another application defined struct and use of sizeof(SSL). If this is a concern might it be useful to both: * Implement an API call that allows an application program to check the sizeof(SSL) it was compiled with against the runtime libraries implementation size (preferably in a convenient way, mostly assisted by header files and man page copy'n'paste snippet with a view of being future proof). * Reserve some extra headroom in the struct, if you think you need to increase the size during the lifetime of the ABI compatibility you wish to retain. * Document any restriction placed on the programmer when using the library. For example if storage for a specific type is not to be allocated statically (at compiled time). If you increase the size of the struct those applications that do allocate a fixed amount of storage based on openssl-1.0.0 will find that the OpenSSL library is scribbling on memory when it accesses the locations at the highest offsets of the new larger structure. The application will not have allocated quite enough memory and so random problems will occur. Can I suggest you combine the storage area used by these flags so no size increase is necessary. The extra instruction Logical And/Or masking of a register value can be done very cheaply and the patch does not appear to affect any critical performance path with bulk transfer. Darryl __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
[openssl.org #1891] SSL_shutdown() corner case issues
I hope I am using the RT correctly in this manner.
At this time I am seeking approval / consensus / agreement in principal
on this patch inclusion. Once that is reached then I'd like to work on
related matters about this issue like updating man pages / API
documentation, having a new FAQ item, updating example code in openssl
builtin apps, authoring a release note comment, all the stuff I don't
need to do for a private OpenSSL release but we do for the public one.
This ticket is request submission of a patch:
http://marc.info/?l=openssl-devm=115154030723033q=p3 [Direct
download for patch *.diff format from previous email attachment, applied
to openssl-0.9.8b ]
http://marc.info/?t=11515400401r=1w=2 [Path covering email thread]
Here is a posting made of a tool called sslregress which can expose
this specific problem and verify the fix:
http://marc.info/?t=11512908121r=1w=2 [The thread discussing
sslregress]
http://marc.info/?t=11515396263r=1w=2 [The thread with the patch]
Here are related threads of discussion over time on the matter:
http://marc.info/?l=openssl-usersm=115088475305680w=2 [Post my me
on the initial problem]
http://marc.info/?t=12124981201r=1w=2 [Thor Lance came to
similar conclusions to me about a fix from his viewpoint and accepts my
more detail analysis of the problem as being more correct]
http://marc.info/?t=11910906151r=1w=2 [Discussion about the
issues around SSL_shutdown]
http://marc.info/?l=openssl-devm=116417462427122w=2 [2nd call for
assistance for inclusion Nov 2006]
http://marc.info/?l=openssl-devm=116525974320575w=2 [3rd call for
assistance for inclusion Dec 2006]
The issue in question is over problems in relation to SSL_shutdown()
functionality and also API inadequacies surrounding that issue:
* The SSL_shutdown() is not handling all possible soft-error conditions
correctly, specifically if the lower-layer buffer (BIO/kernel) is under
a full-buffer on write condition (EAGAIN) whilst SSL_shutdown() attempts
to send/push-down the ssl shutdown notify packet from the SSL stream.
My memory is hazy of the exact details of this problem, as my main
driving issue was the API inadequacy. The thought was that is the last
byte of the encoded ssl shutdown notify packet did not make it into
the BIO/kernel buffer the first time, then this is never sent and
therefore connections may hang open.
* As an OpenSSL libssl.so API user many application want to know when
the OpenSSL library no longer requires use of the write half of a
socket. This is so that a TCP shutdown(sockfd, SHUTDOWN_WR) can be
issued reliability once that shutdown progress checkpoint has been
passed. This is an API inadequacy.
The SSL_shutdown() method is just a restartable state machine, that
enters the following logic sequence each time it is called:
* Mark the SSL handle as not allowing any more application data to be
sent to it (if not already marked).
* Flush all existing application data packets from BIO to kernel (if
data exists and the flush did not complete return -1/WANT_WRITE and
return to caller).
* Write out / enqueue the shutdown notify packet (it not already
enqueued during a previous SSL_shutdown invocation).
* Flush shutdown notify packet from libssl to BIO/Kernel layer below
(if data still exists after flush; i.e. the flush did not complete
return -1/WANT_WRITE and return to caller)
* A this point due to the shutdown notify having been flushed 100% to
from BIO to kernel we may be allowed to return the value 0 from
SSL_shutdown(), if the following conditions are met; a) we have not
previously returned 0 from SSL_shutdown before; b) we never cover up /
mask a hard-error condition, e.g. -1/SSL_ERROR_{SSL,WHATEVER}; c) we
would have returned -1/WANT_READ (sorry this is a superfluous rule and
self-referencing to emphasis the point)
* Read in / process packets from socket (if there are application data
packets or no data exists return -1/WANT_READ by default or 0 according
the the rules on zero-return above)
* We received the shutdown notify packet, mark SSL handle that it has
been received.
* Return successful SSL_shutdown() state (always return 1 regardless of
any zero-return rules above, it is legal for the API to completely skip
returning zero if we get to here in the state machine).
I hope most OpenSSL API users will agree that the above does not
actually change the externally visible mechanics of how SSL_shutdown
works. For all intents and purposes it works the same as it did before
but applications may now commonly see -1/WANT_READ getting returned and
-1/WANT_WRITE very rarely getting returned, the simplest modification to
their existing application code to get old return code would be to treat
the -1/WANT_READ and -1/WANT_WRITE soft-errors as-if zero was returned.
A simple adjustment to make once understood.
Impact on existing code users:
* My guess
