From 77174598920a05826a28d8a0bd87a3af43d3f4d8 Mon Sep 17 00:00:00 2001 From: Viktor Dukhovni Date: Thu, 16 Jul 2020 23:30:43 -0200 Subject: [PATCH] Avoid errors with a priori inapplicable protocol bounds The 'MinProtocol' and 'MaxProtocol' configuration commands now silently ignore TLS protocol version bounds when configurign DTLS-based contexts, and conversely, silently ignore DTLS protocol version bounds when configuring TLS-based contexts. The commands can be repeated to set bounds of both types. The same applies with the corresponding "min_protocol" and "max_protocol" command-line switches, in case some application uses both TLS and DTLS. SSL_CTX instances that are created for a fixed protocol version (e.g. TLSv1_server_method()) also silently ignore version bounds. Previously attempts to apply bounds to these protocol versions would result in an error. Now only the "version-flexible" SSL_CTX instances are subject to limits in configuration files in command-line options. Expected to resolve #12394 Reviewed-by: Paul Dale GH: #12472 --- CHANGES.md | 16 ++++++++++++++++ doc/man3/SSL_CONF_cmd.pod | 29 +++++++++++++++++++++-------- doc/man5/config.pod | 7 ++++++- ssl/ssl_conf.c | 7 +++++++ ssl/statem/statem_lib.c | 34 +++++++++++++++++++--------------- 5 files changed, 69 insertions(+), 24 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 5ff188c18c..14694739ae 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -23,6 +23,22 @@ OpenSSL 3.0 ### Changes between 1.1.1 and 3.0 [xx XXX xxxx] + * The 'MinProtocol' and 'MaxProtocol' configuration commands now silently + ignore TLS protocol version bounds when configuring DTLS-based contexts, and + conversely, silently ignore DTLS protocol version bounds when configuring + TLS-based contexts. The commands can be repeated to set bounds of both + types. The same applies with the corresponding "min_protocol" and + "max_protocol" command-line switches, in case some application uses both TLS + and DTLS. + + SSL_CTX instances that are created for a fixed protocol version (e.g. + TLSv1_server_method()) also silently ignore version bounds. Previously + attempts to apply bounds to these protocol versions would result in an + error. Now only the "version-flexible" SSL_CTX instances are subject to + limits in configuration files in command-line options. + + *Viktor Dukhovni* + * Deprecated the `ENGINE` API. Engines should be replaced with providers going forward. diff --git a/doc/man3/SSL_CONF_cmd.pod b/doc/man3/SSL_CONF_cmd.pod index 753d6778df..97ebff047f 100644 --- a/doc/man3/SSL_CONF_cmd.pod +++ b/doc/man3/SSL_CONF_cmd.pod @@ -178,12 +178,17 @@ See L for more information. =item B<-min_protocol> I, B<-max_protocol> I -Sets the minimum and maximum supported protocol. Currently supported -protocol values are B, B, B, B, B -for TLS and B, B for DTLS, and B for no limit. -If either bound is not specified then only the other bound applies, -if specified. To restrict the supported protocol versions use these -commands rather than the deprecated alternative commands below. +Sets the minimum and maximum supported protocol. +Currently supported protocol values are B, B, B, +B, B for TLS; B, B for DTLS, and B +for no limit. +If either the lower or upper bound is not specified then only the other bound +applies, if specified. +If your application supports both TLS and DTLS you can specify any of these +options twice, once with a bound for TLS and again with an appropriate bound +for DTLS. +To restrict the supported protocol versions use these commands rather than the +deprecated alternative commands below. =item B<-record_padding> I @@ -389,7 +394,11 @@ This sets the minimum supported SSL, TLS or DTLS version. Currently supported protocol values are B, B, B, B, B, B and B. -The value B will disable the limit. +The SSL and TLS bounds apply only to TLS-based contexts, while the DTLS bounds +apply only to DTLS-based contexts. +The command can be repeated with one instance setting a TLS bound, and the +other setting a DTLS bound. +The value B applies to both types of contexts and disables the limits. =item B @@ -397,7 +406,11 @@ This sets the maximum supported SSL, TLS or DTLS version. Currently supported protocol values are B, B, B, B, B, B and B. -The value B will disable the limit. +The SSL and TLS bounds apply only to TLS-based contexts, while the DTLS bounds +apply only to DTLS-based contexts. +The command can be repeated with one instance setting a TLS bound, and the +other setting a DTLS bound. +The value B applies to both types of contexts and disables the limits. =item B diff --git a/doc/man5/config.pod b/doc/man5/config.pod index 58948b4b78..2618cef588 100644 --- a/doc/man5/config.pod +++ b/doc/man5/config.pod @@ -299,10 +299,15 @@ section with the configuration for that name. For example: The configuration name B has a special meaning. If it exists, it is applied whenever an B object is created. For example, -to impose a system-wide minimum on protocol version: +to impose system-wide minimum TLS and DTLS protocol versions: [tls_system_default] MinProtocol = TLSv1.2 + MinProtocol = DTLSv1.2 + +The minimum TLS protocol is applied to B objects that are TLS-based, +and the minimum DTLS protocol to those are DTLS-based. +The same applies also to maximum versions set with B. Each configuration section consists of name/value pairs that are parsed by B, which will be called by SSL_CTX_config() or diff --git a/ssl/ssl_conf.c b/ssl/ssl_conf.c index aefe8ad203..fe9b8ec3ea 100644 --- a/ssl/ssl_conf.c +++ b/ssl/ssl_conf.c @@ -303,6 +303,13 @@ static int protocol_from_string(const char *value) const char *name; int version; }; + /* + * Note: To avoid breaking previously valid configurations, we must retain + * legacy entries in this table even if the underlying protocol is no + * longer supported. This also means that the constants SSL3_VERSION, ... + * need to be retained indefinitely. This table can only grow, never + * shrink. + */ static const struct protocol_versions versions[] = { {"None", 0}, {"SSLv3", SSL3_VERSION}, diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index de8212747f..d8aab20e92 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -1679,11 +1679,22 @@ int ssl_check_version_downgrade(SSL *s) */ int ssl_set_version_bound(int method_version, int version, int *bound) { + int valid_tls; + int valid_dtls; + if (version == 0) { *bound = version; return 1; } + valid_tls = version >= SSL3_VERSION && version <= TLS_MAX_VERSION_INTERNAL; + valid_dtls = + DTLS_VERSION_LE(version, DTLS_MAX_VERSION_INTERNAL) && + DTLS_VERSION_GE(version, DTLS1_BAD_VER); + + if (!valid_tls && !valid_dtls) + return 0; + /*- * Restrict TLS methods to TLS protocol versions. * Restrict DTLS methods to DTLS protocol versions. @@ -1694,31 +1705,24 @@ int ssl_set_version_bound(int method_version, int version, int *bound) * configurations. If the MIN (supported) version ever rises, the user's * "floor" remains valid even if no longer available. We don't expect the * MAX ceiling to ever get lower, so making that variable makes sense. + * + * We ignore attempts to set bounds on version-inflexible methods, + * returning success. */ switch (method_version) { default: - /* - * XXX For fixed version methods, should we always fail and not set any - * bounds, always succeed and not set any bounds, or set the bounds and - * arrange to fail later if they are not met? At present fixed-version - * methods are not subject to controls that disable individual protocol - * versions. - */ - return 0; + break; case TLS_ANY_VERSION: - if (version < SSL3_VERSION || version > TLS_MAX_VERSION_INTERNAL) - return 0; + if (valid_tls) + *bound = version; break; case DTLS_ANY_VERSION: - if (DTLS_VERSION_GT(version, DTLS_MAX_VERSION_INTERNAL) || - DTLS_VERSION_LT(version, DTLS1_BAD_VER)) - return 0; + if (valid_dtls) + *bound = version; break; } - - *bound = version; return 1; } -- 2.39.5