diff --git a/providers/os/connection/docker/file.go b/providers/os/connection/docker/file.go index db0567abd8..b83ce611f9 100644 --- a/providers/os/connection/docker/file.go +++ b/providers/os/connection/docker/file.go @@ -84,7 +84,7 @@ func (f *File) ReadAt(b []byte, off int64) (n int, err error) { } func (f *File) Readdir(count int) (res []os.FileInfo, err error) { - return nil, errors.New("not implemented") + return f.catFs.ReadDir(f.path) } func (f *File) Readdirnames(n int) ([]string, error) { diff --git a/providers/os/connection/docker/filesystem.go b/providers/os/connection/docker/filesystem.go index 633fb3d0bf..1372425491 100644 --- a/providers/os/connection/docker/filesystem.go +++ b/providers/os/connection/docker/filesystem.go @@ -90,3 +90,7 @@ func (fs *FS) Chtimes(name string, atime time.Time, mtime time.Time) error { func (fs *FS) Chown(name string, uid, gid int) error { return errors.New("chown not implemented") } + +func (fs *FS) ReadDir(name string) ([]os.FileInfo, error) { + return fs.catFS.ReadDir(name) +} diff --git a/providers/os/connection/ssh/cat/cat.go b/providers/os/connection/ssh/cat/cat.go index 565f2a2fe7..28de2a9408 100644 --- a/providers/os/connection/ssh/cat/cat.go +++ b/providers/os/connection/ssh/cat/cat.go @@ -107,3 +107,9 @@ func (cat *Fs) Chtimes(name string, atime time.Time, mtime time.Time) error { func (cat *Fs) Chown(name string, uid, gid int) error { return NotImplemented } + +func (cat *Fs) ReadDir(name string) ([]os.FileInfo, error) { + file := NewFile(cat, name, false) + defer file.Close() + return file.Readdir(-1) // -1 is ignored, we read all files +} diff --git a/providers/os/connection/ssh/cat/cat_file.go b/providers/os/connection/ssh/cat/cat_file.go index 3e81a2cdb6..5d14c50e59 100644 --- a/providers/os/connection/ssh/cat/cat_file.go +++ b/providers/os/connection/ssh/cat/cat_file.go @@ -8,6 +8,7 @@ import ( "encoding/base64" "io" "os" + "path/filepath" "strings" "github.com/cockroachdb/errors" @@ -95,7 +96,13 @@ func (f *File) Readdir(count int) (res []os.FileInfo, err error) { res = make([]os.FileInfo, len(names)) for i, name := range names { - res[i], err = f.catfs.Stat(name) + var statPath string + if filepath.IsAbs(name) { + statPath = name + } else { + statPath = filepath.Join(f.path, name) + } + res[i], err = f.catfs.Stat(statPath) if err != nil { return nil, err } diff --git a/providers/os/resources/sshd.go b/providers/os/resources/sshd.go index 592b47c6a9..8259cac6c9 100644 --- a/providers/os/resources/sshd.go +++ b/providers/os/resources/sshd.go @@ -6,6 +6,7 @@ package resources import ( "errors" + "os" "path/filepath" "regexp" "strings" @@ -134,6 +135,12 @@ func (s *mqlSshdConfig) expandGlob(glob string) ([]string, error) { for _, path := range paths { files, err := afs.ReadDir(path) if err != nil { + // If the directory doesn't exist, treat it as "no matches" (empty result) + // This is consistent with standard glob behavior where a non-existent directory + // results in an empty match set, not an error + if os.IsNotExist(err) { + continue + } return nil, err } @@ -165,41 +172,37 @@ func (s *mqlSshdConfig) parse(file *mqlFile) error { file.Path.Data: file, } var allContents strings.Builder - globPathContent := func(glob string) (string, error) { - paths, err := s.expandGlob(glob) - if err != nil { - return "", err - } - - var content strings.Builder - for _, path := range paths { - file, ok := filesIdx[path] - if !ok { - raw, err := CreateResource(s.MqlRuntime, "file", map[string]*llx.RawData{ - "path": llx.StringData(path), - }) - if err != nil { - return "", err - } - file = raw.(*mqlFile) - filesIdx[path] = file - } - fileContent := file.GetContent() - if fileContent.Error != nil { - return "", fileContent.Error + // Function to get file content by path + fileContent := func(path string) (string, error) { + file, ok := filesIdx[path] + if !ok { + raw, err := CreateResource(s.MqlRuntime, "file", map[string]*llx.RawData{ + "path": llx.StringData(path), + }) + if err != nil { + return "", err } + file = raw.(*mqlFile) + filesIdx[path] = file + } - content.WriteString(fileContent.Data) - content.WriteString("\n") + fileContent := file.GetContent() + if fileContent.Error != nil { + return "", fileContent.Error } - res := content.String() - allContents.WriteString(res) - return res, nil + content := fileContent.Data + "\n" + allContents.WriteString(content) + return content, nil + } + + // Function to expand glob patterns + globExpand := func(glob string) ([]string, error) { + return s.expandGlob(glob) } - matchBlocks, err := sshd.ParseBlocks(file.Path.Data, globPathContent) + matchBlocks, err := sshd.ParseBlocksWithGlob(file.Path.Data, fileContent, globExpand) // TODO: check if not ready on I/O if err != nil { s.Params = plugin.TValue[map[string]any]{Error: err, State: plugin.StateIsSet | plugin.StateIsNull} diff --git a/providers/os/resources/sshd/params.go b/providers/os/resources/sshd/params.go index 57f0a5e957..6d99dcc1d9 100644 --- a/providers/os/resources/sshd/params.go +++ b/providers/os/resources/sshd/params.go @@ -85,23 +85,42 @@ func (m MatchBlocks) Flatten() map[string]any { func mergeIncludedBlocks(matchConditions map[string]*MatchBlock, blocks MatchBlocks, curBlock string) { for _, block := range blocks { - // meaning: - // 1. curBlock == "", we can always add all subblocks - // 2. if block == "", we can add it to whatever current block is - // 3. in all other cases the block criteria must match, or we move on - if block.Criteria != curBlock && curBlock != "" && block.Criteria != "" { + if block.Criteria == "" { + // Default block: merge into the current block + existing := matchConditions[curBlock] + if existing == nil { + existing = &MatchBlock{ + Criteria: curBlock, + Params: map[string]any{}, + Context: block.Context, + } + matchConditions[curBlock] = existing + } + if existing.Params == nil { + existing.Params = map[string]any{} + } + for k, v := range block.Params { + if _, ok := existing.Params[k]; !ok { + existing.Params[k] = v + } + } continue } - var existing *MatchBlock - if block.Criteria == "" { - existing = matchConditions[curBlock] - } else { - existing := matchConditions[block.Criteria] - if existing == nil { - matchConditions[block.Criteria] = block - continue + // Match block: always add to global map + existing, ok := matchConditions[block.Criteria] + if !ok { + // Create a new Match block + existing = &MatchBlock{ + Criteria: block.Criteria, + Params: map[string]any{}, + Context: block.Context, } + matchConditions[block.Criteria] = existing + } + + if existing.Params == nil { + existing.Params = map[string]any{} } for k, v := range block.Params { @@ -112,17 +131,147 @@ func mergeIncludedBlocks(matchConditions map[string]*MatchBlock, blocks MatchBlo } } -func ParseBlocks(rootPath string, globPathContent func(string) (string, error)) (MatchBlocks, error) { - content, err := globPathContent(rootPath) +type ( + fileContentFunc func(string) (content string, err error) + globExpandFunc func(string) (paths []string, err error) +) + +// ParseBlocks parses a single SSH config file and returns the match blocks. +// The filePath should be the actual file path (not a glob pattern). +// For Include directives with glob patterns, use ParseBlocksWithGlob instead. +func ParseBlocks(filePath string, content string) (MatchBlocks, error) { + curBlock := &MatchBlock{ + Criteria: "", + Params: map[string]any{}, + Context: Context{ + Path: filePath, + Range: llx.NewRange(), + curLine: 1, + }, + } + matchConditions := map[string]*MatchBlock{ + "": curBlock, + } + + lines := strings.Split(content, "\n") + for curLineIdx, textLine := range lines { + l, err := ParseLine([]rune(textLine)) + if err != nil { + return nil, err + } + + key := l.key + if key == "" { + continue + } + + // handle lower case entries and use proper ssh camel case + if sshKey, ok := SSH_Keywords[strings.ToLower(key)]; ok { + key = sshKey + } + + if key == "Include" { + // Include directives are handled by ParseBlocksWithGlob + // This function only parses single files + log.Warn().Str("file", filePath).Msg("Include directive found in single-file parser, use ParseBlocksWithGlob instead") + continue + } + + if key == "Match" { + // wrap up context on the previous block + curBlock.Context.Range = curBlock.Context.Range.AddLineRange(uint32(curBlock.Context.curLine), uint32(curLineIdx)) + curBlock.Context.curLine = curLineIdx + + // This key is stored in the condition of each block and can be accessed there. + condition := l.args + if b, ok := matchConditions[condition]; ok { + curBlock = b + } else { + curBlock = &MatchBlock{ + Criteria: condition, + Params: map[string]any{}, + Context: Context{ + curLine: curLineIdx + 1, + Path: filePath, + Range: llx.NewRange(), + }, + } + matchConditions[condition] = curBlock + } + continue + } + + setParam(curBlock.Params, key, l.args) + } + + keys := sortx.Keys(matchConditions) + res := make([]*MatchBlock, len(keys)) + i := 0 + for _, key := range keys { + res[i] = matchConditions[key] + i++ + } + + curBlock.Context.Range = curBlock.Context.Range.AddLineRange(uint32(curBlock.Context.curLine), uint32(len(lines))) + + return res, nil +} + +// ParseBlocksWithGlob parses SSH config files, expanding glob patterns in Include directives. +// It expands globs and calls ParseBlocksWithGlobRecursive for each matched file individually, +func ParseBlocksWithGlob(rootPath string, fileContent fileContentFunc, globExpand globExpandFunc) (MatchBlocks, error) { + // First, expand the root path if it's a glob + paths, err := globExpand(rootPath) if err != nil { return nil, err } + // If no paths matched, check if rootPath was a single file (not a glob) or return empty blocks + if len(paths) == 0 { + // Check if rootPath contains a glob pattern + hasGlob := strings.Contains(rootPath, "*") || strings.Contains(rootPath, "?") || strings.Contains(rootPath, "[") + if !hasGlob { + _, err := fileContent(rootPath) + if err != nil { + return nil, err + } + } + return MatchBlocks{}, nil + } + + // Parse each file individually and collect all blocks + // Each file maintains its own context (path, line numbers) + var allBlocks MatchBlocks + + for i, path := range paths { + content, err := fileContent(path) + if err != nil { + if i == 0 && (len(paths) == 1 || path == rootPath) { + return nil, err + } + log.Warn().Err(err).Str("path", path).Msg("unable to read file") + continue + } + + blocks, err := ParseBlocksWithGlobRecursive(path, content, fileContent, globExpand) + if err != nil { + log.Warn().Err(err).Str("path", path).Msg("unable to parse file") + continue + } + + allBlocks = append(allBlocks, blocks...) + } + + return allBlocks, nil +} + +// ParseBlocksWithGlobRecursive parses a single file and recursively handles Include directives. +func ParseBlocksWithGlobRecursive(filePath string, content string, fileContent fileContentFunc, globExpand globExpandFunc) (MatchBlocks, error) { curBlock := &MatchBlock{ Criteria: "", Params: map[string]any{}, Context: Context{ - Path: rootPath, + Path: filePath, Range: llx.NewRange(), curLine: 1, }, @@ -150,15 +299,31 @@ func ParseBlocks(rootPath string, globPathContent func(string) (string, error)) if key == "Include" { // FIXME: parse multi-keys properly - paths := strings.Split(l.args, " ") + includePaths := strings.Split(l.args, " ") - for _, path := range paths { - subBlocks, err := ParseBlocks(path, globPathContent) + for _, includePath := range includePaths { + // Expand glob pattern if present + expandedPaths, err := globExpand(includePath) if err != nil { - log.Warn().Err(err).Msg("unable to parse Include directive") + log.Warn().Err(err).Str("path", includePath).Msg("unable to expand Include directive") continue } - mergeIncludedBlocks(matchConditions, subBlocks, curBlock.Criteria) + + // Parse each matched file individually + for _, expandedPath := range expandedPaths { + subContent, err := fileContent(expandedPath) + if err != nil { + log.Warn().Err(err).Str("path", expandedPath).Msg("unable to read included file") + continue + } + + subBlocks, err := ParseBlocksWithGlobRecursive(expandedPath, subContent, fileContent, globExpand) + if err != nil { + log.Warn().Err(err).Str("path", expandedPath).Msg("unable to parse included file") + continue + } + mergeIncludedBlocks(matchConditions, subBlocks, curBlock.Criteria) + } } continue } @@ -179,7 +344,7 @@ func ParseBlocks(rootPath string, globPathContent func(string) (string, error)) Params: map[string]any{}, Context: Context{ curLine: curLineIdx + 1, - Path: rootPath, + Path: filePath, Range: llx.NewRange(), }, } diff --git a/providers/os/resources/sshd/params_test.go b/providers/os/resources/sshd/params_test.go index d97ea2ee07..e9c021ac66 100644 --- a/providers/os/resources/sshd/params_test.go +++ b/providers/os/resources/sshd/params_test.go @@ -15,9 +15,7 @@ func TestSSHParser(t *testing.T) { raw, err := os.ReadFile("./testdata/sshd_config") require.NoError(t, err) - sshParams, err := ParseBlocks(string(raw), func(s string) (string, error) { - return string(raw), nil - }) + sshParams, err := ParseBlocks("./testdata/sshd_config", string(raw)) if err != nil { t.Fatalf("cannot request file %v", err) } @@ -34,9 +32,7 @@ func TestSSHParseCaseInsensitive(t *testing.T) { raw, err := os.ReadFile("./testdata/case_insensitive") require.NoError(t, err) - sshParams, err := ParseBlocks(string(raw), func(s string) (string, error) { - return string(raw), nil - }) + sshParams, err := ParseBlocks("./testdata/case_insensitive", string(raw)) if err != nil { t.Fatalf("cannot request file %v", err) } @@ -47,3 +43,169 @@ func TestSSHParseCaseInsensitive(t *testing.T) { assert.Equal(t, "any", sshParams[0].Params["AddressFamily"]) assert.Equal(t, "0.0.0.0", sshParams[0].Params["ListenAddress"]) } + +func TestSSHParseWithGlob(t *testing.T) { + // Test that glob patterns in Include directives are expanded correctly + // and that each file maintains its own context + + fileContent := func(path string) (string, error) { + content, err := os.ReadFile(path) + if err != nil { + return "", err + } + return string(content), nil + } + + globExpand := func(glob string) ([]string, error) { + // For this test we handle the known patterns explicitly. + var paths []string + switch glob { + case "conf.d/*.conf": + paths = []string{ + "./testdata/conf.d/01_mondoo.conf", + "./testdata/conf.d/02_security.conf", + } + case "subdir/01_*.conf": + paths = []string{"./testdata/subdir/01_custom.conf"} + case "subdir/02_*.conf": + paths = []string{"./testdata/subdir/02_additional.conf"} + case "./testdata/sshd_config_with_include": + paths = []string{"./testdata/sshd_config_with_include"} + default: + if _, err := os.Stat(glob); err == nil { + paths = []string{glob} + } + } + return paths, nil + } + + blocks, err := ParseBlocksWithGlob("./testdata/sshd_config_with_include", fileContent, globExpand) + require.NoError(t, err) + assert.NotNil(t, blocks) + + // Verify that we have blocks from multiple files + // The main file should have blocks, and included files should also contribute + assert.Greater(t, len(blocks), 0, "should have at least one block") + + // Check that params from included files are present + // Find the default block (empty criteria) + var defaultBlock *MatchBlock + for _, block := range blocks { + if block.Criteria == "" { + defaultBlock = block + break + } + } + require.NotNil(t, defaultBlock, "should have a default block") + + // Verify params from main file + assert.Equal(t, "22", defaultBlock.Params["Port"]) + assert.Equal(t, "any", defaultBlock.Params["AddressFamily"]) + assert.Equal(t, "yes", defaultBlock.Params["UsePAM"]) + assert.Equal(t, "yes", defaultBlock.Params["X11Forwarding"]) + + // Verify params from included files (conf.d/*.conf) + assert.Equal(t, "no", defaultBlock.Params["PermitRootLogin"]) + assert.Equal(t, "yes", defaultBlock.Params["PasswordAuthentication"]) + assert.Equal(t, "3", defaultBlock.Params["MaxAuthTries"]) + assert.Equal(t, "30", defaultBlock.Params["LoginGraceTime"]) + + // Verify params from subdirectory files + assert.Contains(t, defaultBlock.Params["Ciphers"], "aes256-gcm@openssh.com") + assert.Contains(t, defaultBlock.Params["MACs"], "hmac-sha2-256-etm@openssh.com") + + // Verify that blocks have correct file paths in their context + // Check that we have blocks from different files + filePaths := make(map[string]bool) + for _, block := range blocks { + filePaths[block.Context.Path] = true + } + + // Should have blocks from main file and included files + assert.Greater(t, len(filePaths), 1, "should have blocks from multiple files") + + // Verify Match blocks from included files + var sftpBlock *MatchBlock + var adminBlock *MatchBlock + for _, block := range blocks { + if block.Criteria == "Group sftp-users" { + sftpBlock = block + } + if block.Criteria == "User admin" { + adminBlock = block + } + } + + require.NotNil(t, sftpBlock, "should have sftp-users match block") + assert.Equal(t, "no", sftpBlock.Params["AllowTcpForwarding"]) + assert.Contains(t, sftpBlock.Context.Path, "01_mondoo.conf") + + require.NotNil(t, adminBlock, "should have admin user match block") + assert.Equal(t, "yes", adminBlock.Params["PermitRootLogin"]) + assert.Equal(t, "no", adminBlock.Params["PasswordAuthentication"]) + assert.Contains(t, adminBlock.Context.Path, "02_security.conf") +} + +func TestSSHParseIncludeInsideMatchBlock(t *testing.T) { + // Test edge case: Include directive inside a Match block + // The included file has a different Match block - both should be present + // This verifies that Match blocks from included files are always added + // to the global map, regardless of where the Include directive appears. + + fileContent := func(path string) (string, error) { + content, err := os.ReadFile(path) + if err != nil { + return "", err + } + return string(content), nil + } + + globExpand := func(glob string) ([]string, error) { + // Handle the nested include file + if glob == "match_include_nested.conf" { + return []string{"./testdata/match_include_nested.conf"}, nil + } + // For other paths, check if file exists + if _, err := os.Stat(glob); err == nil { + return []string{glob}, nil + } + return []string{glob}, nil + } + + mainConfigPath := "./testdata/match_include_main.conf" + blocks, err := ParseBlocksWithGlob(mainConfigPath, fileContent, globExpand) + require.NoError(t, err) + assert.NotNil(t, blocks) + + // Find both Match blocks + var specialBlock *MatchBlock + var adminBlock *MatchBlock + var defaultBlock *MatchBlock + + for _, block := range blocks { + switch block.Criteria { + case "Group special": + specialBlock = block + case "User admin": + adminBlock = block + case "": + defaultBlock = block + } + } + + // Verify default block exists + require.NotNil(t, defaultBlock, "should have default block") + assert.Equal(t, "22", defaultBlock.Params["Port"]) + + // Verify the Match block containing the Include exists + require.NotNil(t, specialBlock, "should have 'Group special' match block") + assert.Equal(t, "30", specialBlock.Params["ClientAliveInterval"]) + + // Verify the Match block from the included file exists (this is the edge case) + // Before the fix, this block would be filtered out because Include was inside + // a Match block with different criteria. + require.NotNil(t, adminBlock, "should have 'User admin' match block from included file") + assert.Equal(t, "yes", adminBlock.Params["PermitRootLogin"]) + assert.Equal(t, "no", adminBlock.Params["PasswordAuthentication"]) + assert.Contains(t, adminBlock.Context.Path, "match_include_nested.conf") +} diff --git a/providers/os/resources/sshd/testdata/conf.d/01_mondoo.conf b/providers/os/resources/sshd/testdata/conf.d/01_mondoo.conf new file mode 100644 index 0000000000..41736f5031 --- /dev/null +++ b/providers/os/resources/sshd/testdata/conf.d/01_mondoo.conf @@ -0,0 +1,8 @@ +# First included config file +PermitRootLogin no +PasswordAuthentication yes + +Match Group sftp-users + AllowTcpForwarding no + ChrootDirectory /home/%u + diff --git a/providers/os/resources/sshd/testdata/conf.d/02_security.conf b/providers/os/resources/sshd/testdata/conf.d/02_security.conf new file mode 100644 index 0000000000..f02243782c --- /dev/null +++ b/providers/os/resources/sshd/testdata/conf.d/02_security.conf @@ -0,0 +1,8 @@ +# Second included config file +MaxAuthTries 3 +LoginGraceTime 30 + +Match User admin + PermitRootLogin yes + PasswordAuthentication no + diff --git a/providers/os/resources/sshd/testdata/match_include_main.conf b/providers/os/resources/sshd/testdata/match_include_main.conf new file mode 100644 index 0000000000..97db8aa9c1 --- /dev/null +++ b/providers/os/resources/sshd/testdata/match_include_main.conf @@ -0,0 +1,6 @@ +# Main config with Match block containing Include +Port 22 +Match Group special + ClientAliveInterval 30 + Include match_include_nested.conf + diff --git a/providers/os/resources/sshd/testdata/match_include_nested.conf b/providers/os/resources/sshd/testdata/match_include_nested.conf new file mode 100644 index 0000000000..e777d098e1 --- /dev/null +++ b/providers/os/resources/sshd/testdata/match_include_nested.conf @@ -0,0 +1,6 @@ +# Included file with different Match block +# This Match block should still be added even though Include is inside another Match block +Match User admin + PermitRootLogin yes + PasswordAuthentication no + diff --git a/providers/os/resources/sshd/testdata/sshd_config_with_include b/providers/os/resources/sshd/testdata/sshd_config_with_include new file mode 100644 index 0000000000..2a40b42cf5 --- /dev/null +++ b/providers/os/resources/sshd/testdata/sshd_config_with_include @@ -0,0 +1,14 @@ +# Main SSH config file with Include directives +Port 22 +AddressFamily any + +# Include all config files from conf.d directory +Include conf.d/*.conf + +# Include files from subdirectory +Include subdir/01_*.conf subdir/02_*.conf + +# Some additional config +UsePAM yes +X11Forwarding yes + diff --git a/providers/os/resources/sshd/testdata/subdir/01_custom.conf b/providers/os/resources/sshd/testdata/subdir/01_custom.conf new file mode 100644 index 0000000000..f3c6b7cf18 --- /dev/null +++ b/providers/os/resources/sshd/testdata/subdir/01_custom.conf @@ -0,0 +1,3 @@ +# Custom config from subdirectory +Ciphers aes256-gcm@openssh.com,aes128-gcm@openssh.com + diff --git a/providers/os/resources/sshd/testdata/subdir/02_additional.conf b/providers/os/resources/sshd/testdata/subdir/02_additional.conf new file mode 100644 index 0000000000..d25d5615c7 --- /dev/null +++ b/providers/os/resources/sshd/testdata/subdir/02_additional.conf @@ -0,0 +1,3 @@ +# Additional config from subdirectory +MACs hmac-sha2-256-etm@openssh.com,hmac-sha2-512-etm@openssh.com +