[ruby-core:83491] Re: [ruby-cvs:67491] naruse:r60310 (trunk): fix OpenSSL::SSL::SSLContext#min_version doesn't work

On 10/22/2017 01:25 AM, [e-mail] wrote:
> naruse	2017-10-22 01:25:19 +0900 (Sun, 22 Oct 2017)
> 
>   New Revision: 60310
> 
>   https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=60310
> 
>   Log:
>     fix OpenSSL::SSL::SSLContext#min_version doesn't work
> 
>   Modified files:
>     trunk/ext/openssl/lib/openssl/ssl.rb
>     trunk/test/openssl/test_ssl.rb
> 

> diff --git a/ext/openssl/lib/openssl/ssl.rb b/ext/openssl/lib/openssl/ssl.rb
> index fb143c94087a..4bbbcf6c26aa 100644
> --- a/ext/openssl/lib/openssl/ssl.rb
> +++ b/ext/openssl/lib/openssl/ssl.rb
> @@ -136,6 +136,7 @@ def initialize(version = nil)
>        # used.
>        def set_params(params={})
>          params = DEFAULT_PARAMS.merge(params)
> +        self.options = params.delete(:options) # set before min_version/max_version
>          params.each{|name, value| self.__send__("#{name}=", value) }
>          if self.verify_mode != OpenSSL::SSL::VERIFY_NONE
>            unless self.ca_file or self.ca_path or self.cert_store

Oops, good catch! The min_version value in SSLContext::DEFAULT_PARAMS
was effectively ignored. It's cherry-picked to upstream, with a new test
case that passes successfully with OpenSSL >= 1.1.0 and
LibreSSL >= 2.6.0.

	https://github.com/ruby/openssl/commit/62af0446569ae842de67b636b0bd0bb84ec2c8be


> @@ -147,7 +148,7 @@ def set_params(params={})
>  
>        # call-seq:
>        #    ctx.min_version = OpenSSL::SSL::TLS1_2_VERSION
> -      #    ctx.min_version = :TLS1_2
> +      #    ctx.min_version = :TLSv1_2
>        #    ctx.min_version = nil
>        #
>        # Sets the lower bound on the supported SSL/TLS protocol version. The
> @@ -166,18 +167,30 @@ def set_params(params={})
>        #   sock = OpenSSL::SSL::SSLSocket.new(tcp_sock, ctx)
>        #   sock.connect # Initiates a connection using either TLS 1.1 or TLS 1.2
>        def min_version=(version)
> +        case version
> +        when nil, Integer
> +        else
> +          version = (METHODS_MAP[version] or
> +            raise ArgumentError, "unknown SSL version `#{version.inspect}'")
> +        end
>          set_minmax_proto_version(version, @max_proto_version ||= nil)
>          @min_proto_version = version
>        end

'TLS1_2' comes from "TLS1_2_VERSION".sub(/_VERSION$/, ""), where
TLS1_2_VERSION is a value defined by OpenSSL and can be passed to
SSL_CTX_set_min_proto_version(). On the other hand, 'TLSv1_2' comes
from the name of a deprecated SSL method, TLSv1_2_method().

It was natural that SSLContext#ssl_version= takes names with 'v' since
it was a method that actually sets an SSL method used by the SSL
context. However, as SSLContext#{min,max}_version have nothing to do
with those SSL methods, I don't think it makes sense to follow their
naming convention. At least, it is odd that they now accept 'SSLv23'.

Unsubscribe: <mailto:[e-mail]?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>