From 56befa68bcf9ece1732d293d73239b90cf6a9c85 Mon Sep 17 00:00:00 2001 From: nick evans Date: Fri, 20 Jan 2023 23:52:57 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=97=91=EF=B8=8F=20Add=20deprecation=20war?= =?UTF-8?q?nings=20to=20.new=20and=20#starttls=20[=F0=9F=9A=A7=20WIP]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * `ssl` was renamed to `tls` in most places, with backwards compatible aliases. Using `ssl` does not print any deprecation warnings. Using both `tls` and `ssl` keywords raises an ArgumentError. * Preparing for a (backwards-incompatible) secure-by-default configuration, `Net::IMAP.default_tls` will determine the value for `tls` when no explicit port or tls setting is provided. Using port 143 will be insecure by default. Using port 993 will be secure by default. Providing no explicit port will use `Net::IMAP.default_tls` with the appropriate port. And providing any other unknown port will use `default_tls` with a warning. 🚧 TODO: should we use a different config var for default tls params when port is 993 and `tls` is unspecified? 🚧 TODO: should we use a different config var for choosing `tls` when `port` is non-standard vs choosing `port` and `tls` when neither are specified? 🚧 TODO: should we use a different var for `default_tls` be used to config params when port is 993 but tls is implicit? Another var? --- lib/net/imap.rb | 44 ++++++++++++++++++++++++++++++++++++++---- lib/net/imap/config.rb | 24 +++++++++++++++++++++++ 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 6a55c5f2..e69d8528 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -800,8 +800,21 @@ def idle_response_timeout; config.idle_response_timeout end # Returns +false+ for a plaintext connection. attr_reader :ssl_ctx_params - # Creates a new Net::IMAP object and connects it to the specified - # +host+. + # Creates a new Net::IMAP object and connects it to the specified +host+. + # + # ==== Default port and SSL + # + # When both both +port+ and +ssl+ are unspecified or +nil+, + # +ssl+ is determined by {config.default_ssl}[rdoc-ref:Config#default_ssl] + # and +port+ is based on that implicit value for +ssl+. + # + # When only one of the two is specified: + # * When +ssl+ is truthy, +port+ defaults to +993+. + # * When +ssl+ is +false+, +port+ defaults to +143+. + # * When +port+ is +993+, +ssl+ defaults to +true+. + # * When +port+ is +143+, +ssl+ defaults to +false+. + # * When +port+ is nonstandard, the default for +ssl+ is determined + # by {config.default_ssl}[rdoc-ref:Config#default_ssl]. # # ==== Options # @@ -829,7 +842,9 @@ def idle_response_timeout; config.idle_response_timeout end # SSL session verification mode. Valid modes include # +OpenSSL::SSL::VERIFY_PEER+ and +OpenSSL::SSL::VERIFY_NONE+. # - # See {OpenSSL::SSL::SSLContext}[https://docs.ruby-lang.org/en/master/OpenSSL/SSL/SSLContext.html] for other valid SSL context params. + # See + # {OpenSSL::SSL::SSLContext}[https://docs.ruby-lang.org/en/master/OpenSSL/SSL/SSLContext.html] + # for other valid SSL context params. # # See DeprecatedClientOptions.new for deprecated SSL arguments. # @@ -910,7 +925,7 @@ def initialize(host, port: nil, ssl: nil, # Config options @host = host @config = Config.new(config, **config_options) - @port = port || (ssl ? SSL_PORT : PORT) + ssl, @port = default_ssl_and_port(ssl, port) @ssl_ctx_params, @ssl_ctx = build_ssl_ctx(ssl) # Basic Client State @@ -2633,6 +2648,27 @@ def remove_response_handler(handler) PORT = 143 # :nodoc: SSL_PORT = 993 # :nodoc: + def default_ssl_and_port(tls, port) + if tls.nil? && port + tls = true if port == SSL_PORT || /\Aimaps\z/i === port + tls = false if port == PORT + elsif port.nil? && !tls.nil? + port = tls ? SSL_PORT : PORT + end + if tls.nil? && port.nil? + tls = config.default_tls.dup.freeze + port = tls ? SSL_PORT : PORT + if tls.nil? + warn "A future version of Net::IMAP::Config#default_tls " \ + "will default to 'true', for secure connections by default. " \ + "Use 'Net::IMAP.new(host, ssl: false)' or " \ + "Net::IMAP.config.default_tls = false' to silence this warning." + end + end + tls &&= tls.respond_to?(:to_hash) ? tls.to_hash : {} + [tls, port] + end + def start_imap_connection @greeting = get_server_greeting @capabilities = capabilities_from_resp_code @greeting diff --git a/lib/net/imap/config.rb b/lib/net/imap/config.rb index 221eb1b2..2b74a3c5 100644 --- a/lib/net/imap/config.rb +++ b/lib/net/imap/config.rb @@ -200,6 +200,29 @@ def self.[](config) # The default value is +5+ seconds. attr_accessor :idle_response_timeout, type: Integer + # :markup: markdown + # + # The default value for the +ssl+ option of Net::IMAP.new, when +port+ is + # unspecified or non-standard. + # + # *Note*: A future release of Net::IMAP will set the default to +true+, as + # per RFC7525[https://tools.ietf.org/html/rfc7525], + # RFC7817[https://tools.ietf.org/html/rfc7817], and + # RFC8314[https://tools.ietf.org/html/rfc8314]. + # + # * Set to +true+ for the secure default without warnings. + # * Set to +:warn+ for the secure default _with_ warnings. + # * Set to +nil+ to use insecure defaults with warnings. + # * Set to +false+ to silence warnings and use insecure defaults. + # + # | Starting with version | The default value is | + # |-------------------------|------------------------| + # | _original_ | +false+ | + # | v0.6 (planned) | +nil+ | + # | v0.7 (planned) | +:warn+ | + # | _eventually_ | +true+ | + attr_accessor :default_ssl + # :markup: markdown # # Whether to use the +SASL-IR+ extension when the server and \SASL @@ -305,6 +328,7 @@ def defaults_hash debug: false, open_timeout: 30, idle_response_timeout: 5, + default_ssl: nil, sasl_ir: true, responses_without_block: :warn, ).freeze