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.

Sign up for GitHub

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

WIP: Support TLS1.3 #31

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

WIP: Support TLS1.3 #31

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
49374a0
Support TLS1.3
micheelengronne Mar 13, 2020
2acc447
attribute to choose TLS version
micheelengronne Mar 13, 2020
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
17 changes: 16 additions & 1 deletion controls/ssl_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@

#######################################################
# Protocol Tests #
# Valid protocols are: tls1.2 #
# Valid protocols are: tls1.2, tls1.3 #
# Invalid protocols are : ssl2, ssl3, tls1.0, tls1.1 #
#######################################################
control 'ssl2' do
Expand Down Expand Up @@ -162,6 +162,21 @@
end
end

control 'tls1.3' do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the addition of TLS 1.3. How to do you envision the use of TLS 1.2 control and TLS 1.3 control in parallel? Either one control will fail.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a great question. Maybe, we should put another attribute to choose between the 2. TLS1.2 is still valid so we canno't remove it yet.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should test if TLS is configured properly.

  • remove control 'tls1.2'
  • create new control 'tls' that verifies that it is either TLS 1.2 or TLS 1.3
  • introduce an parameter to enforce a strict version, eg. tls_version

valid settings for tls_version could be auto (default), tls1.2 or tls1.3

What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. It is semantically better to regroup it on a simple control 'tls'.

title 'Enable TLS 1.3 on exposed ports.'
impact 0.5
only_if { sslports.length > 0 }

sslports.each do |sslport|
# create a description
proc_desc = "on node == #{target_hostname} running #{sslport[:socket].process.inspect} (#{sslport[:socket].pid})"
describe ssl(sslport).protocols('tls1.3') do
it(proc_desc) { should be_enabled }
it { should be_enabled }
end
end
end

#######################################################
# Key Exchange (Kx) Tests #
# Valid Kx(s) are: ECDHE #
Expand Down