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)

(cherry picked from commit 8f8be103fd7d8b5992724d618c99cbddd7dd00d7)

diff --git a/apps/s_client.c b/apps/s_client.c
index 3b6606963d7c3eac0e89755b05f6f21fabcb2c6b..6ededffc5572751e4fbda04516f4f31b4fae8e30 100644 (file)
--- a/apps/s_client.c
+++ b/apps/s_client.c
@@ -1486,6 +1486,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) {
         if (SSL_CTX_config(ctx, ssl_config) == 0) {
             BIO_printf(bio_err, "Error using configuration \"%s\"\n",
@@ -1495,9 +1498,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)) {
@@ -1520,9 +1525,6 @@ int s_client_main(int argc, char **argv)
         SSL_CTX_set_default_read_buffer_len(ctx, read_buf_len);
     }
 
-    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 d7f51cc4f56c2dff9f8dcd1bfb65e2c7703ce89f..521a3dd1d76483a78a580fd45cff6395ce0f207c 100644 (file)
--- a/apps/s_server.c
+++ b/apps/s_server.c
@@ -1615,6 +1615,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",
@@ -1623,9 +1627,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) {
@@ -1687,8 +1693,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)) {