On Wed, Sep 09, 2015 at 11:17:55AM -0500, Brent Cook wrote:
> On Wed, Sep 9, 2015 at 10:15 AM, Todd C. Miller
> <[email protected]> wrote:
> > On Wed, 09 Sep 2015 10:02:17 -0400, Lawrence Teo wrote:
> >> In s_time.c, NO_SHUTDOWN is always defined, so there is no need for a
> >> bunch of NO_SHUTDOWN #ifdef blocks.
> >
> > I'm less sure about this as without calling SSL_shutdown() the
> > client is not notified. I suppose that's intentional as s_time is
> > just for timing connections?
> >
> > - todd
> >
>
> OK, who has a camera looking over my shoulder? I was just looking at this :)
Oops! Please pay no attention to the drone buzzing behind you. :)
> TBH, I'd rather this were a flag rather than a define. Yes, a knob,
> but this is a benchmark that really should be able to benchmark a full
> TLS connection and shutdown to be accurate. The default behavior of
> faking out the shutdown state machine does make the this run about 25%
> faster, but I would have never known that if not for playing with the
> define.
Thank you all for the feedback. I agree that a flag would be preferred
over recompiling openssl(1).
Here's a diff that adds a flag called -no_shutdown (the underscore is
there to match the -no_* flags used by other openssl(1) commands).
The diff also changes the behavior of s_time so that it will perform
a proper full shutdown by default (i.e. if -no_shutdown is not
specified).
Thoughts?
Index: openssl.1
===================================================================
RCS file: /cvs/src/usr.bin/openssl/openssl.1,v
retrieving revision 1.19
diff -u -p -u -p -r1.19 openssl.1
--- openssl.1 11 Aug 2015 05:01:03 -0000 1.19
+++ openssl.1 10 Sep 2015 01:51:18 -0000
@@ -7074,6 +7074,7 @@ unknown cipher suites a client says it s
.Op Fl key Ar keyfile
.Op Fl nbio
.Op Fl new
+.Op Fl no_shutdown
.Op Fl reuse
.Op Fl time Ar seconds
.Op Fl verify Ar depth
@@ -7135,6 +7136,10 @@ nor
.Fl reuse
are specified,
they are both on by default and executed in sequence.
+.It Fl no_shutdown
+Shutdown the connection without sending a
+.Dq close notify
+shutdown alert to the server.
.It Fl reuse
Performs the timing test using the same session ID;
this can be used as a test that session caching is working.
Index: s_time.c
===================================================================
RCS file: /cvs/src/usr.bin/openssl/s_time.c,v
retrieving revision 1.9
diff -u -p -u -p -r1.9 s_time.c
--- s_time.c 22 Aug 2015 16:36:05 -0000 1.9
+++ s_time.c 10 Sep 2015 01:58:17 -0000
@@ -56,8 +56,6 @@
* [including the GNU Public Licence.]
*/
-#define NO_SHUTDOWN
-
/*-----------------------------------------
s_time - SSL client connection timer program
Written and donated by Larry Streepy <[email protected]>
@@ -114,6 +112,7 @@ struct {
char *keyfile;
int maxtime;
int nbio;
+ int no_shutdown;
int perform;
int verify;
int verify_depth;
@@ -184,6 +183,12 @@ struct option s_time_options[] = {
.value = 1,
},
{
+ .name = "no_shutdown",
+ .desc = "Shutdown the connection without notifying the server",
+ .type = OPTION_FLAG,
+ .opt.flag = &s_time_config.no_shutdown,
+ },
+ {
.name = "reuse",
.desc = "Reuse the same session ID for each connection",
.type = OPTION_VALUE,
@@ -221,7 +226,7 @@ s_time_usage(void)
"usage: s_time "
"[-bugs] [-CAfile file] [-CApath directory] [-cert file]\n"
" [-cipher cipherlist] [-connect host:port] [-key keyfile]\n"
- " [-nbio] [-new] [-reuse] [-time seconds]\n"
+ " [-nbio] [-new] [-no_shutdown] [-reuse] [-time seconds]\n"
" [-verify depth] [-www page]\n\n");
options_usage(s_time_options);
}
@@ -341,11 +346,11 @@ s_time_main(int argc, char **argv)
while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
bytes_read += i;
}
-#ifdef NO_SHUTDOWN
- SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
SSL_RECEIVED_SHUTDOWN);
-#else
- SSL_shutdown(scon);
-#endif
+ if (s_time_config.no_shutdown)
+ SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
+ SSL_RECEIVED_SHUTDOWN);
+ else
+ SSL_shutdown(scon);
shutdown(SSL_get_fd(scon), SHUT_RDWR);
close(SSL_get_fd(scon));
@@ -400,11 +405,11 @@ next:
SSL_write(scon, buf, strlen(buf));
while (SSL_read(scon, buf, sizeof(buf)) > 0);
}
-#ifdef NO_SHUTDOWN
- SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN | SSL_RECEIVED_SHUTDOWN);
-#else
- SSL_shutdown(scon);
-#endif
+ if (s_time_config.no_shutdown)
+ SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
+ SSL_RECEIVED_SHUTDOWN);
+ else
+ SSL_shutdown(scon);
shutdown(SSL_get_fd(scon), SHUT_RDWR);
close(SSL_get_fd(scon));
@@ -434,11 +439,11 @@ next:
while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
bytes_read += i;
}
-#ifdef NO_SHUTDOWN
- SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
SSL_RECEIVED_SHUTDOWN);
-#else
- SSL_shutdown(scon);
-#endif
+ if (s_time_config.no_shutdown)
+ SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
+ SSL_RECEIVED_SHUTDOWN);
+ else
+ SSL_shutdown(scon);
shutdown(SSL_get_fd(scon), SHUT_RDWR);
close(SSL_get_fd(scon));
