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

Proposal: Addition of 2 linux tests #74

Closed
wants to merge 4 commits into from
Closed

Proposal: Addition of 2 linux tests #74

wants to merge 4 commits into from

Conversation

wmunyan
Copy link
Contributor

@wmunyan wmunyan commented Jan 25, 2019

Two new tests are being proposed within the linux schema. These are the "lsmod" and "modprobe" tests. See the proposal for more information and sample content/results.

Two new tests are being proposed within the linux schema.  These are the "lsmod" and "modprobe" tests.  See the proposal for more information and sample content/results.
@wmunyan wmunyan changed the title New Proposal - Linux Additions Proposal: Addition of 2 linux tests Jan 25, 2019
@wmunyan wmunyan added Existing Schema Update A proposal for update(s) to an existing OVAL Test/Object/State Linux Issue related to the Linux schema. labels Jan 25, 2019
@solind
Copy link

solind commented Jan 28, 2019

Hey @wmunyan, these look pretty good (as in, reasonable and doable and important)! I'll take a closer look soon to insure they'll work as intended.

@solind
Copy link

solind commented Jan 28, 2019

Hi Bill,

I think the lsmod_item/used_by should not represent a single string in the implied form of:

[count] [module-list]

... but rather, it should have @max_occurs="unlimited", with each instance containing the name of the dependent module. For none, implementations would add an entity with status="does not exist".

WDYT?

@solind
Copy link

solind commented Jan 28, 2019

Incidentally, this would probably mean also adding an EntityItemIntType for use_count, since as you noted the dependent modules are not always listed.

@wmunyan
Copy link
Contributor Author

wmunyan commented Jan 29, 2019

@solind That sounds totally fine. I will make the schema updates and re-work my implementation to get some results including that update. Thanks very much for the quick feedback!

Bill M added 2 commits January 28, 2019 21:23
Split up "use_count" and "used_by" into 2 fields.
Description updates for "use_count" and "used_by" elements.
@solind
Copy link

solind commented Feb 20, 2019

Hi Bill, I attached a sample result for the lsmod_test. Let me know if I've missed anything.
results.txt

I also have some questions about the modprobe_test/object:

(1) I see you want to limit the module_name/@operator to "equals" ... that's not very OVAL-ish, though. Aren't the possible input values limited to the same module names returned by the lsmod command? If so, there's no inherent reason we have to limit the input values.

(2) I wasn't seeing any output using the command for randomly-selected modules, so I ran a command to try them all:

lsmod | grep -v ^Module | awk '{print $1}' | xargs -I{} modprobe -n -v {}

... and there was still no output!

Under what conditions would there be any output from that command?

@wmunyan
Copy link
Contributor Author

wmunyan commented Feb 25, 2019

David,
Thanks for the output from the lsmod_test; not too much to go wrong on that one, its pretty straightforward. For the modprobe_test, I agree, the "equals" constraint isn't all that "OVAL-ish", but the modprobe command didnt really allow too much flexibility out of the box.

I asked to some of our linux guys if there was a way to determine the list of all modules that were "loadable" and they pointed me to some information:
"By default modprobe loads modules from subdirectories located in the /lib/modules/$(uname -r) directory. Usually the files have an extension of .ko, so they can be listed like this:

find /lib/modules/$(uname -r) -type f -name '*.ko*'

That command also takes into account compressed files, such as .ko.xz

It may be that, to support operations other than "equals", a collector would need to parse the output of the above command. The name of the .ko file is the name of the module, such that a module named foo would be loaded from foo.ko.

If that makes sense, and perhaps it'd be good to get some of the linux folks to chime in, I can make the necessary changes to the PR and re-commit.

@solind
Copy link

solind commented Feb 25, 2019

Hi Bill,

One other issue is that without any kind of name validation, you can get a result like this:
results.txt

I'm not sure that's desirable either. So... hopefully we can get @isimluk to weight in with a recommendation.

@isimluk
Copy link

isimluk commented Mar 19, 2019

Hello @wmunyan and @solind, please accept my apology for delay in my response. I finally, got to review this.

That being said, lsmod part seems to be totally fine. Thank You!

Let me now focus on modprobe part.

@isimluk
Copy link

isimluk commented Mar 19, 2019

I haven't found any materials discussing use-cases for this new tests. Am I right, understanding that these probes are introduced so we can easily see whether certain kernel module is

  • not loaded
  • not loadable

?

Are there any more use-cases we envision for these probes?


My concern with the current modprobe test is that it is context dependent. modprobe -a -v will behave differently provided given kernel module was loaded or not. For example, if you have btusb loaded, but you have blacklisted its loading going forward, you won't be able to verify that using modprobe -a -v btusb... as this command will not show anything since you happen to have btusb loaded. Ilustration


# modprobe -n -v btusb      # here we see nothing, leading to false negative finding
# rmmod btusb 
# modprobe -n -v btusb      # here we see btusb blacklisted
install /bin/true

I am afraid, having the probe context dependent may become pain point and we should strive to avoid that.
Sooner or later this hits some user and they will need

  • to learn to run the scan & mitigation twice in row (just to be sure). Because failures will mostly appear to be random to them.
  • or to instruct content writers to write OVAL in a way that lsmod test and modprobe test are always in the same oval definition (that way however, mitigation/remediation script needs to be as well).

I mean this problem may only lead to OVAL writers learning new idiom: using lsmod and modprobe together and thus increasing complexity of their job.

Please don't get mad if I got too carried away, let's discuss what are the use-cases and our options. (By the way, why did we ruled out modprobe --showconfig?)

@wmunyan
Copy link
Contributor Author

wmunyan commented Mar 19, 2019

Simon,
Not mad at all! This is great feedback, thank you! Your use-case question is a fair one, and for our content here at CIS, we are combining the 2 tests -- Our recommendations are asking the specific question you mention: Ensure module X is not loaded and ensure module X is not loadable. We are following the convention you mention, using lsmod and modprobe together.

As for the code sample you provided, I had a question. If the modprobe -n -v btusb command is issued and no output is returned, that would indicate the module is loaded. For our use-case, that's a valid result that would then indicate failure (for our recommendation). If the user does some remediation, issuing the rmmod command, then subsequent assessments would show the install /bin/true result, which would be correct in our use-case. Maybe I am not following the example correctly, but those results seem OK, at least from the perspective of the CIS recommendations.

If you were to use the modprobe --showconfig command, could you provide some information on how you'd use that to determine if a particular module was loaded or not?

I am seeing this as the first part of the output, which looks promising:

[centos@ip-10-3-0-20 ~]$ modprobe --showconfig | more
install cramfs /bin/true
install freevxfs /bin/true
install jffs2 /bin/true
install hfs /bin/true
install hfsplus /bin/true
install squashfs /bin/true
install udf /bin/true

# End of configuration files. Dumping indexes now:
...

Does that output indicate those modules which are not loadable? Would that provide a more reasonable output than the modprobe -a -v?

@isimluk
Copy link

isimluk commented Mar 28, 2019

Bill, You are right that modprobe -n -v btusb being empty can be classified as failure. I missed that originally.

Thinking about this more, perhaps we may want to make the kernel modules probe more high-level. Why should content writer need to know to use both lsmod and modprobe tests? Why should content writer need to know how to parse output of modprobe -n -v btusb and its quirks. What happens when one day modprobe and lsmod change their outputs, do we plan to rework this and release lsmod2 and modprobe2 tests?

What if we introduced kernel_module test, and left the hard logic to the scanners (oval interpreters). Scanners are in sense tied to the platform, they need to be shipped/re-build on the platform. That would allow OVAL to be little more abstract.

That is not to say, Bill, that we should scrape what you were able to put together here. I am just wondering what may be your opinion about this line of thinking?

@wmunyan
Copy link
Contributor Author

wmunyan commented Mar 28, 2019

Simon,
Thanks very much for your review. I completely understand your logic. I did have a version of a "kernel_module" test which would have 2 boolean attributes, such as "loaded" and "loadable", and then allowing interpreters to collect the information appropriately to populate those columns. I hadn't proposed that because it seemed a little more specifically tied to CIS benchmarks and the types of things our recommendations look for. I can add that test to the schema as an alternative; maybe some folks have some thoughts on other elements to add to a "kernel_module" test that would help make it more feature-complete.

I am on PTO until the 2nd week in April, so I apologize both that this response was delayed, and responses until after April 5 will be delayed as well.

Cheers!

@isimluk
Copy link

isimluk commented Apr 25, 2019

Repeating what was mentioned on the OVAL Board call today.

This proposal is good as it is. However, if there is time to propose unified single and abstract test for probing kernel modules I see certain upside of it for the content writers.

I am sorry for late response.

@wmunyan
Copy link
Contributor Author

wmunyan commented Apr 27, 2019

All,
I've added a couple of items to this commit:

  1. I created, in the Linux schema, a new kernelmodule construct, which allows an interpreter to collect and evaluate the loaded and loadable status of kernel modules, per what was discussed on the OVAL board call.
  2. I added a new construct to the Unix schema, the sshd elements, to collect/evaluate SSHD parameters based on collection via sshd -T [parameter-name].

Attaching updated test content and results as well.

Linux-Kernel-Module-Test-Content-and-Results-v3.zip
Unix-SSHD-Test-Content-and-Results-v1.zip

@wmunyan wmunyan added the UNIX Issue related to the UNIX schema. label May 8, 2019
@solind
Copy link

solind commented May 24, 2019

Hey @wmunyan, where can I get the XSD changes comprising these tests? At your fork of this repository?

@wmunyan
Copy link
Contributor Author

wmunyan commented May 25, 2019

@solind yes you can find the XSDs in my fork.

@solind
Copy link

solind commented May 29, 2019

Hi @wmunyan, it looks like you want to add three new Linux tests -- lsmod, modprobe and kernelmodule. Is that right? I thought the new kernelmodule test was supposed to supplant the other two. If there is a need to collect size, use_count or used_by information, that could be added to the kernelmodule_[state/item]. Wouldn't a consolidated test be preferred?

@solind
Copy link

solind commented May 30, 2019

Also, @wmunyan, there's a var_check="none exist" in your SSHD test content ... since that value of CheckEnumeration was deprecated in OVAL 5.3 and given the language in the OVAL specification document, I'm not really sure what that's attempting to express...

EDIT: According to the OVAL specification section 5.3.6.4, I think what you want is "none satisfy"?

@wmunyan
Copy link
Contributor Author

wmunyan commented May 30, 2019

@solind i do feel like a consolidated kernel module test would be preferred, but I guess I was leaving all 3 in there to illicit more comments. If folks feel like the single test is best, I can remove the other 2 and re-commit.

Second, thanks for the content comment on the sshd test. I'll change that to "none satisfy"

@zport zport added this to the Complete first proposal milestone Jun 13, 2019
@wmunyan
Copy link
Contributor Author

wmunyan commented Jun 13, 2019

Hello everyone, I have modified the proposal, which is rendering this pull request obsolete. Please see the Pull Request for updated information.

The short version:

  • Remove lsmod and modprobe from the proposal
  • Keep kernelmodule in the proposal
  • Add the new sestatus test to the proposal
  • Move the unix-specific sshd test to its own proposal.

I believe this pull request can be closed without merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Existing Schema Update A proposal for update(s) to an existing OVAL Test/Object/State Linux Issue related to the Linux schema. UNIX Issue related to the UNIX schema.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants