New Unix SSHD test proposal

[wmunyan]

Added a new unix sshd test to collect and evaluate named sshd parameters. Content to follow, and will be added to issue.

[solind]

LGTM, WDYT @isimluk?

This will also need to be converted into a PR into the develop branch.

[isimluk]

LGTM.

Consensus call initiated: http://lists.cisecurity.org/pipermail/oval_developer_lists.cisecurity.org/Week-of-Mon-20200210/000616.html

[wmunyan]

Do I get to vote on my own proposal? If so, I think it looks great :)

[johnulmer-oval]

As far as it goes, the schema look good to me. However, it is possible (and I think it actually happens) to configure more than one instance of the sshd daemon. For instance, see - https://access.redhat.com/solutions/1166283. It might be useful to add an instance element of some sort (points to config file(s) that was used to start the sshd instance? Maybe?) to the object, state, and item schema. Unfortunately, I'm not sure how best to do that. Different OSes may use different mechanisms to do the multiple sshd daemons trick. 'Will continue to dig.

[isimluk]

@wmunyan, the 14 days consensus call period concluded. @johnulmer-oval raised important point, about having multiple instances of sshd available on the assessed system. What are Your thoughts on the subject? Is that anything that could be addressed by changes to this proposal?

[wmunyan]

Agreed. Would it be appropriate to add an element, config_file to the object|state|item? For the object element, I would propose the value either be optional or allow nillable="true". Then leaving the element out or setting xsi:nil="true" would indicate usage of the default ssh config. When a value is present, that indicates the addition of the -f option to the sshd -T command in the proposal.

Thoughts on that idea?

[johnulmer-oval]

Regarding: having a 'config_file' element in the OVAL object, state, and item elements
While there is a default or typical location of sshd config files, they can reside anywhere and can be named anything. Ergo, a content author cannot have any real foreknowledge of existing config files past the default locations and names. Consequently, it seems to me, the cases to be handled in the OVAL Object are: 1- handle default location and name for ssh config files, and, 2 - handle ALL running ssh configurations. This smells more like a boolean behavior to me. I'd default to handling all running ssh configurations. (Something like - use 'ps -ef' to get processes named 'sshd'. Parse cmd arguments to determine location of configurations. Parse identified config files. If no -f argument is found, and if 'sshd -T' is supported, config may be retrieved via 'sshd -T'. Otherwise, got to do it the long way.)

If this makes sense, the OVAL object needs only a boolean to indicate targeting either just the default location/config or all running sshd configurations.

The state and item would need 'config_file' elements.

I think....
Maybe....

[solind]

Hi @johnulmer-oval , I think probably using process_object and variables is how (in OVAL) someone would find the config file -- but your idea of a shortcut certainly would make things easier for a content-writer, and your point about all SSH configurations being security-relevant is taken.

I wouldn't object to either approach -- I'd say one would add an xsi:nilable filepath entity, and the other would use a behaviors entity.

[johnulmer-oval]

@solind, I agree with your approach to gathering system chars for this test (variables + process object). While I like the idea of a "short-cut", I don't want to stray from the conventional methods for constructing content.

[wmunyan]

@solind, @johnulmer-oval do you envision the behaviors entity on the object to be something like:

<behaviors default_config_only="(true|false)"/>

With the default value assumed to be false, to indicate that, if the behaviors element is not present, collect all SSH configurations?

[johnulmer-oval]

@solind , I my thinking, the creation of a new sshd test would be most useful if it was a short cut, so to speak, around a more tedious and error prone approach. We do not currently 'need' a new sshd test. We can do everything we need to check sshd configurations with the existing process test and the various file content tests. (Find the processes. Find their config files. Parse config files. Populate sshd items.) The value in a new sshd test is that we can reduce the complexity of that OVAL content chunk and let the SCAP/OVAL tool developers do the design work. So, if we know what we want checked regarding sshd instances, I think (as in I may be wrong) we could have a pretty concise OVAL test definition that is easy for a content author to employ.

That gets me to @wmunyan, that was pretty much what I was thinking. Let the SCAP/OVAL tool developers do the work and put the intelligence in the scanner. Leave the content author to write a pretty concise test and move on.

Maybe... unless I'm wrong.... have been before....

[solind]

Speaking for myself, I am not certain that there's a lot of value in writing tests simply to make them more usable for content authors to write. That is because, at least in my organization, content writing is done programmatically.

I thought the idea behind this test was to use sshd to actually interpret and then output the effective configuration. THAT is a valuable addition, because the effective configuration cannot really be determined from the configuration file(s) alone (in part because it depends on sshd's own defaults).

As to the behaviors, @wmunyan, I would prefer something more declarative, like:

<behaviors collect="default_instance | all_running_instances"/>

We should also consider, an advantage of using the approach of actually specifying a file path in the object is that it makes it possible to inspect a configuration without that configuration being actively tied to a running process. This could be nice, for example, if you're scanning an offline image.

In light of this consideration, the behavior I propose would allow collection of the default configuration even if sshd is not running, and likewise implies it would not collect that configuration in the event no sshd process was running.

EDIT: that last sentence should read "... and likewise implies that it would collect that configuration in the event no sshd process was running" -- whereas in the event all_running_instances is used, it would NOT be collected if there was no corresponding process.

So subtle, I sometimes even confuse myself. :D

[johnulmer-oval]

@solind and @wmunyan, I happy with whatever y'all think is appropriate. My only real concern was that the current schema addresses only the default config. As long as we address the wider situation, I'm happy. Adding a behavior seems to do that. @solind , I like the 'default_instance' | 'all running' approach.

[wmunyan]

• Change name to sshdconf_(test|object|state|item)
• Propose to add a filepath to _object. This filepath represents an sshd configuration file to interrogate. If this element is marked xsi:nil=true it would use the default (we would let sshd tell us what that default filepath is).

[matejak]

I will share here a context of the test, and I hope that it will help to the discussion:

1. There is the runtime vs configuration aspect: I see risks in the approach when the scanner is affected by running processes - although you can determine the configuration file from the process arguments without user input, there are other problems.

• You can't guarantee that only the processes that should be scanned will be running, and those that should be ignored won't be running. Anybody can run an sshd process, processes generally come and go, and I wouldn't be comfortable with the scan being so sensitive to what processes are running at that particular time.
• When a system is hosting containers, you won't be able to pair the real path to the config file with the process easily, as containers basically run in chroot jails.

2. Config file location: Default config files could be given by content authors, but I see value of exposing location of config file as an XCCDF value, so it could be customized for the purpose of the scan.
3. Config file parsing: Textfilecontent would be just enough - the sshd config file format is simple. However, there is one caveat - the trend seems to be moving towards the config file being mostly empty except the Include directive, that would include e.g. all files from /etc/ssh/sshd_config.d/, as one can observe e.g. in Fedora. This has advantages - individual configuration changes s.a. hardening can be administered in separate files s.a. /etc/ssh/sshd_config.d/hardening.conf, which could be appreciated by administrators. However, parsing that include directive and constructing a set of textfilecontent tests would result in a complex check content.
   A specialized SSHD test would hide all that complexity - I imagine that it would need just the path to the main config file, and it would then resolve all the inclusion, ordering and other things internally, probably using functions from the OpenSSH library.

So to sum it up, I would drop the runtime aspect of this PR, and I would transfer the responsibility of scanning the correct SSHD configuration to those who perform the scan.
I see that test that would accept a config file path and that would allow for checking that a configuration key matches a particular value would lead to clean and reliable SSHD configuration tests.

[solind]

In that case simply adding the path+filename/filepath to the (renamed) object should work, right @matejak ?

The existing proposal doesn't have any process dependencies.

And also, I agree with your thinking.

[matejak]

The sshd -T [NAME] is probably incorrect - the OpenSSH client doesn't expect any arguments if supplied with the -T option, and it dumps configuration keys and respective values. I suggest that the -f <main config file> is added to the documentation.

[matejak]

In that case simply adding the path+filename/filepath to the (renamed) object should work, right @matejak ?

The existing proposal doesn't have any process dependencies.

Right, I was reacting to the discussion that we had, and there is no runtime aspect visible in the proposal, so it doesn't make sense in the context of the PR.
I agree with your proposal to just add the path to the main config file.

[wmunyan]

Question: If we add the (path/filename|filepath) elements to this test, would it follow to add unix-def:FileBehaviors to the object as well, to keep consistency with other "file driven" tests?

[solind]

Hey @wmunyan , question for you...

If SshdObject/filepath@xsi:nil="true", then what should the SshdItem/filepath value be? @status="does not exist"?

Or should the item/filepath also be nilable?

[wmunyan]

Since the filepath element in the sshd_object indicates "If xsi:nil="true", then collect from the default filepath at /etc/ssh/sshd_config".... if that's the case, maybe the sshd_item element value is /etc/ssh/sshd_config ? Or maybe @status="not collected"? Or make item/filepath also nillable.... I am good with any of the suggestions. If a decision is made, I'll just add it to the item documentation.

[solind]

It's fine with me to document that object filepath@xsi:nil="true" -> item filepath="/etc/ssh/sshd_config". I see it already says that in the object documentation; I was looking at the item documentation (where it's not mentioned).

[wmunyan]

I updated the documentation for the filepath element in the system characteristics schema

[solind]

Nice! Thanks!

ETA: I think this is ready to submit into develop.