Skip to content
Snippets Groups Projects
Unverified Commit 018c6de1 authored by Stan Hu's avatar Stan Hu
Browse files

Validate that SMTP settings do not enable both TLS and STARTTLS

GitLab versions 15.10.4 and up shipped with
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116925 in order
to fix a Ruby 3 upgrade issue that made it impossible to disable
STARTTLS if the SMTP server advertised support for it.

However, this change enforced the constraint that both SMTP TLS and
STARTTLS cannot be enabled simultaneously. This follows the behavior
of the net-smtp gem, which raises an exception if a user attempts to
enable both.

This constraint was added because as described in
https://github.com/mikel/mail/pull/1536, the logic for
enabling/disabling TLS and/or STARTTLS is a bit tricky to get right
and missed some important edge cases.

This commit adds a validation step that will throw an error if both
settings appear:

```ruby
gitlab_rails['smtp_tls'] = true
gitlab_rails['smtp_enable_starttls_auto'] = true
```

Previously if SMTP TLS were enabled, STARTTLS was disabled outright.

If `gitlab_rails['smtp_tls']` is enabled, generally the easiest way to
get things working is to set
`gitlab_rails['smtp_enable_starttls_auto']` to `false`.

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/409835

Also see https://gitlab.com/gitlab-org/omnibus-gitlab/-/merge_requests/6858

Changelog: changed
parent a2a70fe3
No related branches found
No related tags found
No related merge requests found
Loading
Loading
@@ -34,6 +34,7 @@ module GitlabRails
parse_incoming_email_logfile
parse_service_desk_email_logfile
parse_maximum_request_duration
validate_smtp_settings!
end
 
def parse_directories
Loading
Loading
@@ -361,6 +362,10 @@ module GitlabRails
raise "The maximum request duration needs to be smaller than the worker timeout (#{worker_timeout}s)"
end
 
def validate_smtp_settings!
SmtpHelper.validate_smtp_settings!(Gitlab['gitlab_rails'])
end
def public_path
"#{Gitlab['node']['package']['install-dir']}/embedded/service/gitlab-rails/public"
end
Loading
Loading
# frozen_string_literal: true
class SmtpHelper
def self.validate_smtp_settings!(rails_config)
return unless rails_config['smtp_enable']
return unless rails_config['smtp_tls'] && rails_config['smtp_enable_starttls_auto']
raise "gitlab_rails['smtp_tls'] and gitlab_rails['smtp_enable_starttls_auto'] are mutually exclusive." \
" Set one of them to false. SMTP providers usually use port 465 for TLS and port 587 for STARTTLS."
end
end
Loading
Loading
@@ -1232,6 +1232,58 @@ RSpec.describe 'gitlab::gitlab-rails' do
}
end
end
context 'when STARTTLS is enabled' do
before do
stub_gitlab_rb(
gitlab_rails: {
smtp_enable: true,
smtp_enable_starttls_auto: true
}
)
end
it 'enables STARTTLS in the settings' do
expect(chef_run).to render_file('/var/opt/gitlab/gitlab-rails/etc/smtp_settings.rb').with_content { |content|
expect(content).not_to include('tls =')
expect(content).to include('enable_starttls_auto: true')
}
end
end
context 'when SMTP TLS is enabled' do
before do
stub_gitlab_rb(
gitlab_rails: {
smtp_enable: true,
smtp_tls: true
}
)
end
it 'enables SMTP TLS in the settings' do
expect(chef_run).to render_file('/var/opt/gitlab/gitlab-rails/etc/smtp_settings.rb').with_content { |content|
expect(content).to include('tls: true')
expect(content).not_to include('enable_starttls_auto')
}
end
end
context 'when TLS and STARTTLS are enabled' do
before do
stub_gitlab_rb(
gitlab_rails: {
smtp_enable: true,
smtp_tls: true,
smtp_enable_starttls_auto: true
}
)
end
it 'raises an exception' do
expect { chef_run }.to raise_error(RuntimeError)
end
end
end
 
describe 'cleaning up the legacy sidekiq log symlink' do
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment