Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

INCOMPATIBLE CHANGE: Verify the server's certificate by default. #12

Merged
merged 1 commit into from
Oct 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 22 additions & 12 deletions lib/net/smtp.rb
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,10 @@ class << self
alias default_ssl_port default_tls_port
end

def SMTP.default_ssl_context
OpenSSL::SSL::SSLContext.new
def SMTP.default_ssl_context(verify_peer=true)
context = OpenSSL::SSL::SSLContext.new
context.verify_mode = verify_peer ? OpenSSL::SSL::VERIFY_PEER : OpenSSL::SSL::VERIFY_NONE
context
end

#
Expand Down Expand Up @@ -295,7 +297,7 @@ def tls?
# Enables SMTP/TLS (SMTPS: SMTP over direct TLS connection) for
# this object. Must be called before the connection is established
# to have any effect. +context+ is a OpenSSL::SSL::SSLContext object.
def enable_tls(context = SMTP.default_ssl_context)
def enable_tls(context = nil)
raise 'openssl library not installed' unless defined?(OpenSSL)
raise ArgumentError, "SMTPS and STARTTLS is exclusive" if @starttls == :always
@tls = true
Expand Down Expand Up @@ -332,7 +334,7 @@ def starttls_auto?

# Enables SMTP/TLS (STARTTLS) for this object.
# +context+ is a OpenSSL::SSL::SSLContext object.
def enable_starttls(context = SMTP.default_ssl_context)
def enable_starttls(context = nil)
raise 'openssl library not installed' unless defined?(OpenSSL)
raise ArgumentError, "SMTPS and STARTTLS is exclusive" if @tls
@starttls = :always
Expand All @@ -341,7 +343,7 @@ def enable_starttls(context = SMTP.default_ssl_context)

# Enables SMTP/TLS (STARTTLS) for this object if server accepts.
# +context+ is a OpenSSL::SSL::SSLContext object.
def enable_starttls_auto(context = SMTP.default_ssl_context)
def enable_starttls_auto(context = nil)
raise 'openssl library not installed' unless defined?(OpenSSL)
raise ArgumentError, "SMTPS and STARTTLS is exclusive" if @tls
@starttls = :auto
Expand Down Expand Up @@ -404,14 +406,14 @@ def debug_output=(arg)

#
# :call-seq:
# start(address, port = nil, helo: 'localhost', user: nil, secret: nil, authtype: nil) { |smtp| ... }
# start(address, port = nil, helo: 'localhost', user: nil, secret: nil, authtype: nil, tls_verify: true) { |smtp| ... }
# start(address, port = nil, helo = 'localhost', user = nil, secret = nil, authtype = nil) { |smtp| ... }
#
# Creates a new Net::SMTP object and connects to the server.
#
# This method is equivalent to:
#
# Net::SMTP.new(address, port).start(helo: helo_domain, user: account, secret: password, authtype: authtype)
# Net::SMTP.new(address, port).start(helo: helo_domain, user: account, secret: password, authtype: authtype, tls_verify: flag)
#
# === Example
#
Expand Down Expand Up @@ -441,6 +443,7 @@ def debug_output=(arg)
# or other authentication token; and +authtype+ is the authentication
# type, one of :plain, :login, or :cram_md5. See the discussion of
# SMTP Authentication in the overview notes.
# If +tls_verify+ is true, verify the server's certificate. The default is true.
#
# === Errors
#
Expand All @@ -456,14 +459,14 @@ def debug_output=(arg)
# * IOError
#
def SMTP.start(address, port = nil, *args, helo: nil,
user: nil, secret: nil, password: nil, authtype: nil,
user: nil, secret: nil, password: nil, authtype: nil, tls_verify: true,
&block)
raise ArgumentError, "wrong number of arguments (given #{args.size + 2}, expected 1..6)" if args.size > 4
helo ||= args[0] || 'localhost'
user ||= args[1]
secret ||= password || args[2]
authtype ||= args[3]
new(address, port).start(helo: helo, user: user, secret: secret, authtype: authtype, &block)
new(address, port).start(helo: helo, user: user, secret: secret, authtype: authtype, tls_verify: tls_verify, &block)
end

# +true+ if the SMTP session has been started.
Expand All @@ -473,7 +476,7 @@ def started?

#
# :call-seq:
# start(helo: 'localhost', user: nil, secret: nil, authtype: nil) { |smtp| ... }
# start(helo: 'localhost', user: nil, secret: nil, authtype: nil, tls_verify: true) { |smtp| ... }
# start(helo = 'localhost', user = nil, secret = nil, authtype = nil) { |smtp| ... }
#
# Opens a TCP connection and starts the SMTP session.
Expand All @@ -488,6 +491,7 @@ def started?
# the type of authentication to attempt; it must be one of
# :login, :plain, and :cram_md5. See the notes on SMTP Authentication
# in the overview.
# If +tls_verify+ is true, verify the server's certificate. The default is true.
#
# === Block Usage
#
Expand Down Expand Up @@ -527,12 +531,18 @@ def started?
# * IOError
#
def start(*args, helo: nil,
user: nil, secret: nil, password: nil, authtype: nil)
user: nil, secret: nil, password: nil, authtype: nil, tls_verify: true)
raise ArgumentError, "wrong number of arguments (given #{args.size}, expected 0..4)" if args.size > 4
helo ||= args[0] || 'localhost'
user ||= args[1]
secret ||= password || args[2]
authtype ||= args[3]
if @tls && @ssl_context_tls.nil?
@ssl_context_tls = SMTP.default_ssl_context(tls_verify)
end
if @starttls && @ssl_context_starttls.nil?
@ssl_context_starttls = SMTP.default_ssl_context(tls_verify)
end
if block_given?
begin
do_start helo, user, secret, authtype
Expand Down Expand Up @@ -578,7 +588,7 @@ def do_start(helo_domain, user, secret, authtype)
"STARTTLS is not supported on this server"
end
starttls
@socket = new_internet_message_io(tlsconnect(s, @ssl_context_starttls || SMTP.default_ssl_context))
@socket = new_internet_message_io(tlsconnect(s, @ssl_context_starttls))
# helo response may be different after STARTTLS
do_helo helo_domain
end
Expand Down
2 changes: 1 addition & 1 deletion test/net/smtp/test_smtp.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ def test_tls_connect
smtp = Net::SMTP.new("localhost", servers[0].local_address.ip_port)
smtp.enable_tls
smtp.open_timeout = 1
smtp.start do
smtp.start(tls_verify: false) do
end
ensure
sock.close if sock
Expand Down
18 changes: 16 additions & 2 deletions test/net/smtp/test_sslcontext.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ def tlsconnect(*)

def ssl_socket(socket, context)
@__ssl_context = context
super
s = super
s.define_singleton_method(:post_connection_check){|_|}
s
end
end

Expand Down Expand Up @@ -60,7 +62,7 @@ def start_smtpd(starttls)
def test_default
smtp = MySMTP.new(start_smtpd(true))
smtp.start
assert_not_nil(smtp.__ssl_context)
assert_equal(OpenSSL::SSL::VERIFY_PEER, smtp.__ssl_context.verify_mode)
end

def test_enable_tls
Expand Down Expand Up @@ -96,5 +98,17 @@ def test_enable_starttls_before_disable_tls
smtp.start
assert_equal(context, smtp.__ssl_context)
end

def test_start_with_tls_verify_true
smtp = MySMTP.new(start_smtpd(true))
smtp.start(tls_verify: true)
assert_equal(OpenSSL::SSL::VERIFY_PEER, smtp.__ssl_context.verify_mode)
end

def test_start_with_tls_verify_false
smtp = MySMTP.new(start_smtpd(true))
smtp.start(tls_verify: false)
assert_equal(OpenSSL::SSL::VERIFY_NONE, smtp.__ssl_context.verify_mode)
end
end
end