Re: [openssl.org #3129] Openssl not clearing session ticket upon handshake failure
On 9/18/2013 11:23 AM, Stephen Henson via RT wrote: > Is this the session ticket or the session ID causing the problem? A server > shouldn't just disconnect if it sees a ticket it doesn't like it should just > issue a new one. Presumably it is the session ticket. I haven't yet captured such a poison session. I agree a server shouldn't disconnect if it sees a ticket it doesn't like, but buggy servers exist. > What happens if you disable tickets with -no_ticket? > This problem has manifested with a production service against Amazon ELB. The last time it reproduced was three days ago, the time before that was three weeks prior. The last reproduction, I got enough telemetry to point the finger at session resumption and was able to confirm that OpenSSL will incorrectly reuse a session ticket when the server drops the connection. I now have a test program connecting every second. If/when the problem recurs, that program will keep a copy of the poison session on disk and I should be able to answer such questions. __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [openssl.org #3129] AutoReply: Openssl not clearing session ticket upon handshake failure
My proposed fix does not work. It isn't legitimate to just remove the
session.
An updated proposed fix is attached.
diff -ru ../openssl-1.0.1e-orig/apps/s_client.c ./apps/s_client.c
--- ../openssl-1.0.1e-orig/apps/s_client.c 2013-02-11 07:26:04.0
-0800
+++ ./apps/s_client.c 2013-09-16 10:43:51.589324000 -0700
@@ -1891,6 +1891,12 @@
print_stuff(bio_c_out,con,full_log);
SSL_shutdown(con);
SHUTDOWN(SSL_get_fd(con));
+ if (reconnect)
+ {
+ reconnect--;
+ BIO_printf(bio_c_out,"reconnect\n");
+ goto re_start;
+ }
end:
if (con != NULL)
{
diff -ru ../openssl-1.0.1e-orig/ssl/s3_clnt.c ./ssl/s3_clnt.c
--- ../openssl-1.0.1e-orig/ssl/s3_clnt.c2013-02-11 07:26:04.0
-0800
+++ ./ssl/s3_clnt.c 2013-09-17 15:56:53.531223000 -0700
@@ -646,6 +646,11 @@
BUF_MEM_free(buf);
if (cb != NULL)
cb(s,SSL_CB_CONNECT_EXIT,ret);
+ if (ret < 0 && s->session != NULL)
+ {
+ SSL_CTX_remove_session(s->ctx,s->session);
+ s->new_session=1;
+ }
return(ret);
}
[openssl.org #3129] Openssl not clearing session ticket upon handshake failure
Openssl 1.0.1e is not clearing the session ticket upon handshake
failure, contrary to the recommendation in RFC 5077 section 3.2 paragraph 4.
I am seeing that after some sort of event, Amazon ELB will respond to a
TLS 1.0 handshake containing a session ticket that it had handed out
prior to the event by closing the connection. When my client application
tries to reconnect, openssl will once again send the session ticket,
causing Amazon ELB to once again close the connection. This leads to an
hours-long inability to reconnect, until my client application is
manually restarted, removing knowledge of the now-poison ticket.
Attached is a patch which causes SSL3 to clear the session upon any
handshake failure. I have not attempted to do the corresponding fix to
DTLS. Also included is a change to s_client I used for testing,
extending the -reconnect option to also act upon handshake failure.
diff -ru ../openssl-1.0.1e-orig/apps/s_client.c ./apps/s_client.c
--- ../openssl-1.0.1e-orig/apps/s_client.c 2013-02-11 07:26:04.0
-0800
+++ ./apps/s_client.c 2013-09-16 10:43:51.589324000 -0700
@@ -1891,6 +1891,12 @@
print_stuff(bio_c_out,con,full_log);
SSL_shutdown(con);
SHUTDOWN(SSL_get_fd(con));
+ if (reconnect)
+ {
+ reconnect--;
+ BIO_printf(bio_c_out,"reconnect\n");
+ goto re_start;
+ }
end:
if (con != NULL)
{
diff -ru ../openssl-1.0.1e-orig/ssl/s3_clnt.c ./ssl/s3_clnt.c
--- ../openssl-1.0.1e-orig/ssl/s3_clnt.c2013-02-11 07:26:04.0
-0800
+++ ./ssl/s3_clnt.c 2013-09-16 10:35:05.808437000 -0700
@@ -646,6 +646,12 @@
BUF_MEM_free(buf);
if (cb != NULL)
cb(s,SSL_CB_CONNECT_EXIT,ret);
+ if (ret < 0 && s->session != NULL)
+ {
+ SSL_CTX_remove_session(s->ctx,s->session);
+ SSL_SESSION_free(s->session);
+ s->session=NULL;
+ }
return(ret);
}
[openssl.org #2942] threads(3) gives wrong signature for CRYPTO_set_dynlock_create_callback()
In openssl 1.0.1c, the doc/crypto/threads.pod documentatin gives the wrong signature for CRYPTO_set_dynlock_create_callback(). It documents: void CRYPTO_set_dynlock_create_callback(struct CRYPTO_dynlock_value * (*dyn_create_function)(char *file, int line)); It should be: void CRYPTO_set_dynlock_create_callback(struct CRYPTO_dynlock_value * (*dyn_create_function)(const char *file, int line)); __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [openssl.org #2749] SSL_shutdown() doesn't need to ever return 0
On 4/25/2012 1:21 AM, Darryl Miles via RT wrote: > 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. That was deliberate. The patched implementation conforms to the interface contract of the existing documentation. If the corresponding change were made to the documentation, then callers written to the new documentation would fail when run against downrev/existing versions of the library. Put another way, the implementation change is appropriate for a minor release. The obvious corresponding documentation change is only appropriate for a major release. > 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. The current documentation does not permit the caller to infer that it is safe to do a TCP shutdown(SHUT_WR) from a 0 return value. Neither does the implementation--it will return a 0 before the output BIO has been completely flushed. > > 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 :) As I mentioned above, neither does the existing return value of "0" indicate this. If one wants to encode assumptions about the actual behavior of the existing TLS protocol and that one is not currently doing a handshake, one could use a return value of "-1" plus a SSL_get_error() of SSL_ERROR_WANT_READ to indicate this. __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
[openssl.org #2749] SSL_shutdown() doesn't need to ever return 0
There's no good reason for SSL_shutdown() to ever return a value of 0.
The attached patch simplifies things.
--- openssl-1.0.1-beta3-0orig/ssl/s3_lib.c 2012-02-10 12:08:49.0
-0800
+++ openssl-1.0.1-beta3/ssl/s3_lib.c2012-03-02 11:19:53.847954000 -0800
@@ -4112,7 +4112,7 @@
if (s->s3->alert_dispatch)
return(-1); /* return WANT_WRITE */
}
- else if (s->s3->alert_dispatch)
+ if (s->s3->alert_dispatch)
{
/* resend it if not sent */
#if 1
@@ -4127,7 +4127,7 @@
}
#endif
}
- else if (!(s->shutdown & SSL_RECEIVED_SHUTDOWN))
+ if (!(s->shutdown & SSL_RECEIVED_SHUTDOWN))
{
/* If we are waiting for a close from our peer, we are closed */
s->method->ssl_read_bytes(s,0,NULL,0,0);
@@ -4137,11 +4137,7 @@
}
}
- if ((s->shutdown == (SSL_SENT_SHUTDOWN|SSL_RECEIVED_SHUTDOWN)) &&
- !s->s3->alert_dispatch)
- return(1);
- else
- return(0);
+ return(1);
}
int ssl3_write(SSL *s, const void *buf, int len)
Re: [openssl.org #2740] AutoReply: infinite loop in nonblocking SSL_shutdown() upon permanent error
While having SSL_want_read() and SSL_want_write() return nonzero after a permanent error is a poor interface, the documentation does state that it is necessary to check the return of SSL_get_error(). So this ticket can be closed out as invalid. __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
[openssl.org #2740] infinite loop in nonblocking SSL_shutdown() upon permanent error
ssl3_shutdown() incorrectly indicates SSL_want_read() or
SSL_want_write() when the underlying read/write results in a permanent
error. This means that callers of nonblocking SSL_shutdown() will go
into an infinite loop retrying the shutdown.
This bug appears in both OpenSSL 0.9.8t and 1.0.1-beta3.
Attached are a sample nonblocking server and a client that triggers the
infinite loop. In my testing it loops wanting read, but with the
underlying socket returning end of file. In production, I've seen a real
server loop wanting write but with the underlying socket failing with EPIPE.
#include
#include
#include
#include
#include
//#include
#include
#include
int main(int argc, char **argv)
{
SSL_library_init();
SSL_CTX *ctx=SSL_CTX_new(SSLv3_client_method());
if (!ctx) goto err;
SSL *con=SSL_new(ctx);
if (!con) goto err;
int s = socket(AF_INET, SOCK_STREAM, 0);
if (s == -1) {
perror("socket");
exit(1);
}
struct sockaddr_in sa;
sa.sin_family = AF_INET;
sa.sin_port = htons(10001);
sa.sin_addr.s_addr=htonl(0x7f01);
if (connect(s, (struct sockaddr *)&sa, sizeof(sa)) == -1) {
perror("connect");
exit(1);
}
if (!SSL_set_fd(con, s)) goto err;
if (!SSL_connect(con)) goto err;
fprintf(stdout, "Got handshake\n");
exit(0);
err:
ERR_print_errors_fp(stdout);
exit(1);
}
#include
#include
#include
#include
#include
#include
#include
//#include
#include
#include
int main(int argc, char **argv)
{
SSL_library_init();
SSL_CTX *ctx=SSL_CTX_new(SSLv23_server_method());
if (!ctx) goto err;
SSL_load_error_strings();
SSL_library_init();
SSL_CTX_set_options(ctx, SSL_OP_ALL | SSL_OP_NO_SSLv2);
SSL_CTX_set_mode(ctx, SSL_MODE_ENABLE_PARTIAL_WRITE |
SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER);
SSL_CTX_set_cipher_list(ctx, "DEFAULT:-SSLv2:-LOW:-EXPORT");
SSL_CTX_set_session_cache_mode(ctx, SSL_SESS_CACHE_SERVER);
SSL_CTX_set_session_id_context(ctx, (const unsigned char *)"nbserver", 7);
if (SSL_CTX_use_PrivateKey_file(ctx, "cert.pem", SSL_FILETYPE_PEM) <= 0) {
fprintf(stderr, "Cannot load private key\n");
goto err;
}
if (SSL_CTX_use_certificate_chain_file(ctx, "cert.pem") <= 0) {
fprintf(stderr, "Cannot load server certificate\n");
goto err;
}
if (SSL_CTX_check_private_key(ctx) <= 0) {
fprintf(stderr, "Certificate/key verification error\n");
goto err;
}
SSL *con=SSL_new(ctx);
if (!con) goto err;
SSL_set_verify(con, SSL_VERIFY_NONE, 0);
int s = socket(AF_INET, SOCK_STREAM, 0);
if (s == -1) {
perror("socket");
exit(1);
}
struct sockaddr_in sa;
sa.sin_family = AF_INET;
sa.sin_port = htons(10001);
sa.sin_addr.s_addr=htonl(0x7f01);
if (bind(s, (struct sockaddr *)&sa, sizeof(sa)) == -1) {
perror("bind");
exit(1);
}
if (listen(s, 5) == -1) {
perror("listen");
exit(1);
}
s = accept(s, (struct sockaddr*)0, 0);
if (s == -1) {
perror("accept");
exit(1);
}
if (!SSL_set_fd(con, s)) goto err;
if (!SSL_accept(con)) goto err;
fprintf(stdout, "Got handshake\n");
sleep(3);
unsigned long on = 1;
ioctl(s, FIONBIO, &on);
int r;
while ((r = SSL_shutdown(con)) != 1) {
struct pollfd pfd;
pfd.fd = s;
const char *want;
if (r == 0) continue;
if (SSL_want_write(con)) {
want = "write";
pfd.events = POLLOUT | POLLHUP | POLLERR;
} else if (SSL_want_read(con)) {
want = "read";
pfd.events = POLLIN | POLLERR;
} else {
fprintf(stdout, "Permanent error on shutdown");
break;
}
fprintf(stdout, "want=%s\n", want);
fflush(stdout);
poll(&pfd, 1, -1);
}
exit(0);
err:
ERR_print_errors_fp(stdout);
exit(1);
}
[openssl.org #1766] [PATCH] s_client -reconnect and -starttls don't work together
openssl s_client -starttls smtp -reconnect
doesn't work, because it doesn't do the starttls protocol work after
reconnecting. Fix:
[EMAIL PROTECTED] openssl-0.9.8i]$ diff -ru apps/s_client.c~ apps/s_client.c
--- apps/s_client.c~2008-06-04 13:11:17.0 -0700
+++ apps/s_client.c 2008-10-24 10:42:21.0 -0700
@@ -1023,7 +1023,7 @@
{
BIO_printf(bio_err,"%s",mbuf);
/* We don't need to know any more */
- starttls_proto = PROTO_OFF;
+ if (reconnect == 0)
starttls_proto = PROTO_OFF;
}
if (reconnect)
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #842] [PATCH] Reduce probability of duplicate serial numbers
What is the status of this request? What needs to be done in order for somebody to check in code? __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #842] [PATCH] Reduce probability of duplicate serial numbers
If you want to address incorrect cookbooks, you could error out on zero or you could replace zero with the current time. Or you could pester the cookbook authors. I'm not sure a warning would help much. In any case, it would be nice if someone could check in the patches so far. __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #842] [PATCH] Reduce probability of duplicate serial numbers
Stephen Henson via RT wrote: >One would be the perl front end CA.pl > Patch attached >Another would be if non-standard scripts initialize the serial number >file either for 'ca' or the 'x509' utility. > > My original patch fixed the -CAcreateserial switch. Not much we can do about scripts not shipped with OpenSSL, but most folk seem to be using the standard apps. >With regard to 0 being a non conforming serial number. RFC3280 in >4.1.2.2 says serialNumber must be positive (which would exclude 0) and >in the next sentence non-negative (which wouldn't). > > The second sentence doesn't contradict the first one, it just fails to (redundantly) require CAs to not generate that one particular nonconforming value. The last paragraph also implies that zero serial numbers are nonconforming. But the problem with 0 is not so much its nonconformance but the fact that as a default it (or any other fixed integer) has a good chance at failing the uniqueness requirement. __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #842] [PATCH] Reduce probability of duplicate serial numbers
Stephen Henson via RT wrote: > I'm not sure how portable that patch is as it stands. What would be the portability problem? The code is already calling time() in order to calculate the expiration dates. >As a portable alternative we could use a large random number for the >serial number, for example a 159 bit one has negligible chance of >duplicates. > > That seems to be overkill--64 bits would be more than sufficient. >I'd be interested to know how people are managing to create duplicate >serial numbers: that is what commands and or scripts are being used to >do this. > > I've asked someone with more direct experience with this to comment on the ticket. As I understand it, there are at least two situations: One, the CA somehow loses its serial number file--it rebuilds the box, regenerates the CA cert from scratch, or whatever. Two, the user generates a self-signed cert with "openssl req -x509" and no serial number options, resulting in a cert with (nonconforming) serial number 0. __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
[openssl.org #842] [PATCH] Reduce probability of duplicate serial numbers
The attached patch causes serial numbers to default to the current time, significantly reducing the chance of duplicate serial numbers from a given issuer. I have filed the necessary TSA notification. Mozilla gets a constant stream of problem reports caused by OpenSSL-generated certs with duplicate serial numbers. We would much appreciate you could fix this non-standards-conforming behavior, preferably by integrating this patch. __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
