Re: [openssl.org #1585] NIST CMAC, AES-CCM and AES-GCM modes
AES_CCM has a real well duh! type bug on systems where long is 32 bits. The compiler will probably tell you where it is. Sorry - fix it a couple of days. Peter Peter Waltenberg __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
[openssl.org #1587] DTLS: ChangeCipherSpec is causing handshake message sequence number to increment
Hi,
Since ChangeCipherSpec is not of handshake message type, the handshake
message sequence number should not be incremented. Only the record level
sequence number shall be incremented.
Proposed patch attached modifies both the TX side and the RX side.
Thanks,
Alex
Hi,Since ChangeCipherSpec is not of handshake message type, the handshake message sequence number should not be incremented. Only the record level sequence number shall be incremented.Proposed patch attached modifies both the TX side and the RX side.
Thanks,Alex
Index: ssl/d1_both.c
===
RCS file: /data1/Repository/openssl/ssl/d1_both.c,v
retrieving revision 1.4.2.4
diff -u -r1.4.2.4 d1_both.c
--- ssl/d1_both.c 30 Sep 2007 21:20:59 - 1.4.2.4
+++ ssl/d1_both.c 11 Oct 2007 00:13:25 -
@@ -532,7 +532,7 @@
int i,al;
struct hm_header_st msg_hdr;
unsigned long overlap;
-
+
/* see if we have the required fragment already */
if (dtls1_retrieve_buffered_fragment(s, l))
{
@@ -814,20 +814,27 @@
{
p=(unsigned char *)s-init_buf-data;
*p++=SSL3_MT_CCS;
- s-d1-handshake_write_seq = s-d1-next_handshake_write_seq;
- s-d1-next_handshake_write_seq++;
- s-init_num=DTLS1_CCS_HEADER_LENGTH;
-
if (s-client_version == DTLS1_BAD_VER)
{
+ s-d1-handshake_write_seq = s-d1-next_handshake_write_seq;
+s-d1-next_handshake_write_seq++;
+s-init_num=DTLS1_CCS_HEADER_LENGTH;
+
s2n(s-d1-handshake_write_seq,p);
s-init_num+=2;
- }
- s-init_off=0;
+s-init_off=0;
- dtls1_set_message_header_int(s, SSL3_MT_CCS, 0,
- s-d1-handshake_write_seq, 0, 0);
+dtls1_set_message_header_int(s, SSL3_MT_CCS, 0,
+s-d1-handshake_write_seq, 0, 0);
+ }
+else
+{
+s-init_num=DTLS1_CCS_HEADER_LENGTH;
+s-init_off=0;
+dtls1_set_message_header_int(s, SSL3_MT_CCS, 0,
+0, 0, 0);
+}
/* buffer the message to handle re-xmits */
dtls1_buffer_message(s, 1);
Index: ssl/d1_pkt.c
===
RCS file: /data1/Repository/openssl/ssl/d1_pkt.c,v
retrieving revision 1.4.2.9
diff -u -r1.4.2.9 d1_pkt.c
--- ssl/d1_pkt.c 3 Oct 2007 10:18:06 - 1.4.2.9
+++ ssl/d1_pkt.c 11 Oct 2007 00:09:39 -
@@ -1003,11 +1003,14 @@
if (!ssl3_do_change_cipher_spec(s))
goto err;
- /* do this whenever CCS is processed */
- dtls1_reset_seq_numbers(s, SSL3_CC_READ);
+/* do this whenever CCS is processed */
+dtls1_reset_seq_numbers(s, SSL3_CC_READ);
- /* handshake read seq is reset upon handshake completion */
- s-d1-handshake_read_seq++;
+ if (s-client_version == DTLS1_BAD_VER)
+ {
+/* handshake read seq is reset upon handshake completion */
+s-d1-handshake_read_seq++;
+};
goto start;
}
Re: make SSL_shutdown work with non-blocking BIOs
Darryl Miles wrote:
David Schwartz wrote:
If I'm misunderstanding the man page and/or the source code
please speak up.
My man page says:
If the underlying BIO is non-blocking, SSL_shutdown() will also
Yes but what SSL_shutdown() actually does is always return 0
This discussion a week ago made me wonder how SSL_shutdown() could work
in our non-blocking I/O codebase.
I just got around to check, and lo, I found the following code snippet:
// HACK! -- There was a bug where 'ssl-handler' took 100% CPU between remote
// disconnect and local peer close(). This was not investigated to the bitter
// end, but the severe suspicion exists that the OpenSSL library does not
// react properly with a half-open connection (read side closed, write side
open)
// with non-blocking sockets. The following 5 lines are a least-invasive fix.
// The original control flow of the program is still intact, even though a
// part is now short-circuited here.
if (st-ssl_in_closed) {
shutdown(st-local_socket, SHUT_RD); // Tell local endpoint that we don't
read anymore
st-local_in_closed = true; // Fake lack of SSL-output-still-to-be-sent
...
So I can add one more voice to the choir: the current SSL_shutdown() API
appears to give trouble to every non-blocking developer (I remember I
lost serious time noticing + tracking down this 100% CPU bug), and
afterwards things still don't really work right.
With regards,
Nanno
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
Nanno Langstraat wrote: So I can add one more voice to the choir: the current SSL_shutdown() API appears to give trouble to every non-blocking developer (I remember I lost serious time noticing + tracking down this 100% CPU bug), and afterwards things still don't really work right. I can't immediately think of a reason why you'd get 100% CPU, except with a badly constructed event loop (even with non-blocking I/O in use) much as I'd like to see your support :). The issue I am claiming is that the event loop has no way of knowing if the SSL_shutdown() managed to write and commit the entire shutdown notify packet into the BIO layer as the SSL_shutdown() return value will never indicate a -1/WANT_WRITE error condition, it always returns a value as-if everything has proceeded fine. It also never returns -1/WANT_READ when it has successfully committed the shutdown notify packet to the BIO layer but it now waiting to receive and decode a shutdown notify packet from the remote end. Knowing the exact state is important for non-blocking, the matters that concern a non-blocking user are: 1) Retrying the SSL_shutdown() when all the necessary data isn't yet been written as we need to get the send shutdown notify packet on the wire. 2) Knowing that all the necessary data has been written into the BIO layer so we can now take action to shutdown the sending side of the socket (at socket level, like with shutdown()) and cleanup that half, whilst leaving open the receiving half since we still wish to see the recvd shutdown notify packet. 3) Allowing an application driven timeout to start from the moment we know we committed the send shutdown notify packet to the BIO layer, so we can enforce a timeout based on when we wrote the data to the wire as opposed to when we wanted to start shutdown proceedings. Maybe you got 100% CPU because you were using SSL_shutdown() to look for a return value of 1, meaning its all over. My application uses SSL_read() even after the SSL_shutdown() has been called once. There are 2 reasons for this: 1) It is necessary to read (and discard/sink it if you don't want it) all the encrypted application data that appears before the shutdown. If you stopped calling SSL_read() then I guess 100% CPU results because FD_ISSET(fd, fdread) always signals there is data and calling SSL_shutdown() alone may not clear it because there is application data waiting to be read that is stalling the inbound stream processing. 2) SSL_read() already has a return value -1/ZERO_RETURN which indicates end-of-stream. You may then call SSL_shutdown() to look to see if 1 is returned or not. Or even SSL_get_shutdown() and take whatever security action your application needs to take in the event of an improperly shutdown stream. So as you can see my guess would be that your application has an incorrect event loop in relation to handling SSL_shutdown(). Darryl __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: make SSL_shutdown work with non-blocking BIOs
Darryl Miles wrote: 2) SSL_read() already has a return value -1/ZERO_RETURN which indicates end-of-stream. You may then call SSL_shutdown() to look to see if 1 is returned or not. Or even SSL_get_shutdown() and take whatever security action your application needs to take in the event of an improperly shutdown stream. This isn't quite true. I turn off readahead at my initial call to SSL_shutdown() with SSL_set_read_ahead(ssl, 0). Then in my event processor which runs off the readability indicator for the socket that is being shutdown in this handler I call SSL_shutdown() first but if it is still returning 0, I then call SSL_read() a number of times to sink the application data that may exist, I then call SSL_shutdown() again (before processing the last error if there was one to SSL_read()). Turning the read_ahead off may not be necessary for you, I do it because in my app its possible to see unencrypted data behind the last SSL byte (or possible another request to setup a new SSL session). I also only allow SSL_read() to be called a handful of times (in the shutdown handling detailed above) in a tight loop before it returns back to a main event loop this is to stop a form of denial of service where the receiving CPU is lower spec to the sending CPU which may cause starvation of other connections, this would be because its sinking the data and there isn't the usual disconnection due to too much invalid data that you'd get if it were in the main event loop doing request processing. Turning readahead off ensures OpenSSL only reads just as much data as it needs to and my readability events stay armed within the BIO layer if I choose to not read application data to exhaustion. Darryl __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
