🐛 Fix sshd sshd config check (globPathPattern used as file)#6318
🐛 Fix sshd sshd config check (globPathPattern used as file)#6318
Conversation
…sshd_security_check
| // This ensures that glob patterns use the actual file path instead of the glob pattern | ||
| actualPath := rootPath | ||
| if len(actualPaths) > 0 { | ||
| actualPath = actualPaths[0] |
There was a problem hiding this comment.
What about the other matches?
Taking the example from your PR description, let's assume the glob *.conf would match 01_mondoo.conf and 02_mondoo.conf.
From reading the code, I think we would not detect the 2nd file.
What do you think about expanding the glob outside this function, and calling the function for every file found?
| @@ -15,8 +15,8 @@ func TestSSHParser(t *testing.T) { | |||
| raw, err := os.ReadFile("./testdata/sshd_config") | |||
There was a problem hiding this comment.
Would it be possible to expand the testdata and split the single file up into at least two includes in subdirectories?
This way, we could add the glob case to the test cases.
There was a problem hiding this comment.
@czunker I changed the approach now, and added test for multiple includes. I also verified using the script to reproduce. When I rebuild cnquery with my changes, the problem is gone.
|
I tried it with a container, but couldn't parse the config: The file layout looks like this: The sshd config has an |
|
I checked the config. One of the included files has another I commented that one out, but no change. |
I think the unimplemented error here is because you are running it via docker connection, not locally, and the method is not implemented. But I will make a fix for this too as it's a valid use case. |
|
@czunker I got same not implemented error when checking with docker connection. I added implementation to the missing method. |
| var statPath string | ||
| if filepath.IsAbs(name) { | ||
| statPath = name | ||
| } else { | ||
| statPath = filepath.Join(f.path, name) | ||
| } | ||
| res[i], err = f.catfs.Stat(statPath) |
There was a problem hiding this comment.
This function was not used until now, but I made the change so it's backward compatible.
…sshd_security_check
@Bajusz15 Thanks for adding this to the container connection. The error is gone, but the blocks are empty: The default block works: Anything else I can try? |
No, I need to investigate what happens here with your example. I was only focusing on the bug this ticket was trying to solve. |
I believe this is correct, if there are no |
|
As discussed, I was missing parts of the config. And I can also query the group: |
Issue: sshd config parser stores file patterns in Context.Path (as path), causing "file not found" errors, because the pattern is treated as a file.
When parsing sshd configuration files that use Include directives with glob patterns (e.g., Include /etc/ssh/sshd_config.d/.conf), the parser was storing the literal glob pattern string in the
MatchBlock.Context.Pathfield instead of the actual expanded file paths.Because of this:
When sshd.config.blocks is accessed, matchBlocks2Resources() attempts to create file resources using
cur.Context.Path, sinceContext.Pathcontained the literal glob pattern (e.g., /etc/ssh/sshd_config.d/.conf) instead of the actual file path (e.g., /etc/ssh/sshd_config.d/01-mondoo.conf), it resulted in errors like:file '/etc/ssh/sshd_config.d/*.conf' not found
Root Cause:
ParseBlocks() was setting Context.Path to the rootPath parameter (which could be a glob pattern) without tracking which actual files were expanded
expandGlob() returned an error when directories didn't exist, instead of treating it as "no matches" (standard glob behavior)
Example:
SSHD config contains: Include /etc/ssh/sshd_config.d/.conf
Directory exists with file: /etc/ssh/sshd_config.d/01-mondoo.conf
Parser creates MatchBlock with Context.Path = "/etc/ssh/sshd_config.d/.conf" (literal glob)
How to reproduce:
Run the following script:
PS. Since then, it was discovered for docker connection some file system methods were not supported, so I added implementations.