Improve linux-security ssh PermitRootLogin test #340
Assignees
Comments
arlimus
added a commit
to mondoohq/cnquery
that referenced
this issue
Feb 2, 2024
which also supports multiple values to be set to the field, adressing the first half of: mondoohq/cnspec-policies#340 Signed-off-by: Dominik Richter <[email protected]>
|
Heya, Thank you for bringing this up @mbainter ! I pushed a small change to the provider to support multiple values in the field, so that the policy can switch to checking Throughout this, however, I also found that we are going to add support to explicitly access match groups. Your use-case has shown that these will affect all the fields and getting them nicely structured is going to be awesome. I'll keep you posted in this issue... |
|
That will do it. I was interested in verifying that nothing allowed root ssh, so this is perfect. |
arlimus
added a commit
to mondoohq/cnquery
that referenced
this issue
Feb 5, 2024
This introduces support for querying individual blocks in SSHd configs. It's a more direct way of adressing feedback in mondoohq/cnspec-policies#340 by exposing the underlying block entirely, while also supporting aggregate values in the existing params structure. Example: Let's assume we have an existing `/etc/ssh/sshd_config` on our system with a bunch of existing configuration. If we added a new match group at the end of the file like this: ```ini Match Group sftp-users X11Forwarding no PermitRootLogin no AllowTCPForwarding yes ``` We can now query this match block both explicitly and implicitly. Implicitly it's (already) represented in the existing `params` field: ```coffee > sshd.config.params.AllowTcpForwarding "no,yes" ``` In the above example you can see, that we already had this field set above the match block with the value set to `no`. After adding our match group, it was additionally set to `yes`. The field aggregates both values. This implicit access to config values has already existed in MQL as the default behavior. With the new `blocks` field, we are extending implicit match block access to become explicit: ```coffee > sshd.config.blocks sshd.config.blocks: [ 0: sshd.config.matchBlock criteria="" 1: sshd.config.matchBlock criteria="Group sftp-users" ] ``` This first match block is the default block, which is always present. It has no criteria set and applies to everything. The second match block has a `criteria` field that shows it only matches for `Group sftp-users`. You can easily access its configuration: ```coffee > sshd.config.blocks { criteria params } sshd.config.blocks: [ 0: { criteria: "" params: { AllowTcpForwarding: "no" ... } } 1: { criteria: "Group sftp-users" params: { AllowTcpForwarding: "yes" PermitRootLogin: "no" X11Forwarding: "no" } } ] ``` In this example you can see that each block contains its own set of parameters. These are now restricted to the configuration of the block only. Thus the `AllowTcpForwarding` setting is not an aggregate of values anymore, it now only contains the value defined in the block. Added Note: As a consequence of this change we are now also consistently structuring the `Match` field in the `sshd.config.params` structure to behave like all other fields: It combines any match group separated by commas: ```coffee > sshd.config.params.Match sshd.config.params[Match]: "Group sftp-users,User myservice" ``` Signed-off-by: Dominik Richter <[email protected]>
|
I have opened a PR to add support for explicit match blocks in the resource: mondoohq/cnquery#3194 This will help address similar problems across all fields in the config and make it easier to reason with match blocks. |
chris-rock
pushed a commit
to mondoohq/cnquery
that referenced
this issue
Feb 5, 2024
* ⭐ introduce sshd.config.blocks This introduces support for querying individual blocks in SSHd configs. It's a more direct way of adressing feedback in mondoohq/cnspec-policies#340 by exposing the underlying block entirely, while also supporting aggregate values in the existing params structure. Example: Let's assume we have an existing `/etc/ssh/sshd_config` on our system with a bunch of existing configuration. If we added a new match group at the end of the file like this: ```ini Match Group sftp-users X11Forwarding no PermitRootLogin no AllowTCPForwarding yes ``` We can now query this match block both explicitly and implicitly. Implicitly it's (already) represented in the existing `params` field: ```coffee > sshd.config.params.AllowTcpForwarding "no,yes" ``` In the above example you can see, that we already had this field set above the match block with the value set to `no`. After adding our match group, it was additionally set to `yes`. The field aggregates both values. This implicit access to config values has already existed in MQL as the default behavior. With the new `blocks` field, we are extending implicit match block access to become explicit: ```coffee > sshd.config.blocks sshd.config.blocks: [ 0: sshd.config.matchBlock criteria="" 1: sshd.config.matchBlock criteria="Group sftp-users" ] ``` This first match block is the default block, which is always present. It has no criteria set and applies to everything. The second match block has a `criteria` field that shows it only matches for `Group sftp-users`. You can easily access its configuration: ```coffee > sshd.config.blocks { criteria params } sshd.config.blocks: [ 0: { criteria: "" params: { AllowTcpForwarding: "no" ... } } 1: { criteria: "Group sftp-users" params: { AllowTcpForwarding: "yes" PermitRootLogin: "no" X11Forwarding: "no" } } ] ``` In this example you can see that each block contains its own set of parameters. These are now restricted to the configuration of the block only. Thus the `AllowTcpForwarding` setting is not an aggregate of values anymore, it now only contains the value defined in the block. Added Note: As a consequence of this change we are now also consistently structuring the `Match` field in the `sshd.config.params` structure to behave like all other fields: It combines any match group separated by commas: ```coffee > sshd.config.params.Match sshd.config.params[Match]: "Group sftp-users,User myservice" ``` Signed-off-by: Dominik Richter <[email protected]> * 🟢 fix ssh params test Signed-off-by: Dominik Richter <[email protected]> * 🟢 fix mqlc tests for new fields Signed-off-by: Dominik Richter <[email protected]> --------- Signed-off-by: Dominik Richter <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
The current test doesn't know how to handle multiple settings for PermitRootLogin. I already reported a bug in cnspec because it can't differentiate things like match groups. The policy gets a result of "no,no" because it sees it as two of the same option, so it fails the test.
This should loop over a list if it exists and ensure all of them are set to no. Ideally, cnspec would give you the match groups as collections of options, which can be tested in turn and reported individually.
To Reproduce
Steps to reproduce the behavior:
Match Group games
PermitRootLogin no
sshd.config.params[PermitRootLogin]Expected behavior
That output should pass the test, but it does not, because it is looking for "no" and doesn't know how to handle multiple results.
Desktop (please complete the following information):
The text was updated successfully, but these errors were encountered: