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

[bugfix] Allow multiple maxlogin specifications in /etc/security/limi… #1487

Merged
merged 1 commit into from
Oct 3, 2016

Conversation

radzy
Copy link

@radzy radzy commented Sep 16, 2016

…ts.d #1329

Signed-off-by: T.O. Radzy Radzykewycz radzy@windriver.com

@radzy
Copy link
Author

radzy commented Sep 16, 2016

This change allows multiple maxlogin specifications in /etc/security/limits.d/*.conf , but still does not handle the ordering requirement. The multiple specifications must all pass for the rule to pass, so there are still anomalous failures.

I can't figure out how to deal with the ordering requirement using textfilecontent54, since there are no ordered conditions available and no way to choose "the last".

@mpreisler mpreisler added this to the 0.1.31 milestone Sep 29, 2016
@mpreisler mpreisler added the enhancement General enhancements to the project. label Sep 29, 2016
@mpreisler
Copy link
Member

This could benefit from OVALProject/Sandbox#146 but for now this is probably the best we can do.

@mpreisler mpreisler self-assigned this Sep 29, 2016
@@ -54,7 +54,7 @@
<ind:path>/etc/security/limits.d</ind:path>
<ind:filename operation="pattern match">^.*\.conf$</ind:filename>
<ind:pattern operation="pattern match">^[\s]*\*[\s]+(?:(?:hard)|(?:-))[\s]+maxlogins</ind:pattern>
<ind:instance datatype="int">1</ind:instance>
<ind:instance datatype="int" operation="greater than or equal">0</ind:instance>
Copy link
Member

Choose a reason for hiding this comment

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

I am probably missing something. Could you please explain why you used 0 here and not 1?

Copy link
Author

Choose a reason for hiding this comment

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

Hmmm. Doesn't look like you're missing anything.

I was playing with the idea of removing the negate in the criterion. That would have made this "operation="equal">0<

My best guess right now is that when I backed that out, I forgot to change the number back to 1, and only changed the operation. I do recall testing it after having done that, and the tests doing what I expected. But perhaps I made a mistake in that testing.

I'll test it again with the current code. If that passes, I'll say so. Otherwise (expected), I'll upload a revision.

@mpreisler
Copy link
Member

(Apologize for the slow review. Slipped my radar.)

@radzy
Copy link
Author

radzy commented Sep 29, 2016

@mpreisler : Not sure I understand this. My current testing shows identical results for 0 or 1. I'll test a bit more in the morning, and upload a revised commit regardless.

…ts.d ComplianceAsCode#1329

Signed-off-by: T.O. Radzy Radzykewycz <radzy@windriver.com>
@radzy
Copy link
Author

radzy commented Sep 30, 2016

@mpreisler : Well, I ran the tests again, and they showed identical results for the two conditions. So I suspect that means a bug in OpenSCAP.

Since it is clearer, and logically correct, with "1", I've uploaded a new version.

I'll take a look at OpenSCAP to see if I can find the cause of this.

@mpreisler
Copy link
Member

OK, in the meantime I think we can merge this.

@mpreisler mpreisler merged commit f7e5127 into ComplianceAsCode:master Oct 3, 2016
@radzy
Copy link
Author

radzy commented Oct 3, 2016

Thanks, @mpreisler

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement General enhancements to the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants