Re: [openssl-dev] [PATCH] Do not offer options like -ssl2, -tls1, -dtls if they are not compiled in

[Ángel González]

Thanks for your promptly response, Viktor.
Viktor Dukhovni wrote:
> > On Mar 3, 2016, at 8:07 PM, Ángel González 
> > wrote:
> > 
> > They were showed in the help, but providing them failed with an
> > “unknown option” error, and showed the help which listed it
> > as a valid option.
> The patch is not right.  For example, when TLSv1 is disabled, it is
> not the case that TLSv1.1 and TLSv1.2 are disabled.  

When OPENSSL_NO_TLS1 is disabled, the -tls1_2, -tls1_1 and -tls1
options to s_client are not parsed. See lines 958-964:
> #ifndef OPENSSL_NO_TLS1
> else if (strcmp(*argv, "-tls1_2") == 0)
> meth = TLSv1_2_client_method();
> else if (strcmp(*argv, "-tls1_1") == 0)
> meth = TLSv1_1_client_method();
> else if (strcmp(*argv, "-tls1") == 0)
> meth = TLSv1_client_method();
> #endif

I agree it doesn't seem the best name to control tls 1.2, but I assumed
that they were all using some shared functions so that OPENSSL_NO_TLS1
meant you couldn't use any TLS x function. Also note that there are no
other OPENSSL_NO_TLS* macros which would apply to the minor versions
(the most similar is OPENSSL_NO_TLS1_2_CLIENT).
Do you have more information about *what* is the right behavior here?
Sadly, the macros don't seem to be documented.



> Secondly disabled
> features should report that the feature is disabled, not a bad usage
> message, as would be the case with a mistyped option.

I agree it's a much more sensible way of erroring out, and I would be
happy to prepare a patch that does that. Do note however that such is
the way s_client works, see lines 878-1124 where dozens of argparsing
strcmps are guarded by #ifdefs (as well as on sc_usage() function).
I tried to fix the inconsistency in the least disruptive way.


Additionally, do you have any preference about the branch? I prepared
the patch against the stable branch, since it's the one on which I
noticed the problem, but perhaps you prefer it against to master
instead.

Best regards
-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

Re: [openssl-dev] [PATCH] Do not offer options like -ssl2, -tls1, -dtls if they are not compiled in

[Viktor Dukhovni]

> On Mar 3, 2016, at 8:07 PM, Ángel González  wrote:
> 
> They were showed in the help, but providing them failed with an
> “unknown option” error, and showed the help which listed it
> as a valid option.

The patch is not right.  For example, when TLSv1 is disabled, it is
not the case that TLSv1.1 and TLSv1.2 are disabled.  Secondly disabled
features should report that the feature is disabled, not a bad usage
message, as would be the case with a mistyped option.

> Patch against the stable 1.0.2 branch.
> 
>  apps/s_client.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/apps/s_client.c b/apps/s_client.c
> index 0c1102b..f68c581 100644
> --- a/apps/s_client.c
> +++ b/apps/s_client.c
> @@ -376,16 +376,22 @@ static void sc_usage(void)
> " -srp_strength int - minimal length in bits for N
> (default %d).\n",
> SRP_MINIMAL_N);
>  #endif
> +#ifndef OPENSSL_NO_SSL2
>  BIO_printf(bio_err, " -ssl2 - just use SSLv2\n");
> +#endif
>  #ifndef OPENSSL_NO_SSL3_METHOD
>  BIO_printf(bio_err, " -ssl3 - just use SSLv3\n");
>  #endif
> +#ifndef OPENSSL_NO_TLS1
>  BIO_printf(bio_err, " -tls1_2   - just use TLSv1.2\n");
>  BIO_printf(bio_err, " -tls1_1   - just use TLSv1.1\n");
>  BIO_printf(bio_err, " -tls1 - just use TLSv1\n");
> +#endif
> +#ifndef OPENSSL_NO_DTLS1
>  BIO_printf(bio_err, " -dtls1- just use DTLSv1\n");
> -BIO_printf(bio_err, " -fallback_scsv - send TLS_FALLBACK_SCSV\n");
>  BIO_printf(bio_err, " -mtu  - set the link layer MTU\n");
> +#endif
> +BIO_printf(bio_err, " -fallback_scsv - send TLS_FALLBACK_SCSV\n");
>  BIO_printf(bio_err,
> " -no_tls1_2/-no_tls1_1/-no_tls1/-no_ssl3/-no_ssl2 -
> turn off that protocol\n");
>  BIO_printf(bio_err,
> -- 
> 2.7.2
> -- 
> openssl-dev mailing list
> To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

-- 
Viktor.

-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

[openssl-dev] [PATCH] Do not offer options like -ssl2, -tls1, -dtls if they are not compiled in

[Ángel González]

They were showed in the help, but providing them failed with an
“unknown option” error, and showed the help which listed it
as a valid option.

---

Patch against the stable 1.0.2 branch.

 apps/s_client.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/apps/s_client.c b/apps/s_client.c
index 0c1102b..f68c581 100644
--- a/apps/s_client.c
+++ b/apps/s_client.c
@@ -376,16 +376,22 @@ static void sc_usage(void)
" -srp_strength int - minimal length in bits for N
(default %d).\n",
SRP_MINIMAL_N);
 #endif
+#ifndef OPENSSL_NO_SSL2
 BIO_printf(bio_err, " -ssl2 - just use SSLv2\n");
+#endif
 #ifndef OPENSSL_NO_SSL3_METHOD
 BIO_printf(bio_err, " -ssl3 - just use SSLv3\n");
 #endif
+#ifndef OPENSSL_NO_TLS1
 BIO_printf(bio_err, " -tls1_2   - just use TLSv1.2\n");
 BIO_printf(bio_err, " -tls1_1   - just use TLSv1.1\n");
 BIO_printf(bio_err, " -tls1 - just use TLSv1\n");
+#endif
+#ifndef OPENSSL_NO_DTLS1
 BIO_printf(bio_err, " -dtls1- just use DTLSv1\n");
-BIO_printf(bio_err, " -fallback_scsv - send TLS_FALLBACK_SCSV\n");
 BIO_printf(bio_err, " -mtu  - set the link layer MTU\n");
+#endif
+BIO_printf(bio_err, " -fallback_scsv - send TLS_FALLBACK_SCSV\n");
 BIO_printf(bio_err,
" -no_tls1_2/-no_tls1_1/-no_tls1/-no_ssl3/-no_ssl2 -
turn off that protocol\n");
 BIO_printf(bio_err,
-- 
2.7.2
-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev