Skip to content

Commit

Permalink
🗑️ Add deprecation warnings to .new and #starttls [🚧 WIP]
Browse files Browse the repository at this point in the history
* `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?
  • Loading branch information
nevans committed Oct 5, 2023
1 parent 2066e7f commit cf830ef
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 10 deletions.
60 changes: 53 additions & 7 deletions lib/net/imap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,19 @@ class << self
alias default_imap_port default_port
alias default_imaps_port default_tls_port
alias default_ssl_port default_tls_port

# The default value for the +tls+ option of ::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
# +false+ to globally silence warnings and use insecure defaults.
attr_accessor :default_tls
alias default_ssl default_tls
end

# Returns the initial greeting the server, an UntaggedResponse.
Expand Down Expand Up @@ -745,16 +758,28 @@ class << self
# Accepts the following options:
#
# [port]
# Port number. Defaults to 993 when +ssl+ is truthy, and 143 otherwise.
# Port number. Defaults to 143 when +tls+ is false, 993 when +tls+ is
# truthy. Based on ::default_tls when both +port+ and +tls+ are nil.
#
# [ssl]
# [tls]
# If +true+, the connection will use TLS with the default params set by
# {OpenSSL::SSL::SSLContext#set_params}[https://docs.ruby-lang.org/en/master/OpenSSL/SSL/SSLContext.html#method-i-set_params].
# If +ssl+ is a hash, it's passed to
# If +tls+ is a hash, it's passed to
# {OpenSSL::SSL::SSLContext#set_params}[https://docs.ruby-lang.org/en/master/OpenSSL/SSL/SSLContext.html#method-i-set_params];
# the keys are names of attribute assignment methods on
# SSLContext[https://docs.ruby-lang.org/en/master/OpenSSL/SSL/SSLContext.html].
#
# When <tt>port: 993</tt>, +tls+ defaults to +true+.
# When <tt>port: 143</tt>, +tls+ defaults to +false+.
# When port is unspecified or non-standard, +tls+ defaults to
# ::default_tls. When ::default_tls is also +nil+, a warning is printed
# and the connection does _not_ use TLS.
#
# When +nil+ or unassigned a default value is assigned: the default is
# +true+ if <tt>port: 993</tt>, +false+ if <tt>port: 143</tt>, and
# ::default_tls when +port+ is unspecified or non-standard. When
# ::default_tls is +nil+, a back
#
# [open_timeout]
# Seconds to wait until a connection is opened
# [idle_response_timeout]
Expand Down Expand Up @@ -810,15 +835,15 @@ class << self
# [Net::IMAP::ByeResponseError]
# Connected to the host successfully, but it immediately said goodbye.
#
def initialize(host, port: nil, ssl: nil,
def initialize(host, port: nil, tls: nil,
open_timeout: 30, idle_response_timeout: 5)
super()
# Config options
@host = host
@port = port || (ssl ? SSL_PORT : PORT)
tls, @port = default_tls_and_port(tls, port)
@open_timeout = Integer(open_timeout)
@idle_response_timeout = Integer(idle_response_timeout)
@ssl_ctx_params, @ssl_ctx = build_ssl_ctx(ssl)
@ssl_ctx_params, @ssl_ctx = build_ssl_ctx(tls)

# Basic Client State
@utf8_strings = false
Expand Down Expand Up @@ -933,7 +958,7 @@ def capabilities
# servers will drop all <tt>AUTH=</tt> mechanisms from #capabilities after
# the connection has authenticated.
#
# imap = Net::IMAP.new(hostname, ssl: false)
# imap = Net::IMAP.new(hostname, tls: false)
# imap.capabilities # => ["IMAP4REV1", "LOGINDISABLED"]
# imap.auth_mechanisms # => []
#
Expand Down Expand Up @@ -2352,6 +2377,27 @@ def remove_response_handler(handler)

@@debug = false

def default_tls_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 = self.class.default_tls.dup.freeze
port = tls ? SSL_PORT : PORT
if tls.nil?
warn "A future version of Net::IMAP.default_tls " \
"will default to 'true', for secure connections by default. " \
"Use 'Net::IMAP.new(host, tls: false)' or set " \
"Net::IMAP.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
Expand Down
29 changes: 26 additions & 3 deletions lib/net/imap/deprecated_client_options.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@ class IMAP < Protocol

# This module handles deprecated arguments to various Net::IMAP methods.
module DeprecatedClientOptions
UNDEF = Module.new.freeze
private_constant :UNDEF

# :call-seq:
# Net::IMAP.new(host, **options) # standard keyword options
# Net::IMAP.new(host, ssl: nil, **options) # ssl => tls
# Net::IMAP.new(host, options) # obsolete hash options
# Net::IMAP.new(host, port) # obsolete port argument
# Net::IMAP.new(host, port, usessl, certs = nil, verify = true) # deprecated SSL arguments
Expand All @@ -19,6 +22,13 @@ module DeprecatedClientOptions
# Using obsolete arguments does not a warning. Obsolete arguments will be
# deprecated by a future release.
#
# If +ssl+ is given, it is silently converted to the +tls+ keyword
# argument. Combining both +ssl+ and +tls+ raises an ArgumentError. Both
# of the following behave identically:
#
# Net::IMAP.new("imap.example.com", port: 993, ssl: {ca_path: "path/to/certs"})
# Net::IMAP.new("imap.example.com", port: 993, tls: {ca_path: "path/to/certs"})
#
# If a second positional argument is given and it is a hash (or is
# convertable via +#to_hash+), it is converted to keyword arguments.
#
Expand Down Expand Up @@ -71,6 +81,7 @@ module DeprecatedClientOptions
#
def initialize(host, port_or_options = nil, *deprecated, **options)
if port_or_options.nil? && deprecated.empty?
translate_ssl_to_tls(options)
super host, **options
elsif options.any?
# Net::IMAP.new(host, *__invalid__, **options)
Expand All @@ -79,15 +90,17 @@ def initialize(host, port_or_options = nil, *deprecated, **options)
# Net::IMAP.new(host, options, *__invalid__)
raise ArgumentError, "Do not use deprecated SSL params with options hash"
elsif port_or_options.respond_to?(:to_hash)
super host, **Hash.try_convert(port_or_options)
options = Hash.try_convert(port_or_options)
translate_ssl_to_tls(options)
super host, **options
elsif deprecated.empty?
super host, port: port_or_options
elsif deprecated.shift
warn "DEPRECATED: Call Net::IMAP.new with keyword options", uplevel: 1
super host, port: port_or_options, ssl: create_ssl_params(*deprecated)
super host, port: port_or_options, tls: create_ssl_params(*deprecated)
else
warn "DEPRECATED: Call Net::IMAP.new with keyword options", uplevel: 1
super host, port: port_or_options, ssl: false
super host, port: port_or_options, tls: false
end
end

Expand Down Expand Up @@ -120,7 +133,17 @@ def starttls(*deprecated, **options)

private

def translate_ssl_to_tls(options)
return unless options.key?(:ssl)
if options.key?(:tls)
raise ArgumentError, "conflicting :ssl and :tls keyword arguments"
end
options.merge!(tls: options.delete(:ssl))
end

def create_ssl_params(certs = nil, verify = true)
certs = nil if certs == UNDEF
verify = true if verify == UNDEF
params = {}
if certs
if File.file?(certs)
Expand Down

0 comments on commit cf830ef

Please sign in to comment.