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

Uses netstat to detect open ports on AIX #2210

Merged
merged 2 commits into from
Oct 10, 2017
Merged

Conversation

cattywampus
Copy link
Contributor

@cattywampus cattywampus commented Oct 3, 2017

Related to Issue #1936 - The lsof command is only provided in the AIX expansion pack so not all systems may have it installed. However netstat is available, but in true AIX fashion the command line arguments and output is not consistent with the Linux flavor of netstat.

This PR creates a dedicated AixPorts implementation that defers to using netstat as the default port scanner for AIX and falls back to lsof if necessary. The implementation of this differs from the LinuxPorts netstat parser because:

  • The argument flags are different
  • The address information in the output is structured and parsed differently
  • The output doesn't contain process/pid information
  • Getting process/pid information requires an additional command

Disclaimer: I am by no means an AIX expert. I pieced this solution together based on this blog post. I am completely open to changing any of this to make it better. All feedback welcome.

I created some mocked unit tests for aix and the port resource. I have also tested this by using bin/inspec exec with a port control pointing to a real AIX 7.1 server. Let me know if there is additional tests that need to be written to cover these changes.

@cattywampus cattywampus requested a review from a team as a code owner October 3, 2017 22:45
@cattywampus cattywampus changed the title WIP: Uses netstat to detect open ports on AIX Uses netstat to detect open ports on AIX Oct 4, 2017
@adamleff
Copy link
Contributor

adamleff commented Oct 9, 2017

@cattywampus how are you feeling on this PR? Is this ready for review now? I hesitated to review since I knew it was a WIP, and then I was traveling so I never got back around to looking more deeply in it.

Also, can you rebase / force push your branch up again so Travis will re-test? There was a period where Travis wasn't testing things because of a misconfiguration, and it appears this is one of them.

Signed-off-by: Keith Walters <keith.walters@cattywamp.us>
Signed-off-by: Keith Walters <keith.walters@cattywamp.us>
@cattywampus
Copy link
Contributor Author

@adamleff It's ready to be run through the gauntlet. Rebased and Travis is now passing. Yay!

Copy link
Contributor

@adamleff adamleff left a comment

Choose a reason for hiding this comment

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

@cattywampus this looks solid! I don't know enough about AIX to say this is a good approach, but the code looks great and matches the style of the rest of this resource. Thank you for your contribution!

@adamleff adamleff requested a review from a team October 9, 2017 15:50
@adamleff adamleff added the Type: Enhancement Improves an existing feature label Oct 9, 2017
Copy link
Contributor

@arlimus arlimus left a comment

Choose a reason for hiding this comment

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

Wow this looks great @cattywampus thank you so much for adding it!! 😁

@arlimus arlimus merged commit 2a8d6e0 into inspec:master Oct 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Improves an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants