s_client, s_server: do generic SSL configuration first, specialization after

We did the SSL_CONF_cmd() pass last of all things that could affect
the SSL ctx.  However, the results of this, for example:

    -max_protocol TLSv1.3 -tls1_2

... would mean that the protocol min got set to TLSv1.2 and the
protocol max to TLSv1.3, when they should clearly both be TLSv1.2.

However, if we see the SSL_CONF_cmd() switches as generic and those
internal to s_client and s_server as specialisations, we get something
that makes a little more sense.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5679)

diff --git a/apps/s_client.c b/apps/s_client.c
index 1ed853d14b710c820ae2d27a2b2022c75862eeec..4b7df0cef3649db0c77f2d557d41e13a5a64ea82 100644 (file)
--- a/apps/s_client.c
+++ b/apps/s_client.c
@@ -1672,6 +1672,9 @@ int s_client_main(int argc, char **argv)
     if (sdebug)
         ssl_ctx_security_debug(ctx, sdebug);
 
+    if (!config_ctx(cctx, ssl_args, ctx))
+        goto end;
+
     if (ssl_config != NULL) {
         if (SSL_CTX_config(ctx, ssl_config) == 0) {
             BIO_printf(bio_err, "Error using configuration \"%s\"\n",
@@ -1681,9 +1684,11 @@ int s_client_main(int argc, char **argv)
         }
     }
 
-    if (SSL_CTX_set_min_proto_version(ctx, min_version) == 0)
+    if (min_version != 0
+        && SSL_CTX_set_min_proto_version(ctx, min_version) == 0)
         goto end;
-    if (SSL_CTX_set_max_proto_version(ctx, max_version) == 0)
+    if (max_version != 0
+        && SSL_CTX_set_max_proto_version(ctx, max_version) == 0)
         goto end;
 
     if (vpmtouched && !SSL_CTX_set1_param(ctx, vpm)) {
@@ -1729,9 +1734,6 @@ int s_client_main(int argc, char **argv)
         goto end;
     }
 
-    if (!config_ctx(cctx, ssl_args, ctx))
-        goto end;
-
     if (!ssl_load_stores(ctx, vfyCApath, vfyCAfile, chCApath, chCAfile,
                          crls, crl_download)) {
         BIO_printf(bio_err, "Error loading store locations\n");

diff --git a/apps/s_server.c b/apps/s_server.c
index d21631e8e6aacd31eddce1eb0eb23e20e472edd1..bfa1345e6cd05d563268a791edf975759b0312de 100644 (file)
--- a/apps/s_server.c
+++ b/apps/s_server.c
@@ -1755,6 +1755,10 @@ int s_server_main(int argc, char *argv[])
     }
     if (sdebug)
         ssl_ctx_security_debug(ctx, sdebug);
+
+    if (!config_ctx(cctx, ssl_args, ctx))
+        goto end;
+
     if (ssl_config) {
         if (SSL_CTX_config(ctx, ssl_config) == 0) {
             BIO_printf(bio_err, "Error using configuration \"%s\"\n",
@@ -1763,9 +1767,11 @@ int s_server_main(int argc, char *argv[])
             goto end;
         }
     }
-    if (SSL_CTX_set_min_proto_version(ctx, min_version) == 0)
+    if (min_version != 0
+        && SSL_CTX_set_min_proto_version(ctx, min_version) == 0)
         goto end;
-    if (SSL_CTX_set_max_proto_version(ctx, max_version) == 0)
+    if (max_version != 0
+        && SSL_CTX_set_max_proto_version(ctx, max_version) == 0)
         goto end;
 
     if (session_id_prefix) {
@@ -1841,8 +1847,6 @@ int s_server_main(int argc, char *argv[])
     }
 
     ssl_ctx_add_crls(ctx, crls, 0);
-    if (!config_ctx(cctx, ssl_args, ctx))
-        goto end;
 
     if (!ssl_load_stores(ctx, vfyCApath, vfyCAfile, chCApath, chCAfile,
                          crls, crl_download)) {