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

ssl resource fix and speed improvement #1027

Merged
merged 1 commit into from
Sep 7, 2016
Merged

Conversation

alexpop
Copy link
Contributor

@alexpop alexpop commented Sep 7, 2016

  • Handled hostname differently for WinRM::Connection. Without this, remote windows scans are skipped with error: Cannot determine host for SSL test. Please specify it or use a different target.
  • Parallelize protocol checks to speed up scanning. For each port, five protocols checks are done. Since most ports are not SSL or blocked by firewall, scans can take minutes. Before parallelizing the calls, I scanned a vanilla Windows 2012r2 server in 90 seconds. After applying the change in this PR, it got down to 22 seconds.

@alexpop alexpop added Type: Bug Feature not working as expected Type: Enhancement Improves an existing feature labels Sep 7, 2016
@@ -44,6 +46,11 @@ class SSL < Inspec.resource(1)
def initialize(opts = {})
@host = opts[:host] ||
inspec.backend.instance_variable_get(:@hostname)
# FIXME: This can be removed when/if @hostname is available as a property for 'Train::Transports::WinRM::Connection'
# Train enhancement request for this here: https://github.com/chef/train/issues/128
if @host.nil? && inspec.backend.class.to_s == 'Train::Transports::WinRM::Connection'
Copy link
Contributor

Choose a reason for hiding this comment

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

It feels like we do not want to add this here, but instead we want to improve train directly. Could you push a PR for train, so that we can depend on that?

Copy link
Contributor

Choose a reason for hiding this comment

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

Please get this merged first. We can remove it in the second cycle and improve train. I don't think we should block this change because of a minor adjustment.

parallelize protocol checks to speed up the scan
@chris-rock
Copy link
Contributor

The FIXME is followed up in inspec/train#128

@chris-rock chris-rock merged commit 1268a28 into master Sep 7, 2016
@chris-rock chris-rock deleted the ap/ssl-improvements branch September 7, 2016 09:09
@chris-rock chris-rock modified the milestone: 0.34.0 Sep 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Feature not working as expected Type: Enhancement Improves an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants